diff --git a/client/build_test.go b/client/build_test.go index 461d6cbcb..1cd04222f 100644 --- a/client/build_test.go +++ b/client/build_test.go @@ -52,6 +52,8 @@ func TestClientGatewayIntegration(t *testing.T) { testNoBuildID, testUnknownBuildID, testClientGatewayContainerExecPipe, + testClientGatewayContainerExecPipeRelease, + testClientGatewayContainerExecPipeSignalKill, testClientGatewayContainerCancelOnRelease, testClientGatewayContainerPID1Fail, testClientGatewayContainerPID1Exit, @@ -398,6 +400,35 @@ func testClientGatewayContainerCancelOnRelease(t *testing.T, sb integration.Sand // process together all started via `Exec` into the same container. // We are mimicing: `echo testing | cat | cat > /tmp/foo && cat /tmp/foo` func testClientGatewayContainerExecPipe(t *testing.T, sb integration.Sandbox) { + testClientGatewayContainerExecPipeWithCleanup(t, sb, func(ctx context.Context, ctr client.Container, pid1 client.ContainerProcess, stdin *io.PipeWriter) { + stdin.Close() + pid1.Wait() + ctr.Release(context.WithoutCancel(ctx)) + }) +} + +// testClientGatewayContainerExecPipeRelease verifies that releasing the +// container while pid1 still has an open stdin reader does not leave runc +// blocked on the stdin copy. +func testClientGatewayContainerExecPipeRelease(t *testing.T, sb integration.Sandbox) { + testClientGatewayContainerExecPipeWithCleanup(t, sb, func(ctx context.Context, ctr client.Container, pid1 client.ContainerProcess, _ *io.PipeWriter) { + ctr.Release(context.WithoutCancel(ctx)) + pid1.Wait() + }) +} + +// testClientGatewayContainerExecPipeSignalKill verifies that SIGKILL delivered +// via the process Signal channel unblocks runc when pid1 still has an open +// stdin reader. +func testClientGatewayContainerExecPipeSignalKill(t *testing.T, sb integration.Sandbox) { + testClientGatewayContainerExecPipeWithCleanup(t, sb, func(ctx context.Context, ctr client.Container, pid1 client.ContainerProcess, _ *io.PipeWriter) { + pid1.Signal(ctx, syscall.SIGKILL) + pid1.Wait() + ctr.Release(context.WithoutCancel(ctx)) + }) +} + +func testClientGatewayContainerExecPipeWithCleanup(t *testing.T, sb integration.Sandbox, cleanup func(ctx context.Context, ctr client.Container, pid1 client.ContainerProcess, stdin *io.PipeWriter)) { requiresLinux(t) ctx := sb.Context() @@ -437,19 +468,17 @@ func testClientGatewayContainerExecPipe(t *testing.T, sb integration.Sandbox) { } // background pid1 process that starts container + pid1StdinR, pid1StdinW := io.Pipe() pid1, err := ctr.Start(ctx, client.StartRequest{ - Args: []string{"sleep", "10"}, + Args: []string{"cat"}, + Stdin: pid1StdinR, }) if err != nil { ctr.Release(context.WithoutCancel(ctx)) return nil, err } - defer func() { - // cancel pid1 - ctr.Release(context.WithoutCancel(ctx)) - pid1.Wait() - }() + defer cleanup(ctx, ctr, pid1, pid1StdinW) // first part is `echo testing | cat` stdin2 := bytes.NewBuffer([]byte("testing")) diff --git a/executor/runcexecutor/executor_linux.go b/executor/runcexecutor/executor_linux.go index cd374b791..f5097e59a 100644 --- a/executor/runcexecutor/executor_linux.go +++ b/executor/runcexecutor/executor_linux.go @@ -83,7 +83,34 @@ func (w *runcExecutor) callWithIO(ctx context.Context, process executor.ProcessI }) if !process.Meta.Tty { - return call(ctx, startedCh, &forwardIO{stdin: process.Stdin, stdout: process.Stdout, stderr: process.Stderr}, killer.pidfile) + runcIO := &forwardIO{stdout: process.Stdout, stderr: process.Stderr} + if process.Stdin != nil { + // Forward stdin through an os.Pipe rather than handing the + // caller's io.ReadCloser to runc directly. When cmd.Stdin is + // an *os.File, exec.Cmd dup2s it into the child and cmd.Wait + // returns as soon as the runc subprocess exits. Otherwise + // exec.Cmd spawns an internal goroutine that blocks on the + // caller's Reader and prevents cmd.Wait from returning after + // the in-container process is killed. Stdin is closed in a + // defer after call() returns, matching the tty path and the + // natural-exit cleanup. + pr, pw, err := os.Pipe() + if err != nil { + return errors.Wrap(err, "failed to create stdin pipe") + } + runcIO.stdin = pr + defer pr.Close() + defer process.Stdin.Close() + eg.Go(func() error { + defer pw.Close() + _, err := io.Copy(pw, process.Stdin) + if errors.Is(err, io.ErrClosedPipe) || errors.Is(err, os.ErrClosed) { + return nil + } + return err + }) + } + return call(ctx, startedCh, runcIO, killer.pidfile) } ptm, ptsName, err := console.NewPty()