Skip to content

Commit 16edf8b

Browse files
crazy-maxcrazy-max
committed
builder: conditional warning for wcow
Signed-off-by: CrazyMax <[email protected]>
1 parent fd22746 commit 16edf8b

2 files changed

Lines changed: 105 additions & 20 deletions

File tree

‎cmd/docker/builder.go‎

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -34,43 +34,50 @@ func newBuilderError(warn bool, err error) error {
3434
if pluginmanager.IsNotFound(err) {
3535
return errors.New(errorMsg)
3636
}
37-
return fmt.Errorf("%w\n\n%s", err, errorMsg)
37+
if err != nil {
38+
return fmt.Errorf("%w\n\n%s", err, errorMsg)
39+
}
40+
return fmt.Errorf("%s", errorMsg)
3841
}
3942

4043
func processBuilder(dockerCli command.Cli, cmd *cobra.Command, args, osargs []string) ([]string, []string, error) {
44+
var useLegacy bool
45+
var useBuilder bool
46+
4147
// check DOCKER_BUILDKIT env var is present and
42-
// if not assume we want to use a builder
43-
var enforcedBuilder bool
48+
// if not assume we want to use the builder component
4449
if v, ok := os.LookupEnv("DOCKER_BUILDKIT"); ok {
4550
enabled, err := strconv.ParseBool(v)
4651
if err != nil {
4752
return args, osargs, errors.Wrap(err, "DOCKER_BUILDKIT environment variable expects boolean value")
4853
}
4954
if !enabled {
50-
return args, osargs, nil
55+
useLegacy = true
56+
} else {
57+
useBuilder = true
5158
}
52-
enforcedBuilder = true
5359
}
5460

5561
// if a builder alias is defined, use it instead
5662
// of the default one
57-
isAlias := false
5863
builderAlias := builderDefaultPlugin
5964
aliasMap := dockerCli.ConfigFile().Aliases
6065
if v, ok := aliasMap[keyBuilderAlias]; ok {
61-
isAlias = true
66+
useBuilder = true
6267
builderAlias = v
6368
}
6469

65-
// wcow build command must use the legacy builder for buildx
66-
// if not opt-in through a builder alias
67-
if !isAlias && dockerCli.ServerInfo().OSType == "windows" {
70+
// is this a build that should be forwarded to the builder?
71+
fwargs, fwosargs, forwarded := forwardBuilder(builderAlias, args, osargs)
72+
if !forwarded {
6873
return args, osargs, nil
6974
}
7075

71-
// are we using a cmd that should be forwarded to the builder?
72-
fwargs, fwosargs, forwarded := forwardBuilder(builderAlias, args, osargs)
73-
if !forwarded {
76+
if useLegacy {
77+
// display warning if not wcow and continue
78+
if dockerCli.ServerInfo().OSType != "windows" {
79+
_, _ = fmt.Fprintln(dockerCli.Err(), newBuilderError(true, nil))
80+
}
7481
return args, osargs, nil
7582
}
7683

@@ -80,9 +87,9 @@ func processBuilder(dockerCli command.Cli, cmd *cobra.Command, args, osargs []st
8087
perr = plugin.Err
8188
}
8289
if perr != nil {
83-
// if builder enforced with DOCKER_BUILDKIT=1, cmd fails if plugin missing or broken
84-
if enforcedBuilder {
85-
return fwargs, fwosargs, newBuilderError(false, perr)
90+
// if builder enforced with DOCKER_BUILDKIT=1, cmd must fail if plugin missing or broken
91+
if useBuilder {
92+
return args, osargs, newBuilderError(false, perr)
8693
}
8794
// otherwise, display warning and continue
8895
_, _ = fmt.Fprintln(dockerCli.Err(), newBuilderError(true, perr))

‎cmd/docker/builder_test.go‎

Lines changed: 82 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,27 @@ package main
33
import (
44
"bytes"
55
"os"
6+
"runtime"
67
"testing"
78

89
"github.com/docker/cli/cli/command"
10+
"github.com/docker/cli/internal/test/output"
911
"gotest.tools/v3/assert"
1012
"gotest.tools/v3/env"
1113
"gotest.tools/v3/fs"
1214
)
1315

14-
func TestBuild(t *testing.T) {
16+
var pluginFilename = "docker-buildx"
17+
18+
func init() {
19+
if runtime.GOOS == "windows" {
20+
pluginFilename = pluginFilename + ".exe"
21+
}
22+
}
23+
24+
func TestBuildWithBuilder(t *testing.T) {
1525
dir := fs.NewDir(t, t.Name(),
16-
fs.WithFile("docker-buildx", `#!/bin/sh
26+
fs.WithFile(pluginFilename, `#!/bin/sh
1727
echo '{"SchemaVersion":"0.1.0","Vendor":"Docker Inc.","Version":"v0.6.3","ShortDescription":"Build with BuildKit"}'`, fs.WithMode(0777)),
1828
)
1929
defer dir.Remove()
@@ -36,10 +46,44 @@ echo '{"SchemaVersion":"0.1.0","Vendor":"Docker Inc.","Version":"v0.6.3","ShortD
3646

3747
func TestBuildkitDisabled(t *testing.T) {
3848
defer env.Patch(t, "DOCKER_BUILDKIT", "0")()
39-
var b bytes.Buffer
4049

41-
dockerCli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(&b))
50+
dir := fs.NewDir(t, t.Name(),
51+
fs.WithFile(pluginFilename, `#!/bin/sh exit 1`, fs.WithMode(0777)),
52+
)
53+
defer dir.Remove()
54+
55+
b := bytes.NewBuffer(nil)
56+
57+
dockerCli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(b))
58+
assert.NilError(t, err)
59+
dockerCli.ConfigFile().CLIPluginsExtraDirs = []string{dir.Path()}
60+
61+
tcmd := newDockerCommand(dockerCli)
62+
tcmd.SetArgs([]string{"build", "."})
63+
64+
cmd, args, err := tcmd.HandleGlobalFlags()
65+
assert.NilError(t, err)
66+
67+
args, os.Args, err = processBuilder(dockerCli, cmd, args, os.Args)
68+
assert.NilError(t, err)
69+
assert.DeepEqual(t, []string{"build", "."}, args)
70+
71+
output.Assert(t, b.String(), map[int]func(string) error{
72+
0: output.Suffix("DEPRECATED: The legacy builder is deprecated and will be removed in a future release."),
73+
})
74+
}
75+
76+
func TestBuilderBroken(t *testing.T) {
77+
dir := fs.NewDir(t, t.Name(),
78+
fs.WithFile(pluginFilename, `#!/bin/sh exit 1`, fs.WithMode(0777)),
79+
)
80+
defer dir.Remove()
81+
82+
b := bytes.NewBuffer(nil)
83+
84+
dockerCli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(b))
4285
assert.NilError(t, err)
86+
dockerCli.ConfigFile().CLIPluginsExtraDirs = []string{dir.Path()}
4387

4488
tcmd := newDockerCommand(dockerCli)
4589
tcmd.SetArgs([]string{"build", "."})
@@ -50,4 +94,38 @@ func TestBuildkitDisabled(t *testing.T) {
5094
args, os.Args, err = processBuilder(dockerCli, cmd, args, os.Args)
5195
assert.NilError(t, err)
5296
assert.DeepEqual(t, []string{"build", "."}, args)
97+
98+
output.Assert(t, b.String(), map[int]func(string) error{
99+
0: output.Prefix("failed to fetch metadata:"),
100+
2: output.Suffix("DEPRECATED: The legacy builder is deprecated and will be removed in a future release."),
101+
})
102+
}
103+
104+
func TestBuilderBrokenEnforced(t *testing.T) {
105+
defer env.Patch(t, "DOCKER_BUILDKIT", "1")()
106+
107+
dir := fs.NewDir(t, t.Name(),
108+
fs.WithFile(pluginFilename, `#!/bin/sh exit 1`, fs.WithMode(0777)),
109+
)
110+
defer dir.Remove()
111+
112+
b := bytes.NewBuffer(nil)
113+
114+
dockerCli, err := command.NewDockerCli(command.WithInputStream(discard), command.WithCombinedStreams(b))
115+
assert.NilError(t, err)
116+
dockerCli.ConfigFile().CLIPluginsExtraDirs = []string{dir.Path()}
117+
118+
tcmd := newDockerCommand(dockerCli)
119+
tcmd.SetArgs([]string{"build", "."})
120+
121+
cmd, args, err := tcmd.HandleGlobalFlags()
122+
assert.NilError(t, err)
123+
124+
args, os.Args, err = processBuilder(dockerCli, cmd, args, os.Args)
125+
assert.DeepEqual(t, []string{"build", "."}, args)
126+
127+
output.Assert(t, err.Error(), map[int]func(string) error{
128+
0: output.Prefix("failed to fetch metadata:"),
129+
2: output.Suffix("ERROR: BuildKit is enabled but the buildx component is missing or broken."),
130+
})
53131
}

0 commit comments

Comments
 (0)