builder: conditional warning for wcow and quiet flag

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>

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

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
 	}
 

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."),
+	})
 }

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})