From 56113fc3bffbc30d3b0023c76fd88595e98bc980 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 | 140 +++++++++++++++++++++++++++++++++ 4 files changed, 309 insertions(+), 20 deletions(-) diff --git a/build/opt.go b/build/opt.go index cf9c1098be09..67a524286bf3 100644 --- a/build/opt.go +++ b/build/opt.go @@ -481,8 +481,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 421bd911b77d..c7c47a3e2df6 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 index 994b9a2339cf..1fa28503e071 100644 --- a/util/dockerutil/client_test.go +++ b/util/dockerutil/client_test.go @@ -1,10 +1,19 @@ package dockerutil import ( + "context" + "encoding/json" "io" + "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" ) @@ -71,3 +80,134 @@ func TestWaitingWriterCloseWaitsForLoader(t *testing.T) { require.NoError(t, <-closed) <-copyDone } + +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()) +}