Merge pull request moby#48078 from vvoland/c8d-buildkit-tag-event

c8d/build: Log `image tag` event when image was built with Buildkit

api/server/backend/build/backend.go

@@ -88,11 +88,9 @@ func (b *Backend) Build(ctx context.Context, config backend.BuildConfig) (string
 		}
 	}
 
-	if !useBuildKit {
-		stdout := config.ProgressWriter.StdoutFormatter
-		fmt.Fprintf(stdout, "Successfully built %s\n", stringid.TruncateID(imageID))
-	}
 	if imageID != "" && !useBuildKit {
+		stdout := config.ProgressWriter.StdoutFormatter
+		_, _ = fmt.Fprintf(stdout, "Successfully built %s\n", stringid.TruncateID(imageID))
 		err = tagImages(ctx, b.imageComponent, config.ProgressWriter.StdoutFormatter, image.ID(imageID), tags)
 	}
 	return imageID, err

builder/builder-next/builder.go

@@ -77,24 +77,24 @@ var cacheFields = map[string]bool{
 
 // Opt is option struct required for creating the builder
 type Opt struct {
-	SessionManager        *session.Manager
-	Root                  string
-	EngineID              string
-	Dist                  images.DistributionServices
-	ImageTagger           mobyexporter.ImageTagger
-	NetworkController     *libnetwork.Controller
-	DefaultCgroupParent   string
-	RegistryHosts         docker.RegistryHosts
-	BuilderConfig         config.BuilderConfig
-	Rootless              bool
-	IdentityMapping       idtools.IdentityMapping
-	DNSConfig             config.DNSConfig
-	ApparmorProfile       string
-	UseSnapshotter        bool
-	Snapshotter           string
-	ContainerdAddress     string
-	ContainerdNamespace   string
-	ImageExportedCallback exporter.ImageExportedByBuildkit
+	SessionManager      *session.Manager
+	Root                string
+	EngineID            string
+	Dist                images.DistributionServices
+	ImageTagger         mobyexporter.ImageTagger
+	NetworkController   *libnetwork.Controller
+	DefaultCgroupParent string
+	RegistryHosts       docker.RegistryHosts
+	BuilderConfig       config.BuilderConfig
+	Rootless            bool
+	IdentityMapping     idtools.IdentityMapping
+	DNSConfig           config.DNSConfig
+	ApparmorProfile     string
+	UseSnapshotter      bool
+	Snapshotter         string
+	ContainerdAddress   string
+	ContainerdNamespace string
+	Callbacks           exporter.BuildkitCallbacks
 }
 
 // Builder can build using BuildKit backend

builder/builder-next/controller.go

@@ -149,7 +149,7 @@ func newSnapshotterController(ctx context.Context, rt http.RoundTripper, opt Opt
 	}
 	wo.Executor = exec
 
-	w, err := mobyworker.NewContainerdWorker(ctx, wo, opt.ImageExportedCallback)
+	w, err := mobyworker.NewContainerdWorker(ctx, wo, opt.Callbacks)
 	if err != nil {
 		return nil, err
 	}
@@ -332,7 +332,8 @@ func newGraphDriverController(ctx context.Context, rt http.RoundTripper, opt Opt
 		Differ:                differ,
 		ImageTagger:           opt.ImageTagger,
 		LeaseManager:          lm,
-		ImageExportedCallback: opt.ImageExportedCallback,
+		ImageExportedCallback: opt.Callbacks.Exported,
+		// Callbacks.Named is not used here because the tag operation is handled directly by the image service.
 	})
 	if err != nil {
 		return nil, err

builder/builder-next/exporter/mobyexporter/export.go

@@ -10,7 +10,6 @@ import (
 	"github.com/containerd/containerd/leases"
 	"github.com/containerd/log"
 	distref "github.com/distribution/reference"
-	builderexporter "github.com/docker/docker/builder/builder-next/exporter"
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/layer"
 	"github.com/moby/buildkit/exporter"
@@ -38,7 +37,7 @@ type Opt struct {
 	ImageTagger           ImageTagger
 	ContentStore          content.Store
 	LeaseManager          leases.Manager
-	ImageExportedCallback builderexporter.ImageExportedByBuildkit
+	ImageExportedCallback func(ctx context.Context, id string, desc ocispec.Descriptor)
 }
 
 type imageExporter struct {

builder/builder-next/exporter/wrapper.go

@@ -4,26 +4,38 @@ import (
 	"context"
 	"strings"
 
+	"github.com/containerd/log"
+	"github.com/distribution/reference"
 	"github.com/docker/docker/builder/builder-next/exporter/overrides"
 	"github.com/moby/buildkit/exporter"
 	"github.com/moby/buildkit/exporter/containerimage/exptypes"
 
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
-type ImageExportedByBuildkit = func(ctx context.Context, id string, desc ocispec.Descriptor) error
+type BuildkitCallbacks struct {
+	// Exported is a Called when an image is exported by buildkit.
+	Exported func(ctx context.Context, id string, desc ocispec.Descriptor)
+
+	// Named is a callback that is called when an image is created in the
+	// containerd image store by buildkit.
+	Named func(ctx context.Context, ref reference.NamedTagged, desc ocispec.Descriptor)
+}
 
 // Wraps the containerimage exporter's Resolve method to apply moby-specific
 // overrides to the exporter attributes.
 type imageExporterMobyWrapper struct {
-	exp      exporter.Exporter
-	callback ImageExportedByBuildkit
+	exp       exporter.Exporter
+	callbacks BuildkitCallbacks
 }
 
 // NewWrapper returns an exporter wrapper that applies moby specific attributes
 // and hooks the export process.
-func NewWrapper(exp exporter.Exporter, callback ImageExportedByBuildkit) (exporter.Exporter, error) {
-	return &imageExporterMobyWrapper{exp: exp, callback: callback}, nil
+func NewWrapper(exp exporter.Exporter, callbacks BuildkitCallbacks) (exporter.Exporter, error) {
+	return &imageExporterMobyWrapper{
+		exp:       exp,
+		callbacks: callbacks,
+	}, nil
 }
 
 // Resolve applies moby specific attributes to the request.
@@ -46,12 +58,15 @@ func (e *imageExporterMobyWrapper) Resolve(ctx context.Context, id int, exporter
 		return nil, err
 	}
 
-	return &imageExporterInstanceWrapper{ExporterInstance: inst, callback: e.callback}, nil
+	return &imageExporterInstanceWrapper{
+		ExporterInstance: inst,
+		callbacks:        e.callbacks,
+	}, nil
 }
 
 type imageExporterInstanceWrapper struct {
 	exporter.ExporterInstance
-	callback ImageExportedByBuildkit
+	callbacks BuildkitCallbacks
 }
 
 func (i *imageExporterInstanceWrapper) Export(ctx context.Context, src *exporter.Source, inlineCache exptypes.InlineCache, sessionID string) (map[string]string, exporter.DescriptorReference, error) {
@@ -62,8 +77,26 @@ func (i *imageExporterInstanceWrapper) Export(ctx context.Context, src *exporter
 
 	desc := ref.Descriptor()
 	imageID := out[exptypes.ExporterImageDigestKey]
-	if i.callback != nil {
-		i.callback(ctx, imageID, desc)
+	if i.callbacks.Exported != nil {
+		i.callbacks.Exported(ctx, imageID, desc)
+	}
+
+	if i.callbacks.Named != nil {
+		for _, name := range strings.Split(out[string(exptypes.OptKeyName)], ",") {
+			ref, err := reference.ParseNormalizedNamed(name)
+			if err != nil {
+				// Shouldn't happen, but log if it does and continue.
+				log.G(ctx).WithFields(log.Fields{
+					"name":  name,
+					"error": err,
+				}).Warn("image named with invalid reference produced by buildkit")
+				continue
+			}
+
+			namedTagged := reference.TagNameOnly(ref).(reference.NamedTagged)
+			i.callbacks.Named(ctx, namedTagged, desc)
+		}
 	}
+
 	return out, ref, nil
 }

builder/builder-next/worker/containerdworker.go

@@ -3,37 +3,37 @@ package worker
 import (
 	"context"
 
-	mobyexporter "github.com/docker/docker/builder/builder-next/exporter"
+	"github.com/docker/docker/builder/builder-next/exporter"
 	"github.com/moby/buildkit/client"
-	"github.com/moby/buildkit/exporter"
+	bkexporter "github.com/moby/buildkit/exporter"
 	"github.com/moby/buildkit/session"
 	"github.com/moby/buildkit/worker/base"
 )
 
 // ContainerdWorker is a local worker instance with dedicated snapshotter, cache, and so on.
 type ContainerdWorker struct {
 	*base.Worker
-	callback mobyexporter.ImageExportedByBuildkit
+	callbacks exporter.BuildkitCallbacks
 }
 
 // NewContainerdWorker instantiates a local worker.
-func NewContainerdWorker(ctx context.Context, wo base.WorkerOpt, callback mobyexporter.ImageExportedByBuildkit) (*ContainerdWorker, error) {
+func NewContainerdWorker(ctx context.Context, wo base.WorkerOpt, callbacks exporter.BuildkitCallbacks) (*ContainerdWorker, error) {
 	bw, err := base.NewWorker(ctx, wo)
 	if err != nil {
 		return nil, err
 	}
-	return &ContainerdWorker{Worker: bw, callback: callback}, nil
+	return &ContainerdWorker{Worker: bw, callbacks: callbacks}, nil
 }
 
 // Exporter returns exporter by name
-func (w *ContainerdWorker) Exporter(name string, sm *session.Manager) (exporter.Exporter, error) {
+func (w *ContainerdWorker) Exporter(name string, sm *session.Manager) (bkexporter.Exporter, error) {
 	switch name {
-	case mobyexporter.Moby:
+	case exporter.Moby:
 		exp, err := w.Worker.Exporter(client.ExporterImage, sm)
 		if err != nil {
 			return nil, err
 		}
-		return mobyexporter.NewWrapper(exp, w.callback)
+		return exporter.NewWrapper(exp, w.callbacks)
 	default:
 		return w.Worker.Exporter(name, sm)
 	}

cmd/dockerd/daemon.go

@@ -35,6 +35,7 @@ import (
 	systemrouter "github.com/docker/docker/api/server/router/system"
 	"github.com/docker/docker/api/server/router/volume"
 	buildkit "github.com/docker/docker/builder/builder-next"
+	"github.com/docker/docker/builder/builder-next/exporter"
 	"github.com/docker/docker/builder/dockerfile"
 	"github.com/docker/docker/cmd/dockerd/debug"
 	"github.com/docker/docker/cmd/dockerd/trap"
@@ -430,24 +431,27 @@ func newRouterOptions(ctx context.Context, config *config.Config, d *daemon.Daem
 	cgroupParent := newCgroupParent(config)
 
 	bk, err := buildkit.New(ctx, buildkit.Opt{
-		SessionManager:        sm,
-		Root:                  filepath.Join(config.Root, "buildkit"),
-		EngineID:              d.ID(),
-		Dist:                  d.DistributionServices(),
-		ImageTagger:           d.ImageService(),
-		NetworkController:     d.NetworkController(),
-		DefaultCgroupParent:   cgroupParent,
-		RegistryHosts:         d.RegistryHosts,
-		BuilderConfig:         config.Builder,
-		Rootless:              daemon.Rootless(config),
-		IdentityMapping:       d.IdentityMapping(),
-		DNSConfig:             config.DNSConfig,
-		ApparmorProfile:       daemon.DefaultApparmorProfile(),
-		UseSnapshotter:        d.UsesSnapshotter(),
-		Snapshotter:           d.ImageService().StorageDriver(),
-		ContainerdAddress:     config.ContainerdAddr,
-		ContainerdNamespace:   config.ContainerdNamespace,
-		ImageExportedCallback: d.ImageExportedByBuildkit,
+		SessionManager:      sm,
+		Root:                filepath.Join(config.Root, "buildkit"),
+		EngineID:            d.ID(),
+		Dist:                d.DistributionServices(),
+		ImageTagger:         d.ImageService(),
+		NetworkController:   d.NetworkController(),
+		DefaultCgroupParent: cgroupParent,
+		RegistryHosts:       d.RegistryHosts,
+		BuilderConfig:       config.Builder,
+		Rootless:            daemon.Rootless(config),
+		IdentityMapping:     d.IdentityMapping(),
+		DNSConfig:           config.DNSConfig,
+		ApparmorProfile:     daemon.DefaultApparmorProfile(),
+		UseSnapshotter:      d.UsesSnapshotter(),
+		Snapshotter:         d.ImageService().StorageDriver(),
+		ContainerdAddress:   config.ContainerdAddr,
+		ContainerdNamespace: config.ContainerdNamespace,
+		Callbacks: exporter.BuildkitCallbacks{
+			Exported: d.ImageExportedByBuildkit,
+			Named:    d.ImageNamedByBuildkit,
+		},
 	})
 	if err != nil {
 		return routerOptions{}, err

daemon/build.go

@@ -3,14 +3,24 @@ package daemon
 import (
 	"context"
 
+	"github.com/distribution/reference"
+	"github.com/docker/docker/api/types/events"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
 // ImageExportedByBuildkit is a callback that is called when an image is exported by buildkit.
 // This is used to log the image creation event for untagged images.
 // When no tag is given, buildkit doesn't call the image service so it has no
 // way of knowing the image was created.
-func (daemon *Daemon) ImageExportedByBuildkit(ctx context.Context, id string, desc ocispec.Descriptor) error {
-	daemon.imageService.LogImageEvent(id, id, "create")
-	return nil
+func (daemon *Daemon) ImageExportedByBuildkit(ctx context.Context, id string, desc ocispec.Descriptor) {
+	daemon.imageService.LogImageEvent(id, id, events.ActionCreate)
+}
+
+// ImageNamedByBuildkit is a callback that is called when an image is tagged by buildkit.
+// Note: It is only called if the buildkit didn't call the image service itself to perform the tagging.
+// Currently this only happens when the containerd image store is used.
+func (daemon *Daemon) ImageNamedByBuildkit(ctx context.Context, ref reference.NamedTagged, desc ocispec.Descriptor) {
+	id := desc.Digest.String()
+	name := reference.FamiliarString(ref)
+	daemon.imageService.LogImageEvent(id, name, events.ActionTag)
 }