commands: simplify passing stdin to the build when the monitor is configured

The monitor needs stdin to run and isn't compatible with loading a
context or dockerfile from stdin. We already disallow this combination
and, with the removal of the remote controller, there's no way to use
stdin during the build when invoke is configured.

This just removes the extra code to allow forwarding stdin to the build
when the monitor is configured to simplify that section of code.

Signed-off-by: Jonathan A. Sternberg <[email protected]>

Author: jsternberg

commands/build.go

@@ -425,7 +425,7 @@ func runBasicBuild(ctx context.Context, dockerCli command.Cli, opts *cbuild.Opti
 	return resp, dfmap, err
 }
 
-func runControllerBuild(ctx context.Context, dockerCli command.Cli, opts *cbuild.Options, options buildOptions, printer *progress.Printer) (*client.SolveResponse, *build.Inputs, error) {
+func runControllerBuild(ctx context.Context, dockerCli command.Cli, opts *cbuild.Options, options buildOptions, printer *progress.Printer) (_ *client.SolveResponse, _ *build.Inputs, retErr error) {
 	if options.invokeConfig != nil && (options.dockerfileName == "-" || options.contextPath == "-") {
 		// stdin must be usable for monitor
 		return nil, nil, errors.Errorf("Dockerfile or context from stdin is not supported with invoke")
@@ -434,29 +434,12 @@ func runControllerBuild(ctx context.Context, dockerCli command.Cli, opts *cbuild
 	c := local.NewController(ctx, dockerCli)
 	defer c.Close()
 
-	var (
-		ref    string
-		retErr error
-
-		f  *ioset.SingleForwarder
-		pr io.ReadCloser
-		pw io.WriteCloser
-	)
-
+	var in io.ReadCloser
 	if options.invokeConfig == nil {
-		pr = dockerCli.In()
-	} else {
-		f = ioset.NewSingleForwarder()
-		f.SetReader(dockerCli.In())
-		pr, pw = io.Pipe()
-		f.SetWriter(pw, func() io.WriteCloser {
-			pw.Close() // propagate EOF
-			logrus.Debug("propagating stdin close")
-			return nil
-		})
+		in = dockerCli.In()
 	}
 
-	resp, inputs, err := c.Build(ctx, opts, pr, printer)
+	resp, inputs, err := c.Build(ctx, opts, in, printer)
 	if err != nil {
 		var be *controllererrors.BuildError
 		if errors.As(err, &be) {
@@ -467,28 +450,26 @@ func runControllerBuild(ctx context.Context, dockerCli command.Cli, opts *cbuild
 		}
 	}
 
-	if options.invokeConfig != nil {
-		if err := pw.Close(); err != nil {
-			logrus.Debug("failed to close stdin pipe writer")
-		}
-		if err := pr.Close(); err != nil {
-			logrus.Debug("failed to close stdin pipe reader")
-		}
-	}
-
 	if options.invokeConfig != nil && options.invokeConfig.needsDebug(retErr) {
 		// Print errors before launching monitor
 		if err := printError(retErr, printer); err != nil {
 			logrus.Warnf("failed to print error information: %v", err)
 		}
 
-		pr2, pw2 := io.Pipe()
-		f.SetWriter(pw2, func() io.WriteCloser {
-			pw2.Close() // propagate EOF
+		pr, pw := io.Pipe()
+
+		f := ioset.NewSingleForwarder()
+		f.SetReader(dockerCli.In())
+		f.SetWriter(pw, func() io.WriteCloser {
+			pw.Close() // propagate EOF
 			return nil
 		})
-		monitorBuildResult, err := options.invokeConfig.runDebug(ctx, ref, opts, c, pr2, os.Stdout, os.Stderr, printer)
-		if err := pw2.Close(); err != nil {
+
+		// TODO: ref was never set to a value in the original code. Removed the variable to
+		// reduce confusion but it also probably means this call is wrong in some way.
+		// This area should be removed during the refactor anyway so it doesn't matter that much.
+		monitorBuildResult, err := options.invokeConfig.runDebug(ctx, "", opts, c, pr, os.Stdout, os.Stderr, printer)
+		if err := pw.Close(); err != nil {
 			logrus.Debug("failed to close monitor stdin pipe reader")
 		}
 		if err != nil {

monitor/monitor.go

@@ -346,10 +346,6 @@ func (m *monitor) invoke(ctx context.Context, pid string, cfg *controllerapi.Inv
 
 func (m *monitor) Close() error {
 	m.cancelRunningProcesses()
-	// if m.buildConfig.resultCtx != nil {
-	// 	b.buildConfig.resultCtx.Done()
-	// }
-	// TODO: cancel ongoing builds?
 	return nil
 }