Skip to content

Commit 47b6915

Browse files
committed
fix: hoist signal handling from prompt confirmation
Signed-off-by: Alano Terblanche <18033717+Benehiko@users.noreply.github.com>
1 parent c23a404 commit 47b6915

4 files changed

Lines changed: 26 additions & 19 deletions

File tree

cli/command/utils.go

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,9 @@ import (
99
"fmt"
1010
"io"
1111
"os"
12-
"os/signal"
1312
"path/filepath"
1413
"runtime"
1514
"strings"
16-
"syscall"
1715

1816
"github.com/docker/cli/cli/streams"
1917
"github.com/docker/docker/api/types/filters"
@@ -103,11 +101,6 @@ func PromptForConfirmation(ctx context.Context, ins io.Reader, outs io.Writer, m
103101

104102
result := make(chan bool)
105103

106-
// Catch the termination signal and exit the prompt gracefully.
107-
// The caller is responsible for properly handling the termination.
108-
notifyCtx, notifyCancel := signal.NotifyContext(ctx, syscall.SIGINT, syscall.SIGTERM)
109-
defer notifyCancel()
110-
111104
go func() {
112105
var res bool
113106
scanner := bufio.NewScanner(ins)
@@ -121,8 +114,7 @@ func PromptForConfirmation(ctx context.Context, ins io.Reader, outs io.Writer, m
121114
}()
122115

123116
select {
124-
case <-notifyCtx.Done():
125-
// print a newline on termination
117+
case <-ctx.Done():
126118
_, _ = fmt.Fprintln(outs, "")
127119
return false, ErrPromptTerminated
128120
case r := <-result:

cli/command/utils_test.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"fmt"
88
"io"
99
"os"
10+
"os/signal"
1011
"path/filepath"
1112
"strings"
1213
"syscall"
@@ -135,6 +136,9 @@ func TestPromptForConfirmation(t *testing.T) {
135136
}, promptResult{false, nil}},
136137
} {
137138
t.Run("case="+tc.desc, func(t *testing.T) {
139+
notifyCtx, notifyCancel := signal.NotifyContext(ctx, syscall.SIGINT, syscall.SIGTERM)
140+
t.Cleanup(notifyCancel)
141+
138142
buf.Reset()
139143
promptReader, promptWriter = io.Pipe()
140144

@@ -145,7 +149,7 @@ func TestPromptForConfirmation(t *testing.T) {
145149

146150
result := make(chan promptResult, 1)
147151
go func() {
148-
r, err := command.PromptForConfirmation(ctx, promptReader, promptOut, "")
152+
r, err := command.PromptForConfirmation(notifyCtx, promptReader, promptOut, "")
149153
result <- promptResult{r, err}
150154
}()
151155

cmd/docker/docker.go

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,12 @@ import (
2828
)
2929

3030
func main() {
31-
ctx := context.Background()
31+
baseCtx, cancel := context.WithCancel(context.Background())
32+
defer cancel()
33+
34+
ctx, cancelNotify := signal.NotifyContext(baseCtx, platformsignals.TerminationSignals...)
35+
defer cancelNotify()
36+
3237
dockerCli, err := command.NewDockerCli(command.WithBaseContext(ctx))
3338
if err != nil {
3439
fmt.Fprintln(os.Stderr, err)
@@ -223,7 +228,7 @@ func setValidateArgs(dockerCli command.Cli, cmd *cobra.Command) {
223228
})
224229
}
225230

226-
func tryPluginRun(dockerCli command.Cli, cmd *cobra.Command, subcommand string, envs []string) error {
231+
func tryPluginRun(ctx context.Context, dockerCli command.Cli, cmd *cobra.Command, subcommand string, envs []string) error {
227232
plugincmd, err := pluginmanager.PluginRunCommand(dockerCli, subcommand, cmd)
228233
if err != nil {
229234
return err
@@ -241,11 +246,15 @@ func tryPluginRun(dockerCli command.Cli, cmd *cobra.Command, subcommand string,
241246

242247
// Background signal handling logic: block on the signals channel, and
243248
// notify the plugin via the PluginServer (or signal) as appropriate.
244-
const exitLimit = 3
245-
signals := make(chan os.Signal, exitLimit)
246-
signal.Notify(signals, platformsignals.TerminationSignals...)
249+
const exitLimit = 2
250+
247251
go func() {
248252
retries := 0
253+
<-ctx.Done()
254+
255+
signals := make(chan os.Signal, exitLimit)
256+
signal.Notify(signals, platformsignals.TerminationSignals...)
257+
249258
for range signals {
250259
// If stdin is a TTY, the kernel will forward
251260
// signals to the subprocess because the shared
@@ -333,7 +342,7 @@ func runDocker(ctx context.Context, dockerCli *command.DockerCli) error {
333342
ccmd, _, err := cmd.Find(args)
334343
subCommand = ccmd
335344
if err != nil || pluginmanager.IsPluginCommand(ccmd) {
336-
err := tryPluginRun(dockerCli, cmd, args[0], envs)
345+
err := tryPluginRun(ctx, dockerCli, cmd, args[0], envs)
337346
if err == nil {
338347
if dockerCli.HooksEnabled() && dockerCli.Out().IsTerminal() && ccmd != nil {
339348
_ = pluginmanager.RunPluginHooks(dockerCli, cmd, ccmd, args[0], args)

internal/test/cmd.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package test
33
import (
44
"context"
55
"os"
6-
"syscall"
76
"testing"
87
"time"
98

@@ -32,8 +31,11 @@ func TerminatePrompt(ctx context.Context, t *testing.T, cmd *cobra.Command, cli
3231
assert.NilError(t, err)
3332
cli.SetIn(streams.NewIn(r))
3433

34+
notifyCtx, notifyCancel := context.WithCancel(ctx)
35+
t.Cleanup(notifyCancel)
36+
3537
go func() {
36-
errChan <- cmd.ExecuteContext(ctx)
38+
errChan <- cmd.ExecuteContext(notifyCtx)
3739
}()
3840

3941
writeCtx, writeCancel := context.WithTimeout(ctx, 100*time.Millisecond)
@@ -66,7 +68,7 @@ func TerminatePrompt(ctx context.Context, t *testing.T, cmd *cobra.Command, cli
6668

6769
// sigint and sigterm are caught by the prompt
6870
// this allows us to gracefully exit the prompt with a 0 exit code
69-
syscall.Kill(syscall.Getpid(), syscall.SIGINT)
71+
notifyCancel()
7072

7173
select {
7274
case <-errCtx.Done():

0 commit comments

Comments
 (0)