Skip to content

Commit cf42238

Browse files
committed
attach: wait for exit code from ContainerWait
Such as with `docker run`, if a user CTRL-Cs while attached to a container, we should forward the signal and wait for the exit from `ContainerWait`, instead of just returning. Signed-off-by: Laura Brehm <[email protected]>
1 parent 788e996 commit cf42238

3 files changed

Lines changed: 13 additions & 17 deletions

File tree

cli/command/container/attach.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,8 @@ func RunAttach(ctx context.Context, dockerCLI command.Cli, containerID string, o
7272
apiClient := dockerCLI.Client()
7373

7474
// request channel to wait for client
75-
resultC, errC := apiClient.ContainerWait(ctx, containerID, "")
75+
waitCtx := context.WithoutCancel(ctx)
76+
resultC, errC := apiClient.ContainerWait(waitCtx, containerID, "")
7677

7778
c, err := inspectContainerAndCheckState(ctx, apiClient, containerID)
7879
if err != nil {
@@ -163,9 +164,6 @@ func getExitStatus(errC <-chan error, resultC <-chan container.WaitResponse) err
163164
return cli.StatusError{StatusCode: int(result.StatusCode)}
164165
}
165166
case err := <-errC:
166-
if errors.Is(err, context.Canceled) {
167-
return nil
168-
}
169167
return err
170168
}
171169

cli/command/container/attach_test.go

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package container
22

33
import (
4-
"context"
54
"io"
65
"testing"
76

@@ -86,11 +85,7 @@ func TestNewAttachCommandErrors(t *testing.T) {
8685
}
8786

8887
func TestGetExitStatus(t *testing.T) {
89-
var (
90-
expectedErr = errors.New("unexpected error")
91-
errC = make(chan error, 1)
92-
resultC = make(chan container.WaitResponse, 1)
93-
)
88+
expectedErr := errors.New("unexpected error")
9489

9590
testcases := []struct {
9691
result *container.WaitResponse
@@ -118,20 +113,20 @@ func TestGetExitStatus(t *testing.T) {
118113
},
119114
expectedError: cli.StatusError{StatusCode: 15},
120115
},
121-
{
122-
err: context.Canceled,
123-
expectedError: nil,
124-
},
125116
}
126117

127118
for _, testcase := range testcases {
119+
errC := make(chan error, 1)
120+
resultC := make(chan container.WaitResponse, 1)
128121
if testcase.err != nil {
129122
errC <- testcase.err
130123
}
131124
if testcase.result != nil {
132125
resultC <- *testcase.result
133126
}
127+
134128
err := getExitStatus(errC, resultC)
129+
135130
if testcase.expectedError == nil {
136131
assert.NilError(t, err)
137132
} else {

e2e/container/attach_test.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@ import (
1111

1212
"github.com/creack/pty"
1313
"github.com/docker/cli/e2e/internal/fixtures"
14+
"github.com/docker/cli/internal/test/environment"
1415
"gotest.tools/v3/assert"
1516
"gotest.tools/v3/icmd"
17+
"gotest.tools/v3/skip"
1618
)
1719

1820
func TestAttachExitCode(t *testing.T) {
@@ -32,7 +34,8 @@ func withStdinNewline(cmd *icmd.Cmd) {
3234

3335
// Regression test for https://github.com/docker/cli/issues/5294
3436
func TestAttachInterrupt(t *testing.T) {
35-
result := icmd.RunCommand("docker", "run", "-d", fixtures.AlpineImage, "sh", "-c", "sleep 5")
37+
skip.If(t, environment.RemoteDaemon())
38+
result := icmd.RunCommand("docker", "run", "-d", fixtures.AlpineImage, "sh", "-c", "while true; do sleep 1; done")
3639
result.Assert(t, icmd.Success)
3740
containerID := strings.TrimSpace(result.Stdout())
3841

@@ -49,6 +52,6 @@ func TestAttachInterrupt(t *testing.T) {
4952
c.Process.Signal(os.Interrupt)
5053

5154
_ = c.Wait()
52-
assert.Equal(t, c.ProcessState.ExitCode(), 0)
53-
assert.Equal(t, d.String(), "")
55+
assert.Equal(t, c.ProcessState.ExitCode(), 130)
56+
assert.Equal(t, d.String(), "exit status 130\n")
5457
}

0 commit comments

Comments
 (0)