Merge pull request moby#48316 from vvoland/c8d-create-or-replace-img

daemon/containerd: Extract `createOrReplaceImage`

daemon/containerd/image_builder.go

@@ -519,18 +519,11 @@ func (i *ImageService) createImageOCI(ctx context.Context, imgToCreate imagespec
 		img.Labels[imageLabelClassicBuilderFromScratch] = "1"
 	}
 
-	createdImage, err := i.images.Update(ctx, img)
-	if err != nil {
-		if !cerrdefs.IsNotFound(err) {
-			return "", err
-		}
-
-		if createdImage, err = i.images.Create(ctx, img); err != nil {
-			return "", fmt.Errorf("failed to create new image: %w", err)
-		}
+	if err := i.createOrReplaceImage(ctx, img); err != nil {
+		return "", err
 	}
 
-	id := image.ID(createdImage.Target.Digest)
+	id := image.ID(img.Target.Digest)
 	i.LogImageEvent(id.String(), id.String(), events.ActionCreate)
 
 	if err := i.unpackImage(ctx, i.StorageDriver(), img, manifestDesc); err != nil {

daemon/containerd/image_import.go

@@ -145,8 +145,7 @@ func (i *ImageService) ImportImage(ctx context.Context, ref reference.Named, pla
 		img.Name = danglingImageName(manifestDesc.Digest)
 	}
 
-	err = i.saveImage(ctx, img)
-	if err != nil {
+	if err = i.createOrReplaceImage(ctx, img); err != nil {
 		logger.WithError(err).Debug("failed to save image")
 		return "", err
 	}
@@ -296,21 +295,6 @@ func writeBlobAndReturnDigest(ctx context.Context, cs content.Store, mt string,
 	return digester.Digest(), nil
 }
 
-// saveImage creates an image in the ImageService or updates it if it exists.
-func (i *ImageService) saveImage(ctx context.Context, img images.Image) error {
-	if _, err := i.images.Update(ctx, img); err != nil {
-		if cerrdefs.IsNotFound(err) {
-			if _, err := i.images.Create(ctx, img); err != nil {
-				return errdefs.Unknown(err)
-			}
-		} else {
-			return errdefs.Unknown(err)
-		}
-	}
-
-	return nil
-}
-
 // unpackImage unpacks the platform-specific manifest of a image into the snapshotter.
 func (i *ImageService) unpackImage(ctx context.Context, snapshotter string, img images.Image, manifestDesc ocispec.Descriptor) error {
 	c8dImg, err := i.NewImageManifest(ctx, img, manifestDesc)

daemon/containerd/image_tag.go

@@ -27,8 +27,22 @@ func (i *ImageService) TagImage(ctx context.Context, imageID image.ID, newTag re
 		Labels: targetImage.Labels,
 	}
 
-	_, err = i.images.Create(ctx, newImg)
-	if err != nil {
+	return i.createOrReplaceImage(ctx, newImg)
+}
+
+// createOrReplaceImage creates a new image with the given name and target descriptor.
+// If an image with the same name already exists, it will be replaced.
+// Overwritten image will be persisted as a dangling image if it's a last
+// reference to that image.
+func (i *ImageService) createOrReplaceImage(ctx context.Context, newImg containerdimages.Image) error {
+	// Delete the source dangling image, as it's no longer dangling.
+	// Unless, the image to be created itself is dangling.
+	danglingName := danglingImageName(newImg.Target.Digest)
+
+	// The created image is a dangling image.
+	creatingDangling := newImg.Name == danglingName
+
+	if _, err := i.images.Create(ctx, newImg); err != nil {
 		if !cerrdefs.IsAlreadyExists(err) {
 			return errdefs.System(errors.Wrapf(err, "failed to create image with name %s and target %s", newImg.Name, newImg.Target.Digest.String()))
 		}
@@ -42,8 +56,10 @@ func (i *ImageService) TagImage(ctx context.Context, imageID image.ID, newTag re
 
 		// Check if image we would replace already resolves to the same target.
 		// No need to do anything.
-		if replacedImg.Target.Digest == targetImage.Target.Digest {
-			i.LogImageEvent(imageID.String(), reference.FamiliarString(newTag), events.ActionTag)
+		if replacedImg.Target.Digest == newImg.Target.Digest {
+			if !creatingDangling {
+				i.LogImageEvent(replacedImg.Target.Digest.String(), imageFamiliarName(newImg), events.ActionTag)
+			}
 			return nil
 		}
 
@@ -52,24 +68,26 @@ func (i *ImageService) TagImage(ctx context.Context, imageID image.ID, newTag re
 			return errors.Wrapf(err, "failed to delete previous image %s", replacedImg.Name)
 		}
 
-		if _, err = i.images.Create(context.WithoutCancel(ctx), newImg); err != nil {
+		if _, err := i.images.Create(context.WithoutCancel(ctx), newImg); err != nil {
 			return errdefs.System(errors.Wrapf(err, "failed to create an image %s with target %s after deleting the existing one",
-				newImg.Name, imageID.String()))
+				newImg.Name, newImg.Target.Digest))
 		}
 	}
 
 	logger := log.G(ctx).WithFields(log.Fields{
-		"imageID": imageID.String(),
-		"tag":     newTag.String(),
+		"imageID": newImg.Target.Digest,
+		"tag":     newImg.Name,
 	})
 	logger.Info("image created")
 
-	defer i.LogImageEvent(imageID.String(), reference.FamiliarString(newTag), events.ActionTag)
+	if !creatingDangling {
+		ctx := context.WithoutCancel(ctx)
+		defer i.LogImageEvent(string(newImg.Target.Digest), imageFamiliarName(newImg), events.ActionTag)
 
-	// Delete the source dangling image, as it's no longer dangling.
-	if err := i.images.Delete(context.WithoutCancel(ctx), danglingImageName(targetImage.Target.Digest)); err != nil {
-		if !cerrdefs.IsNotFound(err) {
-			logger.WithError(err).Warn("unexpected error when deleting dangling image")
+		if err := i.images.Delete(ctx, danglingName); err != nil {
+			if !cerrdefs.IsNotFound(err) {
+				logger.WithError(err).Warn("unexpected error when deleting dangling image")
+			}
 		}
 	}