Browse Source

Merge pull request #44781 from vvoland/c8d-tag-upstream

daemon/c8d: Implement `docker tag`
Bjorn Neergaard 2 years ago
parent
commit
5f369ff1d4

+ 2 - 2
api/server/backend/build/backend.go

@@ -21,7 +21,7 @@ import (
 // ImageComponent provides an interface for working with images
 // ImageComponent provides an interface for working with images
 type ImageComponent interface {
 type ImageComponent interface {
 	SquashImage(from string, to string) (string, error)
 	SquashImage(from string, to string) (string, error)
-	TagImageWithReference(image.ID, reference.Named) error
+	TagImage(context.Context, image.ID, reference.Named) error
 }
 }
 
 
 // Builder defines interface for running a build
 // Builder defines interface for running a build
@@ -93,7 +93,7 @@ func (b *Backend) Build(ctx context.Context, config backend.BuildConfig) (string
 		fmt.Fprintf(stdout, "Successfully built %s\n", stringid.TruncateID(imageID))
 		fmt.Fprintf(stdout, "Successfully built %s\n", stringid.TruncateID(imageID))
 	}
 	}
 	if imageID != "" {
 	if imageID != "" {
-		err = tagImages(b.imageComponent, config.ProgressWriter.StdoutFormatter, image.ID(imageID), tags)
+		err = tagImages(ctx, b.imageComponent, config.ProgressWriter.StdoutFormatter, image.ID(imageID), tags)
 	}
 	}
 	return imageID, err
 	return imageID, err
 }
 }

+ 3 - 2
api/server/backend/build/tag.go

@@ -1,6 +1,7 @@
 package build // import "github.com/docker/docker/api/server/backend/build"
 package build // import "github.com/docker/docker/api/server/backend/build"
 
 
 import (
 import (
+	"context"
 	"fmt"
 	"fmt"
 	"io"
 	"io"
 
 
@@ -10,9 +11,9 @@ import (
 )
 )
 
 
 // tagImages creates image tags for the imageID.
 // tagImages creates image tags for the imageID.
-func tagImages(ic ImageComponent, stdout io.Writer, imageID image.ID, repoAndTags []reference.Named) error {
+func tagImages(ctx context.Context, ic ImageComponent, stdout io.Writer, imageID image.ID, repoAndTags []reference.Named) error {
 	for _, rt := range repoAndTags {
 	for _, rt := range repoAndTags {
-		if err := ic.TagImageWithReference(imageID, rt); err != nil {
+		if err := ic.TagImage(ctx, imageID, rt); err != nil {
 			return err
 			return err
 		}
 		}
 		_, _ = fmt.Fprintln(stdout, "Successfully tagged", reference.FamiliarString(rt))
 		_, _ = fmt.Fprintln(stdout, "Successfully tagged", reference.FamiliarString(rt))

+ 35 - 0
api/server/httputils/form.go

@@ -1,9 +1,12 @@
 package httputils // import "github.com/docker/docker/api/server/httputils"
 package httputils // import "github.com/docker/docker/api/server/httputils"
 
 
 import (
 import (
+	"fmt"
 	"net/http"
 	"net/http"
 	"strconv"
 	"strconv"
 	"strings"
 	"strings"
+
+	"github.com/docker/distribution/reference"
 )
 )
 
 
 // BoolValue transforms a form value in different formats into a boolean type.
 // BoolValue transforms a form value in different formats into a boolean type.
@@ -41,6 +44,38 @@ func Int64ValueOrDefault(r *http.Request, field string, def int64) (int64, error
 	return def, nil
 	return def, nil
 }
 }
 
 
+// RepoTagReference parses form values "repo" and "tag" and returns a valid
+// reference with repository and tag.
+// If repo is empty, then a nil reference is returned.
+// If no tag is given, then the default "latest" tag is set.
+func RepoTagReference(repo, tag string) (reference.NamedTagged, error) {
+	if repo == "" {
+		return nil, nil
+	}
+
+	ref, err := reference.ParseNormalizedNamed(repo)
+	if err != nil {
+		return nil, err
+	}
+
+	if _, isDigested := ref.(reference.Digested); isDigested {
+		return nil, fmt.Errorf("cannot import digest reference")
+	}
+
+	if tag != "" {
+		return reference.WithTag(ref, tag)
+	}
+
+	withDefaultTag := reference.TagNameOnly(ref)
+
+	namedTagged, ok := withDefaultTag.(reference.NamedTagged)
+	if !ok {
+		return nil, fmt.Errorf("unexpected reference: %q", ref.String())
+	}
+
+	return namedTagged, nil
+}
+
 // ArchiveOptions stores archive information for different operations.
 // ArchiveOptions stores archive information for different operations.
 type ArchiveOptions struct {
 type ArchiveOptions struct {
 	Name string
 	Name string

+ 6 - 2
api/server/router/container/container_routes.go

@@ -48,10 +48,14 @@ func (s *containerRouter) postCommit(ctx context.Context, w http.ResponseWriter,
 		return err
 		return err
 	}
 	}
 
 
+	ref, err := httputils.RepoTagReference(r.Form.Get("repo"), r.Form.Get("tag"))
+	if err != nil {
+		return errdefs.InvalidParameter(err)
+	}
+
 	commitCfg := &backend.CreateImageConfig{
 	commitCfg := &backend.CreateImageConfig{
 		Pause:   pause,
 		Pause:   pause,
-		Repo:    r.Form.Get("repo"),
-		Tag:     r.Form.Get("tag"),
+		Tag:     ref,
 		Author:  r.Form.Get("author"),
 		Author:  r.Form.Get("author"),
 		Comment: r.Form.Get("comment"),
 		Comment: r.Form.Get("comment"),
 		Config:  config,
 		Config:  config,

+ 1 - 1
api/server/router/image/backend.go

@@ -26,7 +26,7 @@ type imageBackend interface {
 	ImageHistory(ctx context.Context, imageName string) ([]*image.HistoryResponseItem, error)
 	ImageHistory(ctx context.Context, imageName string) ([]*image.HistoryResponseItem, error)
 	Images(ctx context.Context, opts types.ImageListOptions) ([]*types.ImageSummary, error)
 	Images(ctx context.Context, opts types.ImageListOptions) ([]*types.ImageSummary, error)
 	GetImage(ctx context.Context, refOrID string, options image.GetImageOpts) (*dockerimage.Image, error)
 	GetImage(ctx context.Context, refOrID string, options image.GetImageOpts) (*dockerimage.Image, error)
-	TagImage(imageName, repository, tag string) (string, error)
+	TagImage(ctx context.Context, id dockerimage.ID, newRef reference.Named) error
 	ImagesPrune(ctx context.Context, pruneFilters filters.Args) (*types.ImagesPruneReport, error)
 	ImagesPrune(ctx context.Context, pruneFilters filters.Args) (*types.ImagesPruneReport, error)
 }
 }
 
 

+ 16 - 21
api/server/router/image/image_routes.go

@@ -72,25 +72,9 @@ func (ir *imageRouter) postImagesCreate(ctx context.Context, w http.ResponseWrit
 	} else { // import
 	} else { // import
 		src := r.Form.Get("fromSrc")
 		src := r.Form.Get("fromSrc")
 
 
-		var ref reference.Named
-		if repo != "" {
-			var err error
-			ref, err = reference.ParseNormalizedNamed(repo)
-			if err != nil {
-				return errdefs.InvalidParameter(err)
-			}
-			if _, isDigested := ref.(reference.Digested); isDigested {
-				return errdefs.InvalidParameter(errors.New("cannot import digest reference"))
-			}
-
-			if tag != "" {
-				ref, err = reference.WithTag(ref, tag)
-				if err != nil {
-					return errdefs.InvalidParameter(err)
-				}
-			} else {
-				ref = reference.TagNameOnly(ref)
-			}
+		tagRef, err := httputils.RepoTagReference(repo, tag)
+		if err != nil {
+			return errdefs.InvalidParameter(err)
 		}
 		}
 
 
 		if len(comment) == 0 {
 		if len(comment) == 0 {
@@ -121,7 +105,7 @@ func (ir *imageRouter) postImagesCreate(ctx context.Context, w http.ResponseWrit
 		}
 		}
 
 
 		var id image.ID
 		var id image.ID
-		id, progressErr = ir.backend.ImportImage(ctx, ref, platform, comment, layerReader, r.Form["changes"])
+		id, progressErr = ir.backend.ImportImage(ctx, tagRef, platform, comment, layerReader, r.Form["changes"])
 
 
 		if progressErr == nil {
 		if progressErr == nil {
 			output.Write(streamformatter.FormatStatus("", id.String()))
 			output.Write(streamformatter.FormatStatus("", id.String()))
@@ -369,7 +353,18 @@ func (ir *imageRouter) postImagesTag(ctx context.Context, w http.ResponseWriter,
 	if err := httputils.ParseForm(r); err != nil {
 	if err := httputils.ParseForm(r); err != nil {
 		return err
 		return err
 	}
 	}
-	if _, err := ir.backend.TagImage(vars["name"], r.Form.Get("repo"), r.Form.Get("tag")); err != nil {
+
+	ref, err := httputils.RepoTagReference(r.Form.Get("repo"), r.Form.Get("tag"))
+	if ref == nil || err != nil {
+		return errdefs.InvalidParameter(err)
+	}
+
+	img, err := ir.backend.GetImage(ctx, vars["name"], opts.GetImageOpts{})
+	if err != nil {
+		return errdefs.NotFound(err)
+	}
+
+	if err := ir.backend.TagImage(ctx, img.ID(), ref); err != nil {
 		return err
 		return err
 	}
 	}
 	w.WriteHeader(http.StatusCreated)
 	w.WriteHeader(http.StatusCreated)

+ 2 - 2
api/types/backend/backend.go

@@ -5,6 +5,7 @@ import (
 	"io"
 	"io"
 	"time"
 	"time"
 
 
+	"github.com/docker/distribution/reference"
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/container"
 )
 )
 
 
@@ -102,8 +103,7 @@ type ExecProcessConfig struct {
 // CreateImageConfig is the configuration for creating an image from a
 // CreateImageConfig is the configuration for creating an image from a
 // container.
 // container.
 type CreateImageConfig struct {
 type CreateImageConfig struct {
-	Repo    string
-	Tag     string
+	Tag     reference.NamedTagged
 	Pause   bool
 	Pause   bool
 	Author  string
 	Author  string
 	Comment string
 	Comment string

+ 6 - 3
daemon/commit.go

@@ -7,6 +7,7 @@ import (
 	"strings"
 	"strings"
 	"time"
 	"time"
 
 
+	"github.com/docker/distribution/reference"
 	"github.com/docker/docker/api/types/backend"
 	"github.com/docker/docker/api/types/backend"
 	containertypes "github.com/docker/docker/api/types/container"
 	containertypes "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/builder/dockerfile"
 	"github.com/docker/docker/builder/dockerfile"
@@ -119,6 +120,7 @@ func merge(userConf, imageConf *containertypes.Config) error {
 // applying that config over the existing container config.
 // applying that config over the existing container config.
 func (daemon *Daemon) CreateImageFromContainer(ctx context.Context, name string, c *backend.CreateImageConfig) (string, error) {
 func (daemon *Daemon) CreateImageFromContainer(ctx context.Context, name string, c *backend.CreateImageConfig) (string, error) {
 	start := time.Now()
 	start := time.Now()
+
 	container, err := daemon.GetContainer(name)
 	container, err := daemon.GetContainer(name)
 	if err != nil {
 	if err != nil {
 		return "", err
 		return "", err
@@ -169,12 +171,13 @@ func (daemon *Daemon) CreateImageFromContainer(ctx context.Context, name string,
 		return "", err
 		return "", err
 	}
 	}
 
 
-	var imageRef string
-	if c.Repo != "" {
-		imageRef, err = daemon.imageService.TagImage(string(id), c.Repo, c.Tag)
+	imageRef := ""
+	if c.Tag != nil {
+		err = daemon.imageService.TagImage(ctx, id, c.Tag)
 		if err != nil {
 		if err != nil {
 			return "", err
 			return "", err
 		}
 		}
+		imageRef = reference.FamiliarString(c.Tag)
 	}
 	}
 	daemon.LogContainerEventWithAttributes(container, "commit", map[string]string{
 	daemon.LogContainerEventWithAttributes(container, "commit", map[string]string{
 		"comment":  c.Comment,
 		"comment":  c.Comment,

+ 1 - 3
daemon/containerd/image_import.go

@@ -135,9 +135,7 @@ func (i *ImageService) ImportImage(ctx context.Context, ref reference.Named, pla
 		CreatedAt: createdAt,
 		CreatedAt: createdAt,
 	}
 	}
 	if img.Name == "" {
 	if img.Name == "" {
-		// TODO(vvoland): danglingImageName(manifestDesc.Digest)
-		img.Name = "dangling@" + manifestDesc.Digest.String()
-
+		img.Name = danglingImageName(manifestDesc.Digest)
 	}
 	}
 
 
 	err = i.saveImage(ctx, img)
 	err = i.saveImage(ctx, img)

+ 66 - 9
daemon/containerd/image_tag.go

@@ -1,20 +1,77 @@
 package containerd
 package containerd
 
 
 import (
 import (
-	"errors"
+	"context"
 
 
+	cerrdefs "github.com/containerd/containerd/errdefs"
+	containerdimages "github.com/containerd/containerd/images"
 	"github.com/docker/distribution/reference"
 	"github.com/docker/distribution/reference"
 	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/image"
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 )
 
 
-// TagImage creates the tag specified by newTag, pointing to the image named
-// imageName (alternatively, imageName can also be an image ID).
-func (i *ImageService) TagImage(imageName, repository, tag string) (string, error) {
-	return "", errdefs.NotImplemented(errors.New("not implemented"))
-}
+// TagImage creates an image named as newTag and targeting the given descriptor id.
+func (i *ImageService) TagImage(ctx context.Context, imageID image.ID, newTag reference.Named) error {
+	target, err := i.resolveDescriptor(ctx, imageID.String())
+	if err != nil {
+		return errors.Wrapf(err, "failed to resolve image id %q to a descriptor", imageID.String())
+	}
+
+	newImg := containerdimages.Image{
+		Name:   newTag.String(),
+		Target: target,
+	}
+
+	is := i.client.ImageService()
+	_, createErr := is.Create(ctx, newImg)
+	if createErr != 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()))
+		}
+
+		replacedImg, err := is.Get(ctx, newImg.Name)
+		if err != nil {
+			return errdefs.Unknown(errors.Wrapf(err, "creating image %s failed because it already exists, but accessing it also failed", newImg.Name))
+		}
+
+		// Check if image we would replace already resolves to the same target.
+		// No need to do anything.
+		if replacedImg.Target.Digest == target.Digest {
+			return nil
+		}
+
+		// If there already exists an image with this tag, delete it
+		if err := i.softImageDelete(ctx, replacedImg); err != nil {
+			return errors.Wrapf(err, "failed to delete previous image %s", replacedImg.Name)
+		}
+
+		if _, err = is.Create(context.Background(), 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()))
+		}
+	}
+
+	logger := logrus.WithFields(logrus.Fields{
+		"imageID": imageID.String(),
+		"tag":     newTag.String(),
+	})
+	logger.Info("image created")
+
+	// The tag succeeded, check if the source image is dangling
+	sourceDanglingImg, err := is.Get(context.Background(), danglingImageName(target.Digest))
+	if err != nil {
+		if !cerrdefs.IsNotFound(err) {
+			logger.WithError(err).Warn("unexpected error when checking if source image is dangling")
+		}
+		return nil
+	}
+
+	// Delete the source dangling image, as it's no longer dangling.
+	if err := is.Delete(context.Background(), sourceDanglingImg.Name); err != nil {
+		logger.WithError(err).Warn("unexpected error when deleting dangling image")
+	}
 
 
-// TagImageWithReference adds the given reference to the image ID provided.
-func (i *ImageService) TagImageWithReference(imageID image.ID, newTag reference.Named) error {
-	return errdefs.NotImplemented(errors.New("not implemented"))
+	return nil
 }
 }

+ 61 - 0
daemon/containerd/soft_delete.go

@@ -0,0 +1,61 @@
+package containerd
+
+import (
+	"context"
+
+	cerrdefs "github.com/containerd/containerd/errdefs"
+	containerdimages "github.com/containerd/containerd/images"
+	"github.com/docker/docker/errdefs"
+	"github.com/opencontainers/go-digest"
+	"github.com/pkg/errors"
+)
+
+// softImageDelete deletes the image, making sure that there are other images
+// that reference the content of the deleted image.
+// If no other image exists, a dangling one is created.
+func (i *ImageService) softImageDelete(ctx context.Context, img containerdimages.Image) error {
+	is := i.client.ImageService()
+
+	// If the image already exists, persist it as dangling image
+	// but only if no other image has the same target.
+	digest := img.Target.Digest.String()
+	imgs, err := is.List(ctx, "target.digest=="+digest)
+	if err != nil {
+		return errdefs.System(errors.Wrapf(err, "failed to check if there are images targeting digest %s", digest))
+	}
+
+	// From this point explicitly ignore the passed context
+	// and don't allow to interrupt operation in the middle.
+
+	// Create dangling image if this is the last image pointing to this target.
+	if len(imgs) == 1 {
+		danglingImage := img
+
+		danglingImage.Name = danglingImageName(img.Target.Digest)
+		delete(danglingImage.Labels, "io.containerd.image.name")
+		delete(danglingImage.Labels, "org.opencontainers.image.ref.name")
+
+		_, err = is.Create(context.Background(), danglingImage)
+
+		// Error out in case we couldn't persist the old image.
+		// If it already exists, then just continue.
+		if err != nil && !cerrdefs.IsAlreadyExists(err) {
+			return errdefs.System(errors.Wrapf(err, "failed to create a dangling image for the replaced image %s with digest %s",
+				danglingImage.Name, danglingImage.Target.Digest.String()))
+		}
+	}
+
+	// Free the target name.
+	err = is.Delete(context.Background(), img.Name)
+	if err != nil {
+		if !cerrdefs.IsNotFound(err) {
+			return errdefs.System(errors.Wrapf(err, "failed to delete image %s which existed a moment before", img.Name))
+		}
+	}
+
+	return nil
+}
+
+func danglingImageName(digest digest.Digest) string {
+	return "moby-dangling@" + digest.String()
+}

+ 1 - 2
daemon/image_service.go

@@ -37,8 +37,7 @@ type ImageService interface {
 	CountImages() int
 	CountImages() int
 	ImagesPrune(ctx context.Context, pruneFilters filters.Args) (*types.ImagesPruneReport, error)
 	ImagesPrune(ctx context.Context, pruneFilters filters.Args) (*types.ImagesPruneReport, error)
 	ImportImage(ctx context.Context, ref reference.Named, platform *v1.Platform, msg string, layerReader io.Reader, changes []string) (image.ID, error)
 	ImportImage(ctx context.Context, ref reference.Named, platform *v1.Platform, msg string, layerReader io.Reader, changes []string) (image.ID, error)
-	TagImage(imageName, repository, tag string) (string, error)
-	TagImageWithReference(imageID image.ID, newTag reference.Named) error
+	TagImage(ctx context.Context, imageID image.ID, newTag reference.Named) error
 	GetImage(ctx context.Context, refOrID string, options imagetype.GetImageOpts) (*image.Image, error)
 	GetImage(ctx context.Context, refOrID string, options imagetype.GetImageOpts) (*image.Image, error)
 	ImageHistory(ctx context.Context, name string) ([]*imagetype.HistoryResponseItem, error)
 	ImageHistory(ctx context.Context, name string) ([]*imagetype.HistoryResponseItem, error)
 	CommitImage(ctx context.Context, c backend.CommitConfig) (image.ID, error)
 	CommitImage(ctx context.Context, c backend.CommitConfig) (image.ID, error)

+ 1 - 1
daemon/images/image_import.go

@@ -80,7 +80,7 @@ func (i *ImageService) ImportImage(ctx context.Context, newRef reference.Named,
 	}
 	}
 
 
 	if newRef != nil {
 	if newRef != nil {
-		if err := i.TagImageWithReference(id, newRef); err != nil {
+		if err := i.TagImage(ctx, id, newRef); err != nil {
 			return "", err
 			return "", err
 		}
 		}
 	}
 	}

+ 2 - 26
daemon/images/image_tag.go

@@ -4,35 +4,11 @@ import (
 	"context"
 	"context"
 
 
 	"github.com/docker/distribution/reference"
 	"github.com/docker/distribution/reference"
-	imagetypes "github.com/docker/docker/api/types/image"
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/image"
 )
 )
 
 
-// TagImage creates the tag specified by newTag, pointing to the image named
-// imageName (alternatively, imageName can also be an image ID).
-func (i *ImageService) TagImage(imageName, repository, tag string) (string, error) {
-	ctx := context.TODO()
-	img, err := i.GetImage(ctx, imageName, imagetypes.GetImageOpts{})
-	if err != nil {
-		return "", err
-	}
-
-	newTag, err := reference.ParseNormalizedNamed(repository)
-	if err != nil {
-		return "", err
-	}
-	if tag != "" {
-		if newTag, err = reference.WithTag(reference.TrimNamed(newTag), tag); err != nil {
-			return "", err
-		}
-	}
-
-	err = i.TagImageWithReference(img.ID(), newTag)
-	return reference.FamiliarString(newTag), err
-}
-
-// TagImageWithReference adds the given reference to the image ID provided.
-func (i *ImageService) TagImageWithReference(imageID image.ID, newTag reference.Named) error {
+// TagImage adds the given reference to the image ID provided.
+func (i *ImageService) TagImage(ctx context.Context, imageID image.ID, newTag reference.Named) error {
 	if err := i.referenceStore.AddTag(newTag, imageID.Digest(), true); err != nil {
 	if err := i.referenceStore.AddTag(newTag, imageID.Digest(), true); err != nil {
 		return err
 		return err
 	}
 	}