From 997846918e461102f90269018be0db93f4c3ecf0 Mon Sep 17 00:00:00 2001 From: CrazyMax Date: Thu, 3 Nov 2022 23:35:45 +0100 Subject: [PATCH] build: keep "buildx install" behavior Signed-off-by: CrazyMax --- cmd/docker/builder.go | 12 ++-- cmd/docker/builder_test.go | 133 +++++++++++++++++++++++-------------- 2 files changed, 91 insertions(+), 54 deletions(-) diff --git a/cmd/docker/builder.go b/cmd/docker/builder.go index f416931c7f76..7d463e4a767d 100644 --- a/cmd/docker/builder.go +++ b/cmd/docker/builder.go @@ -45,7 +45,7 @@ func newBuilderError(warn bool, err error) error { //nolint:gocyclo func processBuilder(dockerCli command.Cli, cmd *cobra.Command, args, osargs []string) ([]string, []string, []string, error) { - var useLegacy, useBuilder bool + var useLegacy, useBuilder, useAlias bool var envs []string // check DOCKER_BUILDKIT env var is present and @@ -68,6 +68,7 @@ func processBuilder(dockerCli command.Cli, cmd *cobra.Command, args, osargs []st aliasMap := dockerCli.ConfigFile().Aliases if v, ok := aliasMap[keyBuilderAlias]; ok { useBuilder = true + useAlias = true builderAlias = v } @@ -110,10 +111,11 @@ func processBuilder(dockerCli command.Cli, cmd *cobra.Command, args, osargs []st // always create a local docker image (default context builder). This is // for better backward compatibility in case where a user could switch to // a docker container builder with "docker buildx --use foo" which does - // not --load by default. Also makes sure that an arbitrary builder name is - // not being set in the command line or in the environment before setting - // the default context in env vars. - if forwarded && !hasBuilderName(args, os.Environ()) { + // not --load by default. Also makes sure that an arbitrary builder name + // is not being set in the command line or in the environment before + // setting the default context and keep "buildx install" behavior if being + // set (builder alias). + if forwarded && !useAlias && !hasBuilderName(args, os.Environ()) { envs = append([]string{"BUILDX_BUILDER=" + dockerCli.CurrentContext()}, envs...) } diff --git a/cmd/docker/builder_test.go b/cmd/docker/builder_test.go index 80c380fc030d..9a8b40c62284 100644 --- a/cmd/docker/builder_test.go +++ b/cmd/docker/builder_test.go @@ -3,9 +3,11 @@ package main import ( "bytes" "os" + "path/filepath" "testing" "github.com/docker/cli/cli/command" + "github.com/docker/cli/cli/context/store" "github.com/docker/cli/cli/flags" "github.com/docker/cli/internal/test/output" "gotest.tools/v3/assert" @@ -14,36 +16,38 @@ import ( var pluginFilename = "docker-buildx" -func TestBuildWithBuildx(t *testing.T) { - dir := fs.NewDir(t, t.Name(), - fs.WithFile(pluginFilename, `#!/bin/sh -echo '{"SchemaVersion":"0.1.0","Vendor":"Docker Inc.","Version":"v0.6.3","ShortDescription":"Build with BuildKit"}'`, fs.WithMode(0777)), - ) - defer dir.Remove() - - var b bytes.Buffer - - t.Setenv("DOCKER_CONTEXT", "default") - dockerCli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(&b)) - assert.NilError(t, err) - assert.NilError(t, dockerCli.Initialize(flags.NewClientOptions())) - dockerCli.ConfigFile().CLIPluginsExtraDirs = []string{dir.Path()} - - tcmd := newDockerCommand(dockerCli) - tcmd.SetArgs([]string{"build", "."}) - - cmd, args, err := tcmd.HandleGlobalFlags() - assert.NilError(t, err) - - var envs []string - args, os.Args, envs, err = processBuilder(dockerCli, cmd, args, os.Args) - assert.NilError(t, err) - assert.DeepEqual(t, []string{builderDefaultPlugin, "build", "."}, args) - assert.DeepEqual(t, []string{"BUILDX_BUILDER=default"}, envs) -} - -func TestBuildWithBuildxAndBuilder(t *testing.T) { - t.Setenv("BUILDX_BUILDER", "mybuilder") +func TestBuildWithBuilder(t *testing.T) { + testcases := []struct { + name string + context string + builder string + alias bool + expectedEnvs []string + }{ + { + name: "default", + context: "default", + alias: false, + expectedEnvs: []string{"BUILDX_BUILDER=default"}, + }, + { + name: "custom context", + context: "foo", + alias: false, + expectedEnvs: []string{"BUILDX_BUILDER=foo"}, + }, + { + name: "custom builder name", + builder: "mybuilder", + alias: false, + expectedEnvs: nil, + }, + { + name: "buildx install", + alias: true, + expectedEnvs: nil, + }, + } dir := fs.NewDir(t, t.Name(), fs.WithFile(pluginFilename, `#!/bin/sh @@ -51,25 +55,56 @@ echo '{"SchemaVersion":"0.1.0","Vendor":"Docker Inc.","Version":"v0.6.3","ShortD ) defer dir.Remove() - var b bytes.Buffer - - t.Setenv("DOCKER_CONTEXT", "default") - dockerCli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(&b)) - assert.NilError(t, err) - assert.NilError(t, dockerCli.Initialize(flags.NewClientOptions())) - dockerCli.ConfigFile().CLIPluginsExtraDirs = []string{dir.Path()} - - tcmd := newDockerCommand(dockerCli) - tcmd.SetArgs([]string{"build", "."}) - - cmd, args, err := tcmd.HandleGlobalFlags() - assert.NilError(t, err) - - var envs []string - args, os.Args, envs, err = processBuilder(dockerCli, cmd, args, os.Args) - assert.NilError(t, err) - assert.DeepEqual(t, []string{builderDefaultPlugin, "build", "."}, args) - assert.Check(t, len(envs) == 0) + for _, tt := range testcases { + tt := tt + t.Run(tt.name, func(t *testing.T) { + if tt.builder != "" { + t.Setenv("BUILDX_BUILDER", tt.builder) + } + + var b bytes.Buffer + dockerCli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(&b)) + assert.NilError(t, err) + assert.NilError(t, dockerCli.Initialize(flags.NewClientOptions())) + + if tt.context != "" { + if tt.context != command.DefaultContextName { + assert.NilError(t, dockerCli.ContextStore().CreateOrUpdate(store.Metadata{ + Name: tt.context, + Endpoints: map[string]interface{}{ + "docker": map[string]interface{}{ + "host": "unix://" + filepath.Join(t.TempDir(), "docker.sock"), + }, + }, + })) + } + opts := flags.NewClientOptions() + opts.Common.Context = tt.context + assert.NilError(t, dockerCli.Initialize(opts)) + } + + dockerCli.ConfigFile().CLIPluginsExtraDirs = []string{dir.Path()} + if tt.alias { + dockerCli.ConfigFile().Aliases = map[string]string{"builder": "buildx"} + } + + tcmd := newDockerCommand(dockerCli) + tcmd.SetArgs([]string{"build", "."}) + + cmd, args, err := tcmd.HandleGlobalFlags() + assert.NilError(t, err) + + var envs []string + args, os.Args, envs, err = processBuilder(dockerCli, cmd, args, os.Args) + assert.NilError(t, err) + assert.DeepEqual(t, []string{builderDefaultPlugin, "build", "."}, args) + if tt.expectedEnvs != nil { + assert.DeepEqual(t, tt.expectedEnvs, envs) + } else { + assert.Check(t, len(envs) == 0) + } + }) + } } func TestBuildkitDisabled(t *testing.T) {