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

refactor: mutualize builder logic #865

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions bake/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import (
"context"
"strings"

"github.com/docker/buildx/build"
"github.com/docker/buildx/driver"
"github.com/docker/buildx/util/builderutil"
"github.com/docker/buildx/util/progress"
"github.com/moby/buildkit/client"
"github.com/moby/buildkit/client/llb"
Expand All @@ -20,7 +20,7 @@ type Input struct {
URL string
}

func ReadRemoteFiles(ctx context.Context, dis []build.DriverInfo, url string, names []string, pw progress.Writer) ([]File, *Input, error) {
func ReadRemoteFiles(ctx context.Context, drivers []builderutil.Driver, url string, names []string, pw progress.Writer) ([]File, *Input, error) {
var filename string
st, ok := detectGitContext(url)
if !ok {
Expand All @@ -33,8 +33,8 @@ func ReadRemoteFiles(ctx context.Context, dis []build.DriverInfo, url string, na
inp := &Input{State: st, URL: url}
var files []File

var di *build.DriverInfo
for _, d := range dis {
var di *builderutil.Driver
for _, d := range drivers {
if d.Err == nil {
di = &d
continue
Expand Down
93 changes: 10 additions & 83 deletions build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@ import (
"github.com/containerd/containerd/images"
"github.com/containerd/containerd/platforms"
"github.com/docker/buildx/driver"
"github.com/docker/buildx/store/storeutil"
"github.com/docker/buildx/util/builderutil"
"github.com/docker/buildx/util/imagetools"
"github.com/docker/buildx/util/progress"
"github.com/docker/buildx/util/resolver"
"github.com/docker/cli/opts"
"github.com/docker/distribution/reference"
"github.com/docker/docker/api/types"
dockerclient "github.com/docker/docker/client"
"github.com/docker/docker/pkg/jsonmessage"
"github.com/docker/docker/pkg/urlutil"
"github.com/moby/buildkit/client"
Expand Down Expand Up @@ -81,20 +82,8 @@ type Inputs struct {
DockerfileInline string
}

type DriverInfo struct {
Driver driver.Driver
Name string
Platform []specs.Platform
Err error
ImageOpt imagetools.Opt
}

type DockerAPI interface {
DockerAPI(name string) (dockerclient.APIClient, error)
}

func filterAvailableDrivers(drivers []DriverInfo) ([]DriverInfo, error) {
out := make([]DriverInfo, 0, len(drivers))
func filterAvailableDrivers(drivers []builderutil.Driver) ([]builderutil.Driver, error) {
out := make([]builderutil.Driver, 0, len(drivers))
err := errors.Errorf("no drivers found")
for _, di := range drivers {
if di.Err == nil && di.Driver != nil {
Expand Down Expand Up @@ -140,7 +129,7 @@ func allIndexes(l int) []int {
return out
}

func ensureBooted(ctx context.Context, drivers []DriverInfo, idxs []int, pw progress.Writer) ([]*client.Client, error) {
func ensureBooted(ctx context.Context, drivers []builderutil.Driver, idxs []int, pw progress.Writer) ([]*client.Client, error) {
clients := make([]*client.Client, len(drivers))

baseCtx := ctx
Expand Down Expand Up @@ -186,7 +175,7 @@ func splitToDriverPairs(availablePlatforms map[string]int, opt map[string]Option
return m
}

func resolveDrivers(ctx context.Context, drivers []DriverInfo, opt map[string]Options, pw progress.Writer) (map[string][]driverPair, []*client.Client, error) {
func resolveDrivers(ctx context.Context, drivers []builderutil.Driver, opt map[string]Options, pw progress.Writer) (map[string][]driverPair, []*client.Client, error) {
dps, clients, err := resolveDriversBase(ctx, drivers, opt, pw)
if err != nil {
return nil, nil, err
Expand Down Expand Up @@ -227,10 +216,10 @@ func resolveDrivers(ctx context.Context, drivers []DriverInfo, opt map[string]Op
return dps, clients, nil
}

func resolveDriversBase(ctx context.Context, drivers []DriverInfo, opt map[string]Options, pw progress.Writer) (map[string][]driverPair, []*client.Client, error) {
func resolveDriversBase(ctx context.Context, drivers []builderutil.Driver, opt map[string]Options, pw progress.Writer) (map[string][]driverPair, []*client.Client, error) {
availablePlatforms := map[string]int{}
for i, d := range drivers {
for _, p := range d.Platform {
for _, p := range d.Platforms {
availablePlatforms[platforms.Format(p)] = i
}
}
Expand Down Expand Up @@ -580,7 +569,7 @@ func toSolveOpt(ctx context.Context, d driver.Driver, multiDriver bool, opt Opti
return &so, releaseF, nil
}

func Build(ctx context.Context, drivers []DriverInfo, opt map[string]Options, docker DockerAPI, configDir string, w progress.Writer) (resp map[string]*client.SolveResponse, err error) {
func Build(ctx context.Context, drivers []builderutil.Driver, opt map[string]Options, docker *storeutil.DockerClient, configDir string, w progress.Writer) (resp map[string]*client.SolveResponse, err error) {
if len(drivers) == 0 {
return nil, errors.Errorf("driver required for build")
}
Expand Down Expand Up @@ -633,7 +622,7 @@ func Build(ctx context.Context, drivers []DriverInfo, opt map[string]Options, do
}
opt.Platforms = dp.platforms
so, release, err := toSolveOpt(ctx, d, multiDriver, opt, dp.bopts, configDir, w, func(name string) (io.WriteCloser, func(), error) {
return newDockerLoader(ctx, docker, name, w)
return docker.LoadImage(ctx, name, w)
})
if err != nil {
return nil, err
Expand Down Expand Up @@ -1094,40 +1083,6 @@ func notSupported(d driver.Driver, f driver.Feature) error {

type dockerLoadCallback func(name string) (io.WriteCloser, func(), error)

func newDockerLoader(ctx context.Context, d DockerAPI, name string, status progress.Writer) (io.WriteCloser, func(), error) {
c, err := d.DockerAPI(name)
if err != nil {
return nil, nil, err
}

pr, pw := io.Pipe()
done := make(chan struct{})

ctx, cancel := context.WithCancel(ctx)
var w *waitingWriter
w = &waitingWriter{
PipeWriter: pw,
f: func() {
resp, err := c.ImageLoad(ctx, pr, false)
defer close(done)
if err != nil {
pr.CloseWithError(err)
w.mu.Lock()
w.err = err
w.mu.Unlock()
return
}
prog := progress.WithPrefix(status, "", false)
progress.FromReader(prog, "importing to docker", resp.Body)
},
done: done,
cancel: cancel,
}
return w, func() {
pr.Close()
}, nil
}

func noDefaultLoad() bool {
v, ok := os.LookupEnv("BUILDX_NO_DEFAULT_LOAD")
if !ok {
Expand All @@ -1140,34 +1095,6 @@ func noDefaultLoad() bool {
return b
}

type waitingWriter struct {
*io.PipeWriter
f func()
once sync.Once
mu sync.Mutex
err error
done chan struct{}
cancel func()
}

func (w *waitingWriter) Write(dt []byte) (int, error) {
w.once.Do(func() {
go w.f()
})
return w.PipeWriter.Write(dt)
}

func (w *waitingWriter) Close() error {
err := w.PipeWriter.Close()
<-w.done
if err == nil {
w.mu.Lock()
defer w.mu.Unlock()
return w.err
}
return err
}

// handle https://github.com/moby/moby/pull/10858
func handleLowercaseDockerfile(dir, p string) string {
if filepath.Base(p) != "Dockerfile" {
Expand Down
20 changes: 17 additions & 3 deletions commands/bake.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"github.com/containerd/containerd/platforms"
"github.com/docker/buildx/bake"
"github.com/docker/buildx/build"
"github.com/docker/buildx/store/storeutil"
"github.com/docker/buildx/util/builderutil"
"github.com/docker/buildx/util/confutil"
"github.com/docker/buildx/util/progress"
"github.com/docker/buildx/util/tracing"
Expand Down Expand Up @@ -87,16 +89,28 @@ func runBake(dockerCli command.Cli, targets []string, in bakeOptions) (err error
}
}()

dis, err := getInstanceOrDefault(ctx, dockerCli, in.builder, contextPathHash)
txn, release, err := storeutil.GetStore(dockerCli)
if err != nil {
return err
}
defer release()

builder, err := builderutil.New(dockerCli, txn, in.builder)
if err != nil {
return err
}
if err = builder.Validate(); err != nil {
return err
}
if err = builder.LoadDrivers(ctx, false, contextPathHash); err != nil {
return err
}

var files []bake.File
var inp *bake.Input

if url != "" {
files, inp, err = bake.ReadRemoteFiles(ctx, dis, url, in.files, printer)
files, inp, err = bake.ReadRemoteFiles(ctx, builder.Drivers, url, in.files, printer)
} else {
files, err = bake.ReadLocalFiles(in.files)
}
Expand Down Expand Up @@ -146,7 +160,7 @@ func runBake(dockerCli command.Cli, targets []string, in bakeOptions) (err error
return nil
}

resp, err := build.Build(ctx, dis, bo, dockerAPI(dockerCli), confutil.ConfigDir(dockerCli), printer)
resp, err := build.Build(ctx, builder.Drivers, bo, storeutil.NewDockerClient(dockerCli), confutil.ConfigDir(dockerCli), printer)
if err != nil {
return err
}
Expand Down
30 changes: 22 additions & 8 deletions commands/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"strings"

"github.com/docker/buildx/build"
"github.com/docker/buildx/store/storeutil"
"github.com/docker/buildx/util/builderutil"
"github.com/docker/buildx/util/buildflags"
"github.com/docker/buildx/util/confutil"
"github.com/docker/buildx/util/platformutil"
Expand Down Expand Up @@ -204,7 +206,24 @@ func runBuild(dockerCli command.Cli, in buildOptions) (err error) {
contextPathHash = in.contextPath
}

imageID, err := buildTargets(ctx, dockerCli, map[string]build.Options{defaultTargetName: opts}, in.progress, contextPathHash, in.builder, in.metadataFile)
txn, release, err := storeutil.GetStore(dockerCli)
if err != nil {
return err
}
defer release()

builder, err := builderutil.New(dockerCli, txn, in.builder)
if err != nil {
return err
}
if err = builder.Validate(); err != nil {
return err
}
if err = builder.LoadDrivers(ctx, false, contextPathHash); err != nil {
return err
}

imageID, err := buildTargets(ctx, dockerCli, builder.Drivers, map[string]build.Options{defaultTargetName: opts}, in.progress, in.metadataFile)
if err != nil {
return err
}
Expand All @@ -215,18 +234,13 @@ func runBuild(dockerCli command.Cli, in buildOptions) (err error) {
return nil
}

func buildTargets(ctx context.Context, dockerCli command.Cli, opts map[string]build.Options, progressMode, contextPathHash, instance string, metadataFile string) (imageID string, err error) {
dis, err := getInstanceOrDefault(ctx, dockerCli, instance, contextPathHash)
if err != nil {
return "", err
}

func buildTargets(ctx context.Context, dockerCli command.Cli, drivers []builderutil.Driver, opts map[string]build.Options, progressMode string, metadataFile string) (imageID string, err error) {
ctx2, cancel := context.WithCancel(context.TODO())
defer cancel()

printer := progress.NewPrinter(ctx2, os.Stderr, progressMode)

resp, err := build.Build(ctx, dis, opts, dockerAPI(dockerCli), confutil.ConfigDir(dockerCli), printer)
resp, err := build.Build(ctx, drivers, opts, storeutil.NewDockerClient(dockerCli), confutil.ConfigDir(dockerCli), printer)
err1 := printer.Wait()
if err == nil {
err = err1
Expand Down
38 changes: 27 additions & 11 deletions commands/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ import (
"github.com/docker/buildx/driver"
"github.com/docker/buildx/store"
"github.com/docker/buildx/store/storeutil"
"github.com/docker/buildx/util/builderutil"
"github.com/docker/buildx/util/cobrautil"
"github.com/docker/cli/cli"
"github.com/docker/cli/cli/command"
"github.com/docker/cli/opts"
"github.com/google/shlex"
"github.com/moby/buildkit/util/appcontext"
"github.com/pkg/errors"
Expand Down Expand Up @@ -143,7 +145,6 @@ func runCreate(dockerCli command.Cli, in createOptions, args []string) error {
if dockerCli.CurrentContext() == "default" && dockerCli.DockerEndpoint().TLSData != nil {
return errors.Errorf("could not create a builder instance with TLS data loaded from environment. Please use `docker context create <context-name>` to create a context for current environment and then create a builder instance with `docker buildx create <context-name>`")
}

ep, err = storeutil.GetCurrentEndpoint(dockerCli)
if err != nil {
return err
Expand Down Expand Up @@ -185,17 +186,17 @@ func runCreate(dockerCli command.Cli, in createOptions, args []string) error {
}
}

ngi := &nginfo{ng: ng}

timeoutCtx, cancel := context.WithTimeout(ctx, 20*time.Second)
defer cancel()

if err = loadNodeGroupData(timeoutCtx, dockerCli, ngi); err != nil {
return err
}

if in.bootstrap {
if _, err = boot(ctx, ngi); err != nil {
builder, err := builderutil.New(dockerCli, txn, ng.Name)
if err != nil {
return err
}
timeoutCtx, cancel := context.WithTimeout(ctx, 20*time.Second)
defer cancel()
if err = builder.LoadDrivers(timeoutCtx, true, ""); err != nil {
return err
}
if _, err = builder.Boot(ctx); err != nil {
return err
}
}
Expand Down Expand Up @@ -263,3 +264,18 @@ func csvToMap(in []string) (map[string]string, error) {
}
return m, nil
}

func validateEndpoint(dockerCli command.Cli, ep string) (string, error) {
dem, err := storeutil.GetDockerEndpoint(dockerCli, ep)
if err == nil && dem != nil {
if ep == "default" {
return dem.Host, nil
}
return ep, nil
}
h, err := opts.ParseHost(true, ep)
if err != nil {
return "", errors.Wrapf(err, "failed to parse endpoint %s", ep)
}
return h, nil
}
Loading