Skip to content

Return image ID in PullImageResponse.image_ref to stay compatible with future k8s versions #549

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
awmirantis merged 2 commits into from
May 19, 2026
Merged

Return image ID in PullImageResponse.image_ref to stay compatible with future k8s versions #549

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4372887
PullImage: return image ID in PullImageResponse.image_ref
stlaz Mar 12, 2026
b599f39
core/stats_windows: fix logging format strings
stlaz Mar 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 4 additions & 9 deletions core/image.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,12 +155,12 @@ func (ds *dockerService) PullImage(
return nil, filterHTTPError(err, image.Image)
}

Comment thread
stlaz marked this conversation as resolved.
imageRef, err := getImageRef(ds.client, image.Image)
imageID, err := getImageID(ds.client, image.Image)
if err != nil {
return nil, err
}

return &runtimeapi.PullImageResponse{ImageRef: imageRef}, nil
return &runtimeapi.PullImageResponse{ImageRef: imageID}, nil
}

// RemoveImage removes the image.
Expand Down Expand Up @@ -202,8 +202,8 @@ func (ds *dockerService) RemoveImage(
return &runtimeapi.RemoveImageResponse{}, nil
}

// getImageRef returns the image digest if exists, or else returns the image ID.
func getImageRef(client libdocker.DockerClientInterface, image string) (string, error) {
// getImageID returns the image ID.
func getImageID(client libdocker.DockerClientInterface, image string) (string, error) {
img, err := client.InspectImageByRef(image)
if err != nil {
return "", err
Expand All @@ -212,11 +212,6 @@ func getImageRef(client libdocker.DockerClientInterface, image string) (string,
return "", fmt.Errorf("unable to inspect image %s", image)
}

// Returns the digest if it exist.
if len(img.RepoDigests) > 0 {
return img.RepoDigests[0], nil
}

return img.ID, nil
}

Expand Down
40 changes: 35 additions & 5 deletions core/image_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package core

import (
"fmt"
"reflect"
"testing"

dockertypes "github.com/docker/docker/api/types"
Expand Down Expand Up @@ -138,17 +139,19 @@ func TestRemoveImage(t *testing.T) {
}
}

func TestPullWithJSONError(t *testing.T) {
func TestPullImage(t *testing.T) {
Comment thread
stlaz marked this conversation as resolved.
ds, fakeDocker, _ := newTestDockerService()
tests := map[string]struct {
image *runtimeapi.ImageSpec
err error
expectedError string
expectImage bool
}{
"Json error": {
&runtimeapi.ImageSpec{Image: "ubuntu"},
&jsonmessage.JSONError{Code: 50, Message: "Json error"},
"Json error",
false,
},
"Bad gateway": {
&runtimeapi.ImageSpec{Image: "ubuntu"},
Expand All @@ -157,15 +160,42 @@ func TestPullWithJSONError(t *testing.T) {
Message: "<!doctype html>\n<html class=\"no-js\" lang=\"\">\n <head>\n </head>\n <body>\n <h1>Oops, there was an error!</h1>\n <p>We have been contacted of this error, feel free to check out <a href=\"http://status.docker.com/\">status.docker.com</a>\n to see if there is a bigger issue.</p>\n\n </body>\n</html>",
},
"RegistryUnavailable",
false,
},
"Successful Pull": {
&runtimeapi.ImageSpec{Image: "ubuntu"},
nil,
"",
true,
},
}
for key, test := range tests {
fakeDocker.InjectError("pull", test.err)
_, err := ds.PullImage(
if test.err != nil {
fakeDocker.InjectError("pull", test.err)
}

gotResp, gotErr := ds.PullImage(
getTestCTX(),
&runtimeapi.PullImageRequest{Image: test.image, Auth: &runtimeapi.AuthConfig{}},
)
require.Error(t, err, fmt.Sprintf("TestCase [%s]", key))
assert.Contains(t, err.Error(), test.expectedError)
if (len(test.expectedError) > 0) != (gotErr != nil) {
t.Fatalf("expected err %q but got %v", test.expectedError, gotErr)
}

if len(test.expectedError) > 0 {
require.Error(t, gotErr, fmt.Sprintf("TestCase [%s]", key))
assert.Contains(t, gotErr.Error(), test.expectedError)
}

if test.expectImage {
expectedResp := &runtimeapi.PullImageResponse{
ImageRef: libdocker.FakePullImageIDMapping(test.image.Image),
}

if !reflect.DeepEqual(gotResp, expectedResp) {
t.Errorf("expected pull response %v, got %v", expectedResp, gotResp)
}
}

}
}
8 changes: 4 additions & 4 deletions core/stats_windows.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@ func (ds *dockerService) getContainerStats(container *runtimeapi.Container) (*ru
// That will typically happen with init-containers in Exited state. Docker still knows about them but the HCS does not.
// As we don't want to block stats retrieval for other containers, we only log errors.
if !hcsshim.IsNotExist(err) && !hcsshim.IsAlreadyStopped(err) {
logrus.Info("Error opening container for ID: %d (stats will be missing): %v", containerID, err)
logrus.Infof("Error opening container for ID: %s (stats will be missing): %v", containerID, err)
}
return nil, nil
}
defer func() {
closeErr := hcsshimContainer.Close()
if closeErr != nil {
logrus.Errorf("Error closing container %d: %v", containerID, err)
logrus.Errorf("Error closing container %s: %v", containerID, closeErr)
}
}()

Expand All @@ -56,8 +56,8 @@ func (ds *dockerService) getContainerStats(container *runtimeapi.Container) (*ru
// These hcs errors do not have helpers exposed in public package so need to query for the known codes
// https://github.com/microsoft/hcsshim/blob/master/internal/hcs/errors.go
// PR to expose helpers in hcsshim: https://github.com/microsoft/hcsshim/pull/933
logrus.Info(
"Container %dis not in a state that stats can be accessed. This occurs when the container is created but not started: %v",
logrus.Infof(
"Container %s is not in a state that stats can be accessed. This occurs when the container is created but not started: %v",
containerID,
err,
)
Expand Down
35 changes: 29 additions & 6 deletions libdocker/fake_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ limitations under the License.
package libdocker

import (
"crypto/sha256"
"crypto/sha512"
"encoding/json"
"fmt"
"hash/fnv"
Expand Down Expand Up @@ -696,6 +698,10 @@ func (f *FakeDockerClient) PullImage(
authJson, _ := json.Marshal(auth)
inspect := createImageInspectFromRef(image)
f.ImageInspects[image] = inspect
f.ImageInspects[inspect.ID] = inspect
Comment thread
stlaz marked this conversation as resolved.
for _, repoDigest := range inspect.RepoDigests {
f.ImageInspects[repoDigest] = inspect
}
f.appendPulled(fmt.Sprintf("%s using %s", image, string(authJson)))
f.Images = append(f.Images, *createImageFromImageInspect(*inspect))
f.ImagesPulled = append(f.ImagesPulled, image)
Expand Down Expand Up @@ -842,8 +848,9 @@ func (f *FakeDockerClient) ResizeContainerTTY(id string, height, width uint) err

func createImageInspectFromRef(ref string) *dockertypes.ImageInspect {
return &dockertypes.ImageInspect{
ID: ref,
RepoTags: []string{ref},
ID: FakePullImageIDMapping(ref),
Comment thread
awmirantis marked this conversation as resolved.
RepoTags: []string{ref},
RepoDigests: []string{fakePullImageDigestMapping(ref)},
Comment thread
stlaz marked this conversation as resolved.
// Image size is required to be non-zero for CRI integration.
VirtualSize: fakeImageSize,
Size: fakeImageSize,
Expand All @@ -853,8 +860,9 @@ func createImageInspectFromRef(ref string) *dockertypes.ImageInspect {

func createImageInspectFromImage(image dockerimagetypes.Summary) *dockertypes.ImageInspect {
return &dockertypes.ImageInspect{
ID: image.ID,
RepoTags: image.RepoTags,
ID: image.ID,
RepoTags: image.RepoTags,
RepoDigests: image.RepoDigests,
// Image size is required to be non-zero for CRI integration.
VirtualSize: fakeImageSize,
Size: fakeImageSize,
Expand All @@ -864,8 +872,9 @@ func createImageInspectFromImage(image dockerimagetypes.Summary) *dockertypes.Im

func createImageFromImageInspect(inspect dockertypes.ImageInspect) *dockerimagetypes.Summary {
return &dockerimagetypes.Summary{
ID: inspect.ID,
RepoTags: inspect.RepoTags,
ID: inspect.ID,
RepoTags: inspect.RepoTags,
RepoDigests: inspect.RepoDigests,
// Image size is required to be non-zero for CRI integration.
VirtualSize: fakeImageSize,
Size: fakeImageSize,
Expand Down Expand Up @@ -927,3 +936,17 @@ func (f *FakeDockerClient) GetContainerStats(id string) (*dockercontainer.StatsR
}
return stats, nil
}

// FakePullImageIDMapping is used by the fake docker client to map an image ref
// to image ID during PullImage operation
func FakePullImageIDMapping(imageRef string) string {
return fmt.Sprintf("sha256:%x", sha256.Sum256([]byte(imageRef)))
}

func fakePullImageDigestMapping(imageName string) string {
noDigestImageName, _, found := strings.Cut(imageName, "@")
if found {
return imageName
}
return fmt.Sprintf("%s@sha512:%x", noDigestImageName, sha512.Sum512([]byte(imageName)))
}
Loading