Merge pull request #3220 from jsternberg/monitor-driven-build

monitor: move remaining controller functionality into monitor

Author: tonistiigi

build/build.go

@@ -311,11 +311,15 @@ func toRepoOnly(in string) (string, error) {
 	return strings.Join(out, ","), nil
 }
 
+type Handler struct {
+	OnResult func(driverIdx int, rCtx *ResultHandle)
+}
+
 func Build(ctx context.Context, nodes []builder.Node, opts map[string]Options, docker *dockerutil.Client, cfg *confutil.Config, w progress.Writer) (resp map[string]*client.SolveResponse, err error) {
 	return BuildWithResultHandler(ctx, nodes, opts, docker, cfg, w, nil)
 }
 
-func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opts map[string]Options, docker *dockerutil.Client, cfg *confutil.Config, w progress.Writer, resultHandleFunc func(driverIdx int, rCtx *ResultHandle)) (resp map[string]*client.SolveResponse, err error) {
+func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opts map[string]Options, docker *dockerutil.Client, cfg *confutil.Config, w progress.Writer, bh *Handler) (resp map[string]*client.SolveResponse, err error) {
 	if len(nodes) == 0 {
 		return nil, errors.Errorf("driver required for build")
 	}
@@ -509,10 +513,10 @@ func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opts map[
 					buildRef := fmt.Sprintf("%s/%s/%s", node.Builder, node.Name, so.Ref)
 
 					var rr *client.SolveResponse
-					if resultHandleFunc != nil {
+					if bh != nil && bh.OnResult != nil {
 						var resultHandle *ResultHandle
 						resultHandle, rr, err = NewResultHandle(ctx, cc, *so, "buildx", buildFunc, ch)
-						resultHandleFunc(dp.driverIndex, resultHandle)
+						bh.OnResult(dp.driverIndex, resultHandle)
 					} else {
 						span, ctx := tracing.StartSpan(ctx, "build")
 						rr, err = c.Build(ctx, *so, "buildx", buildFunc, ch)

commands/build.go

@@ -23,7 +23,6 @@ import (
 	"github.com/docker/buildx/commands/debug"
 	cbuild "github.com/docker/buildx/controller/build"
 	controllererrors "github.com/docker/buildx/controller/errdefs"
-	"github.com/docker/buildx/controller/local"
 	controllerapi "github.com/docker/buildx/controller/pb"
 	"github.com/docker/buildx/monitor"
 	"github.com/docker/buildx/store"
@@ -32,7 +31,6 @@ import (
 	"github.com/docker/buildx/util/cobrautil"
 	"github.com/docker/buildx/util/confutil"
 	"github.com/docker/buildx/util/desktop"
-	"github.com/docker/buildx/util/ioset"
 	"github.com/docker/buildx/util/metricutil"
 	"github.com/docker/buildx/util/osutil"
 	"github.com/docker/buildx/util/progress"
@@ -418,11 +416,7 @@ func getImageID(resp map[string]string) string {
 }
 
 func runBasicBuild(ctx context.Context, dockerCli command.Cli, opts *cbuild.Options, printer *progress.Printer) (*client.SolveResponse, *build.Inputs, error) {
-	resp, res, dfmap, err := cbuild.RunBuild(ctx, dockerCli, opts, dockerCli.In(), printer, false)
-	if res != nil {
-		res.Done()
-	}
-	return resp, dfmap, err
+	return cbuild.RunBuild(ctx, dockerCli, opts, dockerCli.In(), printer, nil)
 }
 
 func runControllerBuild(ctx context.Context, dockerCli command.Cli, opts *cbuild.Options, options buildOptions, printer *progress.Printer) (_ *client.SolveResponse, _ *build.Inputs, retErr error) {
@@ -433,33 +427,32 @@ func runControllerBuild(ctx context.Context, dockerCli command.Cli, opts *cbuild
 
 	var (
 		in io.ReadCloser
-		f  *ioset.SingleForwarder
+		m  *monitor.Monitor
+		bh build.Handler
 	)
 	if options.invokeConfig == nil {
 		in = dockerCli.In()
 	} else {
-		f = ioset.NewSingleForwarder()
-		f.SetReader(dockerCli.In())
+		m = monitor.New(&options.invokeConfig.InvokeConfig, dockerCli.In(), os.Stdout, os.Stderr, printer)
+		defer m.Close()
+
+		bh = m.Handler()
 	}
 
 	for {
-		c := local.NewController(ctx, dockerCli)
-
-		resp, inputs, err := c.Build(ctx, opts, in, printer)
+		resp, inputs, err := cbuild.RunBuild(ctx, dockerCli, opts, in, printer, &bh)
 		if err != nil {
 			var be *controllererrors.BuildError
 			if errors.As(err, &be) {
 				retErr = err
 				// We can proceed to monitor
 			} else {
-				c.Close()
 				return nil, nil, errors.Wrapf(err, "failed to build")
 			}
 		}
 
-		if options.invokeConfig != nil {
-			if err := runMonitorIfNeeded(ctx, options.invokeConfig, retErr, c, f, os.Stdout, os.Stderr, printer); err != nil {
-				c.Close()
+		if m != nil {
+			if err := m.Run(ctx, err); err != nil {
 				if errors.Is(err, monitor.ErrReload) {
 					retErr = nil
 					continue
@@ -468,55 +461,10 @@ func runControllerBuild(ctx context.Context, dockerCli command.Cli, opts *cbuild
 			}
 		}
 
-		c.Close()
 		return resp, inputs, err
 	}
 }
 
-func runMonitorIfNeeded(ctx context.Context, cfg *invokeConfig, retErr error, c *local.Controller, stdin *ioset.SingleForwarder, stdout io.WriteCloser, stderr console.File, printer *progress.Printer) error {
-	if !cfg.needsDebug(retErr) {
-		return nil
-	}
-
-	// Print errors before launching monitor
-	if err := printError(retErr, printer); err != nil {
-		logrus.Warnf("failed to print error information: %v", err)
-	}
-
-	pr, pw := io.Pipe()
-	stdin.SetWriter(pw, func() io.WriteCloser {
-		pw.Close() // propagate EOF
-		return nil
-	})
-
-	con := console.Current()
-	if err := con.SetRaw(); err != nil {
-		return errors.Errorf("failed to configure terminal: %v", err)
-	}
-	defer con.Reset()
-
-	monitorErr := monitor.RunMonitor(ctx, &cfg.InvokeConfig, c, pr, stdout, stderr, printer)
-	if err := pw.Close(); err != nil {
-		logrus.Debug("failed to close monitor stdin pipe reader")
-	}
-	return monitorErr
-}
-
-func printError(err error, printer *progress.Printer) error {
-	if err == nil {
-		return nil
-	}
-	if err := printer.Pause(); err != nil {
-		return err
-	}
-	defer printer.Unpause()
-	for _, s := range errdefs.Sources(err) {
-		s.Print(os.Stderr)
-	}
-	fmt.Fprintf(os.Stderr, "ERROR: %v\n", err)
-	return nil
-}
-
 func newDebuggableBuild(dockerCli command.Cli, rootOpts *rootOptions) debug.DebuggableCmd {
 	return &debuggableBuild{dockerCli: dockerCli, rootOpts: rootOpts}
 }
@@ -973,23 +921,21 @@ func printValue(w io.Writer, printer callFunc, version string, format string, re
 
 type invokeConfig struct {
 	controllerapi.InvokeConfig
-	onFlag     string
 	invokeFlag string
 }
 
-func (cfg *invokeConfig) needsDebug(retErr error) bool {
-	switch cfg.onFlag {
+func (cfg *invokeConfig) parseInvokeConfig(invoke, on string) error {
+	switch on {
 	case "always":
-		return true
+		cfg.SuspendOn = controllerapi.SuspendAlways
 	case "error":
-		return retErr != nil
+		cfg.SuspendOn = controllerapi.SuspendError
 	default:
-		return cfg.invokeFlag != ""
+		if invoke != "" {
+			cfg.SuspendOn = controllerapi.SuspendAlways
+		}
 	}
-}
 
-func (cfg *invokeConfig) parseInvokeConfig(invoke, on string) error {
-	cfg.onFlag = on
 	cfg.invokeFlag = invoke
 	cfg.Tty = true
 	cfg.NoCmd = true

commands/debug/root.go

@@ -1,18 +1,8 @@
 package debug
 
 import (
-	"context"
-	"os"
-
-	"github.com/containerd/console"
-	"github.com/docker/buildx/controller/local"
-	controllerapi "github.com/docker/buildx/controller/pb"
-	"github.com/docker/buildx/monitor"
 	"github.com/docker/buildx/util/cobrautil"
-	"github.com/docker/buildx/util/progress"
 	"github.com/docker/cli/cli/command"
-	"github.com/moby/buildkit/util/progress/progressui"
-	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 )
 
@@ -38,27 +28,6 @@ func RootCmd(dockerCli command.Cli, children ...DebuggableCmd) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "debug",
 		Short: "Start debugger",
-		Args:  cobra.NoArgs,
-		RunE: func(cmd *cobra.Command, args []string) error {
-			printer, err := progress.NewPrinter(context.TODO(), os.Stderr, progressui.DisplayMode(progressMode))
-			if err != nil {
-				return err
-			}
-
-			ctx := context.TODO()
-			c := local.NewController(ctx, dockerCli)
-
-			con := console.Current()
-			if err := con.SetRaw(); err != nil {
-				return errors.Errorf("failed to configure terminal: %v", err)
-			}
-
-			err = monitor.RunMonitor(ctx, &controllerapi.InvokeConfig{
-				Tty: true,
-			}, c, dockerCli.In(), os.Stdout, os.Stderr, printer)
-			con.Reset()
-			return err
-		},
 	}
 	cobrautil.MarkCommandExperimental(cmd)
 

controller/build/build.go

@@ -5,10 +5,10 @@ import (
 	"io"
 	"path/filepath"
 	"strings"
-	"sync"
 
 	"github.com/docker/buildx/build"
 	"github.com/docker/buildx/builder"
+	"github.com/docker/buildx/controller/errdefs"
 	controllerapi "github.com/docker/buildx/controller/pb"
 	"github.com/docker/buildx/store"
 	"github.com/docker/buildx/store/storeutil"
@@ -34,9 +34,9 @@ const defaultTargetName = "default"
 // NOTE: When an error happens during the build and this function acquires the debuggable *build.ResultHandle,
 // this function returns it in addition to the error (i.e. it does "return nil, res, err"). The caller can
 // inspect the result and debug the cause of that error.
-func RunBuild(ctx context.Context, dockerCli command.Cli, in *Options, inStream io.Reader, progress progress.Writer, generateResult bool) (*client.SolveResponse, *build.ResultHandle, *build.Inputs, error) {
+func RunBuild(ctx context.Context, dockerCli command.Cli, in *Options, inStream io.Reader, progress progress.Writer, bh *build.Handler) (*client.SolveResponse, *build.Inputs, error) {
 	if in.NoCache && len(in.NoCacheFilter) > 0 {
-		return nil, nil, nil, errors.Errorf("--no-cache and --no-cache-filter cannot currently be used together")
+		return nil, nil, errors.Errorf("--no-cache and --no-cache-filter cannot currently be used together")
 	}
 
 	contexts := map[string]build.NamedContext{}
@@ -70,7 +70,7 @@ func RunBuild(ctx context.Context, dockerCli command.Cli, in *Options, inStream
 
 	platforms, err := platformutil.Parse(in.Platforms)
 	if err != nil {
-		return nil, nil, nil, err
+		return nil, nil, err
 	}
 	opts.Platforms = platforms
 
@@ -81,7 +81,7 @@ func RunBuild(ctx context.Context, dockerCli command.Cli, in *Options, inStream
 
 	secrets, err := controllerapi.CreateSecrets(in.Secrets)
 	if err != nil {
-		return nil, nil, nil, err
+		return nil, nil, err
 	}
 	opts.Session = append(opts.Session, secrets)
 
@@ -91,13 +91,13 @@ func RunBuild(ctx context.Context, dockerCli command.Cli, in *Options, inStream
 	}
 	ssh, err := controllerapi.CreateSSH(sshSpecs)
 	if err != nil {
-		return nil, nil, nil, err
+		return nil, nil, err
 	}
 	opts.Session = append(opts.Session, ssh)
 
 	outputs, _, err := controllerapi.CreateExports(in.Exports)
 	if err != nil {
-		return nil, nil, nil, err
+		return nil, nil, err
 	}
 	if in.ExportPush {
 		var pushUsed bool
@@ -136,7 +136,7 @@ func RunBuild(ctx context.Context, dockerCli command.Cli, in *Options, inStream
 
 	annotations, err := buildflags.ParseAnnotations(in.Annotations)
 	if err != nil {
-		return nil, nil, nil, errors.Wrap(err, "parse annotations")
+		return nil, nil, errors.Wrap(err, "parse annotations")
 	}
 
 	for _, o := range outputs {
@@ -156,7 +156,7 @@ func RunBuild(ctx context.Context, dockerCli command.Cli, in *Options, inStream
 
 	allow, err := buildflags.ParseEntitlements(in.Allow)
 	if err != nil {
-		return nil, nil, nil, err
+		return nil, nil, err
 	}
 	opts.Allow = allow
 
@@ -180,56 +180,40 @@ func RunBuild(ctx context.Context, dockerCli command.Cli, in *Options, inStream
 		builder.WithContextPathHash(contextPathHash),
 	)
 	if err != nil {
-		return nil, nil, nil, err
+		return nil, nil, err
 	}
 	if err = updateLastActivity(dockerCli, b.NodeGroup); err != nil {
-		return nil, nil, nil, errors.Wrapf(err, "failed to update builder last activity time")
+		return nil, nil, errors.Wrapf(err, "failed to update builder last activity time")
 	}
 	nodes, err := b.LoadNodes(ctx)
 	if err != nil {
-		return nil, nil, nil, err
+		return nil, nil, err
 	}
 
 	var inputs *build.Inputs
 	buildOptions := map[string]build.Options{defaultTargetName: opts}
-	resp, res, err := buildTargets(ctx, dockerCli, nodes, buildOptions, progress, generateResult)
+	resp, err := buildTargets(ctx, dockerCli, nodes, buildOptions, progress, bh)
 	err = wrapBuildError(err, false)
 	if err != nil {
-		// NOTE: buildTargets can return *build.ResultHandle even on error.
-		return nil, res, nil, err
+		return nil, nil, errdefs.WrapBuild(err)
 	}
 	if i, ok := buildOptions[defaultTargetName]; ok {
 		inputs = &i.Inputs
 	}
-	return resp, res, inputs, nil
+	return resp, inputs, nil
 }
 
 // buildTargets runs the specified build and returns the result.
 //
 // NOTE: When an error happens during the build and this function acquires the debuggable *build.ResultHandle,
 // this function returns it in addition to the error (i.e. it does "return nil, res, err"). The caller can
 // inspect the result and debug the cause of that error.
-func buildTargets(ctx context.Context, dockerCli command.Cli, nodes []builder.Node, opts map[string]build.Options, progress progress.Writer, generateResult bool) (*client.SolveResponse, *build.ResultHandle, error) {
-	var res *build.ResultHandle
-	var resp map[string]*client.SolveResponse
-	var err error
-	if generateResult {
-		var mu sync.Mutex
-		var idx int
-		resp, err = build.BuildWithResultHandler(ctx, nodes, opts, dockerutil.NewClient(dockerCli), confutil.NewConfig(dockerCli), progress, func(driverIndex int, gotRes *build.ResultHandle) {
-			mu.Lock()
-			defer mu.Unlock()
-			if res == nil || driverIndex < idx {
-				idx, res = driverIndex, gotRes
-			}
-		})
-	} else {
-		resp, err = build.Build(ctx, nodes, opts, dockerutil.NewClient(dockerCli), confutil.NewConfig(dockerCli), progress)
-	}
+func buildTargets(ctx context.Context, dockerCli command.Cli, nodes []builder.Node, opts map[string]build.Options, progress progress.Writer, bh *build.Handler) (*client.SolveResponse, error) {
+	resp, err := build.BuildWithResultHandler(ctx, nodes, opts, dockerutil.NewClient(dockerCli), confutil.NewConfig(dockerCli), progress, bh)
 	if err != nil {
-		return nil, res, err
+		return nil, err
 	}
-	return resp[defaultTargetName], res, err
+	return resp[defaultTargetName], err
 }
 
 func wrapBuildError(err error, bake bool) error {