From e3d621fec5d75739ec715bd62faa551ee9781377 Mon Sep 17 00:00:00 2001 From: Ilya Dmitrichenko Date: Tue, 13 Dec 2022 13:45:06 +0000 Subject: [PATCH 1/4] builder: move node filtering functionality - add builder method - allow filtering on load to simplify invocations Signed-off-by: Ilya Dmitrichenko --- build/build.go | 22 ---------------------- builder/node.go | 27 +++++++++++++++++++++++++++ commands/bake.go | 2 +- commands/build.go | 2 +- commands/diskusage.go | 7 +------ commands/prune.go | 7 +------ 6 files changed, 31 insertions(+), 36 deletions(-) diff --git a/build/build.go b/build/build.go index a0e2974d..c42586d2 100644 --- a/build/build.go +++ b/build/build.go @@ -112,23 +112,6 @@ type NamedContext struct { State *llb.State } -func filterAvailableNodes(nodes []builder.Node) ([]builder.Node, error) { - out := make([]builder.Node, 0, len(nodes)) - err := errors.Errorf("no drivers found") - for _, n := range nodes { - if n.Err == nil && n.Driver != nil { - out = append(out, n) - } - if n.Err != nil { - err = n.Err - } - } - if len(out) > 0 { - return out, nil - } - return nil, err -} - type driverPair struct { driverIndex int platforms []specs.Platform @@ -802,11 +785,6 @@ func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opt map[s return nil, errors.Errorf("driver required for build") } - nodes, err = filterAvailableNodes(nodes) - if err != nil { - return nil, errors.Wrapf(err, "no valid drivers found") - } - var noMobyDriver driver.Driver for _, n := range nodes { if !n.Driver.IsMobyDriver() { diff --git a/builder/node.go b/builder/node.go index f565738d..3b1393e4 100644 --- a/builder/node.go +++ b/builder/node.go @@ -34,6 +34,24 @@ func (b *Builder) Nodes() []Node { return b.nodes } +// AvailableNodes returns only nodes that are available. +func (b *Builder) AvailableNodes() ([]Node, error) { + out := make([]Node, 0, len(b.nodes)) + err := errors.Errorf("no drivers found") + for _, n := range b.nodes { + if n.Err == nil && n.Driver != nil { + out = append(out, n) + } + if n.Err != nil { + err = n.Err + } + } + if len(out) > 0 { + return out, nil + } + return nil, err +} + // LoadNodes loads and returns nodes for this builder. // TODO: this should be a method on a Node object and lazy load data for each driver. func (b *Builder) LoadNodes(ctx context.Context, withData bool) (_ []Node, err error) { @@ -163,6 +181,15 @@ func (b *Builder) LoadNodes(ctx context.Context, withData bool) (_ []Node, err e return b.nodes, nil } +func (b *Builder) LoadAvailableNodes(ctx context.Context, withData bool) (_ []Node, err error) { + _, err = b.LoadNodes(ctx, withData) + if err != nil { + return nil, err + } + + return b.AvailableNodes() +} + func (n *Node) loadData(ctx context.Context) error { if n.Driver == nil { return nil diff --git a/commands/bake.go b/commands/bake.go index e36d36b9..2eccb111 100644 --- a/commands/bake.go +++ b/commands/bake.go @@ -114,7 +114,7 @@ func runBake(dockerCli command.Cli, targets []string, in bakeOptions) (err error if err = updateLastActivity(dockerCli, b.NodeGroup); err != nil { return errors.Wrapf(err, "failed to update builder last activity time") } - nodes, err = b.LoadNodes(ctx, false) + nodes, err = b.LoadAvailableNodes(ctx, false) if err != nil { return err } diff --git a/commands/build.go b/commands/build.go index e8e3c231..99e820e3 100644 --- a/commands/build.go +++ b/commands/build.go @@ -266,7 +266,7 @@ func runBuild(dockerCli command.Cli, in buildOptions) (err error) { if err = updateLastActivity(dockerCli, b.NodeGroup); err != nil { return errors.Wrapf(err, "failed to update builder last activity time") } - nodes, err := b.LoadNodes(ctx, false) + nodes, err := b.LoadAvailableNodes(ctx, false) if err != nil { return err } diff --git a/commands/diskusage.go b/commands/diskusage.go index d4bd7dda..7dd8b4f1 100644 --- a/commands/diskusage.go +++ b/commands/diskusage.go @@ -38,15 +38,10 @@ func runDiskUsage(dockerCli command.Cli, opts duOptions) error { return err } - nodes, err := b.LoadNodes(ctx, false) + nodes, err := b.LoadAvailableNodes(ctx, false) if err != nil { return err } - for _, node := range nodes { - if node.Err != nil { - return node.Err - } - } out := make([][]*client.UsageInfo, len(nodes)) diff --git a/commands/prune.go b/commands/prune.go index 653ffd84..697bfbe5 100644 --- a/commands/prune.go +++ b/commands/prune.go @@ -59,15 +59,10 @@ func runPrune(dockerCli command.Cli, opts pruneOptions) error { return err } - nodes, err := b.LoadNodes(ctx, false) + nodes, err := b.LoadAvailableNodes(ctx, false) if err != nil { return err } - for _, node := range nodes { - if node.Err != nil { - return node.Err - } - } ch := make(chan client.UsageInfo) printed := make(chan struct{}) From 61adfa576aa8589a1863381255d66a3290990469 Mon Sep 17 00:00:00 2001 From: Ilya Dmitrichenko Date: Tue, 13 Dec 2022 14:08:50 +0000 Subject: [PATCH 2/4] options: move Options types from build package Signed-off-by: Ilya Dmitrichenko --- bake/bake.go | 29 ++++++++++---------- build/build.go | 68 +++++++--------------------------------------- commands/build.go | 22 ++++++++------- commands/print.go | 4 +-- options/options.go | 61 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 100 insertions(+), 84 deletions(-) create mode 100644 options/options.go diff --git a/bake/bake.go b/bake/bake.go index 423d074c..3467cedc 100644 --- a/bake/bake.go +++ b/bake/bake.go @@ -14,7 +14,8 @@ import ( "strings" "github.com/docker/buildx/bake/hclparser" - "github.com/docker/buildx/build" + "github.com/docker/buildx/options" + "github.com/docker/buildx/util/buildflags" "github.com/docker/buildx/util/platformutil" "github.com/docker/cli/cli/config" @@ -760,8 +761,8 @@ func (t *Target) AddOverrides(overrides map[string]Override) error { return nil } -func TargetsToBuildOpt(m map[string]*Target, inp *Input) (map[string]build.Options, error) { - m2 := make(map[string]build.Options, len(m)) +func TargetsToBuildOpt(m map[string]*Target, inp *Input) (map[string]options.Options, error) { + m2 := make(map[string]options.Options, len(m)) for k, v := range m { bo, err := toBuildOpt(v, inp) if err != nil { @@ -772,14 +773,14 @@ func TargetsToBuildOpt(m map[string]*Target, inp *Input) (map[string]build.Optio return m2, nil } -func updateContext(t *build.Inputs, inp *Input) { +func updateContext(t *options.Inputs, inp *Input) { if inp == nil || inp.State == nil { return } for k, v := range t.NamedContexts { if v.Path == "." { - t.NamedContexts[k] = build.NamedContext{Path: inp.URL} + t.NamedContexts[k] = options.NamedContext{Path: inp.URL} } if strings.HasPrefix(v.Path, "cwd://") || strings.HasPrefix(v.Path, "target:") || strings.HasPrefix(v.Path, "docker-image:") { continue @@ -788,7 +789,7 @@ func updateContext(t *build.Inputs, inp *Input) { continue } st := llb.Scratch().File(llb.Copy(*inp.State, v.Path, "/"), llb.WithCustomNamef("set context %s to %s", k, v.Path)) - t.NamedContexts[k] = build.NamedContext{State: &st} + t.NamedContexts[k] = options.NamedContext{State: &st} } if t.ContextPath == "." { @@ -808,7 +809,7 @@ func updateContext(t *build.Inputs, inp *Input) { // validateContextsEntitlements is a basic check to ensure contexts do not // escape local directories when loaded from remote sources. This is to be // replaced with proper entitlements support in the future. -func validateContextsEntitlements(t build.Inputs, inp *Input) error { +func validateContextsEntitlements(t options.Inputs, inp *Input) error { if inp == nil || inp.State == nil { return nil } @@ -858,7 +859,7 @@ func checkPath(p string) error { return nil } -func toBuildOpt(t *Target, inp *Input) (*build.Options, error) { +func toBuildOpt(t *Target, inp *Input) (*options.Options, error) { if v := t.Context; v != nil && *v == "-" { return nil, errors.Errorf("context from stdin not allowed in bake") } @@ -895,7 +896,7 @@ func toBuildOpt(t *Target, inp *Input) (*build.Options, error) { networkMode = *t.NetworkMode } - bi := build.Inputs{ + bi := options.Inputs{ ContextPath: contextPath, DockerfilePath: dockerfilePath, NamedContexts: toNamedContexts(t.Contexts), @@ -909,7 +910,7 @@ func toBuildOpt(t *Target, inp *Input) (*build.Options, error) { } for k, v := range bi.NamedContexts { if strings.HasPrefix(v.Path, "cwd://") { - bi.NamedContexts[k] = build.NamedContext{Path: path.Clean(strings.TrimPrefix(v.Path, "cwd://"))} + bi.NamedContexts[k] = options.NamedContext{Path: path.Clean(strings.TrimPrefix(v.Path, "cwd://"))} } } @@ -919,7 +920,7 @@ func toBuildOpt(t *Target, inp *Input) (*build.Options, error) { t.Context = &bi.ContextPath - bo := &build.Options{ + bo := &options.Options{ Inputs: bi, Tags: t.Tags, BuildArgs: t.Args, @@ -1057,10 +1058,10 @@ func sliceEqual(s1, s2 []string) bool { return true } -func toNamedContexts(m map[string]string) map[string]build.NamedContext { - m2 := make(map[string]build.NamedContext, len(m)) +func toNamedContexts(m map[string]string) map[string]options.NamedContext { + m2 := make(map[string]options.NamedContext, len(m)) for k, v := range m { - m2[k] = build.NamedContext{Path: v} + m2[k] = options.NamedContext{Path: v} } return m2 } diff --git a/build/build.go b/build/build.go index c42586d2..bf61e4e0 100644 --- a/build/build.go +++ b/build/build.go @@ -23,13 +23,14 @@ import ( "github.com/containerd/containerd/images" "github.com/containerd/containerd/platforms" "github.com/docker/buildx/builder" + "github.com/docker/buildx/options" + "github.com/docker/buildx/driver" "github.com/docker/buildx/util/dockerutil" "github.com/docker/buildx/util/imagetools" "github.com/docker/buildx/util/progress" "github.com/docker/buildx/util/resolver" "github.com/docker/buildx/util/waitmap" - "github.com/docker/cli/opts" "github.com/docker/distribution/reference" "github.com/docker/docker/api/types" "github.com/docker/docker/builder/remotecontext/urlutil" @@ -39,7 +40,6 @@ import ( "github.com/moby/buildkit/exporter/containerimage/exptypes" "github.com/moby/buildkit/frontend/attestations" gateway "github.com/moby/buildkit/frontend/gateway/client" - "github.com/moby/buildkit/session" "github.com/moby/buildkit/session/upload/uploadprovider" "github.com/moby/buildkit/solver/errdefs" "github.com/moby/buildkit/solver/pb" @@ -64,54 +64,6 @@ const ( printFallbackImage = "docker/dockerfile-upstream:1.4-outline@sha256:627443ff4e2d0f635d429cfc1da5388bcd5a70949c38adcd3cd7c4e5df67c73c" ) -type Options struct { - Inputs Inputs - - Allow []entitlements.Entitlement - Attests map[string]*string - BuildArgs map[string]string - CacheFrom []client.CacheOptionsEntry - CacheTo []client.CacheOptionsEntry - CgroupParent string - Exports []client.ExportEntry - ExtraHosts []string - ImageIDFile string - Labels map[string]string - NetworkMode string - NoCache bool - NoCacheFilter []string - Platforms []specs.Platform - Pull bool - Session []session.Attachable - ShmSize opts.MemBytes - Tags []string - Target string - Ulimits *opts.UlimitOpt - - // Linked marks this target as exclusively linked (not requested by the user). - Linked bool - PrintFunc *PrintFunc -} - -type PrintFunc struct { - Name string - Format string -} - -type Inputs struct { - ContextPath string - DockerfilePath string - InStream io.Reader - ContextState *llb.State - DockerfileInline string - NamedContexts map[string]NamedContext -} - -type NamedContext struct { - Path string - State *llb.State -} - type driverPair struct { driverIndex int platforms []specs.Platform @@ -168,7 +120,7 @@ func ensureBooted(ctx context.Context, nodes []builder.Node, idxs []int, pw prog return clients, nil } -func splitToDriverPairs(availablePlatforms map[string]int, opt map[string]Options) map[string][]driverPair { +func splitToDriverPairs(availablePlatforms map[string]int, opt map[string]options.Options) map[string][]driverPair { m := map[string][]driverPair{} for k, opt := range opt { mm := map[int][]specs.Platform{} @@ -192,7 +144,7 @@ func splitToDriverPairs(availablePlatforms map[string]int, opt map[string]Option return m } -func resolveDrivers(ctx context.Context, nodes []builder.Node, opt map[string]Options, pw progress.Writer) (map[string][]driverPair, []*client.Client, error) { +func resolveDrivers(ctx context.Context, nodes []builder.Node, opt map[string]options.Options, pw progress.Writer) (map[string][]driverPair, []*client.Client, error) { dps, clients, err := resolveDriversBase(ctx, nodes, opt, pw) if err != nil { return nil, nil, err @@ -233,7 +185,7 @@ func resolveDrivers(ctx context.Context, nodes []builder.Node, opt map[string]Op return dps, clients, nil } -func resolveDriversBase(ctx context.Context, nodes []builder.Node, opt map[string]Options, pw progress.Writer) (map[string][]driverPair, []*client.Client, error) { +func resolveDriversBase(ctx context.Context, nodes []builder.Node, opt map[string]options.Options, pw progress.Writer) (map[string][]driverPair, []*client.Client, error) { availablePlatforms := map[string]int{} for i, node := range nodes { for _, p := range node.Platforms { @@ -339,7 +291,7 @@ func toRepoOnly(in string) (string, error) { return strings.Join(out, ","), nil } -func toSolveOpt(ctx context.Context, node builder.Node, multiDriver bool, opt Options, bopts gateway.BuildOpts, configDir string, pw progress.Writer, dl dockerLoadCallback) (solveOpt *client.SolveOpt, release func(), err error) { +func toSolveOpt(ctx context.Context, node builder.Node, multiDriver bool, opt options.Options, bopts gateway.BuildOpts, configDir string, pw progress.Writer, dl dockerLoadCallback) (solveOpt *client.SolveOpt, release func(), err error) { nodeDriver := node.Driver defers := make([]func(), 0, 2) releaseF := func() { @@ -776,11 +728,11 @@ func Invoke(ctx context.Context, cfg ContainerConfig) error { return err } -func Build(ctx context.Context, nodes []builder.Node, opt map[string]Options, docker *dockerutil.Client, configDir string, w progress.Writer) (resp map[string]*client.SolveResponse, err error) { +func Build(ctx context.Context, nodes []builder.Node, opt map[string]options.Options, docker *dockerutil.Client, configDir string, w progress.Writer) (resp map[string]*client.SolveResponse, err error) { return BuildWithResultHandler(ctx, nodes, opt, docker, configDir, w, nil, false) } -func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opt map[string]Options, docker *dockerutil.Client, configDir string, w progress.Writer, resultHandleFunc func(driverIndex int, rCtx *ResultContext), allowNoOutput bool) (resp map[string]*client.SolveResponse, err error) { +func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opt map[string]options.Options, docker *dockerutil.Client, configDir string, w progress.Writer, resultHandleFunc func(driverIndex int, rCtx *ResultContext), allowNoOutput bool) (resp map[string]*client.SolveResponse, err error) { if len(nodes) == 0 { return nil, errors.Errorf("driver required for build") } @@ -1339,7 +1291,7 @@ func createTempDockerfile(r io.Reader) (string, error) { return dir, err } -func LoadInputs(ctx context.Context, d driver.Driver, inp Inputs, pw progress.Writer, target *client.SolveOpt) (func(), error) { +func LoadInputs(ctx context.Context, d driver.Driver, inp options.Inputs, pw progress.Writer, target *client.SolveOpt) (func(), error) { if inp.ContextPath == "" { return nil, errors.New("please specify build context (e.g. \".\" for the current directory)") } @@ -1679,7 +1631,7 @@ func tryNodeIdentifier(configDir string) (out string) { return } -func noPrintFunc(opt map[string]Options) bool { +func noPrintFunc(opt map[string]options.Options) bool { for _, v := range opt { if v.PrintFunc != nil { return false diff --git a/commands/build.go b/commands/build.go index 99e820e3..5d76affb 100644 --- a/commands/build.go +++ b/commands/build.go @@ -16,6 +16,8 @@ import ( "github.com/containerd/console" "github.com/docker/buildx/build" + "github.com/docker/buildx/options" + "github.com/docker/buildx/builder" "github.com/docker/buildx/monitor" "github.com/docker/buildx/store" @@ -133,8 +135,8 @@ func runBuild(dockerCli command.Cli, in buildOptions) (err error) { return err } - opts := build.Options{ - Inputs: build.Inputs{ + opts := options.Options{ + Inputs: options.Inputs{ ContextPath: in.contextPath, DockerfilePath: in.dockerfileName, InStream: os.Stdin, @@ -271,7 +273,7 @@ func runBuild(dockerCli command.Cli, in buildOptions) (err error) { return err } - imageID, res, err := buildTargets(ctx, dockerCli, nodes, map[string]build.Options{defaultTargetName: opts}, in.progress, in.metadataFile, in.invoke != "") + imageID, res, err := buildTargets(ctx, dockerCli, nodes, map[string]options.Options{defaultTargetName: opts}, in.progress, in.metadataFile, in.invoke != "") err = wrapBuildError(err, false) if err != nil { return err @@ -288,7 +290,7 @@ func runBuild(dockerCli command.Cli, in buildOptions) (err error) { return errors.Errorf("failed to configure terminal: %v", err) } err = monitor.RunMonitor(ctx, cfg, func(ctx context.Context) (*build.ResultContext, error) { - _, rr, err := buildTargets(ctx, dockerCli, nodes, map[string]build.Options{defaultTargetName: opts}, in.progress, in.metadataFile, true) + _, rr, err := buildTargets(ctx, dockerCli, nodes, map[string]options.Options{defaultTargetName: opts}, in.progress, in.metadataFile, true) return rr, err }, io.NopCloser(os.Stdin), nopCloser{os.Stdout}, nopCloser{os.Stderr}) if err != nil { @@ -309,7 +311,7 @@ type nopCloser struct { func (c nopCloser) Close() error { return nil } -func buildTargets(ctx context.Context, dockerCli command.Cli, nodes []builder.Node, opts map[string]build.Options, progressMode string, metadataFile string, allowNoOutput bool) (imageID string, res *build.ResultContext, err error) { +func buildTargets(ctx context.Context, dockerCli command.Cli, nodes []builder.Node, opts map[string]options.Options, progressMode string, metadataFile string, allowNoOutput bool) (imageID string, res *build.ResultContext, err error) { ctx2, cancel := context.WithCancel(context.TODO()) defer cancel() @@ -623,11 +625,11 @@ func listToMap(values []string, defaultEnv bool) map[string]string { return result } -func parseContextNames(values []string) (map[string]build.NamedContext, error) { +func parseContextNames(values []string) (map[string]options.NamedContext, error) { if len(values) == 0 { return nil, nil } - result := make(map[string]build.NamedContext, len(values)) + result := make(map[string]options.NamedContext, len(values)) for _, value := range values { kv := strings.SplitN(value, "=", 2) if len(kv) != 2 { @@ -638,12 +640,12 @@ func parseContextNames(values []string) (map[string]build.NamedContext, error) { return nil, errors.Wrapf(err, "invalid context name %s", kv[0]) } name := strings.TrimSuffix(reference.FamiliarString(named), ":latest") - result[name] = build.NamedContext{Path: kv[1]} + result[name] = options.NamedContext{Path: kv[1]} } return result, nil } -func parsePrintFunc(str string) (*build.PrintFunc, error) { +func parsePrintFunc(str string) (*options.PrintFunc, error) { if str == "" { return nil, nil } @@ -652,7 +654,7 @@ func parsePrintFunc(str string) (*build.PrintFunc, error) { if err != nil { return nil, err } - f := &build.PrintFunc{} + f := &options.PrintFunc{} for _, field := range fields { parts := strings.SplitN(field, "=", 2) if len(parts) == 2 { diff --git a/commands/print.go b/commands/print.go index 9c7f9460..bcc6a259 100644 --- a/commands/print.go +++ b/commands/print.go @@ -6,14 +6,14 @@ import ( "log" "os" - "github.com/docker/buildx/build" + "github.com/docker/buildx/options" "github.com/docker/docker/api/types/versions" "github.com/moby/buildkit/frontend/subrequests" "github.com/moby/buildkit/frontend/subrequests/outline" "github.com/moby/buildkit/frontend/subrequests/targets" ) -func printResult(f *build.PrintFunc, res map[string]string) error { +func printResult(f *options.PrintFunc, res map[string]string) error { switch f.Name { case "outline": return printValue(outline.PrintOutline, outline.SubrequestsOutlineDefinition.Version, f.Format, res) diff --git a/options/options.go b/options/options.go new file mode 100644 index 00000000..c4bd099a --- /dev/null +++ b/options/options.go @@ -0,0 +1,61 @@ +package options + +import ( + _ "crypto/sha256" // ensure digests can be computed + "io" + + "github.com/docker/cli/opts" + "github.com/moby/buildkit/client" + "github.com/moby/buildkit/client/llb" + "github.com/moby/buildkit/session" + "github.com/moby/buildkit/util/entitlements" + specs "github.com/opencontainers/image-spec/specs-go/v1" +) + +type Options struct { + Inputs Inputs + + Allow []entitlements.Entitlement + Attests map[string]*string + BuildArgs map[string]string + CacheFrom []client.CacheOptionsEntry + CacheTo []client.CacheOptionsEntry + CgroupParent string + Exports []client.ExportEntry + ExtraHosts []string + ImageIDFile string + Labels map[string]string + NetworkMode string + NoCache bool + NoCacheFilter []string + Platforms []specs.Platform + Pull bool + Session []session.Attachable + ShmSize opts.MemBytes + Tags []string + Target string + Ulimits *opts.UlimitOpt + + // Linked marks this target as exclusively linked (not requested by the user). + Linked bool + PrintFunc *PrintFunc +} + +type PrintFunc struct { + Name string + Format string +} + +type Inputs struct { + ContextPath string + DockerfilePath string + InStream io.Reader + ContextState *llb.State + DockerfileInline string + NamedContexts map[string]NamedContext +} + +type NamedContext struct { + Path string + State *llb.State +} From 387cc633c71d1fed57d12907a8e57d390bd911b0 Mon Sep 17 00:00:00 2001 From: Ilya Dmitrichenko Date: Tue, 13 Dec 2022 14:22:06 +0000 Subject: [PATCH 3/4] build: move driver helpers to a separate file This code is very tightly coupled with the rest of the build package, and is not useful elsewhere, so it makes sense to keep it where it is. Separating into another package would require more significant changes to how `driverPair` struct is used in various functions. Signed-off-by: Ilya Dmitrichenko --- build/build.go | 210 ------------------------------------------- build/drivers.go | 230 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 230 insertions(+), 210 deletions(-) create mode 100644 build/drivers.go diff --git a/build/build.go b/build/build.go index bf61e4e0..4418d912 100644 --- a/build/build.go +++ b/build/build.go @@ -64,216 +64,6 @@ const ( printFallbackImage = "docker/dockerfile-upstream:1.4-outline@sha256:627443ff4e2d0f635d429cfc1da5388bcd5a70949c38adcd3cd7c4e5df67c73c" ) -type driverPair struct { - driverIndex int - platforms []specs.Platform - so *client.SolveOpt - bopts gateway.BuildOpts -} - -func driverIndexes(m map[string][]driverPair) []int { - out := make([]int, 0, len(m)) - visited := map[int]struct{}{} - for _, dp := range m { - for _, d := range dp { - if _, ok := visited[d.driverIndex]; ok { - continue - } - visited[d.driverIndex] = struct{}{} - out = append(out, d.driverIndex) - } - } - return out -} - -func allIndexes(l int) []int { - out := make([]int, 0, l) - for i := 0; i < l; i++ { - out = append(out, i) - } - return out -} - -func ensureBooted(ctx context.Context, nodes []builder.Node, idxs []int, pw progress.Writer) ([]*client.Client, error) { - clients := make([]*client.Client, len(nodes)) - - baseCtx := ctx - eg, ctx := errgroup.WithContext(ctx) - - for _, i := range idxs { - func(i int) { - eg.Go(func() error { - c, err := driver.Boot(ctx, baseCtx, nodes[i].Driver, pw) - if err != nil { - return err - } - clients[i] = c - return nil - }) - }(i) - } - - if err := eg.Wait(); err != nil { - return nil, err - } - - return clients, nil -} - -func splitToDriverPairs(availablePlatforms map[string]int, opt map[string]options.Options) map[string][]driverPair { - m := map[string][]driverPair{} - for k, opt := range opt { - mm := map[int][]specs.Platform{} - for _, p := range opt.Platforms { - k := platforms.Format(p) - idx := availablePlatforms[k] // default 0 - pp := mm[idx] - pp = append(pp, p) - mm[idx] = pp - } - // if no platform is specified, use first driver - if len(mm) == 0 { - mm[0] = nil - } - dps := make([]driverPair, 0, 2) - for idx, pp := range mm { - dps = append(dps, driverPair{driverIndex: idx, platforms: pp}) - } - m[k] = dps - } - return m -} - -func resolveDrivers(ctx context.Context, nodes []builder.Node, opt map[string]options.Options, pw progress.Writer) (map[string][]driverPair, []*client.Client, error) { - dps, clients, err := resolveDriversBase(ctx, nodes, opt, pw) - if err != nil { - return nil, nil, err - } - - bopts := make([]gateway.BuildOpts, len(clients)) - - span, ctx := tracing.StartSpan(ctx, "load buildkit capabilities", trace.WithSpanKind(trace.SpanKindInternal)) - - eg, ctx := errgroup.WithContext(ctx) - for i, c := range clients { - if c == nil { - continue - } - - func(i int, c *client.Client) { - eg.Go(func() error { - clients[i].Build(ctx, client.SolveOpt{}, "buildx", func(ctx context.Context, c gateway.Client) (*gateway.Result, error) { - bopts[i] = c.BuildOpts() - return nil, nil - }, nil) - return nil - }) - }(i, c) - } - - err = eg.Wait() - tracing.FinishWithError(span, err) - if err != nil { - return nil, nil, err - } - for key := range dps { - for i, dp := range dps[key] { - dps[key][i].bopts = bopts[dp.driverIndex] - } - } - - return dps, clients, nil -} - -func resolveDriversBase(ctx context.Context, nodes []builder.Node, opt map[string]options.Options, pw progress.Writer) (map[string][]driverPair, []*client.Client, error) { - availablePlatforms := map[string]int{} - for i, node := range nodes { - for _, p := range node.Platforms { - availablePlatforms[platforms.Format(p)] = i - } - } - - undetectedPlatform := false - allPlatforms := map[string]int{} - for _, opt := range opt { - for _, p := range opt.Platforms { - k := platforms.Format(p) - allPlatforms[k] = -1 - if _, ok := availablePlatforms[k]; !ok { - undetectedPlatform = true - } - } - } - - // fast path - if len(nodes) == 1 || len(allPlatforms) == 0 { - m := map[string][]driverPair{} - for k, opt := range opt { - m[k] = []driverPair{{driverIndex: 0, platforms: opt.Platforms}} - } - clients, err := ensureBooted(ctx, nodes, driverIndexes(m), pw) - if err != nil { - return nil, nil, err - } - return m, clients, nil - } - - // map based on existing platforms - if !undetectedPlatform { - m := splitToDriverPairs(availablePlatforms, opt) - clients, err := ensureBooted(ctx, nodes, driverIndexes(m), pw) - if err != nil { - return nil, nil, err - } - return m, clients, nil - } - - // boot all drivers in k - clients, err := ensureBooted(ctx, nodes, allIndexes(len(nodes)), pw) - if err != nil { - return nil, nil, err - } - - eg, ctx := errgroup.WithContext(ctx) - workers := make([][]*client.WorkerInfo, len(clients)) - - for i, c := range clients { - if c == nil { - continue - } - func(i int) { - eg.Go(func() error { - ww, err := clients[i].ListWorkers(ctx) - if err != nil { - return errors.Wrap(err, "listing workers") - } - workers[i] = ww - return nil - }) - - }(i) - } - - if err := eg.Wait(); err != nil { - return nil, nil, err - } - - for i, ww := range workers { - for _, w := range ww { - for _, p := range w.Platforms { - p = platforms.Normalize(p) - ps := platforms.Format(p) - - if _, ok := availablePlatforms[ps]; !ok { - availablePlatforms[ps] = i - } - } - } - } - - return splitToDriverPairs(availablePlatforms, opt), clients, nil -} - func toRepoOnly(in string) (string, error) { m := map[string]struct{}{} p := strings.Split(in, ",") diff --git a/build/drivers.go b/build/drivers.go new file mode 100644 index 00000000..60df0129 --- /dev/null +++ b/build/drivers.go @@ -0,0 +1,230 @@ +package build + +import ( + "context" + _ "crypto/sha256" // ensure digests can be computed + + "github.com/containerd/containerd/platforms" + "github.com/docker/buildx/builder" + "github.com/docker/buildx/options" + + "github.com/docker/buildx/driver" + "github.com/docker/buildx/util/progress" + "github.com/moby/buildkit/client" + gateway "github.com/moby/buildkit/frontend/gateway/client" + "github.com/moby/buildkit/util/tracing" + specs "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/pkg/errors" + "go.opentelemetry.io/otel/trace" + "golang.org/x/sync/errgroup" +) + +type driverPair struct { + driverIndex int + platforms []specs.Platform + so *client.SolveOpt + bopts gateway.BuildOpts +} + +func driverIndexes(m map[string][]driverPair) []int { + out := make([]int, 0, len(m)) + visited := map[int]struct{}{} + for _, dp := range m { + for _, d := range dp { + if _, ok := visited[d.driverIndex]; ok { + continue + } + visited[d.driverIndex] = struct{}{} + out = append(out, d.driverIndex) + } + } + return out +} + +func allIndexes(l int) []int { + out := make([]int, 0, l) + for i := 0; i < l; i++ { + out = append(out, i) + } + return out +} + +func ensureBooted(ctx context.Context, nodes []builder.Node, idxs []int, pw progress.Writer) ([]*client.Client, error) { + clients := make([]*client.Client, len(nodes)) + + baseCtx := ctx + eg, ctx := errgroup.WithContext(ctx) + + for _, i := range idxs { + func(i int) { + eg.Go(func() error { + c, err := driver.Boot(ctx, baseCtx, nodes[i].Driver, pw) + if err != nil { + return err + } + clients[i] = c + return nil + }) + }(i) + } + + if err := eg.Wait(); err != nil { + return nil, err + } + + return clients, nil +} + +func splitToDriverPairs(availablePlatforms map[string]int, opt map[string]options.Options) map[string][]driverPair { + m := map[string][]driverPair{} + for k, opt := range opt { + mm := map[int][]specs.Platform{} + for _, p := range opt.Platforms { + k := platforms.Format(p) + idx := availablePlatforms[k] // default 0 + pp := mm[idx] + pp = append(pp, p) + mm[idx] = pp + } + // if no platform is specified, use first driver + if len(mm) == 0 { + mm[0] = nil + } + dps := make([]driverPair, 0, 2) + for idx, pp := range mm { + dps = append(dps, driverPair{driverIndex: idx, platforms: pp}) + } + m[k] = dps + } + return m +} + +func resolveDrivers(ctx context.Context, nodes []builder.Node, opt map[string]options.Options, pw progress.Writer) (map[string][]driverPair, []*client.Client, error) { + dps, clients, err := resolveDriversBase(ctx, nodes, opt, pw) + if err != nil { + return nil, nil, err + } + + bopts := make([]gateway.BuildOpts, len(clients)) + + span, ctx := tracing.StartSpan(ctx, "load buildkit capabilities", trace.WithSpanKind(trace.SpanKindInternal)) + + eg, ctx := errgroup.WithContext(ctx) + for i, c := range clients { + if c == nil { + continue + } + + func(i int, c *client.Client) { + eg.Go(func() error { + clients[i].Build(ctx, client.SolveOpt{}, "buildx", func(ctx context.Context, c gateway.Client) (*gateway.Result, error) { + bopts[i] = c.BuildOpts() + return nil, nil + }, nil) + return nil + }) + }(i, c) + } + + err = eg.Wait() + tracing.FinishWithError(span, err) + if err != nil { + return nil, nil, err + } + for key := range dps { + for i, dp := range dps[key] { + dps[key][i].bopts = bopts[dp.driverIndex] + } + } + + return dps, clients, nil +} + +func resolveDriversBase(ctx context.Context, nodes []builder.Node, opt map[string]options.Options, pw progress.Writer) (map[string][]driverPair, []*client.Client, error) { + availablePlatforms := map[string]int{} + for i, node := range nodes { + for _, p := range node.Platforms { + availablePlatforms[platforms.Format(p)] = i + } + } + + undetectedPlatform := false + allPlatforms := map[string]int{} + for _, opt := range opt { + for _, p := range opt.Platforms { + k := platforms.Format(p) + allPlatforms[k] = -1 + if _, ok := availablePlatforms[k]; !ok { + undetectedPlatform = true + } + } + } + + // fast path + if len(nodes) == 1 || len(allPlatforms) == 0 { + m := map[string][]driverPair{} + for k, opt := range opt { + m[k] = []driverPair{{driverIndex: 0, platforms: opt.Platforms}} + } + clients, err := ensureBooted(ctx, nodes, driverIndexes(m), pw) + if err != nil { + return nil, nil, err + } + return m, clients, nil + } + + // map based on existing platforms + if !undetectedPlatform { + m := splitToDriverPairs(availablePlatforms, opt) + clients, err := ensureBooted(ctx, nodes, driverIndexes(m), pw) + if err != nil { + return nil, nil, err + } + return m, clients, nil + } + + // boot all drivers in k + clients, err := ensureBooted(ctx, nodes, allIndexes(len(nodes)), pw) + if err != nil { + return nil, nil, err + } + + eg, ctx := errgroup.WithContext(ctx) + workers := make([][]*client.WorkerInfo, len(clients)) + + for i, c := range clients { + if c == nil { + continue + } + func(i int) { + eg.Go(func() error { + ww, err := clients[i].ListWorkers(ctx) + if err != nil { + return errors.Wrap(err, "listing workers") + } + workers[i] = ww + return nil + }) + + }(i) + } + + if err := eg.Wait(); err != nil { + return nil, nil, err + } + + for i, ww := range workers { + for _, w := range ww { + for _, p := range w.Platforms { + p = platforms.Normalize(p) + ps := platforms.Format(p) + + if _, ok := availablePlatforms[ps]; !ok { + availablePlatforms[ps] = i + } + } + } + } + + return splitToDriverPairs(availablePlatforms, opt), clients, nil +} From 08b6d36debf50648c11878a0b13caba018792909 Mon Sep 17 00:00:00 2001 From: Ilya Dmitrichenko Date: Wed, 14 Dec 2022 10:54:58 +0000 Subject: [PATCH 4/4] build: small readability improvements by reversal of conditions This adds some ideomatic earlier returns and loop continuations to avoid deeply nested coditional blocks that are several lines long. Signed-off-by: Ilya Dmitrichenko --- build/build.go | 285 ++++++++++++++++++++++++++----------------------- 1 file changed, 150 insertions(+), 135 deletions(-) diff --git a/build/build.go b/build/build.go index 4418d912..28b5b4f5 100644 --- a/build/build.go +++ b/build/build.go @@ -691,92 +691,99 @@ func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opt map[s return nil } - if pushNames != "" { - progress.Write(pw, fmt.Sprintf("merging manifest list %s", pushNames), func() error { - descs := make([]specs.Descriptor, 0, len(res)) - - for _, r := range res { - s, ok := r.ExporterResponse[exptypes.ExporterImageDigestKey] - if ok { - descs = append(descs, specs.Descriptor{ - Digest: digest.Digest(s), - MediaType: images.MediaTypeDockerSchema2ManifestList, - Size: -1, - }) - } + if pushNames == "" { + return nil + } + + progress.Write(pw, fmt.Sprintf("merging manifest list %s", pushNames), func() error { + descs := make([]specs.Descriptor, 0, len(res)) + + for _, r := range res { + s, ok := r.ExporterResponse[exptypes.ExporterImageDigestKey] + if ok { + descs = append(descs, specs.Descriptor{ + Digest: digest.Digest(s), + MediaType: images.MediaTypeDockerSchema2ManifestList, + Size: -1, + }) } - if len(descs) > 0 { - var imageopt imagetools.Opt - for _, dp := range dps { - imageopt = nodes[dp.driverIndex].ImageOpt - break - } - names := strings.Split(pushNames, ",") + } - if insecurePush { - insecureTrue := true - httpTrue := true - nn, err := reference.ParseNormalizedNamed(names[0]) - if err != nil { - return err - } - imageopt.RegistryConfig = map[string]resolver.RegistryConfig{ - reference.Domain(nn): { - Insecure: &insecureTrue, - PlainHTTP: &httpTrue, - }, - } - } + if len(descs) == 0 { + return nil + } - itpull := imagetools.New(imageopt) + var imageopt imagetools.Opt + for _, dp := range dps { + imageopt = nodes[dp.driverIndex].ImageOpt + break + } + names := strings.Split(pushNames, ",") - ref, err := reference.ParseNormalizedNamed(names[0]) - if err != nil { - return err - } - ref = reference.TagNameOnly(ref) + if insecurePush { + insecureTrue := true + httpTrue := true + nn, err := reference.ParseNormalizedNamed(names[0]) + if err != nil { + return err + } + imageopt.RegistryConfig = map[string]resolver.RegistryConfig{ + reference.Domain(nn): { + Insecure: &insecureTrue, + PlainHTTP: &httpTrue, + }, + } + } - srcs := make([]*imagetools.Source, len(descs)) - for i, desc := range descs { - srcs[i] = &imagetools.Source{ - Desc: desc, - Ref: ref, - } - } + itpull := imagetools.New(imageopt) - dt, desc, err := itpull.Combine(ctx, srcs) - if err != nil { - return err - } - if opt.ImageIDFile != "" { - if err := os.WriteFile(opt.ImageIDFile, []byte(desc.Digest), 0644); err != nil { - return err - } - } + ref, err := reference.ParseNormalizedNamed(names[0]) + if err != nil { + return err + } + ref = reference.TagNameOnly(ref) - itpush := imagetools.New(imageopt) + srcs := make([]*imagetools.Source, len(descs)) + for i, desc := range descs { + srcs[i] = &imagetools.Source{ + Desc: desc, + Ref: ref, + } + } - for _, n := range names { - nn, err := reference.ParseNormalizedNamed(n) - if err != nil { - return err - } - if err := itpush.Push(ctx, nn, desc, dt); err != nil { - return err - } - } + dt, desc, err := itpull.Combine(ctx, srcs) + if err != nil { + return err + } + if opt.ImageIDFile != "" { + if err := os.WriteFile(opt.ImageIDFile, []byte(desc.Digest), 0644); err != nil { + return err + } + } - respMu.Lock() - resp[k] = &client.SolveResponse{ - ExporterResponse: map[string]string{ - "containerimage.digest": desc.Digest.String(), - }, - } - respMu.Unlock() + itpush := imagetools.New(imageopt) + + for _, n := range names { + nn, err := reference.ParseNormalizedNamed(n) + if err != nil { + return err } - return nil - }) - } + if err := itpush.Push(ctx, nn, desc, dt); err != nil { + return err + } + } + + respMu.Lock() + resp[k] = &client.SolveResponse{ + ExporterResponse: map[string]string{ + "containerimage.digest": desc.Digest.String(), + }, + } + respMu.Unlock() + + return nil + }) + return nil }) @@ -866,22 +873,23 @@ func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opt map[s return nil, err } var reqErr *errdefs.UnsupportedSubrequestError - if !isFallback { - if errors.As(err, &reqErr) { - switch reqErr.Name { - case "frontend.outline", "frontend.targets": - isFallback = true - origErr = err - continue - } - return nil, err - } - // buildkit v0.8 vendored in Docker 20.10 does not support typed errors - if strings.Contains(err.Error(), "unsupported request frontend.outline") || strings.Contains(err.Error(), "unsupported request frontend.targets") { + if isFallback { + return nil, err + } + if errors.As(err, &reqErr) { + switch reqErr.Name { + case "frontend.outline", "frontend.targets": isFallback = true origErr = err continue } + return nil, err + } + // buildkit v0.8 vendored in Docker 20.10 does not support typed errors + if strings.Contains(err.Error(), "unsupported request frontend.outline") || strings.Contains(err.Error(), "unsupported request frontend.targets") { + isFallback = true + origErr = err + continue } return nil, err } @@ -908,40 +916,44 @@ func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opt map[s } node := nodes[dp.driverIndex].Driver - if node.IsMobyDriver() { - for _, e := range so.Exports { - if e.Type == "moby" && e.Attrs["push"] != "" { - if ok, _ := strconv.ParseBool(e.Attrs["push"]); ok { - pushNames = e.Attrs["name"] - if pushNames == "" { - return errors.Errorf("tag is needed when pushing to registry") - } - pw := progress.ResetTime(pw) - pushList := strings.Split(pushNames, ",") - for _, name := range pushList { - if err := progress.Wrap(fmt.Sprintf("pushing %s with docker", name), pw.Write, func(l progress.SubLogger) error { - return pushWithMoby(ctx, node, name, l) - }); err != nil { - return err - } - } - remoteDigest, err := remoteDigestWithMoby(ctx, node, pushList[0]) - if err == nil && remoteDigest != "" { - // old daemons might not have containerimage.config.digest set - // in response so use containerimage.digest value for it if available - if _, ok := rr.ExporterResponse[exptypes.ExporterImageConfigDigestKey]; !ok { - if v, ok := rr.ExporterResponse[exptypes.ExporterImageDigestKey]; ok { - rr.ExporterResponse[exptypes.ExporterImageConfigDigestKey] = v - } - } - rr.ExporterResponse[exptypes.ExporterImageDigestKey] = remoteDigest - } else if err != nil { - return err + if !node.IsMobyDriver() { + return nil + } + + for _, e := range so.Exports { + if e.Type == "moby" && e.Attrs["push"] != "" { + if ok, _ := strconv.ParseBool(e.Attrs["push"]); !ok { + continue + } + pushNames = e.Attrs["name"] + if pushNames == "" { + return errors.Errorf("tag is needed when pushing to registry") + } + pw := progress.ResetTime(pw) + pushList := strings.Split(pushNames, ",") + for _, name := range pushList { + if err := progress.Wrap(fmt.Sprintf("pushing %s with docker", name), pw.Write, func(l progress.SubLogger) error { + return pushWithMoby(ctx, node, name, l) + }); err != nil { + return err + } + } + remoteDigest, err := remoteDigestWithMoby(ctx, node, pushList[0]) + if err == nil && remoteDigest != "" { + // old daemons might not have containerimage.config.digest set + // in response so use containerimage.digest value for it if available + if _, ok := rr.ExporterResponse[exptypes.ExporterImageConfigDigestKey]; !ok { + if v, ok := rr.ExporterResponse[exptypes.ExporterImageDigestKey]; ok { + rr.ExporterResponse[exptypes.ExporterImageConfigDigestKey] = v } } + rr.ExporterResponse[exptypes.ExporterImageDigestKey] = remoteDigest + } else if err != nil { + return err } } } + return nil }) @@ -1315,27 +1327,30 @@ func waitContextDeps(ctx context.Context, index int, results *waitmap.Map, so *c } delete(so.FrontendAttrs, v) } - if rr.Ref != nil { - st, err := rr.Ref.ToState() + + if rr.Ref == nil { + continue + } + + st, err := rr.Ref.ToState() + if err != nil { + return err + } + so.FrontendInputs[k] = st + so.FrontendAttrs[v] = "input:" + k + metadata := make(map[string][]byte) + if dt, ok := rr.Metadata[exptypes.ExporterImageConfigKey]; ok { + metadata[exptypes.ExporterImageConfigKey] = dt + } + if dt, ok := rr.Metadata[exptypes.ExporterBuildInfo]; ok { + metadata[exptypes.ExporterBuildInfo] = dt + } + if len(metadata) > 0 { + dt, err := json.Marshal(metadata) if err != nil { return err } - so.FrontendInputs[k] = st - so.FrontendAttrs[v] = "input:" + k - metadata := make(map[string][]byte) - if dt, ok := rr.Metadata[exptypes.ExporterImageConfigKey]; ok { - metadata[exptypes.ExporterImageConfigKey] = dt - } - if dt, ok := rr.Metadata[exptypes.ExporterBuildInfo]; ok { - metadata[exptypes.ExporterBuildInfo] = dt - } - if len(metadata) > 0 { - dt, err := json.Marshal(metadata) - if err != nil { - return err - } - so.FrontendAttrs["input-metadata:"+k] = string(dt) - } + so.FrontendAttrs["input-metadata:"+k] = string(dt) } } return nil