Merge pull request #2333 from tonistiigi/v0.13.1-picks

[v0.13] cherry-picks for v0.13.1
docker · Mar 12, 2024 · 7884339 · 7884339
2 parents 37b7ad1 + 7e24604
commit 7884339
Show file tree

Hide file tree

Showing 9 changed files with 106 additions and 12 deletions.
diff --git a/bake/bake_test.go b/bake/bake_test.go
@@ -259,6 +259,25 @@ func TestPushOverride(t *testing.T) {
 
 	require.Equal(t, 1, len(m["app"].Outputs))
 	require.Equal(t, "type=image,push=true", m["app"].Outputs[0])
+
+	fp = File{
+		Name: "docker-bake.hcl",
+		Data: []byte(
+			`target "foo" {
+		  		output = [ "type=local,dest=out" ]
+			}
+			target "bar" {
+			}`),
+	}
+	ctx = context.TODO()
+	m, _, err = ReadTargets(ctx, []File{fp}, []string{"foo", "bar"}, []string{"*.push=true"}, nil)
+	require.NoError(t, err)
+
+	require.Equal(t, 2, len(m))
+	require.Equal(t, 1, len(m["foo"].Outputs))
+	require.Equal(t, []string{"type=local,dest=out"}, m["foo"].Outputs)
+	require.Equal(t, 1, len(m["bar"].Outputs))
+	require.Equal(t, []string{"type=image,push=true"}, m["bar"].Outputs)
 }
 
 func TestReadTargetsCompose(t *testing.T) {

diff --git a/builder/node.go b/builder/node.go
@@ -142,7 +142,7 @@ func (b *Builder) LoadNodes(ctx context.Context, opts ...LoadNodesOption) (_ []N
 					}
 				}
 
-				d, err := driver.GetDriver(ctx, "buildx_buildkit_"+n.Name, factory, n.Endpoint, dockerapi, imageopt.Auth, kcc, n.BuildkitdFlags, n.Files, n.DriverOpts, n.Platforms, b.opts.contextPathHash, lno.dialMeta)
+				d, err := driver.GetDriver(ctx, driver.BuilderName(n.Name), factory, n.Endpoint, dockerapi, imageopt.Auth, kcc, n.BuildkitdFlags, n.Files, n.DriverOpts, n.Platforms, b.opts.contextPathHash, lno.dialMeta)
 				if err != nil {
 					node.Err = err
 					return nil

diff --git a/commands/bake.go b/commands/bake.go
@@ -72,7 +72,7 @@ func runBake(ctx context.Context, dockerCli command.Cli, targets []string, in ba
 
 	overrides := in.overrides
 	if in.exportPush {
-		overrides = append(overrides, "*.output=type=registry")
+		overrides = append(overrides, "*.push=true")
 	}
 	if in.exportLoad {
 		overrides = append(overrides, "*.output=type=docker")

diff --git a/driver/driver.go b/driver/driver.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"io"
 	"net"
+	"strings"
 
 	"github.com/docker/buildx/store"
 	"github.com/docker/buildx/util/progress"
@@ -67,6 +68,19 @@ type Driver interface {
 	Config() InitConfig
 }
 
+const builderNamePrefix = "buildx_buildkit_"
+
+func BuilderName(name string) string {
+	return builderNamePrefix + name
+}
+
+func ParseBuilderName(name string) (string, error) {
+	if !strings.HasPrefix(name, builderNamePrefix) {
+		return "", errors.Errorf("invalid builder name %q, must have %q prefix", name, builderNamePrefix)
+	}
+	return strings.TrimPrefix(name, builderNamePrefix), nil
+}
+
 func Boot(ctx, clientContext context.Context, d *DriverHandle, pw progress.Writer) (*client.Client, error) {
 	try := 0
 	for {

diff --git a/driver/kubernetes/factory.go b/driver/kubernetes/factory.go
@@ -244,10 +244,10 @@ func (f *factory) AllowsInstances() bool {
 // eg. "buildx_buildkit_loving_mendeleev0" -> "loving-mendeleev0"
 func buildxNameToDeploymentName(bx string) (string, error) {
 	// TODO: commands.util.go should not pass "buildx_buildkit_" prefix to drivers
-	if !strings.HasPrefix(bx, "buildx_buildkit_") {
-		return "", errors.Errorf("expected a string with \"buildx_buildkit_\", got %q", bx)
+	s, err := driver.ParseBuilderName(bx)
+	if err != nil {
+		return "", err
 	}
-	s := strings.TrimPrefix(bx, "buildx_buildkit_")
 	s = strings.ReplaceAll(s, "_", "-")
 	return s, nil
 }
diff --git a/driver/kubernetes/factory_test.go b/driver/kubernetes/factory_test.go
@@ -29,7 +29,7 @@ func TestFactory_processDriverOpts(t *testing.T) {
 	}
 
 	cfg := driver.InitConfig{
-		Name:             "buildx_buildkit_test",
+		Name:             driver.BuilderName("test"),
 		KubeClientConfig: &kcc,
 	}
 	f := factory{}

diff --git a/driver/remote/driver.go b/driver/remote/driver.go
@@ -13,6 +13,7 @@ import (
 	util "github.com/docker/buildx/driver/remote/util"
 	"github.com/docker/buildx/util/progress"
 	"github.com/moby/buildkit/client"
+	"github.com/moby/buildkit/client/connhelper"
 	"github.com/moby/buildkit/util/tracing/detect"
 	"github.com/pkg/errors"
 )
@@ -95,7 +96,16 @@ func (d *Driver) Client(ctx context.Context) (*client.Client, error) {
 }
 
 func (d *Driver) Dial(ctx context.Context) (net.Conn, error) {
-	network, addr, ok := strings.Cut(d.InitConfig.EndpointAddr, "://")
+	addr := d.InitConfig.EndpointAddr
+	ch, err := connhelper.GetConnectionHelper(addr)
+	if err != nil {
+		return nil, err
+	}
+	if ch != nil {
+		return ch.ContextDialer(ctx, addr)
+	}
+
+	network, addr, ok := strings.Cut(addr, "://")
 	if !ok {
 		return nil, errors.Errorf("invalid endpoint address: %s", d.InitConfig.EndpointAddr)
 	}

diff --git a/tests/bake.go b/tests/bake.go
@@ -778,11 +778,11 @@ target "default" {
 	outb, err := cmd.CombinedOutput()
 	require.NoError(t, err, string(outb))
 
-	// test registry
-	desc, provider, err := contentutil.ProviderFromRef(target)
-	require.NoError(t, err)
-	_, err = testutil.ReadImages(sb.Context(), provider, desc)
-	require.NoError(t, err)
+	// TODO: test registry when --load case fixed for bake (currently overrides --push)
+	//desc, provider, err := contentutil.ProviderFromRef(target)
+	//require.NoError(t, err)
+	//_, err = testutil.ReadImages(sb.Context(), provider, desc)
+	//require.NoError(t, err)
 
 	// test docker store
 	cmd = dockerCmd(sb, withArgs("image", "inspect", target))

diff --git a/tests/create.go b/tests/create.go
@@ -1,9 +1,13 @@
 package tests
 
 import (
+	"fmt"
+	"os"
 	"strings"
 	"testing"
 
+	"github.com/docker/buildx/driver"
+	"github.com/moby/buildkit/identity"
 	"github.com/moby/buildkit/util/testutil/integration"
 	"github.com/stretchr/testify/require"
 )
@@ -18,6 +22,7 @@ func createCmd(sb integration.Sandbox, opts ...cmdOpt) (string, error) {
 var createTests = []func(t *testing.T, sb integration.Sandbox){
 	testCreateMemoryLimit,
 	testCreateRestartAlways,
+	testCreateRemoteContainer,
 }
 
 func testCreateMemoryLimit(t *testing.T, sb integration.Sandbox) {
@@ -57,3 +62,49 @@ func testCreateRestartAlways(t *testing.T, sb integration.Sandbox) {
 	require.NoError(t, err, out)
 	builderName = strings.TrimSpace(out)
 }
+
+func testCreateRemoteContainer(t *testing.T, sb integration.Sandbox) {
+	if sb.Name() != "docker" {
+		t.Skip("skipping test for non-docker workers")
+	}
+
+	ctnBuilderName := "ctn-builder-" + identity.NewID()
+	remoteBuilderName := "remote-builder-" + identity.NewID()
+	var hasCtnBuilder, hasRemoteBuilder bool
+	t.Cleanup(func() {
+		if hasCtnBuilder {
+			out, err := rmCmd(sb, withArgs(ctnBuilderName))
+			require.NoError(t, err, out)
+		}
+		if hasRemoteBuilder {
+			out, err := rmCmd(sb, withArgs(remoteBuilderName))
+			require.NoError(t, err, out)
+		}
+	})
+
+	out, err := createCmd(sb, withArgs("--driver", "docker-container", "--name", ctnBuilderName))
+	require.NoError(t, err, out)
+	hasCtnBuilder = true
+
+	out, err = inspectCmd(sb, withArgs("--bootstrap", ctnBuilderName))
+	require.NoError(t, err, out)
+
+	cmd := dockerCmd(sb, withArgs("container", "inspect", fmt.Sprintf("%s0", driver.BuilderName(ctnBuilderName))))
+	cmd.Stderr = os.Stderr
+	require.NoError(t, cmd.Run())
+
+	out, err = createCmd(sb, withArgs("--driver", "remote", "--name", remoteBuilderName, fmt.Sprintf("docker-container://%s0", driver.BuilderName(ctnBuilderName))))
+	require.NoError(t, err, out)
+	hasRemoteBuilder = true
+
+	out, err = inspectCmd(sb, withArgs(remoteBuilderName))
+	require.NoError(t, err, out)
+
+	for _, line := range strings.Split(out, "\n") {
+		if v, ok := strings.CutPrefix(line, "Status:"); ok {
+			require.Equal(t, strings.TrimSpace(v), "running")
+			return
+		}
+	}
+	require.Fail(t, "remote builder is not running")
+}