Skip to content

Commit

Permalink
attach: wait for exit code from ContainerWait
Browse files Browse the repository at this point in the history
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]>
(cherry picked from commit 7b46bfc)
Signed-off-by: Laura Brehm <[email protected]>
  • Loading branch information
laurazard committed Jul 26, 2024
1 parent 1b2782e commit a3f618a
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 6 deletions.
3 changes: 2 additions & 1 deletion cli/command/container/attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,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 {
Expand Down
10 changes: 5 additions & 5 deletions cli/command/container/attach_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,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
Expand Down Expand Up @@ -116,13 +112,17 @@ func TestGetExitStatus(t *testing.T) {
}

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 {
Expand Down
38 changes: 38 additions & 0 deletions e2e/container/attach_test.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
package container

import (
"bytes"
"fmt"
"os"
"os/exec"
"strings"
"testing"
"time"

"github.com/creack/pty"
"github.com/docker/cli/e2e/internal/fixtures"
"gotest.tools/v3/assert"
"gotest.tools/v3/icmd"
"gotest.tools/v3/skip"
)

func TestAttachExitCode(t *testing.T) {
Expand All @@ -23,3 +30,34 @@ func TestAttachExitCode(t *testing.T) {
func withStdinNewline(cmd *icmd.Cmd) {
cmd.Stdin = strings.NewReader("\n")
}

// Regression test for https://github.com/docker/cli/issues/5294
func TestAttachInterrupt(t *testing.T) {
// 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())

// run it as such so we can signal it later
c := exec.Command("docker", "attach", containerID)
d := bytes.Buffer{}
c.Stdout = &d
c.Stderr = &d
_, err := pty.Start(c)
assert.NilError(t, err)

// have to wait a bit to give time for the command to execute/print
time.Sleep(500 * time.Millisecond)
c.Process.Signal(os.Interrupt)

_ = c.Wait()
// 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")
}

0 comments on commit a3f618a

Please sign in to comment.