From c44c18e088d669612ff724e3c3212eee00a18d0d Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Tue, 7 Apr 2020 23:24:20 +0200 Subject: [PATCH] docker build: check experimental --platform on pre-run Signed-off-by: Sebastiaan van Stijn (cherry picked from commit a88a1bea2360506a794d71f2be2a7477f8b4ebbb) Signed-off-by: Sebastiaan van Stijn --- cli/command/image/build.go | 29 +++++++++++++++++++++-------- cli/command/image/build_test.go | 9 ++++++++- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/cli/command/image/build.go b/cli/command/image/build.go index eeb8b3eb94..528137cb80 100644 --- a/cli/command/image/build.go +++ b/cli/command/image/build.go @@ -115,6 +115,25 @@ func NewBuildCommand(dockerCli command.Cli) *cobra.Command { }, } + // Wrap the global pre-run to handle non-BuildKit use of the --platform flag. + // + // We're doing it here so that we're only contacting the daemon when actually + // running the command, and not during initialization. + // TODO remove this hack once we no longer support the experimental use of --platform + rootFn := cmd.Root().PersistentPreRunE + cmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + if ok, _ := command.BuildKitEnabled(dockerCli.ServerInfo()); !ok { + f := cmd.Flag("platform") + delete(f.Annotations, "buildkit") + f.Annotations["version"] = []string{"1.32"} + f.Annotations["experimental"] = nil + } + if rootFn != nil { + return rootFn(cmd, args) + } + return nil + } + flags := cmd.Flags() flags.VarP(&options.tags, "tag", "t", "Name and optionally a tag in the 'name:tag' format") @@ -163,14 +182,8 @@ func NewBuildCommand(dockerCli command.Cli) *cobra.Command { command.AddTrustVerificationFlags(flags, &options.untrusted, dockerCli.ContentTrustEnabled()) flags.StringVar(&options.platform, "platform", os.Getenv("DOCKER_DEFAULT_PLATFORM"), "Set platform if server is multi-platform capable") - // Platform is not experimental when BuildKit is used - buildkitEnabled, err := command.BuildKitEnabled(dockerCli.ServerInfo()) - if err == nil && buildkitEnabled { - flags.SetAnnotation("platform", "version", []string{"1.38"}) - } else { - flags.SetAnnotation("platform", "version", []string{"1.32"}) - flags.SetAnnotation("platform", "experimental", nil) - } + flags.SetAnnotation("platform", "version", []string{"1.38"}) + flags.SetAnnotation("platform", "buildkit", nil) flags.BoolVar(&options.squash, "squash", false, "Squash newly built layers into a single new layer") flags.SetAnnotation("squash", "experimental", nil) diff --git a/cli/command/image/build_test.go b/cli/command/image/build_test.go index fced1bfca7..3c8c875eb9 100644 --- a/cli/command/image/build_test.go +++ b/cli/command/image/build_test.go @@ -20,11 +20,13 @@ import ( "github.com/google/go-cmp/cmp" "github.com/moby/buildkit/session/secrets/secretsprovider" "gotest.tools/v3/assert" + "gotest.tools/v3/env" "gotest.tools/v3/fs" "gotest.tools/v3/skip" ) func TestRunBuildDockerfileFromStdinWithCompress(t *testing.T) { + defer env.Patch(t, "DOCKER_BUILDKIT", "0")() buffer := new(bytes.Buffer) fakeBuild := newFakeBuild() fakeImageBuild := func(ctx context.Context, context io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error) { @@ -60,6 +62,7 @@ func TestRunBuildDockerfileFromStdinWithCompress(t *testing.T) { } func TestRunBuildResetsUidAndGidInContext(t *testing.T) { + defer env.Patch(t, "DOCKER_BUILDKIT", "0")() skip.If(t, os.Getuid() != 0, "root is required to chown files") fakeBuild := newFakeBuild() cli := test.NewFakeCli(&fakeClient{imageBuildFunc: fakeBuild.build}) @@ -90,6 +93,7 @@ func TestRunBuildResetsUidAndGidInContext(t *testing.T) { } func TestRunBuildDockerfileOutsideContext(t *testing.T) { + defer env.Patch(t, "DOCKER_BUILDKIT", "0")() dir := fs.NewDir(t, t.Name(), fs.WithFile("data", "data file")) defer dir.Remove() @@ -122,7 +126,8 @@ COPY data /data // TODO: test "context selection" logic directly when runBuild is refactored // to support testing (ex: docker/cli#294) func TestRunBuildFromGitHubSpecialCase(t *testing.T) { - cmd := NewBuildCommand(test.NewFakeCli(nil)) + defer env.Patch(t, "DOCKER_BUILDKIT", "0")() + cmd := NewBuildCommand(test.NewFakeCli(&fakeClient{})) // Clone a small repo that exists so git doesn't prompt for credentials cmd.SetArgs([]string{"github.com/docker/for-win"}) cmd.SetOutput(ioutil.Discard) @@ -135,6 +140,7 @@ func TestRunBuildFromGitHubSpecialCase(t *testing.T) { // starting with `github.com` takes precedence over the `github.com` special // case. func TestRunBuildFromLocalGitHubDir(t *testing.T) { + defer env.Patch(t, "DOCKER_BUILDKIT", "0")() tmpDir, err := ioutil.TempDir("", "docker-build-from-local-dir-") assert.NilError(t, err) defer os.RemoveAll(tmpDir) @@ -154,6 +160,7 @@ func TestRunBuildFromLocalGitHubDir(t *testing.T) { } func TestRunBuildWithSymlinkedContext(t *testing.T) { + defer env.Patch(t, "DOCKER_BUILDKIT", "0")() dockerfile := ` FROM alpine:3.6 RUN echo hello world