Skip to content

Commit

Permalink
build: allow BuildKit to be used on Windows daemons that advertise it
Browse files Browse the repository at this point in the history
Commit 6fef143 switched the CLI to use
BuildKit by default, but as part of that removed the use of the
BuildkitVersion field as returned by Ping.

Some follow-up changes in commits e38e6c5 and
e7a8748 updated the logic for detecting whether
BuildKit should be used or the legacy builder, but hard-coded using the
legacy builder for Windows daemons.

While Windows / WCOW does not yet support BuildKit by default, there is
work in progress to implement it, so we should not hard-code the assumption
that a Windows daemon cannot support BuildKit.

On the daemon-side, [moby@7b153b9] (Docker v23.0) changed the default as
advertised by the daemon to be BuildKit for Linux daemons. That change
still hardcoded BuildKit to be unsupported for Windows daemons (and does
not yet allow overriding the config), but this may change for future
versions of the daemon, or test-builds.

This patch:

- Re-introduces checks for the BuildkitVersion field in the "Ping" response.
- If the Ping response from the daemon advertises that it supports BuildKit,
  the CLI will now use BuildKit as builder.
- If we didn't get a Ping response, or the Ping response did NOT advertise
  that the daemon supported BuildKit, we continue to use the current
  defaults (BuildKit for Linux daemons, and the legacy builder for Windows)
- Handling of the DOCKER_BUILDKIT environment variable is unchanged; for
  CLI.BuildKitEnabled, DOCKER_BUILDKIT always takes precedence, and for
  processBuilder the value is taken into account, but will print a warning
  when BuildKit is disabled and a Linux daemon is used. For Windows daemons,
  no warning is printed.

[moby@7b153b9]: moby/moby@7b153b9

Signed-off-by: Sebastiaan van Stijn <[email protected]>
  • Loading branch information
thaJeztah committed Jun 20, 2024
1 parent 64206ae commit e5d26a8
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 8 deletions.
15 changes: 12 additions & 3 deletions cli/command/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,18 @@ func (cli *DockerCli) BuildKitEnabled() (bool, error) {
if _, ok := aliasMap["builder"]; ok {
return true, nil
}
// otherwise, assume BuildKit is enabled but
// not if wcow reported from server side
return cli.ServerInfo().OSType != "windows", nil

si := cli.ServerInfo()
if si.BuildkitVersion == types.BuilderBuildKit {
// The daemon advertised BuildKit as the preferred builder; this may
// be either a Linux daemon or a Windows daemon with experimental
// BuildKit support enabled.
return true, nil
}

// otherwise, assume BuildKit is enabled for Linux, but disabled for
// Windows / WCOW, which does not yet support BuildKit by default.
return si.OSType != "windows", nil
}

// HooksEnabled returns whether plugin hooks are enabled.
Expand Down
20 changes: 15 additions & 5 deletions cmd/docker/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

pluginmanager "github.com/docker/cli/cli-plugins/manager"
"github.com/docker/cli/cli/command"
"github.com/docker/docker/api/types"
"github.com/pkg/errors"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
Expand Down Expand Up @@ -74,14 +75,23 @@ func processBuilder(dockerCli command.Cli, cmd *cobra.Command, args, osargs []st
return args, osargs, nil, nil
}

// wcow build command must use the legacy builder
// if not opt-in through a builder component
if !useBuilder && dockerCli.ServerInfo().OSType == "windows" {
return args, osargs, nil, nil
if !useBuilder {
// Builder is not explicitly configured as an alias for buildx.
// Detect whether we should use BuildKit, or fallback to the
// legacy builder.
if si := dockerCli.ServerInfo(); si.BuildkitVersion != types.BuilderBuildKit && si.OSType == "windows" {
// The daemon didn't advertise BuildKit as the preferred builder,
// so use the legacy builder, which is still the default for
// Windows / WCOW.
return args, osargs, nil, nil
}
}

if buildKitDisabled {
// display warning if not wcow and continue
// When using a Linux daemon, print a warning that the legacy builder
// is deprecated. For Windows / WCOW, BuildKit is still experimental,
// so we don't print this warning, even if the daemon advertised that
// it supports BuildKit.
if dockerCli.ServerInfo().OSType != "windows" {
_, _ = fmt.Fprintf(dockerCli.Err(), "%s\n\n", buildkitDisabledWarning)
}
Expand Down

0 comments on commit e5d26a8

Please sign in to comment.