diff --git a/cli/command/container/attach.go b/cli/command/container/attach.go index d5303485f0..a4dbe0ec0d 100644 --- a/cli/command/container/attach.go +++ b/cli/command/container/attach.go @@ -72,7 +72,8 @@ func RunAttach(ctx context.Context, dockerCLI command.Cli, containerID string, o apiClient := dockerCLI.Client() // request channel to wait for client - resultC, errC := apiClient.ContainerWait(ctx, containerID, "") + waitCtx := context.WithoutCancel(ctx) + resultC, errC := apiClient.ContainerWait(waitCtx, containerID, "") c, err := inspectContainerAndCheckState(ctx, apiClient, containerID) if err != nil { @@ -163,9 +164,6 @@ func getExitStatus(errC <-chan error, resultC <-chan container.WaitResponse) err return cli.StatusError{StatusCode: int(result.StatusCode)} } case err := <-errC: - if errors.Is(err, context.Canceled) { - return nil - } return err } diff --git a/cli/command/container/attach_test.go b/cli/command/container/attach_test.go index 6ffa9ff3d7..2bbef24a8d 100644 --- a/cli/command/container/attach_test.go +++ b/cli/command/container/attach_test.go @@ -1,7 +1,6 @@ package container import ( - "context" "io" "testing" @@ -86,11 +85,7 @@ func TestNewAttachCommandErrors(t *testing.T) { } func TestGetExitStatus(t *testing.T) { - var ( - expectedErr = errors.New("unexpected error") - errC = make(chan error, 1) - resultC = make(chan container.WaitResponse, 1) - ) + expectedErr := errors.New("unexpected error") testcases := []struct { result *container.WaitResponse @@ -118,20 +113,20 @@ func TestGetExitStatus(t *testing.T) { }, expectedError: cli.StatusError{StatusCode: 15}, }, - { - err: context.Canceled, - expectedError: nil, - }, } for _, testcase := range testcases { + errC := make(chan error, 1) + resultC := make(chan container.WaitResponse, 1) if testcase.err != nil { errC <- testcase.err } if testcase.result != nil { resultC <- *testcase.result } + err := getExitStatus(errC, resultC) + if testcase.expectedError == nil { assert.NilError(t, err) } else { diff --git a/e2e/container/attach_test.go b/e2e/container/attach_test.go index 6822ca60f7..a6c9bc04b5 100644 --- a/e2e/container/attach_test.go +++ b/e2e/container/attach_test.go @@ -13,6 +13,7 @@ import ( "github.com/docker/cli/e2e/internal/fixtures" "gotest.tools/v3/assert" "gotest.tools/v3/icmd" + "gotest.tools/v3/skip" ) func TestAttachExitCode(t *testing.T) { @@ -32,7 +33,13 @@ func withStdinNewline(cmd *icmd.Cmd) { // Regression test for https://github.com/docker/cli/issues/5294 func TestAttachInterrupt(t *testing.T) { - result := icmd.RunCommand("docker", "run", "-d", fixtures.AlpineImage, "sh", "-c", "sleep 5") + // this is a new test, it already did not work (inside dind) when over ssh + // 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) containerID := strings.TrimSpace(result.Stdout()) @@ -49,6 +56,8 @@ func TestAttachInterrupt(t *testing.T) { c.Process.Signal(os.Interrupt) _ = c.Wait() - assert.Equal(t, c.ProcessState.ExitCode(), 0) - assert.Equal(t, d.String(), "") + // the CLI should exit with 33 (the SIGINT was forwarded to the container), and the + // CLI process waited for the container exit and properly captured/set the exit code + assert.Equal(t, c.ProcessState.ExitCode(), 33) + assert.Equal(t, d.String(), "exit status 33\n") }