controller: remove the controller interface

The controller interface is removed and the local controller is used for
only the initial build, invoke, and rebuilds.

Process control has been moved to the monitor.

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

commands/build.go

@@ -21,10 +21,9 @@ import (
 	"github.com/docker/buildx/build"
 	"github.com/docker/buildx/builder"
 	"github.com/docker/buildx/commands/debug"
-	"github.com/docker/buildx/controller"
 	cbuild "github.com/docker/buildx/controller/build"
-	"github.com/docker/buildx/controller/control"
 	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"
@@ -431,28 +430,19 @@ func runControllerBuild(ctx context.Context, dockerCli command.Cli, opts *cbuild
 		// stdin must be usable for monitor
 		return nil, nil, errors.Errorf("Dockerfile or context from stdin is not supported with invoke")
 	}
-	c := controller.NewController(ctx, dockerCli)
-	defer func() {
-		if err := c.Close(); err != nil {
-			logrus.Warnf("failed to close server connection %v", err)
-		}
-	}()
 
-	// NOTE: buildx server has the current working directory different from the client
+	c := local.NewController(ctx, dockerCli)
-	// so we need to resolve paths to abosolute ones in the client.
+	defer c.Close()
-	opts, err := cbuild.ResolveOptionPaths(opts)
-	if err != nil {
-		return nil, nil, err
-	}
 
-	var ref string
+	var (
-	var retErr error
+		ref    string
-	var resp *client.SolveResponse
+		retErr error
-	var inputs *build.Inputs
+
+		f  *ioset.SingleForwarder
+		pr io.ReadCloser
+		pw io.WriteCloser
+	)
 
-	var f *ioset.SingleForwarder
-	var pr io.ReadCloser
-	var pw io.WriteCloser
 	if options.invokeConfig == nil {
 		pr = dockerCli.In()
 	} else {
@@ -466,7 +456,7 @@ func runControllerBuild(ctx context.Context, dockerCli command.Cli, opts *cbuild
 		})
 	}
 
-	resp, inputs, err = c.Build(ctx, opts, pr, printer)
+	resp, inputs, err := c.Build(ctx, opts, pr, printer)
 	if err != nil {
 		var be *controllererrors.BuildError
 		if errors.As(err, &be) {
@@ -508,10 +498,6 @@ func runControllerBuild(ctx context.Context, dockerCli command.Cli, opts *cbuild
 			// Update return values with the last build result from monitor
 			resp, retErr = monitorBuildResult.Resp, monitorBuildResult.Err
 		}
-	} else {
-		if err := c.Close(); err != nil {
-			logrus.Warnf("close error: %v", err)
-		}
 	}
 
 	return resp, inputs, retErr
@@ -1003,13 +989,9 @@ func (cfg *invokeConfig) needsDebug(retErr error) bool {
 	}
 }
 
-func (cfg *invokeConfig) runDebug(ctx context.Context, ref string, options *cbuild.Options, c control.BuildxController, stdin io.ReadCloser, stdout io.WriteCloser, stderr console.File, progress *progress.Printer) (*monitor.MonitorBuildResult, error) {
+func (cfg *invokeConfig) runDebug(ctx context.Context, ref string, options *cbuild.Options, c *local.Controller, stdin io.ReadCloser, stdout io.WriteCloser, stderr console.File, progress *progress.Printer) (*monitor.MonitorBuildResult, error) {
 	con := console.Current()
 	if err := con.SetRaw(); err != nil {
-		// TODO: run disconnect in build command (on error case)
-		if err := c.Close(); err != nil {
-			logrus.Warnf("close error: %v", err)
-		}
 		return nil, errors.Errorf("failed to configure terminal: %v", err)
 	}
 	defer con.Reset()

commands/debug/root.go

@@ -5,7 +5,7 @@ import (
 	"os"
 
 	"github.com/containerd/console"
-	"github.com/docker/buildx/controller"
+	"github.com/docker/buildx/controller/local"
 	controllerapi "github.com/docker/buildx/controller/pb"
 	"github.com/docker/buildx/monitor"
 	"github.com/docker/buildx/util/cobrautil"
@@ -13,7 +13,6 @@ import (
 	"github.com/docker/cli/cli/command"
 	"github.com/moby/buildkit/util/progress/progressui"
 	"github.com/pkg/errors"
-	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
 
@@ -47,12 +46,8 @@ func RootCmd(dockerCli command.Cli, children ...DebuggableCmd) *cobra.Command {
 			}
 
 			ctx := context.TODO()
-			c := controller.NewController(ctx, dockerCli)
+			c := local.NewController(ctx, dockerCli)
-			defer func() {
+
-				if err := c.Close(); err != nil {
-					logrus.Warnf("failed to close server connection %v", err)
-				}
-			}()
 			con := console.Current()
 			if err := con.SetRaw(); err != nil {
 				return errors.Errorf("failed to configure terminal: %v", err)

controller/control/controller.go

@@ -1,26 +0,0 @@
-package control
-
-import (
-	"context"
-	"io"
-
-	"github.com/docker/buildx/build"
-	cbuild "github.com/docker/buildx/controller/build"
-	controllerapi "github.com/docker/buildx/controller/pb"
-	"github.com/docker/buildx/controller/processes"
-	"github.com/docker/buildx/util/progress"
-	"github.com/moby/buildkit/client"
-)
-
-type BuildxController interface {
-	Build(ctx context.Context, options *cbuild.Options, in io.ReadCloser, progress progress.Writer) (resp *client.SolveResponse, inputs *build.Inputs, err error)
-	// Invoke starts an IO session into the specified process.
-	// If pid doesn't match to any running processes, it starts a new process with the specified config.
-	// If there is no container running or InvokeConfig.Rollback is specified, the process will start in a newly created container.
-	// NOTE: If needed, in the future, we can split this API into three APIs (NewContainer, NewProcess and Attach).
-	Invoke(ctx context.Context, pid string, options *controllerapi.InvokeConfig, ioIn io.ReadCloser, ioOut io.WriteCloser, ioErr io.WriteCloser) error
-	Close() error
-	ListProcesses(ctx context.Context) (infos []*processes.ProcessInfo, retErr error)
-	DisconnectProcess(ctx context.Context, pid string) error
-	Inspect(ctx context.Context) *cbuild.Options
-}

controller/controller.go

@@ -1,13 +0,0 @@
-package controller
-
-import (
-	"context"
-
-	"github.com/docker/buildx/controller/control"
-	"github.com/docker/buildx/controller/local"
-	"github.com/docker/cli/cli/command"
-)
-
-func NewController(ctx context.Context, dockerCli command.Cli) control.BuildxController {
-	return local.NewLocalBuildxController(ctx, dockerCli)
-}

controller/local/controller.go

@@ -7,7 +7,6 @@ import (
 
 	"github.com/docker/buildx/build"
 	cbuild "github.com/docker/buildx/controller/build"
-	"github.com/docker/buildx/controller/control"
 	controllererrors "github.com/docker/buildx/controller/errdefs"
 	controllerapi "github.com/docker/buildx/controller/pb"
 	"github.com/docker/buildx/controller/processes"
@@ -18,10 +17,9 @@ import (
 	"github.com/pkg/errors"
 )
 
-func NewLocalBuildxController(ctx context.Context, dockerCli command.Cli) control.BuildxController {
+func NewController(ctx context.Context, dockerCli command.Cli) *Controller {
-	return &localController{
+	return &Controller{
 		dockerCli: dockerCli,
-		processes: processes.NewManager(),
 	}
 }
 
@@ -32,15 +30,14 @@ type buildConfig struct {
 	buildOptions *cbuild.Options
 }
 
-type localController struct {
+type Controller struct {
 	dockerCli   command.Cli
 	buildConfig buildConfig
-	processes   *processes.Manager
 
 	buildOnGoing atomic.Bool
 }
 
-func (b *localController) Build(ctx context.Context, options *cbuild.Options, in io.ReadCloser, progress progress.Writer) (*client.SolveResponse, *build.Inputs, error) {
+func (b *Controller) Build(ctx context.Context, options *cbuild.Options, in io.ReadCloser, progress progress.Writer) (*client.SolveResponse, *build.Inputs, error) {
 	if !b.buildOnGoing.CompareAndSwap(false, true) {
 		return nil, nil, errors.New("build ongoing")
 	}
@@ -63,27 +60,15 @@ func (b *localController) Build(ctx context.Context, options *cbuild.Options, in
 	return resp, dockerfileMappings, nil
 }
 
-func (b *localController) ListProcesses(ctx context.Context) (infos []*processes.ProcessInfo, retErr error) {
+func (b *Controller) Invoke(ctx context.Context, processes *processes.Manager, pid string, cfg *controllerapi.InvokeConfig, ioIn io.ReadCloser, ioOut io.WriteCloser, ioErr io.WriteCloser) error {
-	return b.processes.ListProcesses(), nil
+	proc, ok := processes.Get(pid)
-}
-
-func (b *localController) DisconnectProcess(ctx context.Context, pid string) error {
-	return b.processes.DeleteProcess(pid)
-}
-
-func (b *localController) cancelRunningProcesses() {
-	b.processes.CancelRunningProcesses()
-}
-
-func (b *localController) Invoke(ctx context.Context, pid string, cfg *controllerapi.InvokeConfig, ioIn io.ReadCloser, ioOut io.WriteCloser, ioErr io.WriteCloser) error {
-	proc, ok := b.processes.Get(pid)
 	if !ok {
 		// Start a new process.
 		if b.buildConfig.resultCtx == nil {
 			return errors.New("no build result is registered")
 		}
 		var err error
-		proc, err = b.processes.StartProcess(pid, b.buildConfig.resultCtx, cfg)
+		proc, err = processes.StartProcess(pid, b.buildConfig.resultCtx, cfg)
 		if err != nil {
 			return err
 		}
@@ -103,15 +88,13 @@ func (b *localController) Invoke(ctx context.Context, pid string, cfg *controlle
 	}
 }
 
-func (b *localController) Close() error {
+func (b *Controller) Inspect(ctx context.Context) *cbuild.Options {
-	b.cancelRunningProcesses()
+	return b.buildConfig.buildOptions
+}
+
+func (b *Controller) Close() error {
 	if b.buildConfig.resultCtx != nil {
 		b.buildConfig.resultCtx.Done()
 	}
-	// TODO: cancel ongoing builds?
 	return nil
 }
-
-func (b *localController) Inspect(ctx context.Context) *cbuild.Options {
-	return b.buildConfig.buildOptions
-}

monitor/monitor.go

@@ -12,8 +12,9 @@ import (
 	"github.com/containerd/console"
 	"github.com/docker/buildx/build"
 	cbuild "github.com/docker/buildx/controller/build"
-	"github.com/docker/buildx/controller/control"
+	"github.com/docker/buildx/controller/local"
 	controllerapi "github.com/docker/buildx/controller/pb"
+	"github.com/docker/buildx/controller/processes"
 	"github.com/docker/buildx/monitor/commands"
 	"github.com/docker/buildx/monitor/types"
 	"github.com/docker/buildx/util/ioset"
@@ -32,13 +33,7 @@ type MonitorBuildResult struct {
 }
 
 // RunMonitor provides an interactive session for running and managing containers via specified IO.
-func RunMonitor(ctx context.Context, curRef string, options *cbuild.Options, invokeConfig *controllerapi.InvokeConfig, c control.BuildxController, stdin io.ReadCloser, stdout io.WriteCloser, stderr console.File, progress *progress.Printer) (*MonitorBuildResult, error) {
+func RunMonitor(ctx context.Context, curRef string, options *cbuild.Options, invokeConfig *controllerapi.InvokeConfig, c *local.Controller, stdin io.ReadCloser, stdout io.WriteCloser, stderr console.File, progress *progress.Printer) (*MonitorBuildResult, error) {
-	defer func() {
-		if err := c.Close(); err != nil {
-			logrus.Warnf("close error: %v", err)
-		}
-	}()
-
 	if err := progress.Pause(); err != nil {
 		return nil, err
 	}
@@ -70,8 +65,9 @@ func RunMonitor(ctx context.Context, curRef string, options *cbuild.Options, inv
 	invokeForwarder := ioset.NewForwarder()
 	invokeForwarder.SetIn(&containerIn)
 	m := &monitor{
-		BuildxController: c,
+		c:         c,
-		invokeIO:         invokeForwarder,
+		processes: processes.NewManager(),
+		invokeIO:  invokeForwarder,
 		muxIO: ioset.NewMuxIO(ioset.In{
 			Stdin:  io.NopCloser(stdin),
 			Stdout: nopCloser{stdout},
@@ -84,6 +80,12 @@ func RunMonitor(ctx context.Context, curRef string, options *cbuild.Options, inv
 			return "Switched IO\n"
 		}),
 	}
+
+	defer func() {
+		if err := m.Close(); err != nil {
+			logrus.Warnf("close error: %v", err)
+		}
+	}()
 	m.ref.Store(curRef)
 
 	// Start container automatically
@@ -231,7 +233,7 @@ type readWriter struct {
 }
 
 type monitor struct {
-	control.BuildxController
+	c   *local.Controller
 	ref atomic.Value
 
 	muxIO        *ioset.MuxIO
@@ -240,14 +242,24 @@ type monitor struct {
 	attachedPid  atomic.Value
 
 	lastBuildResult *MonitorBuildResult
+
+	processes *processes.Manager
 }
 
 func (m *monitor) Build(ctx context.Context, options *cbuild.Options, in io.ReadCloser, progress progress.Writer) (resp *client.SolveResponse, input *build.Inputs, err error) {
-	resp, _, err = m.BuildxController.Build(ctx, options, in, progress)
+	resp, _, err = m.c.Build(ctx, options, in, progress)
 	m.lastBuildResult = &MonitorBuildResult{Resp: resp, Err: err} // Record build result
 	return
 }
 
+func (m *monitor) Invoke(ctx context.Context, pid string, cfg *controllerapi.InvokeConfig, ioIn io.ReadCloser, ioOut io.WriteCloser, ioErr io.WriteCloser) error {
+	return m.c.Invoke(ctx, m.processes, pid, cfg, ioIn, ioOut, ioErr)
+}
+
+func (m *monitor) Inspect(ctx context.Context) *cbuild.Options {
+	return m.c.Inspect(ctx)
+}
+
 func (m *monitor) Rollback(ctx context.Context, cfg *controllerapi.InvokeConfig) string {
 	pid := identity.NewID()
 	cfg1 := cfg
@@ -332,6 +344,27 @@ func (m *monitor) invoke(ctx context.Context, pid string, cfg *controllerapi.Inv
 	return err
 }
 
+func (m *monitor) Close() error {
+	m.cancelRunningProcesses()
+	// if m.buildConfig.resultCtx != nil {
+	// 	b.buildConfig.resultCtx.Done()
+	// }
+	// TODO: cancel ongoing builds?
+	return nil
+}
+
+func (m *monitor) ListProcesses(ctx context.Context) (infos []*processes.ProcessInfo, retErr error) {
+	return m.processes.ListProcesses(), nil
+}
+
+func (m *monitor) DisconnectProcess(ctx context.Context, pid string) error {
+	return m.processes.DeleteProcess(pid)
+}
+
+func (m *monitor) cancelRunningProcesses() {
+	m.processes.CancelRunningProcesses()
+}
+
 type nopCloser struct {
 	io.Writer
 }

monitor/types/types.go

@@ -2,14 +2,31 @@ package types
 
 import (
 	"context"
+	"io"
 
-	"github.com/docker/buildx/controller/control"
+	"github.com/docker/buildx/build"
+	cbuild "github.com/docker/buildx/controller/build"
 	controllerapi "github.com/docker/buildx/controller/pb"
+	"github.com/docker/buildx/controller/processes"
+	"github.com/docker/buildx/util/progress"
+	"github.com/moby/buildkit/client"
 )
 
 // Monitor provides APIs for attaching and controlling the buildx server.
 type Monitor interface {
-	control.BuildxController
+	Build(ctx context.Context, options *cbuild.Options, in io.ReadCloser, progress progress.Writer) (resp *client.SolveResponse, inputs *build.Inputs, err error)
+
+	Inspect(ctx context.Context) *cbuild.Options
+
+	// Invoke starts an IO session into the specified process.
+	// If pid doesn't match to any running processes, it starts a new process with the specified config.
+	// If there is no container running or InvokeConfig.Rollback is specified, the process will start in a newly created container.
+	// NOTE: If needed, in the future, we can split this API into three APIs (NewContainer, NewProcess and Attach).
+	Invoke(ctx context.Context, pid string, options *controllerapi.InvokeConfig, ioIn io.ReadCloser, ioOut io.WriteCloser, ioErr io.WriteCloser) error
+
+	ListProcesses(ctx context.Context) (infos []*processes.ProcessInfo, retErr error)
+
+	DisconnectProcess(ctx context.Context, pid string) error
 
 	// Rollback re-runs the interactive container with initial rootfs contents.
 	Rollback(ctx context.Context, cfg *controllerapi.InvokeConfig) string
@@ -25,6 +42,8 @@ type Monitor interface {
 
 	// Detach detaches IO from the container.
 	Detach()
+
+	io.Closer
 }
 
 // CommandInfo is information about a command.