diff --git a/cli/command/image/build.go b/cli/command/image/build.go index 1ec0af48f94c..049b77efbc25 100644 --- a/cli/command/image/build.go +++ b/cli/command/image/build.go @@ -34,7 +34,10 @@ import ( "github.com/spf13/cobra" ) -var errStdinConflict = errors.New("invalid argument: can't use stdin for both build context and dockerfile") +var ( + errStdinConflict = errors.New("invalid argument: can't use stdin for both build context and dockerfile") + warnLegacyBuilder string +) type buildOptions struct { context string @@ -104,6 +107,11 @@ func NewBuildCommand(dockerCli command.Cli) *cobra.Command { Args: cli.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { options.context = args[0] + cmd.VisitParents(func(cmd *cobra.Command) { + if v, ok := cmd.Annotations["_warning_legacy_builder"]; ok { + warnLegacyBuilder = v + } + }) return runBuild(dockerCli, options) }, } @@ -184,6 +192,10 @@ func runBuild(dockerCli command.Cli, options buildOptions) error { remote string ) + if len(warnLegacyBuilder) > 0 && !options.quiet && dockerCli.ServerInfo().OSType != "windows" { + _, _ = fmt.Fprint(dockerCli.Err(), warnLegacyBuilder) + } + if options.stream { _, _ = fmt.Fprint(dockerCli.Err(), `DEPRECATED: The experimental --stream flag has been removed and the build context will be sent non-streaming. Enable BuildKit instead with DOCKER_BUILDKIT=1 diff --git a/cmd/docker/builder.go b/cmd/docker/builder.go index faa7adbeca8d..7578c7a7be48 100644 --- a/cmd/docker/builder.go +++ b/cmd/docker/builder.go @@ -16,6 +16,7 @@ const ( buildxMissingWarning = `DEPRECATED: The legacy builder is deprecated and will be removed in a future release. Install the buildx component to build images with BuildKit: https://docs.docker.com/go/buildx/ + ` buildxMissingError = `ERROR: BuildKit is enabled but the buildx component is missing or broken. @@ -34,43 +35,48 @@ func newBuilderError(warn bool, err error) error { if pluginmanager.IsNotFound(err) { return errors.New(errorMsg) } - return fmt.Errorf("%w\n\n%s", err, errorMsg) + if err != nil { + return fmt.Errorf("%w\n\n%s", err, errorMsg) + } + return fmt.Errorf("%s", errorMsg) } func processBuilder(dockerCli command.Cli, cmd *cobra.Command, args, osargs []string) ([]string, []string, error) { + var useLegacy bool + var useBuilder bool + // check DOCKER_BUILDKIT env var is present and - // if not assume we want to use a builder - var enforcedBuilder bool + // if not assume we want to use the builder component if v, ok := os.LookupEnv("DOCKER_BUILDKIT"); ok { enabled, err := strconv.ParseBool(v) if err != nil { return args, osargs, errors.Wrap(err, "DOCKER_BUILDKIT environment variable expects boolean value") } if !enabled { - return args, osargs, nil + useLegacy = true + } else { + useBuilder = true } - enforcedBuilder = true } // if a builder alias is defined, use it instead // of the default one - isAlias := false builderAlias := builderDefaultPlugin aliasMap := dockerCli.ConfigFile().Aliases if v, ok := aliasMap[keyBuilderAlias]; ok { - isAlias = true + useBuilder = true builderAlias = v } - // wcow build command must use the legacy builder for buildx - // if not opt-in through a builder alias - if !isAlias && dockerCli.ServerInfo().OSType == "windows" { + // is this a build that should be forwarded to the builder? + fwargs, fwosargs, forwarded := forwardBuilder(builderAlias, args, osargs) + if !forwarded { return args, osargs, nil } - // are we using a cmd that should be forwarded to the builder? - fwargs, fwosargs, forwarded := forwardBuilder(builderAlias, args, osargs) - if !forwarded { + if useLegacy { + // set warning msg as annotation that will be displayed in build cmd + cmd.Annotations["_warning_legacy_builder"] = newBuilderError(true, nil).Error() return args, osargs, nil } @@ -80,12 +86,12 @@ func processBuilder(dockerCli command.Cli, cmd *cobra.Command, args, osargs []st perr = plugin.Err } if perr != nil { - // if builder enforced with DOCKER_BUILDKIT=1, cmd fails if plugin missing or broken - if enforcedBuilder { - return fwargs, fwosargs, newBuilderError(false, perr) + // if builder enforced with DOCKER_BUILDKIT=1, cmd must fail if plugin missing or broken + if useBuilder { + return args, osargs, newBuilderError(false, perr) } - // otherwise, display warning and continue - _, _ = fmt.Fprintln(dockerCli.Err(), newBuilderError(true, perr)) + // if not set warning msg as annotation that will be displayed in build cmd + cmd.Annotations["_warning_legacy_builder"] = newBuilderError(true, perr).Error() return args, osargs, nil } diff --git a/cmd/docker/builder_test.go b/cmd/docker/builder_test.go index bc0f3fa0a1da..1eba54c819ee 100644 --- a/cmd/docker/builder_test.go +++ b/cmd/docker/builder_test.go @@ -3,17 +3,27 @@ package main import ( "bytes" "os" + "runtime" "testing" "github.com/docker/cli/cli/command" + "github.com/docker/cli/internal/test/output" "gotest.tools/v3/assert" "gotest.tools/v3/env" "gotest.tools/v3/fs" ) -func TestBuild(t *testing.T) { +var pluginFilename = "docker-buildx" + +func init() { + if runtime.GOOS == "windows" { + pluginFilename = pluginFilename + ".exe" + } +} + +func TestBuildWithBuilder(t *testing.T) { dir := fs.NewDir(t, t.Name(), - fs.WithFile("docker-buildx", `#!/bin/sh + 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() @@ -36,10 +46,45 @@ echo '{"SchemaVersion":"0.1.0","Vendor":"Docker Inc.","Version":"v0.6.3","ShortD func TestBuildkitDisabled(t *testing.T) { defer env.Patch(t, "DOCKER_BUILDKIT", "0")() - var b bytes.Buffer - dockerCli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(&b)) + dir := fs.NewDir(t, t.Name(), + fs.WithFile(pluginFilename, `#!/bin/sh exit 1`, fs.WithMode(0777)), + ) + defer dir.Remove() + + b := bytes.NewBuffer(nil) + + dockerCli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(b)) + assert.NilError(t, err) + dockerCli.ConfigFile().CLIPluginsExtraDirs = []string{dir.Path()} + + tcmd := newDockerCommand(dockerCli) + tcmd.SetArgs([]string{"build", "."}) + + cmd, args, err := tcmd.HandleGlobalFlags() + assert.NilError(t, err) + + args, os.Args, err = processBuilder(dockerCli, cmd, args, os.Args) + assert.NilError(t, err) + assert.DeepEqual(t, []string{"build", "."}, args) + assert.Equal(t, cmd.Annotations["_warning_legacy_builder"], buildxMissingWarning) + + output.Assert(t, cmd.Annotations["_warning_legacy_builder"], map[int]func(string) error{ + 0: output.Suffix("DEPRECATED: The legacy builder is deprecated and will be removed in a future release."), + }) +} + +func TestBuilderBroken(t *testing.T) { + dir := fs.NewDir(t, t.Name(), + fs.WithFile(pluginFilename, `#!/bin/sh exit 1`, fs.WithMode(0777)), + ) + defer dir.Remove() + + b := bytes.NewBuffer(nil) + + dockerCli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(b)) assert.NilError(t, err) + dockerCli.ConfigFile().CLIPluginsExtraDirs = []string{dir.Path()} tcmd := newDockerCommand(dockerCli) tcmd.SetArgs([]string{"build", "."}) @@ -50,4 +95,38 @@ func TestBuildkitDisabled(t *testing.T) { args, os.Args, err = processBuilder(dockerCli, cmd, args, os.Args) assert.NilError(t, err) assert.DeepEqual(t, []string{"build", "."}, args) + + output.Assert(t, cmd.Annotations["_warning_legacy_builder"], map[int]func(string) error{ + 0: output.Prefix("failed to fetch metadata:"), + 2: output.Suffix("DEPRECATED: The legacy builder is deprecated and will be removed in a future release."), + }) +} + +func TestBuilderBrokenEnforced(t *testing.T) { + defer env.Patch(t, "DOCKER_BUILDKIT", "1")() + + dir := fs.NewDir(t, t.Name(), + fs.WithFile(pluginFilename, `#!/bin/sh exit 1`, fs.WithMode(0777)), + ) + defer dir.Remove() + + b := bytes.NewBuffer(nil) + + dockerCli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(b)) + assert.NilError(t, err) + dockerCli.ConfigFile().CLIPluginsExtraDirs = []string{dir.Path()} + + tcmd := newDockerCommand(dockerCli) + tcmd.SetArgs([]string{"build", "."}) + + cmd, args, err := tcmd.HandleGlobalFlags() + assert.NilError(t, err) + + args, os.Args, err = processBuilder(dockerCli, cmd, args, os.Args) + assert.DeepEqual(t, []string{"build", "."}, args) + + output.Assert(t, err.Error(), map[int]func(string) error{ + 0: output.Prefix("failed to fetch metadata:"), + 2: output.Suffix("ERROR: BuildKit is enabled but the buildx component is missing or broken."), + }) } diff --git a/e2e/image/build_test.go b/e2e/image/build_test.go index 2adb1c120bd8..071274a647dc 100644 --- a/e2e/image/build_test.go +++ b/e2e/image/build_test.go @@ -40,6 +40,7 @@ func TestBuildFromContextDirectoryWithTag(t *testing.T) { const buildxMissingWarning = `DEPRECATED: The legacy builder is deprecated and will be removed in a future release. Install the buildx component to build images with BuildKit: https://docs.docker.com/go/buildx/ + ` result.Assert(t, icmd.Expected{Err: buildxMissingWarning})