Merge pull request #1713 from ktock/monitor-buildapi-options

controller: Extract nested CommonOptions on controller API

Author: jedevc

commands/bake.go

@@ -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"
@@ -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) {
@@ -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 {
@@ -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 {
@@ -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
 		}
 	}
@@ -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)
 		},
@@ -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")`)

commands/build.go

@@ -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
 }
 
@@ -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
@@ -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)
@@ -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`)
 
@@ -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")
 
@@ -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
 		}

commands/build_test.go

@@ -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"),
 			},
 		},
 	}

controller/build/build.go

@@ -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")
 	}
 
@@ -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,
@@ -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 {
@@ -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",
@@ -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 {
@@ -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