diff --git a/pkg/kubelet/dockershim/docker_image.go b/pkg/kubelet/dockershim/docker_image.go index e4c450bc8b0..7ba73a803da 100644 --- a/pkg/kubelet/dockershim/docker_image.go +++ b/pkg/kubelet/dockershim/docker_image.go @@ -120,24 +120,27 @@ func (ds *dockerService) RemoveImage(_ context.Context, r *runtimeapi.RemoveImag // TODO: We assume image.Image is image ID here, which is true in the current implementation // of kubelet, but we should still clarify this in CRI. imageInspect, err := ds.client.InspectImageByID(image.Image) - if err == nil && imageInspect != nil && len(imageInspect.RepoTags) > 1 { - for _, tag := range imageInspect.RepoTags { - if _, err := ds.client.RemoveImage(tag, dockertypes.ImageRemoveOptions{PruneChildren: true}); err != nil && !libdocker.IsImageNotFoundError(err) { - return nil, err - } - } - return &runtimeapi.RemoveImageResponse{}, nil - } + // dockerclient.InspectImageByID doesn't work with digest and repoTags, // it is safe to continue removing it since there is another check below. if err != nil && !libdocker.IsImageNotFoundError(err) { return nil, err } - _, err = ds.client.RemoveImage(image.Image, dockertypes.ImageRemoveOptions{PruneChildren: true}) - if err != nil && !libdocker.IsImageNotFoundError(err) { - return nil, err + // An image can have different numbers of RepoTags and RepoDigests. + // Iterating over both of them plus the image ID ensures the image really got removed. + // It also prevents images from being deleted, which actually are deletable using this approach. + var images []string + images = append(images, imageInspect.RepoTags...) + images = append(images, imageInspect.RepoDigests...) + images = append(images, image.Image) + + for _, image := range images { + if _, err := ds.client.RemoveImage(image, dockertypes.ImageRemoveOptions{PruneChildren: true}); err != nil && !libdocker.IsImageNotFoundError(err) { + return nil, err + } } + return &runtimeapi.RemoveImageResponse{}, nil } diff --git a/pkg/kubelet/dockershim/docker_image_test.go b/pkg/kubelet/dockershim/docker_image_test.go index 61c803c2fde..3dbd5ef2b3a 100644 --- a/pkg/kubelet/dockershim/docker_image_test.go +++ b/pkg/kubelet/dockershim/docker_image_test.go @@ -38,15 +38,36 @@ func TestRemoveImage(t *testing.T) { dockertypes.ImageInspect{ID: "1111", RepoTags: []string{"foo"}}, []libdocker.CalledDetail{ libdocker.NewCalledDetail("inspect_image", nil), + libdocker.NewCalledDetail("remove_image", []interface{}{"foo", dockertypes.ImageRemoveOptions{PruneChildren: true}}), libdocker.NewCalledDetail("remove_image", []interface{}{"1111", dockertypes.ImageRemoveOptions{PruneChildren: true}}), }, }, "multiple tags": { - dockertypes.ImageInspect{ID: "1111", RepoTags: []string{"foo", "bar"}}, + dockertypes.ImageInspect{ID: "2222", RepoTags: []string{"foo", "bar"}}, []libdocker.CalledDetail{ libdocker.NewCalledDetail("inspect_image", nil), libdocker.NewCalledDetail("remove_image", []interface{}{"foo", dockertypes.ImageRemoveOptions{PruneChildren: true}}), libdocker.NewCalledDetail("remove_image", []interface{}{"bar", dockertypes.ImageRemoveOptions{PruneChildren: true}}), + libdocker.NewCalledDetail("remove_image", []interface{}{"2222", dockertypes.ImageRemoveOptions{PruneChildren: true}}), + }, + }, + "single tag multiple repo digests": { + dockertypes.ImageInspect{ID: "3333", RepoTags: []string{"foo"}, RepoDigests: []string{"foo@3333", "example.com/foo@3333"}}, + []libdocker.CalledDetail{ + libdocker.NewCalledDetail("inspect_image", nil), + libdocker.NewCalledDetail("remove_image", []interface{}{"foo", dockertypes.ImageRemoveOptions{PruneChildren: true}}), + libdocker.NewCalledDetail("remove_image", []interface{}{"foo@3333", dockertypes.ImageRemoveOptions{PruneChildren: true}}), + libdocker.NewCalledDetail("remove_image", []interface{}{"example.com/foo@3333", dockertypes.ImageRemoveOptions{PruneChildren: true}}), + libdocker.NewCalledDetail("remove_image", []interface{}{"3333", dockertypes.ImageRemoveOptions{PruneChildren: true}}), + }, + }, + "no tags multiple repo digests": { + dockertypes.ImageInspect{ID: "4444", RepoTags: []string{}, RepoDigests: []string{"foo@4444", "example.com/foo@4444"}}, + []libdocker.CalledDetail{ + libdocker.NewCalledDetail("inspect_image", nil), + libdocker.NewCalledDetail("remove_image", []interface{}{"foo@4444", dockertypes.ImageRemoveOptions{PruneChildren: true}}), + libdocker.NewCalledDetail("remove_image", []interface{}{"example.com/foo@4444", dockertypes.ImageRemoveOptions{PruneChildren: true}}), + libdocker.NewCalledDetail("remove_image", []interface{}{"4444", dockertypes.ImageRemoveOptions{PruneChildren: true}}), }, }, }