From 5ee23b60508aab5b623e66cef3dadc13e8d52c69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Fri, 14 Jun 2024 13:13:16 +0200 Subject: [PATCH] builder-next: Add ImageNamedByBuildkit callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When image is built with buildkit with containerd integration the image service has no way of knowing that the image was tagged because buildkit creates the image directly in containerd image store. Add a callback that is called by the exporter wrapper. Signed-off-by: Paweł Gronowski (cherry picked from commit 1506bbcfe814380bc2363e55d10c54a3f04926ab) Signed-off-by: Paweł Gronowski --- api/server/backend/build/backend.go | 6 +-- builder/builder-next/builder.go | 36 ++++++------- builder/builder-next/controller.go | 5 +- .../exporter/mobyexporter/export.go | 3 +- builder/builder-next/exporter/wrapper.go | 51 +++++++++++++++---- .../builder-next/worker/containerdworker.go | 16 +++--- cmd/dockerd/daemon.go | 39 +++++++------- 7 files changed, 95 insertions(+), 61 deletions(-) diff --git a/api/server/backend/build/backend.go b/api/server/backend/build/backend.go index ba05607fa5..63f650781d 100644 --- a/api/server/backend/build/backend.go +++ b/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 diff --git a/builder/builder-next/builder.go b/builder/builder-next/builder.go index 3ea8b90346..c1f4184467 100644 --- a/builder/builder-next/builder.go +++ b/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 diff --git a/builder/builder-next/controller.go b/builder/builder-next/controller.go index e56adbaa0d..b035f1265f 100644 --- a/builder/builder-next/controller.go +++ b/builder/builder-next/controller.go @@ -138,7 +138,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 } @@ -321,7 +321,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 diff --git a/builder/builder-next/exporter/mobyexporter/export.go b/builder/builder-next/exporter/mobyexporter/export.go index dd78e3c165..7f705a75e9 100644 --- a/builder/builder-next/exporter/mobyexporter/export.go +++ b/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 { diff --git a/builder/builder-next/exporter/wrapper.go b/builder/builder-next/exporter/wrapper.go index d954b3a698..c78e261af6 100644 --- a/builder/builder-next/exporter/wrapper.go +++ b/builder/builder-next/exporter/wrapper.go @@ -4,6 +4,8 @@ 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" @@ -11,19 +13,29 @@ import ( ocispec "github.com/opencontainers/image-spec/specs-go/v1" ) -type ImageExportedByBuildkit = func(ctx context.Context, id string, desc ocispec.Descriptor) +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 } diff --git a/builder/builder-next/worker/containerdworker.go b/builder/builder-next/worker/containerdworker.go index 2f640cca69..848932d3fa 100644 --- a/builder/builder-next/worker/containerdworker.go +++ b/builder/builder-next/worker/containerdworker.go @@ -3,9 +3,9 @@ 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" ) @@ -13,27 +13,27 @@ import ( // 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) } diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go index 2b3ffe21ba..16b25e44c8 100644 --- a/cmd/dockerd/daemon.go +++ b/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,26 @@ 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, + }, }) if err != nil { return routerOptions{}, err