Skip to content

Commit dfb8f21

Browse files
laurazardvvoland
authored andcommitted
attach: don't return context cancelled error
In 3f0d90a we introduced a global signal handler and made sure all the contexts passed into command execution get (appropriately) cancelled when we get a SIGINT. Due to that change, and how we use this context during `docker attach`, we started to return the context cancelation error when a user signals the running `docker attach`. Since this is the intended behavior, we shouldn't return an error, so this commit adds checks to ignore this specific error in this case. Also adds a regression test. Signed-off-by: Laura Brehm <[email protected]> (cherry picked from commit 66aa0f6) Signed-off-by: Paweł Gronowski <[email protected]>
1 parent 2e506cb commit dfb8f21

File tree

3 files changed

+39
-1
lines changed

3 files changed

+39
-1
lines changed

cli/command/container/attach.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,8 @@ func RunAttach(ctx context.Context, dockerCLI command.Cli, containerID string, o
146146
detachKeys: options.DetachKeys,
147147
}
148148

149-
if err := streamer.stream(ctx); err != nil {
149+
// if the context was canceled, this was likely intentional and we shouldn't return an error
150+
if err := streamer.stream(ctx); err != nil && !errors.Is(err, context.Canceled) {
150151
return err
151152
}
152153

@@ -163,6 +164,9 @@ func getExitStatus(errC <-chan error, resultC <-chan container.WaitResponse) err
163164
return cli.StatusError{StatusCode: int(result.StatusCode)}
164165
}
165166
case err := <-errC:
167+
if errors.Is(err, context.Canceled) {
168+
return nil
169+
}
166170
return err
167171
}
168172

cli/command/container/attach_test.go

+5
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package container
22

33
import (
4+
"context"
45
"io"
56
"testing"
67

@@ -113,6 +114,10 @@ func TestGetExitStatus(t *testing.T) {
113114
},
114115
expectedError: cli.StatusError{StatusCode: 15},
115116
},
117+
{
118+
err: context.Canceled,
119+
expectedError: nil,
120+
},
116121
}
117122

118123
for _, testcase := range testcases {

e2e/container/attach_test.go

+29
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,17 @@
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"
1016
)
1117

@@ -23,3 +29,26 @@ func TestAttachExitCode(t *testing.T) {
2329
func withStdinNewline(cmd *icmd.Cmd) {
2430
cmd.Stdin = strings.NewReader("\n")
2531
}
32+
33+
// Regression test for https://github.com/docker/cli/issues/5294
34+
func TestAttachInterrupt(t *testing.T) {
35+
result := icmd.RunCommand("docker", "run", "-d", fixtures.AlpineImage, "sh", "-c", "sleep 5")
36+
result.Assert(t, icmd.Success)
37+
containerID := strings.TrimSpace(result.Stdout())
38+
39+
// run it as such so we can signal it later
40+
c := exec.Command("docker", "attach", containerID)
41+
d := bytes.Buffer{}
42+
c.Stdout = &d
43+
c.Stderr = &d
44+
_, err := pty.Start(c)
45+
assert.NilError(t, err)
46+
47+
// have to wait a bit to give time for the command to execute/print
48+
time.Sleep(500 * time.Millisecond)
49+
c.Process.Signal(os.Interrupt)
50+
51+
_ = c.Wait()
52+
assert.Equal(t, c.ProcessState.ExitCode(), 0)
53+
assert.Equal(t, d.String(), "")
54+
}

0 commit comments

Comments
 (0)