From ccc314a823c528dd85fa41169b41d69da4185c79 Mon Sep 17 00:00:00 2001 From: Niklas Gehlen Date: Fri, 5 Apr 2024 15:30:33 +0200 Subject: [PATCH] Implement new driver-opt: default-load This eases build driver migrations, as it allows aligning the default behavior. See also https://docs.docker.com/build/drivers/ Signed-off-by: Niklas Gehlen --- build/build.go | 4 +++ build/opt.go | 12 +++++--- driver/docker-container/driver.go | 2 ++ driver/docker-container/factory.go | 5 ++++ driver/docker/driver.go | 1 + driver/features.go | 2 ++ driver/kubernetes/driver.go | 2 ++ driver/kubernetes/factory.go | 35 ++++++++++++++--------- driver/kubernetes/factory_test.go | 28 +++++++++++-------- driver/remote/driver.go | 2 ++ driver/remote/factory.go | 7 +++++ tests/build.go | 45 ++++++++++++++++++++++++++++++ tests/create.go | 4 +-- 13 files changed, 118 insertions(+), 31 deletions(-) diff --git a/build/build.go b/build/build.go index af269ac35fa..d66762c3582 100644 --- a/build/build.go +++ b/build/build.go @@ -169,6 +169,10 @@ func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opt map[s if noMobyDriver != nil && !noDefaultLoad() && noPrintFunc(opt) { var noOutputTargets []string for name, opt := range opt { + if noMobyDriver.Features(ctx)[driver.DefaultLoad] { + continue + } + if !opt.Linked && len(opt.Exports) == 0 { noOutputTargets = append(noOutputTargets, name) } diff --git a/build/opt.go b/build/opt.go index 0750f4dde42..c340b99110c 100644 --- a/build/opt.go +++ b/build/opt.go @@ -162,10 +162,14 @@ func toSolveOpt(ctx context.Context, node builder.Node, multiDriver bool, opt Op case 1: // valid case 0: - if nodeDriver.IsMobyDriver() && !noDefaultLoad() { - // backwards compat for docker driver only: - // this ensures the build results in a docker image. - opt.Exports = []client.ExportEntry{{Type: "image", Attrs: map[string]string{}}} + if !noDefaultLoad() { + if nodeDriver.IsMobyDriver() { + // backwards compat for docker driver only: + // this ensures the build results in a docker image. + opt.Exports = []client.ExportEntry{{Type: "image", Attrs: map[string]string{}}} + } else if nodeDriver.Features(ctx)[driver.DefaultLoad] { + opt.Exports = []client.ExportEntry{{Type: "docker", Attrs: map[string]string{}}} + } } default: if err := bopts.LLBCaps.Supports(pb.CapMultipleExporters); err != nil { diff --git a/driver/docker-container/driver.go b/driver/docker-container/driver.go index 6a86009fc36..9f77dfe6aed 100644 --- a/driver/docker-container/driver.go +++ b/driver/docker-container/driver.go @@ -56,6 +56,7 @@ type Driver struct { cgroupParent string restartPolicy container.RestartPolicy env []string + defaultLoad bool } func (d *Driver) IsMobyDriver() bool { @@ -423,6 +424,7 @@ func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool { driver.DockerExporter: true, driver.CacheExport: true, driver.MultiPlatform: true, + driver.DefaultLoad: d.defaultLoad, } } diff --git a/driver/docker-container/factory.go b/driver/docker-container/factory.go index 74ba34a8601..18bb2a4a553 100644 --- a/driver/docker-container/factory.go +++ b/driver/docker-container/factory.go @@ -94,6 +94,11 @@ func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver if err != nil { return nil, err } + case k == "default-load": + d.defaultLoad, err = strconv.ParseBool(v) + if err != nil { + return nil, err + } case strings.HasPrefix(k, "env."): envName := strings.TrimPrefix(k, "env.") if envName == "" { diff --git a/driver/docker/driver.go b/driver/docker/driver.go index 742c52c2ae7..2b1481f6e60 100644 --- a/driver/docker/driver.go +++ b/driver/docker/driver.go @@ -93,6 +93,7 @@ func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool { driver.DockerExporter: useContainerdSnapshotter, driver.CacheExport: useContainerdSnapshotter, driver.MultiPlatform: useContainerdSnapshotter, + driver.DefaultLoad: true, } }) return d.features.list diff --git a/driver/features.go b/driver/features.go index 30ee2a48234..09f9dee5c7b 100644 --- a/driver/features.go +++ b/driver/features.go @@ -7,3 +7,5 @@ const DockerExporter Feature = "Docker exporter" const CacheExport Feature = "Cache export" const MultiPlatform Feature = "Multi-platform build" + +const DefaultLoad Feature = "Automatically load images to the Docker Engine image store" diff --git a/driver/kubernetes/driver.go b/driver/kubernetes/driver.go index 73742ddda62..cb38c533757 100644 --- a/driver/kubernetes/driver.go +++ b/driver/kubernetes/driver.go @@ -49,6 +49,7 @@ type Driver struct { podClient clientcorev1.PodInterface configMapClient clientcorev1.ConfigMapInterface podChooser podchooser.PodChooser + defaultLoad bool } func (d *Driver) IsMobyDriver() bool { @@ -229,6 +230,7 @@ func (d *Driver) Features(_ context.Context) map[driver.Feature]bool { driver.DockerExporter: d.DockerAPI != nil, driver.CacheExport: true, driver.MultiPlatform: true, // Untested (needs multiple Driver instances) + driver.DefaultLoad: d.defaultLoad, } } diff --git a/driver/kubernetes/factory.go b/driver/kubernetes/factory.go index c6e2e48e444..f55885cecb3 100644 --- a/driver/kubernetes/factory.go +++ b/driver/kubernetes/factory.go @@ -68,11 +68,13 @@ func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver clientset: clientset, } - deploymentOpt, loadbalance, namespace, err := f.processDriverOpts(deploymentName, namespace, cfg) + deploymentOpt, loadbalance, namespace, defaultLoad, err := f.processDriverOpts(deploymentName, namespace, cfg) if nil != err { return nil, err } + d.defaultLoad = defaultLoad + d.deployment, d.configMaps, err = manifest.NewDeployment(deploymentOpt) if err != nil { return nil, err @@ -100,7 +102,7 @@ func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver return d, nil } -func (f *factory) processDriverOpts(deploymentName string, namespace string, cfg driver.InitConfig) (*manifest.DeploymentOpt, string, string, error) { +func (f *factory) processDriverOpts(deploymentName string, namespace string, cfg driver.InitConfig) (*manifest.DeploymentOpt, string, string, bool, error) { deploymentOpt := &manifest.DeploymentOpt{ Name: deploymentName, Image: bkimage.DefaultImage, @@ -111,6 +113,8 @@ func (f *factory) processDriverOpts(deploymentName string, namespace string, cfg ConfigFiles: cfg.Files, } + defaultLoad := false + deploymentOpt.Qemu.Image = bkimage.QemuImage loadbalance := LoadbalanceSticky @@ -127,7 +131,7 @@ func (f *factory) processDriverOpts(deploymentName string, namespace string, cfg case "replicas": deploymentOpt.Replicas, err = strconv.Atoi(v) if err != nil { - return nil, "", "", err + return nil, "", "", false, err } case "requests.cpu": deploymentOpt.RequestsCPU = v @@ -140,7 +144,7 @@ func (f *factory) processDriverOpts(deploymentName string, namespace string, cfg case "rootless": deploymentOpt.Rootless, err = strconv.ParseBool(v) if err != nil { - return nil, "", "", err + return nil, "", "", false, err } if _, isImage := cfg.DriverOpts["image"]; !isImage { deploymentOpt.Image = bkimage.DefaultRootlessImage @@ -150,17 +154,17 @@ func (f *factory) processDriverOpts(deploymentName string, namespace string, cfg case "nodeselector": deploymentOpt.NodeSelector, err = splitMultiValues(v, ",", "=") if err != nil { - return nil, "", "", errors.Wrap(err, "cannot parse node selector") + return nil, "", "", false, errors.Wrap(err, "cannot parse node selector") } case "annotations": deploymentOpt.CustomAnnotations, err = splitMultiValues(v, ",", "=") if err != nil { - return nil, "", "", errors.Wrap(err, "cannot parse annotations") + return nil, "", "", false, errors.Wrap(err, "cannot parse annotations") } case "labels": deploymentOpt.CustomLabels, err = splitMultiValues(v, ",", "=") if err != nil { - return nil, "", "", errors.Wrap(err, "cannot parse labels") + return nil, "", "", false, errors.Wrap(err, "cannot parse labels") } case "tolerations": ts := strings.Split(v, ";") @@ -185,12 +189,12 @@ func (f *factory) processDriverOpts(deploymentName string, namespace string, cfg case "tolerationSeconds": c, err := strconv.Atoi(kv[1]) if nil != err { - return nil, "", "", err + return nil, "", "", false, err } c64 := int64(c) t.TolerationSeconds = &c64 default: - return nil, "", "", errors.Errorf("invalid tolaration %q", v) + return nil, "", "", false, errors.Errorf("invalid tolaration %q", v) } } } @@ -202,24 +206,29 @@ func (f *factory) processDriverOpts(deploymentName string, namespace string, cfg case LoadbalanceSticky: case LoadbalanceRandom: default: - return nil, "", "", errors.Errorf("invalid loadbalance %q", v) + return nil, "", "", false, errors.Errorf("invalid loadbalance %q", v) } loadbalance = v case "qemu.install": deploymentOpt.Qemu.Install, err = strconv.ParseBool(v) if err != nil { - return nil, "", "", err + return nil, "", "", false, err } case "qemu.image": if v != "" { deploymentOpt.Qemu.Image = v } + case "default-load": + defaultLoad, err = strconv.ParseBool(v) + if err != nil { + return nil, "", "", false, err + } default: - return nil, "", "", errors.Errorf("invalid driver option %s for driver %s", k, DriverName) + return nil, "", "", false, errors.Errorf("invalid driver option %s for driver %s", k, DriverName) } } - return deploymentOpt, loadbalance, namespace, nil + return deploymentOpt, loadbalance, namespace, defaultLoad, nil } func splitMultiValues(in string, itemsep string, kvsep string) (map[string]string, error) { diff --git a/driver/kubernetes/factory_test.go b/driver/kubernetes/factory_test.go index 9b6224bd274..3d6244b1d2c 100644 --- a/driver/kubernetes/factory_test.go +++ b/driver/kubernetes/factory_test.go @@ -52,8 +52,9 @@ func TestFactory_processDriverOpts(t *testing.T) { "loadbalance": "random", "qemu.install": "true", "qemu.image": "qemu:latest", + "default-load": "true", } - r, loadbalance, ns, err := f.processDriverOpts(cfg.Name, "test", cfg) + r, loadbalance, ns, defaultLoad, err := f.processDriverOpts(cfg.Name, "test", cfg) nodeSelectors := map[string]string{ "selector1": "value1", @@ -102,6 +103,7 @@ func TestFactory_processDriverOpts(t *testing.T) { require.Equal(t, LoadbalanceRandom, loadbalance) require.True(t, r.Qemu.Install) require.Equal(t, "qemu:latest", r.Qemu.Image) + require.True(t, defaultLoad) }, ) @@ -109,7 +111,7 @@ func TestFactory_processDriverOpts(t *testing.T) { "NoOptions", func(t *testing.T) { cfg.DriverOpts = map[string]string{} - r, loadbalance, ns, err := f.processDriverOpts(cfg.Name, "test", cfg) + r, loadbalance, ns, defaultLoad, err := f.processDriverOpts(cfg.Name, "test", cfg) require.NoError(t, err) @@ -128,6 +130,7 @@ func TestFactory_processDriverOpts(t *testing.T) { require.Equal(t, LoadbalanceSticky, loadbalance) require.False(t, r.Qemu.Install) require.Equal(t, bkimage.QemuImage, r.Qemu.Image) + require.False(t, defaultLoad) }, ) @@ -138,7 +141,7 @@ func TestFactory_processDriverOpts(t *testing.T) { "loadbalance": "sticky", } - r, loadbalance, ns, err := f.processDriverOpts(cfg.Name, "test", cfg) + r, loadbalance, ns, defaultLoad, err := f.processDriverOpts(cfg.Name, "test", cfg) require.NoError(t, err) @@ -157,6 +160,7 @@ func TestFactory_processDriverOpts(t *testing.T) { require.Equal(t, LoadbalanceSticky, loadbalance) require.False(t, r.Qemu.Install) require.Equal(t, bkimage.QemuImage, r.Qemu.Image) + require.False(t, defaultLoad) }, ) @@ -165,7 +169,7 @@ func TestFactory_processDriverOpts(t *testing.T) { cfg.DriverOpts = map[string]string{ "replicas": "invalid", } - _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) + _, _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) require.Error(t, err) }, ) @@ -175,7 +179,7 @@ func TestFactory_processDriverOpts(t *testing.T) { cfg.DriverOpts = map[string]string{ "rootless": "invalid", } - _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) + _, _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) require.Error(t, err) }, ) @@ -185,7 +189,7 @@ func TestFactory_processDriverOpts(t *testing.T) { cfg.DriverOpts = map[string]string{ "tolerations": "key=foo,value=bar,invalid=foo2", } - _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) + _, _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) require.Error(t, err) }, ) @@ -195,7 +199,7 @@ func TestFactory_processDriverOpts(t *testing.T) { cfg.DriverOpts = map[string]string{ "tolerations": "key=foo,value=bar,tolerationSeconds=invalid", } - _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) + _, _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) require.Error(t, err) }, ) @@ -205,7 +209,7 @@ func TestFactory_processDriverOpts(t *testing.T) { cfg.DriverOpts = map[string]string{ "annotations": "key,value", } - _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) + _, _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) require.Error(t, err) }, ) @@ -215,7 +219,7 @@ func TestFactory_processDriverOpts(t *testing.T) { cfg.DriverOpts = map[string]string{ "labels": "key=value=foo", } - _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) + _, _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) require.Error(t, err) }, ) @@ -225,7 +229,7 @@ func TestFactory_processDriverOpts(t *testing.T) { cfg.DriverOpts = map[string]string{ "loadbalance": "invalid", } - _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) + _, _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) require.Error(t, err) }, ) @@ -235,7 +239,7 @@ func TestFactory_processDriverOpts(t *testing.T) { cfg.DriverOpts = map[string]string{ "qemu.install": "invalid", } - _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) + _, _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) require.Error(t, err) }, ) @@ -245,7 +249,7 @@ func TestFactory_processDriverOpts(t *testing.T) { cfg.DriverOpts = map[string]string{ "invalid": "foo", } - _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) + _, _, _, _, err := f.processDriverOpts(cfg.Name, "test", cfg) require.Error(t, err) }, ) diff --git a/driver/remote/driver.go b/driver/remote/driver.go index b96ba92a71e..56b1078d30b 100644 --- a/driver/remote/driver.go +++ b/driver/remote/driver.go @@ -24,6 +24,7 @@ type Driver struct { // if you add fields, remember to update docs: // https://github.com/docker/docs/blob/main/content/build/drivers/remote.md *tlsOpts + defaultLoad bool } type tlsOpts struct { @@ -149,6 +150,7 @@ func (d *Driver) Features(ctx context.Context) map[driver.Feature]bool { driver.DockerExporter: true, driver.CacheExport: true, driver.MultiPlatform: true, + driver.DefaultLoad: d.defaultLoad, } } diff --git a/driver/remote/factory.go b/driver/remote/factory.go index 4d196dd8481..1c30baad616 100644 --- a/driver/remote/factory.go +++ b/driver/remote/factory.go @@ -4,6 +4,7 @@ import ( "context" "net/url" "path/filepath" + "strconv" "strings" // import connhelpers for special url schemes @@ -80,6 +81,12 @@ func (f *factory) New(ctx context.Context, cfg driver.InitConfig) (driver.Driver } tls.key = v tlsEnabled = true + case "default-load": + parsed, err := strconv.ParseBool(v) + if err != nil { + return nil, err + } + d.defaultLoad = parsed default: return nil, errors.Errorf("invalid driver option %s for remote driver", k) } diff --git a/tests/build.go b/tests/build.go index 5e6cf1c1a85..da790fc15cd 100644 --- a/tests/build.go +++ b/tests/build.go @@ -59,6 +59,7 @@ var buildTests = []func(t *testing.T, sb integration.Sandbox){ testBuildMultiExporters, testBuildLoadPush, testBuildSecret, + testBuildDefaultLoad, } func testBuild(t *testing.T, sb integration.Sandbox) { @@ -750,6 +751,50 @@ COPY --from=build /token / }) } +func testBuildDefaultLoad(t *testing.T, sb integration.Sandbox) { + if !isDockerWorker(sb) { + t.Skip("only testing with docker workers") + } + + tag := "buildx/build:" + identity.NewID() + + var builderName string + t.Cleanup(func() { + if builderName == "" { + return + } + + cmd := dockerCmd(sb, withArgs("image", "rm", tag)) + cmd.Stderr = os.Stderr + require.NoError(t, cmd.Run()) + + out, err := rmCmd(sb, withArgs(builderName)) + require.NoError(t, err, out) + }) + + out, err := createCmd(sb, withArgs( + "--driver", "docker-container", + "--driver-opt", "default-load=true", + )) + require.NoError(t, err, out) + builderName = strings.TrimSpace(out) + + dir := createTestProject(t) + + cmd := buildxCmd(sb, withArgs( + "build", + fmt.Sprintf("-t=%s", tag), + dir, + )) + cmd.Env = append(cmd.Env, "BUILDX_BUILDER="+builderName) + outb, err := cmd.CombinedOutput() + require.NoError(t, err, string(outb)) + + cmd = dockerCmd(sb, withArgs("image", "inspect", tag)) + cmd.Stderr = os.Stderr + require.NoError(t, cmd.Run()) +} + func createTestProject(t *testing.T) string { dockerfile := []byte(` FROM busybox:latest AS base diff --git a/tests/create.go b/tests/create.go index 195691fbbc2..5d2bc23c329 100644 --- a/tests/create.go +++ b/tests/create.go @@ -64,8 +64,8 @@ func testCreateRestartAlways(t *testing.T, sb integration.Sandbox) { } func testCreateRemoteContainer(t *testing.T, sb integration.Sandbox) { - if sb.Name() != "docker" { - t.Skip("skipping test for non-docker workers") + if !isDockerWorker(sb) { + t.Skip("only testing with docker workers") } ctnBuilderName := "ctn-builder-" + identity.NewID()