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()) +}