Skip to content

controller: Extract nested CommonOptions on controller API #1713

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.

Sign up for GitHub

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

Merged
jedevc merged 1 commit into from
Apr 3, 2023
Merged

controller: Extract nested CommonOptions on controller API #1713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 15 additions & 12 deletions commands/bake.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"github.com/docker/buildx/bake"
"github.com/docker/buildx/build"
"github.com/docker/buildx/builder"
controllerapi "github.com/docker/buildx/controller/pb"
"github.com/docker/buildx/util/buildflags"
"github.com/docker/buildx/util/confutil"
"github.com/docker/buildx/util/dockerutil"
Expand All @@ -29,7 +28,11 @@ type bakeOptions struct {
printOnly bool
sbom string
provenance string
controllerapi.CommonOptions

builder string
metadataFile string
exportPush bool
exportLoad bool
}

func runBake(dockerCli command.Cli, targets []string, in bakeOptions, cFlags commonFlags) (err error) {
Expand Down Expand Up @@ -64,12 +67,12 @@ func runBake(dockerCli command.Cli, targets []string, in bakeOptions, cFlags com
}

overrides := in.overrides
if in.ExportPush {
if in.ExportLoad {
if in.exportPush {
if in.exportLoad {
return errors.Errorf("push and load may not be set together at the moment")
}
overrides = append(overrides, "*.push=true")
} else if in.ExportLoad {
} else if in.exportLoad {
overrides = append(overrides, "*.output=type=docker")
}
if cFlags.noCache != nil {
Expand Down Expand Up @@ -97,7 +100,7 @@ func runBake(dockerCli command.Cli, targets []string, in bakeOptions, cFlags com
// instance only needed for reading remote bake files or building
if url != "" || !in.printOnly {
b, err := builder.New(dockerCli,
builder.WithName(in.Builder),
builder.WithName(in.builder),
builder.WithContextPathHash(contextPathHash),
)
if err != nil {
Expand Down Expand Up @@ -193,12 +196,12 @@ func runBake(dockerCli command.Cli, targets []string, in bakeOptions, cFlags com
return wrapBuildError(err, true)
}

if len(in.MetadataFile) > 0 {
if len(in.metadataFile) > 0 {
dt := make(map[string]interface{})
for t, r := range resp {
dt[t] = decodeExporterResponse(r.ExporterResponse)
}
if err := writeMetadataFile(in.MetadataFile, dt); err != nil {
if err := writeMetadataFile(in.metadataFile, dt); err != nil {
return err
}
}
Expand All @@ -222,8 +225,8 @@ func bakeCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command {
if !cmd.Flags().Lookup("pull").Changed {
cFlags.pull = nil
}
options.Builder = rootOpts.builder
options.MetadataFile = cFlags.metadataFile
options.builder = rootOpts.builder
options.metadataFile = cFlags.metadataFile
// Other common flags (noCache, pull and progress) are processed in runBake function.
return runBake(dockerCli, args, options, cFlags)
},
Expand All @@ -232,9 +235,9 @@ func bakeCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command {
flags := cmd.Flags()

flags.StringArrayVarP(&options.files, "file", "f", []string{}, "Build definition file")
flags.BoolVar(&options.ExportLoad, "load", false, `Shorthand for "--set=*.output=type=docker"`)
flags.BoolVar(&options.exportLoad, "load", false, `Shorthand for "--set=*.output=type=docker"`)
flags.BoolVar(&options.printOnly, "print", false, "Print the options without building")
flags.BoolVar(&options.ExportPush, "push", false, `Shorthand for "--set=*.output=type=registry"`)
flags.BoolVar(&options.exportPush, "push", false, `Shorthand for "--set=*.output=type=registry"`)
flags.StringVar(&options.sbom, "sbom", "", `Shorthand for "--set=*.attest=type=sbom"`)
flags.StringVar(&options.provenance, "provenance", "", `Shorthand for "--set=*.attest=type=provenance"`)
flags.StringArrayVar(&options.overrides, "set", nil, `Override target value (e.g., "targetpattern.key=value")`)
Expand Down
35 changes: 23 additions & 12 deletions commands/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,13 @@ type buildOptions struct {
progress string
quiet bool

controllerapi.CommonOptions
builder string
metadataFile string
noCache bool
pull bool
exportPush bool
exportLoad bool

control.ControlOptions
}

Expand All @@ -97,7 +103,12 @@ func (o *buildOptions) toControllerOptions() (controllerapi.BuildOptions, error)
Tags: o.tags,
Target: o.target,
Ulimits: dockerUlimitToControllerUlimit(o.ulimits),
Opts: &o.CommonOptions,
Builder: o.builder,
MetadataFile: o.metadataFile,
NoCache: o.noCache,
Pull: o.pull,
ExportPush: o.exportPush,
ExportLoad: o.exportLoad,
}

// TODO: extract env var parsing to a method easily usable by library consumers
Expand Down Expand Up @@ -225,15 +236,15 @@ func buildCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command {
Args: cli.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
options.contextPath = args[0]
options.Builder = rootOpts.builder
options.MetadataFile = cFlags.metadataFile
options.NoCache = false
options.builder = rootOpts.builder
options.metadataFile = cFlags.metadataFile
options.noCache = false
if cFlags.noCache != nil {
options.NoCache = *cFlags.noCache
options.noCache = *cFlags.noCache
}
options.Pull = false
options.pull = false
if cFlags.pull != nil {
options.Pull = *cFlags.pull
options.pull = *cFlags.pull
}
options.progress = cFlags.progress
cmd.Flags().VisitAll(checkWarnedFlags)
Expand Down Expand Up @@ -274,7 +285,7 @@ func buildCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command {

flags.StringArrayVar(&options.labels, "label", []string{}, "Set metadata for an image")

flags.BoolVar(&options.ExportLoad, "load", false, `Shorthand for "--output=type=docker"`)
flags.BoolVar(&options.exportLoad, "load", false, `Shorthand for "--output=type=docker"`)

flags.StringVar(&options.networkMode, "network", "default", `Set the networking mode for the "RUN" instructions during build`)

Expand All @@ -288,7 +299,7 @@ func buildCmd(dockerCli command.Cli, rootOpts *rootOptions) *cobra.Command {
flags.StringVar(&options.printFunc, "print", "", "Print result of information request (e.g., outline, targets) [experimental]")
}

flags.BoolVar(&options.ExportPush, "push", false, `Shorthand for "--output=type=registry"`)
flags.BoolVar(&options.exportPush, "push", false, `Shorthand for "--output=type=registry"`)

flags.BoolVarP(&options.quiet, "quiet", "q", false, "Suppress the build output and print image ID on success")

Expand Down Expand Up @@ -814,8 +825,8 @@ func resolvePaths(options *controllerapi.BuildOptions) (_ *controllerapi.BuildOp
}
options.SSH = ssh

if options.Opts != nil && options.Opts.MetadataFile != "" {
options.Opts.MetadataFile, err = filepath.Abs(options.Opts.MetadataFile)
if options.MetadataFile != "" {
options.MetadataFile, err = filepath.Abs(options.MetadataFile)
if err != nil {
return nil, err
}
Expand Down
8 changes: 2 additions & 6 deletions commands/build_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,14 +233,10 @@ func TestResolvePaths(t *testing.T) {
{
name: "metadatafile",
options: controllerapi.BuildOptions{
Opts: &controllerapi.CommonOptions{
MetadataFile: "test1",
},
MetadataFile: "test1",
},
want: controllerapi.BuildOptions{
Opts: &controllerapi.CommonOptions{
MetadataFile: filepath.Join(tmpwd, "test1"),
},
MetadataFile: filepath.Join(tmpwd, "test1"),
},
},
}
Expand Down
16 changes: 8 additions & 8 deletions controller/build/build.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import (
const defaultTargetName = "default"

func RunBuild(ctx context.Context, dockerCli command.Cli, in controllerapi.BuildOptions, inStream io.Reader, progressMode string, statusChan chan *client.SolveStatus) (*client.SolveResponse, *build.ResultContext, error) {
if in.Opts.NoCache && len(in.NoCacheFilter) > 0 {
if in.NoCache && len(in.NoCacheFilter) > 0 {
return nil, nil, errors.Errorf("--no-cache and --no-cache-filter cannot currently be used together")
}

Expand All @@ -67,9 +67,9 @@ func RunBuild(ctx context.Context, dockerCli command.Cli, in controllerapi.Build
ExtraHosts: in.ExtraHosts,
Labels: in.Labels,
NetworkMode: in.NetworkMode,
NoCache: in.Opts.NoCache,
NoCache: in.NoCache,
NoCacheFilter: in.NoCacheFilter,
Pull: in.Opts.Pull,
Pull: in.Pull,
ShmSize: dockeropts.MemBytes(in.ShmSize),
Tags: in.Tags,
Target: in.Target,
Expand Down Expand Up @@ -106,8 +106,8 @@ func RunBuild(ctx context.Context, dockerCli command.Cli, in controllerapi.Build
if err != nil {
return nil, nil, err
}
if in.Opts.ExportPush {
if in.Opts.ExportLoad {
if in.ExportPush {
if in.ExportLoad {
return nil, nil, errors.Errorf("push and load may not be set together at the moment")
}
if len(outputs) == 0 {
Expand All @@ -126,7 +126,7 @@ func RunBuild(ctx context.Context, dockerCli command.Cli, in controllerapi.Build
}
}
}
if in.Opts.ExportLoad {
if in.ExportLoad {
if len(outputs) == 0 {
outputs = []client.ExportEntry{{
Type: "docker",
Expand Down Expand Up @@ -160,7 +160,7 @@ func RunBuild(ctx context.Context, dockerCli command.Cli, in controllerapi.Build
}

b, err := builder.New(dockerCli,
builder.WithName(in.Opts.Builder),
builder.WithName(in.Builder),
builder.WithContextPathHash(contextPathHash),
)
if err != nil {
Expand All @@ -174,7 +174,7 @@ func RunBuild(ctx context.Context, dockerCli command.Cli, in controllerapi.Build
return nil, nil, err
}

resp, res, err := buildTargets(ctx, dockerCli, b.NodeGroup, nodes, map[string]build.Options{defaultTargetName: opts}, progressMode, in.Opts.MetadataFile, statusChan)
resp, res, err := buildTargets(ctx, dockerCli, b.NodeGroup, nodes, map[string]build.Options{defaultTargetName: opts}, progressMode, in.MetadataFile, statusChan)
err = wrapBuildError(err, false)
if err != nil {
return nil, nil, err
Expand Down
Loading