diff --git a/pkg/server/image_pull.go b/pkg/server/image_pull.go index 3269dc0d3fbd..43ee706a4a09 100644 --- a/pkg/server/image_pull.go +++ b/pkg/server/image_pull.go @@ -59,6 +59,9 @@ import ( // if we saw an image without snapshots or with in-complete contents during startup, // should we re-pull the image? Or should we remove the entry? // +// yanxuean: We cann't delete image directly, because we don't know if the image +// is pulled by us. There are resource leakage. +// // 2) Containerd suggests user to add entry before pulling the image. However if // an error occurrs during the pulling, should we remove the entry from metadata // store? Or should we leave it there until next startup (resource leakage)? diff --git a/pkg/server/image_remove.go b/pkg/server/image_remove.go index bdf8f7086e1a..c28606a70ce3 100644 --- a/pkg/server/image_remove.go +++ b/pkg/server/image_remove.go @@ -42,7 +42,7 @@ func (c *criContainerdService) RemoveImage(ctx context.Context, r *runtime.Remov return &runtime.RemoveImageResponse{}, nil } - // Exclude out dated image tag. + // Exclude outdated image tag. for i, tag := range image.RepoTags { cImage, err := c.client.GetImage(ctx, tag) if err != nil { @@ -53,14 +53,27 @@ func (c *criContainerdService) RemoveImage(ctx context.Context, r *runtime.Remov } desc, err := cImage.Config(ctx) if err != nil { - return nil, fmt.Errorf("failed to get image %q config descriptor: %v", tag, err) + // We can only get image id by reading Config from content. + // If the config is missing, we will fail to get image id, + // So we will can't remove the image forever, + // and cri-containerd always report the image is ok. + // But we also don't check it by manifest, + // It's possible that two manifest digests have the same image ID in theory. + // In theory it's possible that an image is compressed with different algorithms, + // then they'll have the same uncompressed id - image id, + // but different ids generated from compressed contents - manifest digest. + // So we decide to leave it. + // After all, the user can override the repoTag by pulling image again. + glog.Errorf("can't remove image,failed to get config for Image tag %q,id %q: %v", tag, image.ID, err) + image.RepoTags = append(image.RepoTags[:i], image.RepoTags[i+1:]...) + continue } cID := desc.Digest.String() - if cID == image.ID { + if cID != image.ID { + glog.V(4).Infof("Image tag %q for %q is outdated, it's currently used by %q", tag, image.ID, cID) + image.RepoTags = append(image.RepoTags[:i], image.RepoTags[i+1:]...) continue } - glog.V(4).Infof("Image tag %q for %q is out dated, it's currently used by %q", tag, image.ID, cID) - image.RepoTags = append(image.RepoTags[:i], image.RepoTags[i+1:]...) } // Include all image references, including RepoTag, RepoDigest and id.