Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 32 additions & 9 deletions driver/docker-container/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
18 changes: 12 additions & 6 deletions driver/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
36 changes: 36 additions & 0 deletions driver/manager_test.go
Original file line number Diff line number Diff line change
@@ -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
}
79 changes: 79 additions & 0 deletions tests/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
Loading