diff --git a/build/build.go b/build/build.go index 04665f5207db..b4e1dda1adfe 100644 --- a/build/build.go +++ b/build/build.go @@ -766,8 +766,12 @@ func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opts map[ } node := dp.Node().Driver if node.IsMobyDriver() { + features, err := node.Features(ctx) + if err != nil { + return errors.Wrap(err, "failed to detect driver features") + } for _, e := range so.Exports { - if e.Type == "moby" && e.Attrs["push"] != "" && !node.Features(ctx)[driver.DirectPush] { + if e.Type == "moby" && e.Attrs["push"] != "" && !features[driver.DirectPush] { if ok, _ := strconv.ParseBool(e.Attrs["push"]); ok { pushNames = e.Attrs["name"] if pushNames == "" { diff --git a/build/opt.go b/build/opt.go index fb8b5fe393d2..7351098ee161 100644 --- a/build/opt.go +++ b/build/opt.go @@ -245,6 +245,10 @@ func isPolicyEvaluationError(policies []*policy.Policy, err error) bool { func toSolveOpt(ctx context.Context, np *noderesolver.ResolvedNode, multiDriver bool, opt *Options, bopts gateway.BuildOpts, cfg *confutil.Config, pw progress.Writer, docker *dockerutil.Client) (_ *client.SolveOpt, release func(error), err error) { node := np.Node() nodeDriver := node.Driver + driverFeatures, err := nodeDriver.Features(ctx) + if err != nil { + return nil, nil, errors.Wrap(err, "failed to detect driver features") + } defers := make([]func(error), 0, 2) releaseF := func(inErr error) { for _, f := range defers { @@ -269,7 +273,7 @@ func toSolveOpt(ctx context.Context, np *noderesolver.ResolvedNode, multiDriver } for _, e := range opt.CacheTo { - if e.Type != "inline" && !nodeDriver.Features(ctx)[driver.CacheExport] { + if e.Type != "inline" && !driverFeatures[driver.CacheExport] { return nil, nil, notSupported(driver.CacheExport, nodeDriver, "https://docs.docker.com/go/build-cache-backends/") } } @@ -346,10 +350,10 @@ func toSolveOpt(ctx context.Context, np *noderesolver.ResolvedNode, multiDriver } } - supportAttestations := bopts.LLBCaps.Contains(apicaps.CapID("exporter.image.attestations")) && nodeDriver.Features(ctx)[driver.MultiPlatform] + supportAttestations := bopts.LLBCaps.Contains(apicaps.CapID("exporter.image.attestations")) && driverFeatures[driver.MultiPlatform] if len(attests) > 0 { if !supportAttestations { - if !nodeDriver.Features(ctx)[driver.MultiPlatform] { + if !driverFeatures[driver.MultiPlatform] { return nil, nil, notSupported("Attestation", nodeDriver, "https://docs.docker.com/go/attestations/") } return nil, nil, errors.Errorf("Attestations are not supported by the current BuildKit daemon") @@ -391,7 +395,7 @@ func toSolveOpt(ctx context.Context, np *noderesolver.ResolvedNode, multiDriver // backwards compat for docker driver only: // this ensures the build results in a docker image. opt.Exports = []client.ExportEntry{{Type: "image", Attrs: map[string]string{}}} - } else if nodeDriver.Features(ctx)[driver.DefaultLoad] { + } else if driverFeatures[driver.DefaultLoad] { opt.Exports = []client.ExportEntry{{Type: "docker", Attrs: map[string]string{}}} } } @@ -402,7 +406,7 @@ func toSolveOpt(ctx context.Context, np *noderesolver.ResolvedNode, multiDriver } // check if index annotations are supported by docker driver - if len(opt.Exports) > 0 && opt.CallFunc == nil && len(opt.Annotations) > 0 && nodeDriver.IsMobyDriver() && !nodeDriver.Features(ctx)[driver.MultiPlatform] { + if len(opt.Exports) > 0 && opt.CallFunc == nil && len(opt.Annotations) > 0 && nodeDriver.IsMobyDriver() && !driverFeatures[driver.MultiPlatform] { for _, exp := range opt.Exports { if exp.Type == "image" || exp.Type == "docker" { for ak := range opt.Annotations { @@ -476,12 +480,24 @@ func toSolveOpt(ctx context.Context, np *noderesolver.ResolvedNode, multiDriver // set up exporters for i, e := range so.Exports { - if e.Type == "oci" && !nodeDriver.Features(ctx)[driver.OCIExporter] { + if e.Type == "oci" && !driverFeatures[driver.OCIExporter] { return nil, nil, notSupported(driver.OCIExporter, nodeDriver, "https://docs.docker.com/go/build-exporters/") } if e.Type == "docker" { - features := docker.Features(ctx, e.Attrs["context"]) - if features[dockerutil.OCIImporter] && e.Output == nil { + var features map[dockerutil.Feature]bool + if e.Output == nil { + contextName := e.Attrs["context"] + if nodeDriver.IsMobyDriver() { + // The docker driver loads into its own daemon. + contextName = node.Endpoint + } + var err error + features, err = docker.Features(ctx, contextName) + if err != nil { + return nil, nil, errors.Wrap(err, "failed to detect docker features") + } + } + if features[dockerutil.OCIImporter] { // rely on oci importer if available (which supports // multi-platform images), otherwise fall back to docker so.Exports[i].Type = "oci" @@ -511,14 +527,14 @@ func toSolveOpt(ctx context.Context, np *noderesolver.ResolvedNode, multiDriver so.Exports[i].Attrs["prefer-image-digest"] = "true" } } - } else if !nodeDriver.Features(ctx)[driver.DockerExporter] { + } else if !driverFeatures[driver.DockerExporter] { return nil, nil, notSupported(driver.DockerExporter, nodeDriver, "https://docs.docker.com/go/build-exporters/") } } if e.Type == "image" && nodeDriver.IsMobyDriver() { so.Exports[i].Type = "moby" // The containerd image store resolves images by manifest or index digest. - if nodeDriver.Features(ctx)[driver.PreferImageDigest] { + if driverFeatures[driver.PreferImageDigest] { so.Exports[i].Attrs["prefer-image-digest"] = "true" } if e.Attrs["push"] != "" { @@ -605,7 +621,7 @@ func toSolveOpt(ctx context.Context, np *noderesolver.ResolvedNode, multiDriver for i, p := range opt.Platforms { pp[i] = platforms.FormatAll(p) } - if len(pp) > 1 && !nodeDriver.Features(ctx)[driver.MultiPlatform] { + if len(pp) > 1 && !driverFeatures[driver.MultiPlatform] { return nil, nil, notSupported(driver.MultiPlatform, nodeDriver, "https://docs.docker.com/go/build-multi-platform/") } so.FrontendAttrs["platform"] = strings.Join(pp, ",") diff --git a/build/opt_test.go b/build/opt_test.go index 9327d211d37c..b7442c2dec4f 100644 --- a/build/opt_test.go +++ b/build/opt_test.go @@ -2,13 +2,26 @@ package build import ( "context" + "io" + "net/http" + "net/http/httptest" + "strings" "sync" + "sync/atomic" "testing" + noderesolver "github.com/docker/buildx/build/resolver" + "github.com/docker/buildx/builder" + "github.com/docker/buildx/driver" "github.com/docker/buildx/policy" + "github.com/docker/buildx/store" "github.com/docker/buildx/util/buildflags" + "github.com/docker/buildx/util/confutil" + "github.com/docker/buildx/util/dockerutil" "github.com/docker/buildx/util/ocilayout" "github.com/docker/buildx/util/progress" + "github.com/docker/cli/cli/command" + contextstore "github.com/docker/cli/cli/context/store" "github.com/moby/buildkit/client" "github.com/moby/buildkit/client/ociindex" gateway "github.com/moby/buildkit/frontend/gateway/client" @@ -21,6 +34,176 @@ import ( "github.com/stretchr/testify/require" ) +type exporterTestDriver struct { + driver.Driver + moby bool + features func(context.Context) (map[driver.Feature]bool, error) +} + +func (d exporterTestDriver) Info(context.Context) (*driver.Info, error) { + return &driver.Info{Status: driver.Running}, nil +} + +func (d exporterTestDriver) Client(context.Context, ...client.ClientOpt) (*client.Client, error) { + return nil, nil +} + +func (d exporterTestDriver) IsMobyDriver() bool { + return d.moby +} + +func (d exporterTestDriver) Features(ctx context.Context) (map[driver.Feature]bool, error) { + if d.features != nil { + return d.features(ctx) + } + return map[driver.Feature]bool{driver.DockerExporter: true}, nil +} + +type exporterTestCLI struct { + command.Cli + store contextstore.Store + currentContext string +} + +func (c exporterTestCLI) ContextStore() contextstore.Store { + return c.store +} + +func (c exporterTestCLI) CurrentContext() string { + return c.currentContext +} + +func TestDockerExporterFeatureProbe(t *testing.T) { + var goodCalls, badCalls atomic.Int32 + newServer := func(available bool, calls *atomic.Int32) *httptest.Server { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/_ping") { + w.Header().Set("API-Version", "1.55") + return + } + if !strings.HasSuffix(r.URL.Path, "/info") { + http.NotFound(w, r) + return + } + calls.Add(1) + w.Header().Set("Content-Type", "application/json") + if !available { + w.WriteHeader(http.StatusServiceUnavailable) + _, _ = io.WriteString(w, `{"message":"daemon unavailable"}`) + return + } + _, _ = io.WriteString(w, `{}`) + })) + t.Cleanup(server.Close) + return server + } + good := newServer(true, &goodCalls) + bad := newServer(false, &badCalls) + for _, tt := range []struct { + name string + moby bool + endpoint string + currentContext string + exportContext string + tarball bool + wantError bool + }{ + {name: "docker uses builder", moby: true, endpoint: good.URL, currentContext: bad.URL}, + {name: "docker ignores output context", moby: true, endpoint: good.URL, currentContext: bad.URL, exportContext: bad.URL}, + {name: "docker reports builder failure", moby: true, endpoint: bad.URL, currentContext: good.URL, wantError: true}, + {name: "remote uses output context", endpoint: bad.URL, currentContext: bad.URL, exportContext: good.URL}, + {name: "remote reports current context failure", endpoint: good.URL, currentContext: bad.URL, wantError: true}, + {name: "tarball skips daemon probe", endpoint: bad.URL, currentContext: bad.URL, tarball: true}, + } { + t.Run(tt.name, func(t *testing.T) { + goodCalls.Store(0) + badCalls.Store(0) + nodes, err := noderesolver.Resolve(t.Context(), []builder.Node{{ + Node: store.Node{Endpoint: tt.endpoint}, + Driver: &driver.DriverHandle{Driver: exporterTestDriver{moby: tt.moby}}, + }}, nil, nil) + require.NoError(t, err) + require.Len(t, nodes, 1) + cli := exporterTestCLI{ + store: contextstore.New(t.TempDir(), command.DefaultContextStoreConfig()), + currentContext: tt.currentContext, + } + export := client.ExportEntry{Type: "docker", Attrs: map[string]string{"context": tt.exportContext}} + if tt.tarball { + export.Output = func(map[string]string) (io.WriteCloser, error) { return nil, nil } + } + opt := &Options{ + Inputs: Inputs{ContextPath: "https://example.com/context.tar.gz"}, + Exports: []client.ExportEntry{export}, + Policy: []buildflags.PolicyConfig{{Disabled: true}}, + } + cfg := confutil.NewConfig(nil, confutil.WithDir(t.TempDir())) + so, release, err := toSolveOpt(t.Context(), nodes[0], false, opt, gateway.BuildOpts{}, cfg, testProgressWriter{}, dockerutil.NewClient(cli)) + if tt.wantError { + require.ErrorContains(t, err, "failed to detect docker features") + require.ErrorContains(t, err, "daemon unavailable") + require.EqualValues(t, 1, badCalls.Load()) + require.Zero(t, goodCalls.Load()) + return + } + require.NoError(t, err) + defer release(nil) + require.Zero(t, badCalls.Load()) + if tt.tarball { + require.Zero(t, goodCalls.Load()) + } else { + require.EqualValues(t, 1, goodCalls.Load()) + } + if tt.moby { + require.Equal(t, "moby", so.Exports[0].Type) + } else { + require.Equal(t, "docker", so.Exports[0].Type) + } + }) + } +} + +func TestDriverFeatureFailurePreservesProvenance(t *testing.T) { + t.Setenv(noDefaultAttestationsEnv, "false") + probeErr := errors.New("worker is starting") + calls := 0 + d := exporterTestDriver{ + moby: true, + features: func(context.Context) (map[driver.Feature]bool, error) { + calls++ + if calls == 1 { + return nil, probeErr + } + return map[driver.Feature]bool{driver.MultiPlatform: true}, nil + }, + } + nodes, err := noderesolver.Resolve(t.Context(), []builder.Node{{ + Driver: &driver.DriverHandle{Driver: d}, + }}, nil, nil) + require.NoError(t, err) + require.Len(t, nodes, 1) + opt := &Options{ + Inputs: Inputs{ContextPath: "https://example.com/context.tar.gz"}, + Exports: []client.ExportEntry{{Type: "image", Attrs: map[string]string{}}}, + Platforms: []ocispecs.Platform{{OS: "linux", Architecture: "amd64"}, {OS: "linux", Architecture: "arm64"}}, + Policy: []buildflags.PolicyConfig{{Disabled: true}}, + } + cfg := confutil.NewConfig(nil, confutil.WithDir(t.TempDir())) + bopts := buildOptsWithCaps(apicaps.CapID("exporter.image.attestations")) + so, release, err := toSolveOpt(t.Context(), nodes[0], false, opt, bopts, cfg, testProgressWriter{}, nil) + require.ErrorIs(t, err, probeErr) + require.ErrorContains(t, err, "failed to detect driver features") + require.Nil(t, so) + require.Nil(t, release) + require.Equal(t, 1, calls) + + so, release, err = toSolveOpt(t.Context(), nodes[0], false, opt, bopts, cfg, testProgressWriter{}, nil) + require.NoError(t, err) + defer release(nil) + require.Equal(t, "mode=min,inline-only=true", so.FrontendAttrs["attest:provenance"]) + require.Equal(t, 2, calls) +} + func TestCacheOptions_DerivedVars(t *testing.T) { t.Setenv("ACTIONS_RUNTIME_TOKEN", "sensitive_token") t.Setenv("ACTIONS_CACHE_URL", "https://cache.github.com") diff --git a/commands/inspect.go b/commands/inspect.go index 72d74c9a06eb..05776d0d8c82 100644 --- a/commands/inspect.go +++ b/commands/inspect.go @@ -104,14 +104,18 @@ func runInspect(ctx context.Context, dockerCli command.Cli, in inspectOptions) e } if debug.IsEnabled() { fmt.Fprintf(w, "Features:\n") - features := nodes[i].Driver.Features(ctx) - featKeys := make([]string, 0, len(features)) - for k := range features { - featKeys = append(featKeys, string(k)) - } - sort.Strings(featKeys) - for _, k := range featKeys { - fmt.Fprintf(w, "\t%s:\t%t\n", k, features[driver.Feature(k)]) + features, err := nodes[i].Driver.Features(timeoutCtx) + if err != nil { + fmt.Fprintf(w, "\tError:\t%s\n", err.Error()) + } else { + featKeys := make([]string, 0, len(features)) + for k := range features { + featKeys = append(featKeys, string(k)) + } + sort.Strings(featKeys) + for _, k := range featKeys { + fmt.Fprintf(w, "\t%s:\t%t\n", k, features[driver.Feature(k)]) + } } } if len(nodes[i].Labels) > 0 { diff --git a/driver/cloud/driver.go b/driver/cloud/driver.go index c2e61f75eea4..bde1fff09fe7 100644 --- a/driver/cloud/driver.go +++ b/driver/cloud/driver.go @@ -176,7 +176,7 @@ func (d *Driver) Client(ctx context.Context, opts ...client.ClientOpt) (*client. return c, nil } -func (d *Driver) Features(_ context.Context) map[driver.Feature]bool { +func (d *Driver) Features(_ context.Context) (map[driver.Feature]bool, error) { return map[driver.Feature]bool{ driver.OCIExporter: true, driver.DockerExporter: false, @@ -184,7 +184,7 @@ func (d *Driver) Features(_ context.Context) map[driver.Feature]bool { driver.MultiPlatform: true, driver.DirectPush: true, driver.DefaultLoad: d.defaultLoad, - } + }, nil } func (d *Driver) Factory() driver.Factory { diff --git a/driver/docker-container/driver.go b/driver/docker-container/driver.go index 5800311f40d5..833dfc0f3cdc 100644 --- a/driver/docker-container/driver.go +++ b/driver/docker-container/driver.go @@ -581,7 +581,7 @@ func (d *Driver) Factory() driver.Factory { return d.factory } -func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool { +func (d *Driver) Features(ctx context.Context) (map[driver.Feature]bool, error) { return map[driver.Feature]bool{ driver.OCIExporter: true, driver.DockerExporter: true, @@ -589,7 +589,7 @@ func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool { driver.MultiPlatform: true, driver.DirectPush: true, driver.DefaultLoad: d.defaultLoad, - } + }, nil } func (d *Driver) HostGatewayIP(ctx context.Context) (net.IP, error) { diff --git a/driver/docker/driver.go b/driver/docker/driver.go index 402509b613b7..a805b521bd2c 100644 --- a/driver/docker/driver.go +++ b/driver/docker/driver.go @@ -9,6 +9,7 @@ import ( "github.com/docker/buildx/driver" "github.com/docker/buildx/util/progress" "github.com/moby/buildkit/client" + "github.com/moby/buildkit/util/flightcontrol" dockerclient "github.com/moby/moby/client" "github.com/pkg/errors" ) @@ -19,7 +20,7 @@ type Driver struct { // if you add fields, remember to update docs: // https://github.com/docker/docs/blob/main/content/build/drivers/docker.md - features features + features flightcontrol.CachedGroup[map[driver.Feature]bool] hostGateway hostGateway } @@ -74,24 +75,24 @@ func (d *Driver) Client(ctx context.Context, opts ...client.ClientOpt) (*client. return client.New(ctx, "", opts...) } -type features struct { - once sync.Once - list map[driver.Feature]bool -} - -func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool { - d.features.once.Do(func() { +func (d *Driver) Features(ctx context.Context) (map[driver.Feature]bool, error) { + return d.features.Do(ctx, "", func(ctx context.Context) (map[driver.Feature]bool, error) { + c, err := d.Client(ctx) + if err != nil { + return nil, err + } + defer c.Close() + workers, err := c.ListWorkers(ctx) + if err != nil { + return nil, errors.Wrap(err, "listing workers") + } var useContainerdSnapshotter bool - if c, err := d.Client(ctx); err == nil { - workers, _ := c.ListWorkers(ctx) - for _, w := range workers { - if _, ok := w.Labels["org.mobyproject.buildkit.worker.snapshotter"]; ok { - useContainerdSnapshotter = true - } + for _, w := range workers { + if _, ok := w.Labels["org.mobyproject.buildkit.worker.snapshotter"]; ok { + useContainerdSnapshotter = true } - c.Close() } - d.features.list = map[driver.Feature]bool{ + return map[driver.Feature]bool{ driver.OCIExporter: useContainerdSnapshotter, driver.DockerExporter: useContainerdSnapshotter, driver.CacheExport: useContainerdSnapshotter, @@ -99,9 +100,8 @@ func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool { driver.DirectPush: useContainerdSnapshotter, driver.PreferImageDigest: useContainerdSnapshotter, driver.DefaultLoad: true, - } + }, nil }) - return d.features.list } type hostGateway struct { diff --git a/driver/docker/driver_test.go b/driver/docker/driver_test.go new file mode 100644 index 000000000000..107771514128 --- /dev/null +++ b/driver/docker/driver_test.go @@ -0,0 +1,77 @@ +package docker + +import ( + "context" + "net" + "sync/atomic" + "testing" + "time" + + "github.com/docker/buildx/driver" + control "github.com/moby/buildkit/api/services/control" + types "github.com/moby/buildkit/api/types" + dockerclient "github.com/moby/moby/client" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +type featureTestAPI struct { + dockerclient.APIClient + addr string +} + +func (a featureTestAPI) DialHijack(ctx context.Context, _, _ string, _ map[string][]string) (net.Conn, error) { + var dialer net.Dialer + return dialer.DialContext(ctx, "tcp", a.addr) +} + +type featureTestControl struct { + control.UnimplementedControlServer + calls atomic.Int32 + supported bool +} + +func (c *featureTestControl) ListWorkers(context.Context, *control.ListWorkersRequest) (*control.ListWorkersResponse, error) { + if c.calls.Add(1) == 1 { + return nil, status.Error(codes.Unavailable, "worker is starting") + } + w := &types.WorkerRecord{} + if c.supported { + w.Labels = map[string]string{"org.mobyproject.buildkit.worker.snapshotter": "overlayfs"} + } + return &control.ListWorkersResponse{Record: []*types.WorkerRecord{w}}, nil +} + +func TestFeaturesRetry(t *testing.T) { + for _, supported := range []bool{false, true} { + t.Run(map[bool]string{false: "unsupported", true: "supported"}[supported], func(t *testing.T) { + ctx, cancel := context.WithTimeoutCause(t.Context(), 10*time.Second, context.DeadlineExceeded) + defer cancel() + var lc net.ListenConfig + listener, err := lc.Listen(ctx, "tcp", "127.0.0.1:0") + require.NoError(t, err) + defer listener.Close() + server := grpc.NewServer() + defer server.Stop() + ctl := &featureTestControl{supported: supported} + control.RegisterControlServer(server, ctl) + go server.Serve(listener) + d := &Driver{InitConfig: driver.InitConfig{DockerAPI: featureTestAPI{addr: listener.Addr().String()}}} + features, err := d.Features(ctx) + require.ErrorContains(t, err, "listing workers") + require.Equal(t, codes.Unavailable, status.Code(err)) + require.Nil(t, features) + for range 2 { + features, err = d.Features(ctx) + require.NoError(t, err) + require.True(t, features[driver.DefaultLoad]) + for _, f := range []driver.Feature{driver.OCIExporter, driver.DockerExporter, driver.CacheExport, driver.MultiPlatform, driver.DirectPush, driver.PreferImageDigest} { + require.Equal(t, supported, features[f], f) + } + } + require.EqualValues(t, 2, ctl.calls.Load()) + }) + } +} diff --git a/driver/driver.go b/driver/driver.go index 0fb1264464ca..7996cb0635f4 100644 --- a/driver/driver.go +++ b/driver/driver.go @@ -70,7 +70,7 @@ type Driver interface { Rm(ctx context.Context, force, rmVolume, rmDaemon bool) error Dial(ctx context.Context) (net.Conn, error) Client(ctx context.Context, opts ...client.ClientOpt) (*client.Client, error) - Features(ctx context.Context) map[Feature]bool + Features(ctx context.Context) (map[Feature]bool, error) HostGatewayIP(ctx context.Context) (net.IP, error) IsMobyDriver() bool Config() InitConfig diff --git a/driver/kubernetes/driver.go b/driver/kubernetes/driver.go index 4a59a5d372a0..e0206785e399 100644 --- a/driver/kubernetes/driver.go +++ b/driver/kubernetes/driver.go @@ -416,7 +416,7 @@ func (d *Driver) Factory() driver.Factory { return d.factory } -func (d *Driver) Features(_ context.Context) map[driver.Feature]bool { +func (d *Driver) Features(_ context.Context) (map[driver.Feature]bool, error) { return map[driver.Feature]bool{ driver.OCIExporter: true, driver.DockerExporter: d.DockerAPI != nil, @@ -424,7 +424,7 @@ func (d *Driver) Features(_ context.Context) map[driver.Feature]bool { driver.MultiPlatform: true, // Untested (needs multiple Driver instances) driver.DirectPush: true, driver.DefaultLoad: d.defaultLoad, - } + }, nil } func (d *Driver) HostGatewayIP(_ context.Context) (net.IP, error) { diff --git a/driver/remote/driver.go b/driver/remote/driver.go index 62ed7646f6cb..8dda8a13c7cf 100644 --- a/driver/remote/driver.go +++ b/driver/remote/driver.go @@ -187,7 +187,7 @@ func loadTLS(opts *tlsOpts) (*tls.Config, error) { return cfg, nil } -func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool { +func (d *Driver) Features(ctx context.Context) (map[driver.Feature]bool, error) { return map[driver.Feature]bool{ driver.OCIExporter: true, driver.DockerExporter: true, @@ -195,7 +195,7 @@ func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool { driver.MultiPlatform: true, driver.DirectPush: true, driver.DefaultLoad: d.defaultLoad, - } + }, nil } func (d *Driver) HostGatewayIP(ctx context.Context) (net.IP, error) { diff --git a/util/dockerutil/client.go b/util/dockerutil/client.go index 68ff9a29c281..7a9157a61097 100644 --- a/util/dockerutil/client.go +++ b/util/dockerutil/client.go @@ -7,6 +7,7 @@ import ( "github.com/docker/buildx/util/progress" "github.com/docker/cli/cli/command" + "github.com/moby/buildkit/util/flightcontrol" dockerclient "github.com/moby/moby/client" ) @@ -14,8 +15,7 @@ import ( type Client struct { cli command.Cli - featuresOnce sync.Once - featuresCache map[Feature]bool + features flightcontrol.CachedGroup[map[Feature]bool] } // NewClient initializes a new docker client. @@ -73,23 +73,22 @@ func (c *Client) LoadImage(ctx context.Context, name string, status progress.Wri }, nil } -func (c *Client) Features(ctx context.Context, name string) map[Feature]bool { - c.featuresOnce.Do(func() { - c.featuresCache = c.features(ctx, name) - }) - return c.featuresCache -} - -func (c *Client) features(ctx context.Context, name string) map[Feature]bool { - features := make(map[Feature]bool) - if dapi, err := c.API(name); err == nil { - if res, err := dapi.Info(ctx, dockerclient.InfoOptions{}); err == nil { - if HasOCIImporter(res.Info) { - features[OCIImporter] = true - } - } +func (c *Client) Features(ctx context.Context, name string) (map[Feature]bool, error) { + if name == "" { + name = c.cli.CurrentContext() } - return features + return c.features.Do(ctx, name, func(ctx context.Context) (map[Feature]bool, error) { + dapi, err := c.API(name) + if err != nil { + return nil, err + } + defer dapi.Close() + res, err := dapi.Info(ctx, dockerclient.InfoOptions{}) + if err != nil { + return nil, err + } + return map[Feature]bool{OCIImporter: HasOCIImporter(res.Info)}, nil + }) } type waitingWriter struct { diff --git a/util/dockerutil/client_test.go b/util/dockerutil/client_test.go new file mode 100644 index 000000000000..c7029a4ef624 --- /dev/null +++ b/util/dockerutil/client_test.go @@ -0,0 +1,148 @@ +package dockerutil + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/docker/cli/cli/command" + "github.com/docker/cli/cli/context/store" + "github.com/moby/moby/api/types/system" + "github.com/stretchr/testify/require" +) + +type featureTestCLI struct { + command.Cli + store store.Store + currentContext string +} + +func (c featureTestCLI) ContextStore() store.Store { + return c.store +} + +func (c featureTestCLI) CurrentContext() string { + return c.currentContext +} + +func newFeatureTestServer(t *testing.T, info http.HandlerFunc) *httptest.Server { + t.Helper() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.HasSuffix(r.URL.Path, "/_ping"): + w.Header().Set("API-Version", "1.55") + case strings.HasSuffix(r.URL.Path, "/info"): + w.Header().Set("Content-Type", "application/json") + info(w, r) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(server.Close) + return server +} + +func TestFeaturesRetry(t *testing.T) { + for _, supported := range []bool{false, true} { + t.Run(map[bool]string{false: "unsupported", true: "supported"}[supported], func(t *testing.T) { + var calls atomic.Int32 + server := newFeatureTestServer(t, func(w http.ResponseWriter, r *http.Request) { + if calls.Add(1) == 1 { + w.WriteHeader(http.StatusServiceUnavailable) + _, _ = w.Write([]byte(`{"message":"daemon is starting"}`)) + return + } + info := system.Info{} + if supported { + info.DriverStatus = [][2]string{{"driver-type", "io.containerd.snapshotter.v1"}} + } + _ = json.NewEncoder(w).Encode(info) + }) + c := NewClient(featureTestCLI{store: store.New(t.TempDir(), command.DefaultContextStoreConfig())}) + ctx := t.Context() + features, err := c.Features(ctx, server.URL) + require.ErrorContains(t, err, "daemon is starting") + require.Nil(t, features) + for range 2 { + features, err = c.Features(ctx, server.URL) + require.NoError(t, err) + require.Equal(t, supported, features[OCIImporter]) + } + require.EqualValues(t, 2, calls.Load()) + }) + } +} + +func TestFeaturesContexts(t *testing.T) { + var supportedCalls, unsupportedCalls atomic.Int32 + supported := newFeatureTestServer(t, func(w http.ResponseWriter, r *http.Request) { + supportedCalls.Add(1) + _ = json.NewEncoder(w).Encode(system.Info{DriverStatus: [][2]string{{"driver-type", "io.containerd.snapshotter.v1"}}}) + }) + unsupported := newFeatureTestServer(t, func(w http.ResponseWriter, r *http.Request) { + unsupportedCalls.Add(1) + _ = json.NewEncoder(w).Encode(system.Info{}) + }) + c := NewClient(featureTestCLI{ + store: store.New(t.TempDir(), command.DefaultContextStoreConfig()), + currentContext: supported.URL, + }) + for range 2 { + for _, name := range []string{"", unsupported.URL, supported.URL} { + features, err := c.Features(t.Context(), name) + require.NoError(t, err) + require.Equal(t, name != unsupported.URL, features[OCIImporter]) + } + } + require.EqualValues(t, 1, supportedCalls.Load()) + require.EqualValues(t, 1, unsupportedCalls.Load()) +} + +func TestFeaturesCancellation(t *testing.T) { + entered := make(chan struct{}) + release := make(chan struct{}) + var calls atomic.Int32 + server := newFeatureTestServer(t, func(w http.ResponseWriter, r *http.Request) { + if calls.Add(1) == 1 { + close(entered) + select { + case <-release: + case <-r.Context().Done(): + } + return + } + _ = json.NewEncoder(w).Encode(system.Info{DriverStatus: [][2]string{{"driver-type", "io.containerd.snapshotter.v1"}}}) + }) + t.Cleanup(func() { close(release) }) + c := NewClient(featureTestCLI{store: store.New(t.TempDir(), command.DefaultContextStoreConfig())}) + ctx, cancel := context.WithTimeoutCause(t.Context(), 10*time.Second, context.DeadlineExceeded) + defer cancel() + probeCtx, cancelProbe := context.WithCancelCause(ctx) + defer cancelProbe(context.Canceled) + done := make(chan error, 1) + go func() { + _, err := c.Features(probeCtx, server.URL) + done <- err + }() + select { + case <-entered: + case <-ctx.Done(): + t.Fatal("probe did not start") + } + cancelProbe(context.Canceled) + select { + case err := <-done: + require.ErrorIs(t, err, context.Canceled) + case <-ctx.Done(): + t.Fatal("probe did not stop after cancellation") + } + features, err := c.Features(ctx, server.URL) + require.NoError(t, err) + require.True(t, features[OCIImporter]) + require.EqualValues(t, 2, calls.Load()) +}