diff --git a/cli/command/container/run.go b/cli/command/container/run.go index bb0340ed1b49..5ef3e8b7bf7b 100644 --- a/cli/command/container/run.go +++ b/cli/command/container/run.go @@ -161,7 +161,8 @@ func runContainer(ctx context.Context, dockerCli command.Cli, runOpts *runOption waitDisplayID chan struct{} errCh chan error ) - if !config.AttachStdout && !config.AttachStderr { + attach := config.AttachStdin || config.AttachStdout || config.AttachStderr + if !attach { // Make this asynchronous to allow the client to write to stdin before having to read the ID waitDisplayID = make(chan struct{}) go func() { @@ -169,7 +170,6 @@ func runContainer(ctx context.Context, dockerCli command.Cli, runOpts *runOption _, _ = fmt.Fprintln(stdout, containerID) }() } - attach := config.AttachStdin || config.AttachStdout || config.AttachStderr if attach { detachKeys := dockerCli.ConfigFile().DetachKeys if runOpts.detachKeys != "" { @@ -215,14 +215,22 @@ func runContainer(ctx context.Context, dockerCli command.Cli, runOpts *runOption return toStatusError(err) } - if (config.AttachStdin || config.AttachStdout || config.AttachStderr) && config.Tty && dockerCli.Out().IsTerminal() { + // Detached mode: wait for the id to be displayed and return. + if !attach { + // Detached mode + <-waitDisplayID + return nil + } + + if config.Tty && dockerCli.Out().IsTerminal() { if err := MonitorTtySize(ctx, dockerCli, containerID, false); err != nil { _, _ = fmt.Fprintln(stderr, "Error monitoring TTY size:", err) } } - if errCh != nil { - if err := <-errCh; err != nil { + select { + case err := <-errCh: + if err != nil { if _, ok := err.(term.EscapeError); ok { // The user entered the detach escape sequence. return nil @@ -231,19 +239,20 @@ func runContainer(ctx context.Context, dockerCli command.Cli, runOpts *runOption logrus.Debugf("Error hijack: %s", err) return err } + status := <-statusChan + if status != 0 { + 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 status != 0 { + return cli.StatusError{StatusCode: status} + } } - // Detached mode: wait for the id to be displayed and return. - if !config.AttachStdout && !config.AttachStderr { - // Detached mode - <-waitDisplayID - return nil - } - - status := <-statusChan - if status != 0 { - return cli.StatusError{StatusCode: status} - } return nil } diff --git a/e2e/container/attach_test.go b/e2e/container/attach_test.go index a6c9bc04b534..545d86ed707a 100644 --- a/e2e/container/attach_test.go +++ b/e2e/container/attach_test.go @@ -37,7 +37,6 @@ func TestAttachInterrupt(t *testing.T) { // todo(laurazard): make this test work w/ dind over ssh skip.If(t, strings.Contains(os.Getenv("DOCKER_HOST"), "ssh://")) - // if result := icmd.RunCommand("docker", "run", "-d", fixtures.AlpineImage, "sh", "-c", "trap \"exit 33\" SIGINT; for i in $(seq 100); do sleep 0.1; done; exit 34") result.Assert(t, icmd.Success) diff --git a/e2e/container/run_test.go b/e2e/container/run_test.go index c835f1caf294..f4e591e21d4d 100644 --- a/e2e/container/run_test.go +++ b/e2e/container/run_test.go @@ -3,11 +3,13 @@ package container import ( "bytes" "fmt" + "os/exec" "strings" "syscall" "testing" "time" + "github.com/creack/pty" "github.com/docker/cli/e2e/internal/fixtures" "github.com/docker/cli/internal/test/environment" "github.com/docker/docker/api/types/versions" @@ -38,6 +40,39 @@ func TestRunAttachedFromRemoteImageAndRemove(t *testing.T) { golden.Assert(t, result.Stderr(), "run-attached-from-remote-and-remove.golden") } +func TestRunAttach(t *testing.T) { + skip.If(t, environment.RemoteDaemon()) + t.Parallel() + + streams := []string{"stdin", "stdout", "stderr"} + for _, stream := range streams { + t.Run(stream, func(t *testing.T) { + t.Parallel() + c := exec.Command("docker", "run", "-a", stream, "--rm", "alpine", + "sh", "-c", "sleep 1 && exit 7") + d := bytes.Buffer{} + c.Stdout = &d + c.Stderr = &d + _, err := pty.Start(c) + assert.NilError(t, err) + + done := make(chan error) + go func() { + done <- c.Wait() + }() + + select { + case <-time.After(20 * time.Second): + t.Fatal("docker run took too long, likely hang", d.String()) + case <-done: + } + + assert.Equal(t, c.ProcessState.ExitCode(), 7) + assert.Check(t, is.Contains(d.String(), "exit status 7")) + }) + } +} + // Regression test for https://github.com/docker/cli/issues/5053 func TestRunInvalidEntrypointWithAutoremove(t *testing.T) { environment.SkipIfDaemonNotLinux(t)