Skip to content

Commit a3f618a

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 <laurabrehm@hey.com> (cherry picked from commit 7b46bfc) Signed-off-by: Laura Brehm <laurabrehm@hey.com>
1 parent 1b2782e commit a3f618a

3 files changed

Lines changed: 45 additions & 6 deletions

File tree

cli/command/container/attach.go

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

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

7879
c, err := inspectContainerAndCheckState(ctx, apiClient, containerID)
7980
if err != nil {

cli/command/container/attach_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,7 @@ func TestNewAttachCommandErrors(t *testing.T) {
8181
}
8282

8383
func TestGetExitStatus(t *testing.T) {
84-
var (
85-
expectedErr = errors.New("unexpected error")
86-
errC = make(chan error, 1)
87-
resultC = make(chan container.WaitResponse, 1)
88-
)
84+
expectedErr := errors.New("unexpected error")
8985

9086
testcases := []struct {
9187
result *container.WaitResponse
@@ -116,13 +112,17 @@ func TestGetExitStatus(t *testing.T) {
116112
}
117113

118114
for _, testcase := range testcases {
115+
errC := make(chan error, 1)
116+
resultC := make(chan container.WaitResponse, 1)
119117
if testcase.err != nil {
120118
errC <- testcase.err
121119
}
122120
if testcase.result != nil {
123121
resultC <- *testcase.result
124122
}
123+
125124
err := getExitStatus(errC, resultC)
125+
126126
if testcase.expectedError == nil {
127127
assert.NilError(t, err)
128128
} else {

e2e/container/attach_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,19 @@
11
package container
22

33
import (
4+
"bytes"
45
"fmt"
6+
"os"
7+
"os/exec"
58
"strings"
69
"testing"
10+
"time"
711

12+
"github.com/creack/pty"
813
"github.com/docker/cli/e2e/internal/fixtures"
14+
"gotest.tools/v3/assert"
915
"gotest.tools/v3/icmd"
16+
"gotest.tools/v3/skip"
1017
)
1118

1219
func TestAttachExitCode(t *testing.T) {
@@ -23,3 +30,34 @@ func TestAttachExitCode(t *testing.T) {
2330
func withStdinNewline(cmd *icmd.Cmd) {
2431
cmd.Stdin = strings.NewReader("\n")
2532
}
33+
34+
// Regression test for https://github.com/docker/cli/issues/5294
35+
func TestAttachInterrupt(t *testing.T) {
36+
// this is a new test, it already did not work (inside dind) when over ssh
37+
// todo(laurazard): make this test work w/ dind over ssh
38+
skip.If(t, strings.Contains(os.Getenv("DOCKER_HOST"), "ssh://"))
39+
40+
// if
41+
result := icmd.RunCommand("docker", "run", "-d", fixtures.AlpineImage,
42+
"sh", "-c", "trap \"exit 33\" SIGINT; for i in $(seq 100); do sleep 0.1; done; exit 34")
43+
result.Assert(t, icmd.Success)
44+
containerID := strings.TrimSpace(result.Stdout())
45+
46+
// run it as such so we can signal it later
47+
c := exec.Command("docker", "attach", containerID)
48+
d := bytes.Buffer{}
49+
c.Stdout = &d
50+
c.Stderr = &d
51+
_, err := pty.Start(c)
52+
assert.NilError(t, err)
53+
54+
// have to wait a bit to give time for the command to execute/print
55+
time.Sleep(500 * time.Millisecond)
56+
c.Process.Signal(os.Interrupt)
57+
58+
_ = c.Wait()
59+
// the CLI should exit with 33 (the SIGINT was forwarded to the container), and the
60+
// CLI process waited for the container exit and properly captured/set the exit code
61+
assert.Equal(t, c.ProcessState.ExitCode(), 33)
62+
assert.Equal(t, d.String(), "exit status 33\n")
63+
}

0 commit comments

Comments
 (0)