[v0.37] cherry-picks for v0.37.1

build/build.go

@@ -432,6 +432,41 @@ func toRepoOnly(in string) (string, error) {
 	return strings.Join(out, ","), nil
 }
 
+func prepareMultiDriverExports(so *client.SolveOpt, pushNames *string, insecurePush *bool) error {
+	var pushPrepared bool
+	for i := range so.Exports {
+		e := &so.Exports[i]
+		switch e.Type {
+		case "oci", "tar":
+			return errors.Errorf("%s for multi-node builds currently not supported", e.Type)
+		case "image":
+			if pushPrepared {
+				continue
+			}
+			if ok, _ := strconv.ParseBool(e.Attrs["push"]); !ok {
+				continue
+			}
+			if *pushNames == "" {
+				*pushNames = e.Attrs["name"]
+				if *pushNames == "" {
+					return errors.Errorf("tag is needed when pushing to registry")
+				}
+				if ok, _ := strconv.ParseBool(e.Attrs["registry.insecure"]); ok {
+					*insecurePush = true
+				}
+			}
+			names, err := toRepoOnly(e.Attrs["name"])
+			if err != nil {
+				return err
+			}
+			e.Attrs["name"] = names
+			e.Attrs["push-by-digest"] = "true"
+			pushPrepared = true
+		}
+	}
+	return nil
+}
+
 type (
 	EvaluateFunc func(ctx context.Context, name string, c gateway.Client, res *gateway.Result, opt Options) error
 	Handler      struct {
@@ -566,30 +601,8 @@ func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opts map[
 				node := dp.Node()
 				so := reqForNodes[k][i].so
 				if multiDriver {
-					for i, e := range so.Exports {
-						switch e.Type {
-						case "oci", "tar":
-							return errors.Errorf("%s for multi-node builds currently not supported", e.Type)
-						case "image":
-							if pushNames == "" && e.Attrs["push"] != "" {
-								if ok, _ := strconv.ParseBool(e.Attrs["push"]); ok {
-									pushNames = e.Attrs["name"]
-									if pushNames == "" {
-										return errors.Errorf("tag is needed when pushing to registry")
-									}
-									names, err := toRepoOnly(e.Attrs["name"])
-									if err != nil {
-										return err
-									}
-									if ok, _ := strconv.ParseBool(e.Attrs["registry.insecure"]); ok {
-										insecurePush = true
-									}
-									e.Attrs["name"] = names
-									e.Attrs["push-by-digest"] = "true"
-									so.Exports[i].Attrs = e.Attrs
-								}
-							}
-						}
+					if err := prepareMultiDriverExports(so, &pushNames, &insecurePush); err != nil {
+						return err
 					}
 				}
 
@@ -602,7 +615,8 @@ func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opts map[
 					// shared solver vertices land on the same daemon instance
 					c = linkedClients[node.Name]
 					if c == nil {
-						c, err = dp.Client(ctx)
+						// The shared client must outlive this target's errgroup.
+						c, err = dp.Client(baseCtx)
 						if err == nil {
 							linkedClients[node.Name] = c
 						}

build/build_test.go

@@ -35,6 +35,48 @@ func (d warnOutputDriver) IsMobyDriver() bool {
 	return d.moby
 }
 
+func TestPrepareMultiDriverExportsForEveryNode(t *testing.T) {
+	const taggedName = "registry.example.com/user/app:latest"
+	const secondaryName = "registry.example.com/user/secondary:latest"
+	newSolveOpt := func() *client.SolveOpt {
+		return &client.SolveOpt{
+			Exports: []client.ExportEntry{
+				{
+					Type: "image",
+					Attrs: map[string]string{
+						"name":              taggedName,
+						"push":              "true",
+						"registry.insecure": "true",
+					},
+				},
+				{
+					Type: "image",
+					Attrs: map[string]string{
+						"name": secondaryName,
+						"push": "true",
+					},
+				},
+			},
+		}
+	}
+	solveOpts := []*client.SolveOpt{newSolveOpt(), newSolveOpt()}
+
+	var pushNames string
+	var insecurePush bool
+	for _, so := range solveOpts {
+		require.NoError(t, prepareMultiDriverExports(so, &pushNames, &insecurePush))
+	}
+
+	require.Equal(t, taggedName, pushNames)
+	require.True(t, insecurePush)
+	for _, so := range solveOpts {
+		require.Equal(t, "registry.example.com/user/app", so.Exports[0].Attrs["name"])
+		require.Equal(t, "true", so.Exports[0].Attrs["push-by-digest"])
+		require.Equal(t, secondaryName, so.Exports[1].Attrs["name"])
+		require.NotContains(t, so.Exports[1].Attrs, "push-by-digest")
+	}
+}
+
 func TestWarnOnNoOutput(t *testing.T) {
 	cloudNodes := []builder.Node{{Driver: newWarnOutputDriver("cloud", false)}}
 	mobyNodes := []builder.Node{{Driver: newWarnOutputDriver("docker", true)}}

commands/rm.go

@@ -91,7 +91,7 @@ func runRm(ctx context.Context, dockerCli command.Cli, in rmOptions) error {
 					return err
 				}
 
-				err1 := rm(timeoutCtx, nodes, in)
+				err1 := rm(ctx, nodes, in)
 				if err := txn.Remove(b.Name); err != nil {
 					return err
 				}
@@ -200,7 +200,7 @@ func rmAllInactive(ctx context.Context, txn *store.Txn, dockerCli command.Cli, i
 					return nil
 				}
 				if b.Inactive() {
-					rmerr := rm(timeoutCtx, nodes, in)
+					rmerr := rm(ctx, nodes, in)
 					if err := txn.Remove(b.Name); err != nil {
 						return err
 					}

driver/docker-container/driver.go

@@ -35,9 +35,10 @@ import (
 )
 
 const (
-	volumeStateSuffix       = "_state"
-	buildkitdConfigFile     = "buildkitd.toml"
-	buildkitdStartupTimeout = 20 * time.Second
+	volumeStateSuffix      = "_state"
+	buildkitdConfigFile    = "buildkitd.toml"
+	buildkitdStartupWindow = 20 * time.Second
+	buildkitdReadyTimeout  = 20 * time.Second
 )
 
 type Driver struct {
@@ -525,7 +526,7 @@ func (d *Driver) Client(ctx context.Context, opts ...client.ClientOpt) (*client.
 		}
 		return nil, errors.WithStack(err)
 	}
-	waitDeadline, err := clientWaitDeadline(res.Container.State, time.Now())
+	waitReady, err := clientWaitReady(res.Container.State, time.Now())
 	if err != nil {
 		return nil, err
 	}
@@ -549,11 +550,11 @@ func (d *Driver) Client(ctx context.Context, opts ...client.ClientOpt) (*client.
 		_ = conn.Close()
 		return nil, err
 	}
-	if waitDeadline.IsZero() {
+	if !waitReady {
 		return c, nil
 	}
 
-	waitCtx, cancel := context.WithDeadlineCause(ctx, waitDeadline, errors.WithStack(context.DeadlineExceeded))
+	waitCtx, cancel := context.WithTimeoutCause(ctx, buildkitdReadyTimeout, errors.WithStack(context.DeadlineExceeded))
 	defer cancel()
 	if err := c.Wait(waitCtx); err != nil {
 		_ = c.Close()
@@ -562,22 +563,18 @@ func (d *Driver) Client(ctx context.Context, opts ...client.ClientOpt) (*client.
 	return c, nil
 }
 
-func clientWaitDeadline(state *container.State, now time.Time) (time.Time, error) {
+func clientWaitReady(state *container.State, now time.Time) (bool, error) {
 	if state == nil || !state.Running {
-		return time.Time{}, driver.ErrNotRunning{}
+		return false, driver.ErrNotRunning{}
 	}
 	// Docker reports a container as running before buildkitd has bound its
-	// socket. Wait only during that startup window so an established but broken
-	// builder still returns its connection error promptly.
+	// socket. Use the startup window only to decide whether to wait so an
+	// established but broken builder still returns its connection error promptly.
 	startedAt, err := time.Parse(time.RFC3339Nano, state.StartedAt)
 	if err != nil {
-		return time.Time{}, nil
+		return false, nil
 	}
-	deadline := startedAt.Add(buildkitdStartupTimeout)
-	if !now.Before(deadline) {
-		return time.Time{}, nil
-	}
-	return deadline, nil
+	return now.Before(startedAt.Add(buildkitdStartupWindow)), nil
 }
 
 func (d *Driver) Factory() driver.Factory {

driver/docker-container/driver_test.go

@@ -9,40 +9,35 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestClientWaitDeadline(t *testing.T) {
+func TestClientWaitReady(t *testing.T) {
 	now := time.Now()
+	running := func(startedAt string) *container.State {
+		return &container.State{Running: true, StartedAt: startedAt}
+	}
+	tests := []struct {
+		name     string
+		state    *container.State
+		wantWait bool
+		wantErr  error
+	}{
+		{name: "missing-state-fails-fast", wantErr: driver.ErrNotRunning{}},
+		{name: "stopped-builder-fails-fast", state: &container.State{}, wantErr: driver.ErrNotRunning{}},
+		{name: "established-builder-skips-wait", state: running(now.Add(-2 * buildkitdStartupWindow).Format(time.RFC3339Nano))},
+		{name: "recent-start-builder-waits", state: running(now.Add(-time.Second).Format(time.RFC3339Nano)), wantWait: true},
+		{name: "nearly-expired-startup-window-waits", state: running(now.Add(-buildkitdStartupWindow + time.Nanosecond).Format(time.RFC3339Nano)), wantWait: true},
+		{name: "expired-startup-window-skips-wait", state: running(now.Add(-buildkitdStartupWindow).Format(time.RFC3339Nano))},
+		{name: "invalid-start-time-skips-wait", state: running("invalid")},
+	}
 
-	t.Run("stopped-builder-fails-fast", func(t *testing.T) {
-		deadline, err := clientWaitDeadline(&container.State{}, now)
-		require.ErrorIs(t, err, driver.ErrNotRunning{})
-		require.True(t, deadline.IsZero())
-	})
-
-	t.Run("established-builder-skips-wait", func(t *testing.T) {
-		deadline, err := clientWaitDeadline(&container.State{
-			Running:   true,
-			StartedAt: now.Add(-2 * buildkitdStartupTimeout).Format(time.RFC3339Nano),
-		}, now)
-		require.NoError(t, err)
-		require.True(t, deadline.IsZero())
-	})
-
-	t.Run("recent-start-builder-waits", func(t *testing.T) {
-		startedAt := now.Add(-time.Second)
-		deadline, err := clientWaitDeadline(&container.State{
-			Running:   true,
-			StartedAt: startedAt.Format(time.RFC3339Nano),
-		}, now)
-		require.NoError(t, err)
-		require.True(t, deadline.Equal(startedAt.Add(buildkitdStartupTimeout)))
-	})
-
-	t.Run("invalid-start-time-skips-wait", func(t *testing.T) {
-		deadline, err := clientWaitDeadline(&container.State{
-			Running:   true,
-			StartedAt: "invalid",
-		}, now)
-		require.NoError(t, err)
-		require.True(t, deadline.IsZero())
-	})
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			wait, err := clientWaitReady(tt.state, now)
+			if tt.wantErr != nil {
+				require.ErrorIs(t, err, tt.wantErr)
+			} else {
+				require.NoError(t, err)
+			}
+			require.Equal(t, tt.wantWait, wait)
+		})
+	}
 }

tests/build.go

@@ -67,6 +67,7 @@ var buildTests = []func(t *testing.T, sb integration.Sandbox){
 	testBuildRegistryExport,
 	testBuildRegistryExportAttestations,
 	testBuildRegistryExportNoDefaultOCIArtifact,
+	testBuildMultiNodePushFailureDoesNotPublishTag,
 	testBuildTarExport,
 	testBuildMobyFromLocalImage,
 	testBuildDetailsLink,
@@ -689,6 +690,34 @@ func requireLegacyAttestationStorage(t *testing.T, sb integration.Sandbox, ref s
 	require.NotEmpty(t, mfst.Layers)
 }
 
+func testBuildMultiNodePushFailureDoesNotPublishTag(t *testing.T, sb integration.Sandbox) {
+	if !isRemoteMultiNodeWorker(sb) {
+		t.Skip("only testing with remote multi-node worker")
+	}
+
+	dir := createTestProject(t)
+	err := os.WriteFile(filepath.Join(dir, "Dockerfile"), []byte(`FROM alpine
+ARG TARGETARCH
+RUN if [ "$TARGETARCH" = arm64 ]; then exit 1; fi
+RUN echo "$TARGETARCH" > /platform
+`), 0644)
+	require.NoError(t, err)
+
+	registry, err := sb.NewRegistry()
+	if errors.Is(err, integration.ErrRequirements) {
+		t.Skip(err.Error())
+	}
+	require.NoError(t, err)
+
+	target := registry + "/buildx/partial-push:" + identity.NewID()
+
+	out, err := buildCmd(sb, withArgs("-t", target, "--push", "--platform=linux/amd64,linux/arm64", "--provenance=false", dir))
+	require.Error(t, err, string(out))
+
+	_, _, err = contentutil.ProviderFromRef(sb.Context(), target)
+	require.Error(t, err)
+}
+
 func testImageIDOutput(t *testing.T, sb integration.Sandbox) {
 	dockerfile := []byte(`FROM busybox:latest`)