Skip to content

Commit 269936e

Browse files
committed
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>
1 parent ddf35fc commit 269936e

4 files changed

Lines changed: 317 additions & 20 deletions

File tree

‎build/opt.go‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -480,8 +480,20 @@ func toSolveOpt(ctx context.Context, np *noderesolver.ResolvedNode, multiDriver
480480
return nil, nil, notSupported(driver.OCIExporter, nodeDriver, "https://docs.docker.com/go/build-exporters/")
481481
}
482482
if e.Type == "docker" {
483-
features := docker.Features(ctx, e.Attrs["context"])
484-
if features[dockerutil.OCIImporter] && e.Output == nil {
483+
var features map[dockerutil.Feature]bool
484+
if e.Output == nil {
485+
contextName := e.Attrs["context"]
486+
if nodeDriver.IsMobyDriver() {
487+
// The docker driver loads into its own daemon.
488+
contextName = node.Endpoint
489+
}
490+
var err error
491+
features, err = docker.Features(ctx, contextName)
492+
if err != nil {
493+
return nil, nil, errors.Wrap(err, "failed to detect docker features")
494+
}
495+
}
496+
if features[dockerutil.OCIImporter] {
485497
// rely on oci importer if available (which supports
486498
// multi-platform images), otherwise fall back to docker
487499
so.Exports[i].Type = "oci"

‎build/opt_test.go‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,26 @@ package build
22

33
import (
44
"context"
5+
"io"
6+
"net/http"
7+
"net/http/httptest"
8+
"strings"
59
"sync"
10+
"sync/atomic"
611
"testing"
712

13+
noderesolver "github.com/docker/buildx/build/resolver"
14+
"github.com/docker/buildx/builder"
15+
"github.com/docker/buildx/driver"
816
"github.com/docker/buildx/policy"
17+
"github.com/docker/buildx/store"
918
"github.com/docker/buildx/util/buildflags"
19+
"github.com/docker/buildx/util/confutil"
20+
"github.com/docker/buildx/util/dockerutil"
1021
"github.com/docker/buildx/util/ocilayout"
1122
"github.com/docker/buildx/util/progress"
23+
"github.com/docker/cli/cli/command"
24+
contextstore "github.com/docker/cli/cli/context/store"
1225
"github.com/moby/buildkit/client"
1326
"github.com/moby/buildkit/client/ociindex"
1427
gateway "github.com/moby/buildkit/frontend/gateway/client"
@@ -21,6 +34,131 @@ import (
2134
"github.com/stretchr/testify/require"
2235
)
2336

37+
type exporterTestDriver struct {
38+
driver.Driver
39+
moby bool
40+
}
41+
42+
func (d exporterTestDriver) Info(context.Context) (*driver.Info, error) {
43+
return &driver.Info{Status: driver.Running}, nil
44+
}
45+
46+
func (d exporterTestDriver) Client(context.Context, ...client.ClientOpt) (*client.Client, error) {
47+
return nil, nil
48+
}
49+
50+
func (d exporterTestDriver) IsMobyDriver() bool {
51+
return d.moby
52+
}
53+
54+
func (d exporterTestDriver) Features(context.Context) map[driver.Feature]bool {
55+
return map[driver.Feature]bool{driver.DockerExporter: true}
56+
}
57+
58+
type exporterTestCLI struct {
59+
command.Cli
60+
store contextstore.Store
61+
currentContext string
62+
}
63+
64+
func (c exporterTestCLI) ContextStore() contextstore.Store {
65+
return c.store
66+
}
67+
68+
func (c exporterTestCLI) CurrentContext() string {
69+
return c.currentContext
70+
}
71+
72+
func TestDockerExporterFeatureProbe(t *testing.T) {
73+
var goodCalls, badCalls atomic.Int32
74+
newServer := func(available bool, calls *atomic.Int32) *httptest.Server {
75+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
76+
if strings.HasSuffix(r.URL.Path, "/_ping") {
77+
w.Header().Set("API-Version", "1.55")
78+
return
79+
}
80+
if !strings.HasSuffix(r.URL.Path, "/info") {
81+
http.NotFound(w, r)
82+
return
83+
}
84+
calls.Add(1)
85+
w.Header().Set("Content-Type", "application/json")
86+
if !available {
87+
w.WriteHeader(http.StatusServiceUnavailable)
88+
_, _ = io.WriteString(w, `{"message":"daemon unavailable"}`)
89+
return
90+
}
91+
_, _ = io.WriteString(w, `{}`)
92+
}))
93+
t.Cleanup(server.Close)
94+
return server
95+
}
96+
good := newServer(true, &goodCalls)
97+
bad := newServer(false, &badCalls)
98+
for _, tt := range []struct {
99+
name string
100+
moby bool
101+
endpoint string
102+
currentContext string
103+
exportContext string
104+
tarball bool
105+
wantError bool
106+
}{
107+
{name: "docker uses builder", moby: true, endpoint: good.URL, currentContext: bad.URL},
108+
{name: "docker ignores output context", moby: true, endpoint: good.URL, currentContext: bad.URL, exportContext: bad.URL},
109+
{name: "docker reports builder failure", moby: true, endpoint: bad.URL, currentContext: good.URL, wantError: true},
110+
{name: "remote uses output context", endpoint: bad.URL, currentContext: bad.URL, exportContext: good.URL},
111+
{name: "remote reports current context failure", endpoint: good.URL, currentContext: bad.URL, wantError: true},
112+
{name: "tarball skips daemon probe", endpoint: bad.URL, currentContext: bad.URL, tarball: true},
113+
} {
114+
t.Run(tt.name, func(t *testing.T) {
115+
goodCalls.Store(0)
116+
badCalls.Store(0)
117+
nodes, err := noderesolver.Resolve(t.Context(), []builder.Node{{
118+
Node: store.Node{Endpoint: tt.endpoint},
119+
Driver: &driver.DriverHandle{Driver: exporterTestDriver{moby: tt.moby}},
120+
}}, nil, nil)
121+
require.NoError(t, err)
122+
require.Len(t, nodes, 1)
123+
cli := exporterTestCLI{
124+
store: contextstore.New(t.TempDir(), command.DefaultContextStoreConfig()),
125+
currentContext: tt.currentContext,
126+
}
127+
export := client.ExportEntry{Type: "docker", Attrs: map[string]string{"context": tt.exportContext}}
128+
if tt.tarball {
129+
export.Output = func(map[string]string) (io.WriteCloser, error) { return nil, nil }
130+
}
131+
opt := &Options{
132+
Inputs: Inputs{ContextPath: "https://example.com/context.tar.gz"},
133+
Exports: []client.ExportEntry{export},
134+
Policy: []buildflags.PolicyConfig{{Disabled: true}},
135+
}
136+
cfg := confutil.NewConfig(nil, confutil.WithDir(t.TempDir()))
137+
so, release, err := toSolveOpt(t.Context(), nodes[0], false, opt, gateway.BuildOpts{}, cfg, testProgressWriter{}, dockerutil.NewClient(cli))
138+
if tt.wantError {
139+
require.ErrorContains(t, err, "failed to detect docker features")
140+
require.ErrorContains(t, err, "daemon unavailable")
141+
require.EqualValues(t, 1, badCalls.Load())
142+
require.Zero(t, goodCalls.Load())
143+
return
144+
}
145+
require.NoError(t, err)
146+
defer release(nil)
147+
require.Zero(t, badCalls.Load())
148+
if tt.tarball {
149+
require.Zero(t, goodCalls.Load())
150+
} else {
151+
require.EqualValues(t, 1, goodCalls.Load())
152+
}
153+
if tt.moby {
154+
require.Equal(t, "moby", so.Exports[0].Type)
155+
} else {
156+
require.Equal(t, "docker", so.Exports[0].Type)
157+
}
158+
})
159+
}
160+
}
161+
24162
func TestCacheOptions_DerivedVars(t *testing.T) {
25163
t.Setenv("ACTIONS_RUNTIME_TOKEN", "sensitive_token")
26164
t.Setenv("ACTIONS_CACHE_URL", "https://cache.github.com")

‎util/dockerutil/client.go‎

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,15 @@ import (
77

88
"github.com/docker/buildx/util/progress"
99
"github.com/docker/cli/cli/command"
10+
"github.com/moby/buildkit/util/flightcontrol"
1011
dockerclient "github.com/moby/moby/client"
1112
)
1213

1314
// Client represents an active docker object.
1415
type Client struct {
1516
cli command.Cli
1617

17-
featuresOnce sync.Once
18-
featuresCache map[Feature]bool
18+
features flightcontrol.CachedGroup[map[Feature]bool]
1919
}
2020

2121
// NewClient initializes a new docker client.
@@ -73,23 +73,22 @@ func (c *Client) LoadImage(ctx context.Context, name string, status progress.Wri
7373
}, nil
7474
}
7575

76-
func (c *Client) Features(ctx context.Context, name string) map[Feature]bool {
77-
c.featuresOnce.Do(func() {
78-
c.featuresCache = c.features(ctx, name)
79-
})
80-
return c.featuresCache
81-
}
82-
83-
func (c *Client) features(ctx context.Context, name string) map[Feature]bool {
84-
features := make(map[Feature]bool)
85-
if dapi, err := c.API(name); err == nil {
86-
if res, err := dapi.Info(ctx, dockerclient.InfoOptions{}); err == nil {
87-
if HasOCIImporter(res.Info) {
88-
features[OCIImporter] = true
89-
}
90-
}
76+
func (c *Client) Features(ctx context.Context, name string) (map[Feature]bool, error) {
77+
if name == "" {
78+
name = c.cli.CurrentContext()
9179
}
92-
return features
80+
return c.features.Do(ctx, name, func(ctx context.Context) (map[Feature]bool, error) {
81+
dapi, err := c.API(name)
82+
if err != nil {
83+
return nil, err
84+
}
85+
defer dapi.Close()
86+
res, err := dapi.Info(ctx, dockerclient.InfoOptions{})
87+
if err != nil {
88+
return nil, err
89+
}
90+
return map[Feature]bool{OCIImporter: HasOCIImporter(res.Info)}, nil
91+
})
9392
}
9493

9594
type waitingWriter struct {

‎util/dockerutil/client_test.go‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
package dockerutil
2+
3+
import (
4+
"context"
5+
"encoding/json"
6+
"net/http"
7+
"net/http/httptest"
8+
"strings"
9+
"sync/atomic"
10+
"testing"
11+
"time"
12+
13+
"github.com/docker/cli/cli/command"
14+
"github.com/docker/cli/cli/context/store"
15+
"github.com/moby/moby/api/types/system"
16+
"github.com/stretchr/testify/require"
17+
)
18+
19+
type featureTestCLI struct {
20+
command.Cli
21+
store store.Store
22+
currentContext string
23+
}
24+
25+
func (c featureTestCLI) ContextStore() store.Store {
26+
return c.store
27+
}
28+
29+
func (c featureTestCLI) CurrentContext() string {
30+
return c.currentContext
31+
}
32+
33+
func newFeatureTestServer(t *testing.T, info http.HandlerFunc) *httptest.Server {
34+
t.Helper()
35+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
36+
switch {
37+
case strings.HasSuffix(r.URL.Path, "/_ping"):
38+
w.Header().Set("API-Version", "1.55")
39+
case strings.HasSuffix(r.URL.Path, "/info"):
40+
w.Header().Set("Content-Type", "application/json")
41+
info(w, r)
42+
default:
43+
http.NotFound(w, r)
44+
}
45+
}))
46+
t.Cleanup(server.Close)
47+
return server
48+
}
49+
50+
func TestFeaturesRetry(t *testing.T) {
51+
for _, supported := range []bool{false, true} {
52+
t.Run(map[bool]string{false: "unsupported", true: "supported"}[supported], func(t *testing.T) {
53+
var calls atomic.Int32
54+
server := newFeatureTestServer(t, func(w http.ResponseWriter, r *http.Request) {
55+
if calls.Add(1) == 1 {
56+
w.WriteHeader(http.StatusServiceUnavailable)
57+
_, _ = w.Write([]byte(`{"message":"daemon is starting"}`))
58+
return
59+
}
60+
info := system.Info{}
61+
if supported {
62+
info.DriverStatus = [][2]string{{"driver-type", "io.containerd.snapshotter.v1"}}
63+
}
64+
_ = json.NewEncoder(w).Encode(info)
65+
})
66+
c := NewClient(featureTestCLI{store: store.New(t.TempDir(), command.DefaultContextStoreConfig())})
67+
ctx := t.Context()
68+
features, err := c.Features(ctx, server.URL)
69+
require.ErrorContains(t, err, "daemon is starting")
70+
require.Nil(t, features)
71+
for range 2 {
72+
features, err = c.Features(ctx, server.URL)
73+
require.NoError(t, err)
74+
require.Equal(t, supported, features[OCIImporter])
75+
}
76+
require.EqualValues(t, 2, calls.Load())
77+
})
78+
}
79+
}
80+
81+
func TestFeaturesContexts(t *testing.T) {
82+
var supportedCalls, unsupportedCalls atomic.Int32
83+
supported := newFeatureTestServer(t, func(w http.ResponseWriter, r *http.Request) {
84+
supportedCalls.Add(1)
85+
_ = json.NewEncoder(w).Encode(system.Info{DriverStatus: [][2]string{{"driver-type", "io.containerd.snapshotter.v1"}}})
86+
})
87+
unsupported := newFeatureTestServer(t, func(w http.ResponseWriter, r *http.Request) {
88+
unsupportedCalls.Add(1)
89+
_ = json.NewEncoder(w).Encode(system.Info{})
90+
})
91+
c := NewClient(featureTestCLI{
92+
store: store.New(t.TempDir(), command.DefaultContextStoreConfig()),
93+
currentContext: supported.URL,
94+
})
95+
for range 2 {
96+
for _, name := range []string{"", unsupported.URL, supported.URL} {
97+
features, err := c.Features(t.Context(), name)
98+
require.NoError(t, err)
99+
require.Equal(t, name != unsupported.URL, features[OCIImporter])
100+
}
101+
}
102+
require.EqualValues(t, 1, supportedCalls.Load())
103+
require.EqualValues(t, 1, unsupportedCalls.Load())
104+
}
105+
106+
func TestFeaturesCancellation(t *testing.T) {
107+
entered := make(chan struct{})
108+
release := make(chan struct{})
109+
var calls atomic.Int32
110+
server := newFeatureTestServer(t, func(w http.ResponseWriter, r *http.Request) {
111+
if calls.Add(1) == 1 {
112+
close(entered)
113+
select {
114+
case <-release:
115+
case <-r.Context().Done():
116+
}
117+
return
118+
}
119+
_ = json.NewEncoder(w).Encode(system.Info{DriverStatus: [][2]string{{"driver-type", "io.containerd.snapshotter.v1"}}})
120+
})
121+
t.Cleanup(func() { close(release) })
122+
c := NewClient(featureTestCLI{store: store.New(t.TempDir(), command.DefaultContextStoreConfig())})
123+
ctx, cancel := context.WithTimeoutCause(t.Context(), 10*time.Second, context.DeadlineExceeded)
124+
defer cancel()
125+
probeCtx, cancelProbe := context.WithCancelCause(ctx)
126+
defer cancelProbe(context.Canceled)
127+
done := make(chan error, 1)
128+
go func() {
129+
_, err := c.Features(probeCtx, server.URL)
130+
done <- err
131+
}()
132+
select {
133+
case <-entered:
134+
case <-ctx.Done():
135+
t.Fatal("probe did not start")
136+
}
137+
cancelProbe(context.Canceled)
138+
select {
139+
case err := <-done:
140+
require.ErrorIs(t, err, context.Canceled)
141+
case <-ctx.Done():
142+
t.Fatal("probe did not stop after cancellation")
143+
}
144+
features, err := c.Features(ctx, server.URL)
145+
require.NoError(t, err)
146+
require.True(t, features[OCIImporter])
147+
require.EqualValues(t, 2, calls.Load())
148+
}

0 commit comments

Comments
 (0)