From 1cf3637198385c5cb6afe029e0de9ae9d6228a77 Mon Sep 17 00:00:00 2001
From: Laura Brehm <laurabrehm@hey.com>
Date: Thu, 25 Jul 2024 09:27:45 +0100
Subject: [PATCH] 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 7b46bfc5acd8a8674e0ed82bd649ffe78459c2f1)
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
---
 cli/command/container/attach.go      |  3 ++-
 cli/command/container/attach_test.go | 10 +++++-----
 e2e/container/attach_test.go         | 14 +++++++++++---
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/cli/command/container/attach.go b/cli/command/container/attach.go
index ddbaded4b4..2d3c2a3b21 100644
--- a/cli/command/container/attach.go
+++ b/cli/command/container/attach.go
@@ -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 {
diff --git a/cli/command/container/attach_test.go b/cli/command/container/attach_test.go
index 0ba71ec7de..dfc293ea8f 100644
--- a/cli/command/container/attach_test.go
+++ b/cli/command/container/attach_test.go
@@ -82,11 +82,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
@@ -121,13 +117,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 {
diff --git a/e2e/container/attach_test.go b/e2e/container/attach_test.go
index 6822ca60f7..c48e951471 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,7 @@ 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)
 }