reliably remove image when content missing

Signed-off-by: yason <yan.xuean@zte.com.cn>

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)?

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.