From c27751fcfe5ded9b3e1e6afe0f1c298d058b72c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Mon, 24 Mar 2025 15:27:28 +0100 Subject: [PATCH 1/2] container/run: Fix stdout/err truncation after container exit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a regression introduced by 30c4637f03ecb8855fe6e47f5374c89d50222577 which made the `docker run` command produce potentially truncated stdout/stderr output. Previous implementation stopped the content streaming as soon as the container exited which would potentially truncate a long outputs. This change fixes the issue by only canceling the IO stream immediately if neither stdout nor stderr is attached. Signed-off-by: Paweł Gronowski --- cli/command/container/run.go | 14 ++++++--- e2e/container/run_test.go | 56 ++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/cli/command/container/run.go b/cli/command/container/run.go index 5c541a0ddef4..702669e19b00 100644 --- a/cli/command/container/run.go +++ b/cli/command/container/run.go @@ -238,10 +238,16 @@ func runContainer(ctx context.Context, dockerCli command.Cli, runOpts *runOption return cli.StatusError{StatusCode: status} } case status := <-statusChan: - // notify hijackedIOStreamer that we're exiting and wait - // so that the terminal can be restored. - cancelFun() - <-errCh + // If container exits, output stream processing may not be finished yet, + // we need to keep the streamer running until all output is read. + // However, if stdout or stderr is not attached, we can just exit. + if !config.AttachStdout && !config.AttachStderr { + // Notify hijackedIOStreamer that we're exiting and wait + // so that the terminal can be restored. + cancelFun() + } + <-errCh // Drain channel but don't care about result + if status != 0 { return cli.StatusError{StatusCode: status} } diff --git a/e2e/container/run_test.go b/e2e/container/run_test.go index ec56d03b2b56..d279f2521bf2 100644 --- a/e2e/container/run_test.go +++ b/e2e/container/run_test.go @@ -3,6 +3,8 @@ package container import ( "bytes" "fmt" + "io" + "math/rand" "os/exec" "strings" "syscall" @@ -280,3 +282,57 @@ func TestProcessTermination(t *testing.T) { ExitCode: 0, }) } + +// Adapted from https://github.com/docker/for-mac/issues/7632#issue-2932169772 +// Thanks [@almet](https://github.com/almet)! +func TestRunReadAfterContainerExit(t *testing.T) { + skip.If(t, environment.RemoteDaemon()) + + r := rand.New(rand.NewSource(0x123456)) + + const size = 18933764 + cmd := exec.Command("docker", "run", + "--rm", "-i", + "alpine", + "sh", "-c", "cat -", + ) + + cmd.Stdin = io.LimitReader(r, size) + + var stderr bytes.Buffer + cmd.Stderr = &stderr + + stdout, err := cmd.StdoutPipe() + assert.NilError(t, err) + + err = cmd.Start() + assert.NilError(t, err) + + buffer := make([]byte, 1000) + counter := 0 + totalRead := 0 + + for { + n, err := stdout.Read(buffer) + if n > 0 { + totalRead += n + } + + // Wait 0.1s every megabyte (approx.) + if counter%1000 == 0 { + time.Sleep(100 * time.Millisecond) + } + + if err != nil || n == 0 { + break + } + + counter++ + } + + err = cmd.Wait() + t.Logf("Error: %v", err) + t.Logf("Stderr: %s", stderr.String()) + assert.Check(t, err == nil) + assert.Check(t, is.Equal(totalRead, size)) +} From 5a8120c809db84c14097e685109b330894cdb2b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Mon, 24 Mar 2025 16:28:05 +0100 Subject: [PATCH 2/2] container/run: Fix TestRunAttachTermination MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restore part of the code removed by 966b44183f525a8cca6caeaff2dc5b3f156fd06e that closed the stream. It's required now because the Run command won't finish before the output stream was processed by the caller. Signed-off-by: Paweł Gronowski --- cli/command/container/run_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cli/command/container/run_test.go b/cli/command/container/run_test.go index 186d197e49a1..bf48d3066d35 100644 --- a/cli/command/container/run_test.go +++ b/cli/command/container/run_test.go @@ -147,6 +147,7 @@ func TestRunAttachTermination(t *testing.T) { _ = p.Close() }() + var conn net.Conn killCh := make(chan struct{}) attachCh := make(chan struct{}) fakeCLI := test.NewFakeCli(&fakeClient{ @@ -163,6 +164,7 @@ func TestRunAttachTermination(t *testing.T) { }, containerAttachFunc: func(ctx context.Context, containerID string, options container.AttachOptions) (types.HijackedResponse, error) { server, client := net.Pipe() + conn = server t.Cleanup(func() { _ = server.Close() }) @@ -202,6 +204,7 @@ func TestRunAttachTermination(t *testing.T) { } assert.NilError(t, syscall.Kill(syscall.Getpid(), syscall.SIGTERM)) + conn.Close() select { case <-killCh: