monitor: move remaining controller functionality into monitor

This creates a `Monitor` type that keeps the global state between
monitor invocations and allows the monitor to exist during the build so
it can be utilized for callbacks.

The result handler is now registered with the monitor during the build
and `Run` will use the result if it is present and the configuration
intends the monitor to be invoked with the given result.

Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>

Author: jsternberg

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 {