diff --git a/daemon/containerd/image_builder.go b/daemon/containerd/image_builder.go index c5c331cb04..7f4c61598e 100644 --- a/daemon/containerd/image_builder.go +++ b/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 { diff --git a/daemon/containerd/image_import.go b/daemon/containerd/image_import.go index 8d2baeae99..37960799b4 100644 --- a/daemon/containerd/image_import.go +++ b/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) diff --git a/daemon/containerd/image_tag.go b/daemon/containerd/image_tag.go index 5558d89dcd..b3d3af84b7 100644 --- a/daemon/containerd/image_tag.go +++ b/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") + } } }