Merge pull request docker#2362 from jsternberg/single-tracer-delegate…

…-client driver: initialize tracer delegate in driver handle instead of individual plugins
crazy-max · Mar 29, 2024 · c690d46 · c690d46
2 parents 35781a6 + 5c89004
commit c690d46
Show file tree

Hide file tree

Showing 8 changed files with 48 additions and 65 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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...)
 }