Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set buildx as default builder #3314

Merged
merged 5 commits into from Feb 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
4 changes: 4 additions & 0 deletions Dockerfile
Expand Up @@ -5,6 +5,7 @@ ARG GO_VERSION=1.16.11
ARG XX_VERSION=1.0.0-rc.2
ARG GOVERSIONINFO_VERSION=v1.3.0
ARG GOTESTSUM_VERSION=v1.7.0
ARG BUILDX_VERSION=0.7.1

FROM --platform=$BUILDPLATFORM golang:${GO_VERSION}-${BASE_VARIANT} AS gostable
FROM --platform=$BUILDPLATFORM golang:1.17rc1-${BASE_VARIANT} AS golatest
Expand Down Expand Up @@ -106,6 +107,8 @@ ARG COMPOSE_VERSION=1.29.2
RUN curl -fsSL https://github.com/docker/compose/releases/download/${COMPOSE_VERSION}/docker-compose-$(uname -s)-$(uname -m) -o /usr/local/bin/docker-compose && \
chmod +x /usr/local/bin/docker-compose

FROM docker/buildx-bin:${BUILDX_VERSION} AS buildx

FROM e2e-base-${BASE_VARIANT} AS e2e
ARG NOTARY_VERSION=v0.6.1
ADD --chmod=0755 https://github.com/theupdateframework/notary/releases/download/${NOTARY_VERSION}/notary-Linux-amd64 /usr/local/bin/notary
Expand All @@ -114,6 +117,7 @@ RUN echo 'notary.cert' >> /etc/ca-certificates.conf && update-ca-certificates
COPY --from=gotestsum /out/gotestsum /usr/bin/gotestsum
COPY --from=build /out ./build/
COPY --from=build-plugins /out ./build/
COPY --from=buildx /buildx /usr/libexec/docker/cli-plugins/docker-buildx
COPY . .
ENV DOCKER_BUILDKIT=1
ENV PATH=/go/src/github.com/docker/cli/build:$PATH
Expand Down
30 changes: 30 additions & 0 deletions cli-plugins/manager/manager.go
Expand Up @@ -104,6 +104,36 @@ func listPluginCandidates(dirs []string) (map[string][]string, error) {
return result, nil
}

// GetPlugin returns a plugin on the system by its name
func GetPlugin(name string, dockerCli command.Cli, rootcmd *cobra.Command) (*Plugin, error) {
pluginDirs, err := getPluginDirs(dockerCli)
if err != nil {
return nil, err
}

candidates, err := listPluginCandidates(pluginDirs)
if err != nil {
return nil, err
}

if paths, ok := candidates[name]; ok {
if len(paths) == 0 {
return nil, errPluginNotFound(name)
}
c := &candidate{paths[0]}
p, err := newPlugin(c, rootcmd)
if err != nil {
return nil, err
}
if !IsNotFound(p.Err) {
p.ShadowedPaths = paths[1:]
}
return &p, nil
}

return nil, errPluginNotFound(name)
}

// ListPlugins produces a list of the plugins available on the system
func ListPlugins(dockerCli command.Cli, rootcmd *cobra.Command) ([]Plugin, error) {
pluginDirs, err := getPluginDirs(dockerCli)
Expand Down
23 changes: 23 additions & 0 deletions cli-plugins/manager/manager_test.go
Expand Up @@ -82,6 +82,29 @@ func TestListPluginCandidates(t *testing.T) {
assert.DeepEqual(t, candidates, exp)
}

func TestGetPlugin(t *testing.T) {
dir := fs.NewDir(t, t.Name(),
fs.WithFile("docker-bbb", `
#!/bin/sh
echo '{"SchemaVersion":"0.1.0"}'`, fs.WithMode(0777)),
fs.WithFile("docker-aaa", `
#!/bin/sh
echo '{"SchemaVersion":"0.1.0"}'`, fs.WithMode(0777)),
)
defer dir.Remove()

cli := test.NewFakeCli(nil)
cli.SetConfigFile(&configfile.ConfigFile{CLIPluginsExtraDirs: []string{dir.Path()}})

plugin, err := GetPlugin("bbb", cli, &cobra.Command{})
assert.NilError(t, err)
assert.Equal(t, plugin.Name, "bbb")

_, err = GetPlugin("ccc", cli, &cobra.Command{})
assert.Error(t, err, "Error: No such CLI plugin: ccc")
assert.Assert(t, IsNotFound(err))
}

func TestListPluginsIsSorted(t *testing.T) {
dir := fs.NewDir(t, t.Name(),
fs.WithFile("docker-bbb", `
Expand Down
15 changes: 0 additions & 15 deletions cli/command/cli.go
Expand Up @@ -7,7 +7,6 @@ import (
"os"
"path/filepath"
"runtime"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -169,20 +168,6 @@ func (cli *DockerCli) ContentTrustEnabled() bool {
return cli.contentTrust
}

// BuildKitEnabled returns whether buildkit is enabled either through a daemon setting
// or otherwise the client-side DOCKER_BUILDKIT environment variable
func BuildKitEnabled(si ServerInfo) (bool, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docker/compose relies on this to select the builder to be used.
Maybe restore this function as a dumb return true would make more sense?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the logic was moved inside processBuilder(). Perhaps it makes sense to move that logic back into this function. The default would now be (true) if DOCKER_BUILDKIT is not set?

Wondering if the function also needs to take windows daemon into account 🤔
How exactly is it used in Compose (as in; what does it do if it's "disabled"? or is that only for the Windows case?)

buildkitEnabled := si.BuildkitVersion == types.BuilderBuildKit
if buildkitEnv := os.Getenv("DOCKER_BUILDKIT"); buildkitEnv != "" {
var err error
buildkitEnabled, err = strconv.ParseBool(buildkitEnv)
if err != nil {
return false, errors.Wrap(err, "DOCKER_BUILDKIT environment variable expects boolean value")
}
}
return buildkitEnabled, nil
}

// ManifestStore returns a store for local manifests
func (cli *DockerCli) ManifestStore() manifeststore.Store {
// TODO: support override default location from config file
Expand Down
99 changes: 1 addition & 98 deletions cli/command/image/build.go
Expand Up @@ -5,7 +5,6 @@ import (
"bufio"
"bytes"
"context"
"encoding/csv"
"encoding/json"
"fmt"
"io"
Expand Down Expand Up @@ -57,7 +56,6 @@ type buildOptions struct {
isolation string
quiet bool
noCache bool
progress string
rm bool
forceRm bool
pull bool
Expand All @@ -71,9 +69,6 @@ type buildOptions struct {
stream bool
platform string
untrusted bool
secrets []string
ssh []string
outputs []string
}

// dockerfileFromStdin returns true when the user specified that the Dockerfile
Expand Down Expand Up @@ -118,40 +113,26 @@ func NewBuildCommand(dockerCli command.Cli) *cobra.Command {
flags.VarP(&options.tags, "tag", "t", "Name and optionally a tag in the 'name:tag' format")
flags.Var(&options.buildArgs, "build-arg", "Set build-time variables")
flags.Var(options.ulimits, "ulimit", "Ulimit options")
flags.SetAnnotation("ulimit", "no-buildkit", nil)
flags.StringVarP(&options.dockerfileName, "file", "f", "", "Name of the Dockerfile (Default is 'PATH/Dockerfile')")
flags.VarP(&options.memory, "memory", "m", "Memory limit")
flags.SetAnnotation("memory", "no-buildkit", nil)
flags.Var(&options.memorySwap, "memory-swap", "Swap limit equal to memory plus swap: '-1' to enable unlimited swap")
flags.SetAnnotation("memory-swap", "no-buildkit", nil)
flags.Var(&options.shmSize, "shm-size", "Size of /dev/shm")
flags.SetAnnotation("shm-size", "no-buildkit", nil)
flags.Int64VarP(&options.cpuShares, "cpu-shares", "c", 0, "CPU shares (relative weight)")
flags.SetAnnotation("cpu-shares", "no-buildkit", nil)
flags.Int64Var(&options.cpuPeriod, "cpu-period", 0, "Limit the CPU CFS (Completely Fair Scheduler) period")
flags.SetAnnotation("cpu-period", "no-buildkit", nil)
flags.Int64Var(&options.cpuQuota, "cpu-quota", 0, "Limit the CPU CFS (Completely Fair Scheduler) quota")
flags.SetAnnotation("cpu-quota", "no-buildkit", nil)
flags.StringVar(&options.cpuSetCpus, "cpuset-cpus", "", "CPUs in which to allow execution (0-3, 0,1)")
flags.SetAnnotation("cpuset-cpus", "no-buildkit", nil)
flags.StringVar(&options.cpuSetMems, "cpuset-mems", "", "MEMs in which to allow execution (0-3, 0,1)")
flags.SetAnnotation("cpuset-mems", "no-buildkit", nil)
flags.StringVar(&options.cgroupParent, "cgroup-parent", "", "Optional parent cgroup for the container")
flags.SetAnnotation("cgroup-parent", "no-buildkit", nil)
flags.StringVar(&options.isolation, "isolation", "", "Container isolation technology")
flags.Var(&options.labels, "label", "Set metadata for an image")
flags.BoolVar(&options.noCache, "no-cache", false, "Do not use cache when building the image")
flags.BoolVar(&options.rm, "rm", true, "Remove intermediate containers after a successful build")
flags.SetAnnotation("rm", "no-buildkit", nil)
flags.BoolVar(&options.forceRm, "force-rm", false, "Always remove intermediate containers")
flags.SetAnnotation("force-rm", "no-buildkit", nil)
flags.BoolVarP(&options.quiet, "quiet", "q", false, "Suppress the build output and print image ID on success")
flags.BoolVar(&options.pull, "pull", false, "Always attempt to pull a newer version of the image")
flags.StringSliceVar(&options.cacheFrom, "cache-from", []string{}, "Images to consider as cache sources")
flags.BoolVar(&options.compress, "compress", false, "Compress the build context using gzip")
flags.SetAnnotation("compress", "no-buildkit", nil)
flags.StringSliceVar(&options.securityOpt, "security-opt", []string{}, "Security options")
flags.SetAnnotation("security-opt", "no-buildkit", nil)
flags.StringVar(&options.networkMode, "network", "default", "Set the networking mode for the RUN instructions during build")
flags.SetAnnotation("network", "version", []string{"1.25"})
flags.Var(&options.extraHosts, "add-host", "Add a custom host-to-IP mapping (host:ip)")
Expand All @@ -162,7 +143,6 @@ func NewBuildCommand(dockerCli command.Cli) *cobra.Command {

flags.StringVar(&options.platform, "platform", os.Getenv("DOCKER_DEFAULT_PLATFORM"), "Set platform if server is multi-platform capable")
crazy-max marked this conversation as resolved.
Show resolved Hide resolved
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)
Expand All @@ -171,21 +151,6 @@ func NewBuildCommand(dockerCli command.Cli) *cobra.Command {
flags.BoolVar(&options.stream, "stream", false, "Stream attaches to server to negotiate build context")
flags.MarkHidden("stream")

flags.StringVar(&options.progress, "progress", "auto", "Set type of progress output (auto, plain, tty). Use plain to show container output")
flags.SetAnnotation("progress", "buildkit", nil)

flags.StringArrayVar(&options.secrets, "secret", []string{}, "Secret file to expose to the build (only if BuildKit enabled): id=mysecret,src=/local/secret")
flags.SetAnnotation("secret", "version", []string{"1.39"})
flags.SetAnnotation("secret", "buildkit", nil)

flags.StringArrayVar(&options.ssh, "ssh", []string{}, "SSH agent socket or keys to expose to the build (only if BuildKit enabled) (format: default|<id>[=<socket>|<key>[,<key>]])")
flags.SetAnnotation("ssh", "version", []string{"1.39"})
flags.SetAnnotation("ssh", "buildkit", nil)

flags.StringArrayVarP(&options.outputs, "output", "o", []string{}, "Output destination (format: type=local,dest=path)")
flags.SetAnnotation("output", "version", []string{"1.40"})
flags.SetAnnotation("output", "buildkit", nil)

return cmd
}

Expand All @@ -207,15 +172,8 @@ func (out *lastProgressOutput) WriteProgress(prog progress.Progress) error {

// nolint: gocyclo
func runBuild(dockerCli command.Cli, options buildOptions) error {
buildkitEnabled, err := command.BuildKitEnabled(dockerCli.ServerInfo())
if err != nil {
return err
}
if buildkitEnabled {
return runBuildBuildKit(dockerCli, options)
}

var (
err error
buildCtx io.ReadCloser
dockerfileCtx io.ReadCloser
contextDir string
Expand Down Expand Up @@ -609,58 +567,3 @@ func imageBuildOptions(dockerCli command.Cli, options buildOptions) types.ImageB
Platform: options.platform,
}
}

func parseOutputs(inp []string) ([]types.ImageBuildOutput, error) {
var outs []types.ImageBuildOutput
if len(inp) == 0 {
return nil, nil
}
for _, s := range inp {
csvReader := csv.NewReader(strings.NewReader(s))
fields, err := csvReader.Read()
if err != nil {
return nil, err
}
if len(fields) == 1 && fields[0] == s && !strings.HasPrefix(s, "type=") {
if s == "-" {
outs = append(outs, types.ImageBuildOutput{
Type: "tar",
Attrs: map[string]string{
"dest": s,
},
})
} else {
outs = append(outs, types.ImageBuildOutput{
Type: "local",
Attrs: map[string]string{
"dest": s,
},
})
}
continue
}

out := types.ImageBuildOutput{
Attrs: map[string]string{},
}
for _, field := range fields {
parts := strings.SplitN(field, "=", 2)
if len(parts) != 2 {
return nil, errors.Errorf("invalid value %s", field)
}
key := strings.ToLower(parts[0])
value := parts[1]
switch key {
case "type":
out.Type = value
default:
out.Attrs[key] = value
}
}
if out.Type == "" {
return nil, errors.Errorf("type is required for output")
}
outs = append(outs, out)
}
return outs, nil
}