From 5c89004bb6ff05563b48032dc52a0f8a98e8acd6 Mon Sep 17 00:00:00 2001 From: "Jonathan A. Sternberg" Date: Wed, 27 Mar 2024 15:13:43 -0500 Subject: [PATCH] driver: initialize tracer delegate in driver handle instead of individual plugins This refactors the driver handle to initialize the tracer delegate inside of the driver handle instead of the individual plugins. This provides more uniformity to how the tracer delegate is created by allowing the driver handle to pass additional client options to the drivers when they create the client. It also avoids creating the tracer delegate client multiple times because the driver handle will only initialize the client once. This prevents some drivers, like the remote driver, from accidentally registering multiple clients as tracer delegates. Signed-off-by: Jonathan A. Sternberg --- build/build.go | 6 +++--- build/utils.go | 2 +- driver/docker-container/driver.go | 26 +++++++++----------------- driver/docker/driver.go | 15 +++------------ driver/driver.go | 2 +- driver/kubernetes/driver.go | 20 ++++++-------------- driver/manager.go | 20 +++++++++++++++++++- driver/remote/driver.go | 22 ++++++---------------- 8 files changed, 48 insertions(+), 65 deletions(-) diff --git a/build/build.go b/build/build.go index 05fced84674..af269ac35fa 100644 --- a/build/build.go +++ b/build/build.go @@ -158,7 +158,7 @@ func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opt map[s return nil, errors.Wrapf(err, "no valid drivers found") } - var noMobyDriver driver.Driver + var noMobyDriver *driver.DriverHandle for _, n := range nodes { if !n.Driver.IsMobyDriver() { noMobyDriver = n.Driver @@ -658,7 +658,7 @@ func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opt map[s return resp, nil } -func pushWithMoby(ctx context.Context, d driver.Driver, name string, l progress.SubLogger) error { +func pushWithMoby(ctx context.Context, d *driver.DriverHandle, name string, l progress.SubLogger) error { api := d.Config().DockerAPI if api == nil { return errors.Errorf("invalid empty Docker API reference") // should never happen @@ -738,7 +738,7 @@ func pushWithMoby(ctx context.Context, d driver.Driver, name string, l progress. return nil } -func remoteDigestWithMoby(ctx context.Context, d driver.Driver, name string) (string, error) { +func remoteDigestWithMoby(ctx context.Context, d *driver.DriverHandle, name string) (string, error) { api := d.Config().DockerAPI if api == nil { return "", errors.Errorf("invalid empty Docker API reference") // should never happen diff --git a/build/utils.go b/build/utils.go index 0f3197280fe..8894b592742 100644 --- a/build/utils.go +++ b/build/utils.go @@ -105,7 +105,7 @@ func toBuildkitUlimits(inp *opts.UlimitOpt) (string, error) { return strings.Join(ulimits, ","), nil } -func notSupported(f driver.Feature, d driver.Driver, docs string) error { +func notSupported(f driver.Feature, d *driver.DriverHandle, docs string) error { return errors.Errorf(`%s is not supported for the %s driver. Switch to a different driver, or turn on the containerd image store, and try again. Learn more at %s`, f, d.Factory().Name(), docs) diff --git a/driver/docker-container/driver.go b/driver/docker-container/driver.go index 187f150c577..6a86009fc36 100644 --- a/driver/docker-container/driver.go +++ b/driver/docker-container/driver.go @@ -30,7 +30,6 @@ import ( "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/stdcopy" "github.com/moby/buildkit/client" - "github.com/moby/buildkit/util/tracing/detect" "github.com/pkg/errors" ) @@ -396,28 +395,21 @@ func (d *Driver) Dial(ctx context.Context) (net.Conn, error) { return conn, nil } -func (d *Driver) Client(ctx context.Context) (*client.Client, error) { +func (d *Driver) Client(ctx context.Context, opts ...client.ClientOpt) (*client.Client, error) { conn, err := d.Dial(ctx) if err != nil { return nil, err } - exp, _, err := detect.Exporter() - if err != nil { - return nil, err - } - - var opts []client.ClientOpt var counter int64 - opts = append(opts, client.WithContextDialer(func(context.Context, string) (net.Conn, error) { - if atomic.AddInt64(&counter, 1) > 1 { - return nil, net.ErrClosed - } - return conn, nil - })) - if td, ok := exp.(client.TracerDelegate); ok { - opts = append(opts, client.WithTracerDelegate(td)) - } + opts = append([]client.ClientOpt{ + client.WithContextDialer(func(context.Context, string) (net.Conn, error) { + if atomic.AddInt64(&counter, 1) > 1 { + return nil, net.ErrClosed + } + return conn, nil + }), + }, opts...) return client.New(ctx, "", opts...) } diff --git a/driver/docker/driver.go b/driver/docker/driver.go index 85f4248cd4d..742c52c2ae7 100644 --- a/driver/docker/driver.go +++ b/driver/docker/driver.go @@ -9,7 +9,6 @@ import ( "github.com/docker/buildx/driver" "github.com/docker/buildx/util/progress" "github.com/moby/buildkit/client" - "github.com/moby/buildkit/util/tracing/detect" "github.com/pkg/errors" ) @@ -61,22 +60,14 @@ func (d *Driver) Dial(ctx context.Context) (net.Conn, error) { return d.DockerAPI.DialHijack(ctx, "/grpc", "h2c", d.DialMeta) } -func (d *Driver) Client(ctx context.Context) (*client.Client, error) { - opts := []client.ClientOpt{ +func (d *Driver) Client(ctx context.Context, opts ...client.ClientOpt) (*client.Client, error) { + opts = append([]client.ClientOpt{ client.WithContextDialer(func(context.Context, string) (net.Conn, error) { return d.Dial(ctx) }), client.WithSessionDialer(func(ctx context.Context, proto string, meta map[string][]string) (net.Conn, error) { return d.DockerAPI.DialHijack(ctx, "/session", proto, meta) }), - } - - exp, _, err := detect.Exporter() - if err != nil { - return nil, err - } - if td, ok := exp.(client.TracerDelegate); ok { - opts = append(opts, client.WithTracerDelegate(td)) - } + }, opts...) return client.New(ctx, "", opts...) } diff --git a/driver/driver.go b/driver/driver.go index da49b75f66e..d31ce82dda4 100644 --- a/driver/driver.go +++ b/driver/driver.go @@ -61,7 +61,7 @@ type Driver interface { Stop(ctx context.Context, force bool) error Rm(ctx context.Context, force, rmVolume, rmDaemon bool) error Dial(ctx context.Context) (net.Conn, error) - Client(ctx context.Context) (*client.Client, error) + Client(ctx context.Context, opts ...client.ClientOpt) (*client.Client, error) Features(ctx context.Context) map[Feature]bool HostGatewayIP(ctx context.Context) (net.IP, error) IsMobyDriver() bool diff --git a/driver/kubernetes/driver.go b/driver/kubernetes/driver.go index 29ffb21e08b..73742ddda62 100644 --- a/driver/kubernetes/driver.go +++ b/driver/kubernetes/driver.go @@ -15,7 +15,6 @@ import ( "github.com/docker/buildx/util/platformutil" "github.com/docker/buildx/util/progress" "github.com/moby/buildkit/client" - "github.com/moby/buildkit/util/tracing/detect" "github.com/pkg/errors" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -211,19 +210,12 @@ func (d *Driver) Dial(ctx context.Context) (net.Conn, error) { return conn, nil } -func (d *Driver) Client(ctx context.Context) (*client.Client, error) { - exp, _, err := detect.Exporter() - if err != nil { - return nil, err - } - - var opts []client.ClientOpt - opts = append(opts, client.WithContextDialer(func(context.Context, string) (net.Conn, error) { - return d.Dial(ctx) - })) - if td, ok := exp.(client.TracerDelegate); ok { - opts = append(opts, client.WithTracerDelegate(td)) - } +func (d *Driver) Client(ctx context.Context, opts ...client.ClientOpt) (*client.Client, error) { + opts = append([]client.ClientOpt{ + client.WithContextDialer(func(context.Context, string) (net.Conn, error) { + return d.Dial(ctx) + }), + }, opts...) return client.New(ctx, "", opts...) } diff --git a/driver/manager.go b/driver/manager.go index a4e56e64e74..8e30a242d05 100644 --- a/driver/manager.go +++ b/driver/manager.go @@ -9,6 +9,7 @@ import ( dockerclient "github.com/docker/docker/client" "github.com/moby/buildkit/client" + "github.com/moby/buildkit/util/tracing/detect" specs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" "k8s.io/client-go/rest" @@ -156,11 +157,28 @@ type DriverHandle struct { func (d *DriverHandle) Client(ctx context.Context) (*client.Client, error) { d.once.Do(func() { - d.client, d.err = d.Driver.Client(ctx) + opts, err := d.getClientOptions() + if err != nil { + d.err = err + return + } + d.client, d.err = d.Driver.Client(ctx, opts...) }) return d.client, d.err } +func (d *DriverHandle) getClientOptions() ([]client.ClientOpt, error) { + exp, _, err := detect.Exporter() + if err != nil { + return nil, err + } else if td, ok := exp.(client.TracerDelegate); ok { + return []client.ClientOpt{ + client.WithTracerDelegate(td), + }, nil + } + return nil, nil +} + func (d *DriverHandle) HistoryAPISupported(ctx context.Context) bool { d.historyAPISupportedOnce.Do(func() { if c, err := d.Client(ctx); err == nil { diff --git a/driver/remote/driver.go b/driver/remote/driver.go index 0dc676cf36b..b96ba92a71e 100644 --- a/driver/remote/driver.go +++ b/driver/remote/driver.go @@ -14,7 +14,6 @@ import ( "github.com/docker/buildx/util/progress" "github.com/moby/buildkit/client" "github.com/moby/buildkit/client/connhelper" - "github.com/moby/buildkit/util/tracing/detect" "github.com/pkg/errors" ) @@ -77,21 +76,12 @@ func (d *Driver) Rm(ctx context.Context, force, rmVolume, rmDaemon bool) error { return nil } -func (d *Driver) Client(ctx context.Context) (*client.Client, error) { - opts := []client.ClientOpt{} - - exp, _, err := detect.Exporter() - if err != nil { - return nil, err - } - if td, ok := exp.(client.TracerDelegate); ok { - opts = append(opts, client.WithTracerDelegate(td)) - } - - opts = append(opts, client.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { - return d.Dial(ctx) - })) - +func (d *Driver) Client(ctx context.Context, opts ...client.ClientOpt) (*client.Client, error) { + opts = append([]client.ClientOpt{ + client.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { + return d.Dial(ctx) + }), + }, opts...) return client.New(ctx, "", opts...) }