From 833613b100987078951b3b5144a190f33bf3d71f Mon Sep 17 00:00:00 2001 From: CrazyMax <1951866+crazy-max@users.noreply.github.com> Date: Thu, 20 Aug 2026 13:04:59 +0200 Subject: [PATCH] docker-container: wait for BuildKit before dialing Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com> --- driver/docker-container/driver.go | 41 ++++++++++++---- driver/manager.go | 18 ++++--- driver/manager_test.go | 36 ++++++++++++++ tests/build.go | 79 +++++++++++++++++++++++++++++++ 4 files changed, 159 insertions(+), 15 deletions(-) create mode 100644 driver/manager_test.go diff --git a/driver/docker-container/driver.go b/driver/docker-container/driver.go index a7688606995d..21a4f8668766 100644 --- a/driver/docker-container/driver.go +++ b/driver/docker-container/driver.go @@ -329,13 +329,15 @@ func (d *Driver) wait(ctx context.Context, l progress.SubLogger) error { bufStdout := &bytes.Buffer{} bufStderr := &bytes.Buffer{} if err := d.run(ctx, []string{"buildctl", "debug", "workers"}, bufStdout, bufStderr); err != nil { - if try > 15 { - d.copyLogs(context.TODO(), l) - if bufStdout.Len() != 0 { - l.Log(1, bufStdout.Bytes()) - } - if bufStderr.Len() != 0 { - l.Log(2, bufStderr.Bytes()) + if try > 15 || !isReadinessStartupError(err) { + if l != nil { + d.copyLogs(context.TODO(), l) + if bufStdout.Len() != 0 { + l.Log(1, bufStdout.Bytes()) + } + if bufStderr.Len() != 0 { + l.Log(2, bufStderr.Bytes()) + } } return err } @@ -351,6 +353,20 @@ func (d *Driver) wait(ctx context.Context, l progress.SubLogger) error { } } +func isReadinessStartupError(err error) bool { + if errors.Is(err, errExecExit) { + return true + } + if cerrdefs.IsNotFound(err) || cerrdefs.IsUnavailable(err) { + return true + } + if cerrdefs.IsConflict(err) { + msg := strings.ToLower(err.Error()) + return strings.Contains(msg, "is not running") || strings.Contains(msg, "is restarting") + } + return false +} + func (d *Driver) copyLogs(ctx context.Context, l progress.SubLogger) error { rc, err := d.DockerAPI.ContainerLogs(ctx, d.Name, dockerclient.ContainerLogsOptions{ ShowStdout: true, ShowStderr: true, @@ -403,7 +419,9 @@ func (d *Driver) exec(ctx context.Context, cmd []string) (string, net.Conn, erro return execID, resp.Conn, nil } -func (d *Driver) run(ctx context.Context, cmd []string, stdout, stderr io.Writer) (err error) { +var errExecExit = errors.New("exec exit") + +func (d *Driver) run(ctx context.Context, cmd []string, stdout, stderr *bytes.Buffer) (err error) { id, conn, err := d.exec(ctx, cmd) if err != nil { return err @@ -417,7 +435,7 @@ func (d *Driver) run(ctx context.Context, cmd []string, stdout, stderr io.Writer return err } if resp.ExitCode != 0 { - return errors.Errorf("exit code %d", resp.ExitCode) + return errors.Wrapf(errExecExit, "exit code %d\nstdout: %s\nstderr: %s", resp.ExitCode, strings.TrimSpace(stdout.String()), strings.TrimSpace(stderr.String())) } return nil } @@ -508,6 +526,11 @@ func (d *Driver) Rm(ctx context.Context, force, rmVolume, rmDaemon bool) error { } func (d *Driver) Dial(ctx context.Context) (net.Conn, error) { + // Docker marks the container running before buildkitd has necessarily + // bound its socket, so verify readiness before opening dial-stdio. + if err := d.wait(ctx, nil); err != nil { + return nil, err + } _, conn, err := d.exec(ctx, []string{"buildctl", "dial-stdio"}) if err != nil { return nil, err diff --git a/driver/manager.go b/driver/manager.go index b9ef2c275af5..881b3ac12217 100644 --- a/driver/manager.go +++ b/driver/manager.go @@ -127,17 +127,23 @@ func GetFactories(instanceRequired bool) []Factory { type DriverHandle struct { Driver client *client.Client - err error - once sync.Once + clientMu sync.Mutex historyAPISupportedOnce sync.Once historyAPISupported bool } func (d *DriverHandle) Client(ctx context.Context, opt ...client.ClientOpt) (*client.Client, error) { - d.once.Do(func() { - d.client, d.err = d.Driver.Client(ctx, append(d.getClientOptions(), opt...)...) - }) - return d.client, d.err + d.clientMu.Lock() + defer d.clientMu.Unlock() + if d.client != nil { + return d.client, nil + } + c, err := d.Driver.Client(ctx, append(d.getClientOptions(), opt...)...) + if err != nil { + return nil, err + } + d.client = c + return c, nil } func (d *DriverHandle) UncachedClient(ctx context.Context) (*client.Client, error) { diff --git a/driver/manager_test.go b/driver/manager_test.go new file mode 100644 index 000000000000..a9d552a7e47d --- /dev/null +++ b/driver/manager_test.go @@ -0,0 +1,36 @@ +package driver + +import ( + "context" + "testing" + + "github.com/moby/buildkit/client" + "github.com/stretchr/testify/require" +) + +func TestBootRetriesClientAfterErrNotRunning(t *testing.T) { + d := &retryDriver{client: &client.Client{}} + + c, err := Boot(context.Background(), context.Background(), &DriverHandle{Driver: d}, nil) + require.NoError(t, err) + require.Same(t, d.client, c) + require.Equal(t, 2, d.clientCalls) +} + +type retryDriver struct { + Driver + client *client.Client + clientCalls int +} + +func (d *retryDriver) Info(context.Context) (*Info, error) { + return &Info{Status: Running}, nil +} + +func (d *retryDriver) Client(context.Context, ...client.ClientOpt) (*client.Client, error) { + d.clientCalls++ + if d.clientCalls == 1 { + return nil, ErrNotRunning{} + } + return d.client, nil +} diff --git a/tests/build.go b/tests/build.go index 1b8c7a2504e3..5b7a9a1286e7 100644 --- a/tests/build.go +++ b/tests/build.go @@ -14,12 +14,14 @@ import ( "regexp" "strings" "testing" + "time" "github.com/containerd/containerd/v2/core/content" "github.com/containerd/containerd/v2/plugins/content/local" "github.com/containerd/continuity/fs/fstest" "github.com/containerd/platforms" "github.com/creack/pty" + "github.com/docker/buildx/driver" "github.com/docker/buildx/localstate" "github.com/docker/buildx/util/confutil" "github.com/docker/buildx/util/gitutil" @@ -42,6 +44,7 @@ import ( "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/sync/errgroup" ) func buildCmd(sb integration.Sandbox, opts ...cmdOpt) (string, error) { @@ -94,6 +97,7 @@ var buildTests = []func(t *testing.T, sb integration.Sandbox){ testBuildCheckCallOutput, testBuildExtraHosts, testBuildIndexAnnotationsLoadDocker, + testBuildDockerContainerConcurrentFirstBuild, } func testBuild(t *testing.T, sb integration.Sandbox) { @@ -1831,6 +1835,81 @@ func testBuildIndexAnnotationsLoadDocker(t *testing.T, sb integration.Sandbox) { require.Contains(t, out, "index annotations not supported for single platform export") } +func testBuildDockerContainerConcurrentFirstBuild(t *testing.T, sb integration.Sandbox) { + if !isDockerContainerWorker(sb) { + t.Skip("only testing with docker-container worker") + } + integration.SkipOnPlatform(t, "windows") + + dir := tmpdir(t, + fstest.CreateFile("Dockerfile", []byte("FROM scratch\nCOPY marker /marker\n"), 0o600), + fstest.CreateFile("marker", []byte("hi"), 0o600), + ) + + builderName := "concurrent-" + identity.NewID() + imageName := "buildx-test-buildkit-delay:" + identity.NewID() + imageDir := tmpdir(t, + fstest.CreateFile("Dockerfile", fmt.Appendf(nil, `FROM %s +COPY buildkitd-delay /usr/bin/buildkitd-delay +RUN chmod +x /usr/bin/buildkitd-delay +ENTRYPOINT ["/usr/bin/buildkitd-delay"] +`, buildkitImage), 0o600), + fstest.CreateFile("buildkitd-delay", []byte("#!/bin/sh\nsleep 3\nexec /usr/bin/buildkitd \"$@\"\n"), 0o600), + ) + dockerOut, err := dockerCmd(sb, withArgs("build", "-t", imageName, imageDir)).CombinedOutput() + require.NoError(t, err, string(dockerOut)) + t.Cleanup(func() { + dockerOut, err := dockerCmd(sb, withArgs("image", "rm", "-f", imageName)).CombinedOutput() + require.NoError(t, err, string(dockerOut)) + }) + + out, err := createCmd(sb, withArgs( + "--name", builderName, + "--driver", "docker-container", + "--driver-opt", "image="+imageName, + )) + require.NoError(t, err, out) + t.Cleanup(func() { + out, err := rmCmd(sb, withArgs("-f", builderName)) + require.NoError(t, err, out) + }) + + var eg errgroup.Group + var waited bool + defer func() { + if !waited { + require.NoError(t, eg.Wait()) + } + }() + startBuild := func(name string) { + eg.Go(func() error { + cmd := buildxCmd(sb, withArgs("build", "--progress=quiet", "--output=type=cacheonly", dir)) + cmd.Env = append(cmd.Env, "BUILDX_BUILDER="+builderName) + out, err := cmd.CombinedOutput() + if err != nil { + return errors.Errorf("%s failed: %v\n%s", name, err, string(out)) + } + return nil + }) + } + + startBuild("bootstrap") + containerName := fmt.Sprintf("%s0", driver.BuilderName(builderName)) + require.EventuallyWithT(t, func(c *assert.CollectT) { + cmd := dockerCmd(sb, withArgs("inspect", "-f", "{{.State.Running}}", containerName)) + out, err := cmd.CombinedOutput() + assert.NoError(c, err, string(out)) + assert.Equal(c, "true", strings.TrimSpace(string(out))) + }, 60*time.Second, 20*time.Millisecond, "container %s did not report running", containerName) + + for i := range 5 { + startBuild(fmt.Sprintf("sibling %d", i+1)) + } + err = eg.Wait() + waited = true + require.NoError(t, err) +} + func createTestProject(t *testing.T) string { dockerfile := []byte(` FROM busybox:latest AS base