From 65e46cc6af2b3a7f844a353e5548125d89c3bb83 Mon Sep 17 00:00:00 2001 From: "Jonathan A. Sternberg" Date: Mon, 2 Jun 2025 16:31:32 -0500 Subject: [PATCH] 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 --- commands/build.go | 51 +++++++++++++++------------------------------- monitor/monitor.go | 4 ---- 2 files changed, 16 insertions(+), 39 deletions(-) diff --git a/commands/build.go b/commands/build.go index 60fbb4272..93791a4ac 100644 --- a/commands/build.go +++ b/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 { diff --git a/monitor/monitor.go b/monitor/monitor.go index 6156dc834..50cabd672 100644 --- a/monitor/monitor.go +++ b/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 }