From 269936e60f52f9516851e6eb82111ec906b461cc Mon Sep 17 00:00:00 2001 From: CrazyMax <1951866+crazy-max@users.noreply.github.com> Date: Wed, 16 Sep 2026 15:20:43 +0200 Subject: [PATCH] build: report docker feature detection failures Return daemon probe errors instead of treating them as unsupported features. Cache only successful results per docker context so failed or canceled probes can be retried. Probe the builder endpoint for the docker driver and the load destination for other drivers. Skip daemon detection for tarball exports, and cover retries, cancellation, concurrent lookups, and exporter probe selection. Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com> --- build/opt.go | 16 +++- build/opt_test.go | 138 ++++++++++++++++++++++++++++++ util/dockerutil/client.go | 35 ++++---- util/dockerutil/client_test.go | 148 +++++++++++++++++++++++++++++++++ 4 files changed, 317 insertions(+), 20 deletions(-) create mode 100644 util/dockerutil/client_test.go diff --git a/build/opt.go b/build/opt.go index fb8b5fe393d2..17eef94d1f60 100644 --- a/build/opt.go +++ b/build/opt.go @@ -480,8 +480,20 @@ func toSolveOpt(ctx context.Context, np *noderesolver.ResolvedNode, multiDriver 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" diff --git a/build/opt_test.go b/build/opt_test.go index 9327d211d37c..7dd8bd16b33a 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,131 @@ import ( "github.com/stretchr/testify/require" ) +type exporterTestDriver struct { + driver.Driver + moby bool +} + +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(context.Context) map[driver.Feature]bool { + return map[driver.Feature]bool{driver.DockerExporter: true} +} + +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 TestCacheOptions_DerivedVars(t *testing.T) { t.Setenv("ACTIONS_RUNTIME_TOKEN", "sensitive_token") t.Setenv("ACTIONS_CACHE_URL", "https://cache.github.com") 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()) +}