Skip to content

Commit 7b87094

Browse files
fix: forward SIGTERM to legacy CLI plugins
Signed-off-by: maxpetrusenkoagent <max.petrusenko.agent@gmail.com>
1 parent fb59821 commit 7b87094

4 files changed

Lines changed: 63 additions & 3 deletions

File tree

cli-plugins/socket/socket.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,16 @@ func (pl *PluginServer) Addr() net.Addr {
9090
return pl.l.Addr()
9191
}
9292

93+
// HasConnections reports whether any plugin has connected to the server.
94+
func (pl *PluginServer) HasConnections() bool {
95+
if pl == nil {
96+
return false
97+
}
98+
pl.mu.Lock()
99+
defer pl.mu.Unlock()
100+
return len(pl.conns) > 0
101+
}
102+
93103
// Close ensures that the server is no longer accepting new connections and
94104
// closes all existing connections. Existing connections will receive [io.EOF].
95105
//

cmd/docker/docker.go

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -368,10 +368,22 @@ func tryPluginRun(ctx context.Context, dockerCli command.Cli, cmd *cobra.Command
368368
//
369369
// Repeated invocations result in EINVAL or EBADF,
370370
// but that is fine for our purposes.
371+
hasSocketConnections := srv != nil && srv.HasConnections()
371372
if srv != nil {
372373
_ = srv.Close()
373374
}
374375

376+
// Plugins using the CLI plugin socket are notified by closing the
377+
// server connections above. Older plugins do not connect to the socket,
378+
// so forward the user's termination signal directly to let them shut down
379+
// gracefully instead of leaving them running in the background.
380+
if !force && !hasSocketConnections {
381+
var signalErr errCtxSignalTerminated
382+
if errors.As(context.Cause(ctx), &signalErr) && signalErr.signal != os.Interrupt && plugincmd.Process != nil {
383+
_ = plugincmd.Process.Signal(signalErr.signal)
384+
}
385+
}
386+
375387
// force the process to terminate if it hasn't already
376388
if force {
377389
// Close forceExitCh before Kill so the channel is guaranteed
@@ -387,11 +399,11 @@ func tryPluginRun(ctx context.Context, dockerCli command.Cli, cmd *cobra.Command
387399
retries := 0
388400
// catch the first signal through context cancellation
389401
<-ctx.Done()
390-
tryTerminatePlugin(false)
391-
392-
// register subsequent signals
402+
// Register subsequent signals before attempting graceful termination, so
403+
// quick repeated signals are still observed while cleanup is in progress.
393404
signals := make(chan os.Signal, exitLimit)
394405
signal.Notify(signals, platformsignals.TerminationSignals...)
406+
tryTerminatePlugin(false)
395407

396408
force := false
397409
for range signals {

e2e/cli-plugins/plugins/presocket/main.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,19 @@ func RootCmd(dockerCli command.Cli) *cobra.Command {
5555
},
5656
})
5757

58+
cmd.AddCommand(&cobra.Command{
59+
Use: "test-no-socket-exit-on-signal",
60+
Short: "test command that exits when it receives a termination signal",
61+
RunE: func(cmd *cobra.Command, args []string) error {
62+
signalCh := make(chan os.Signal, 1)
63+
signal.Notify(signalCh, syscall.SIGINT, syscall.SIGTERM)
64+
sig := <-signalCh
65+
_, _ = fmt.Fprintln(dockerCli.Out(), "received", sig)
66+
os.Exit(2)
67+
return nil
68+
},
69+
})
70+
5871
cmd.AddCommand(&cobra.Command{
5972
Use: "test-socket",
6073
Short: "test command that runs until it receives a SIGINT",

e2e/cli-plugins/socket_test.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,31 @@ func TestPluginSocketCommunication(t *testing.T) {
179179
})
180180

181181
t.Run("detached", func(t *testing.T) {
182+
t.Run("the plugin receives SIGTERM sent directly to the CLI", func(t *testing.T) {
183+
cmd := run("presocket", "test-no-socket-exit-on-signal")
184+
command := exec.Command(cmd.Command[0], cmd.Command[1:]...)
185+
t.Log(strings.Join(command.Args, " "))
186+
command.SysProcAttr = &syscall.SysProcAttr{
187+
Setpgid: true,
188+
}
189+
190+
go func() {
191+
<-time.After(time.Second)
192+
// Signal the CLI process directly, not the process group. When a
193+
// plugin is detached and cannot be canceled over the plugin socket,
194+
// the CLI should forward the termination signal to the plugin.
195+
err := syscall.Kill(command.Process.Pid, syscall.SIGTERM)
196+
assert.NilError(t, err, "failed to signal CLI process")
197+
}()
198+
out, err := command.CombinedOutput()
199+
200+
var exitError *exec.ExitError
201+
assert.Assert(t, errors.As(err, &exitError))
202+
assert.Check(t, exitError.Exited())
203+
assert.Check(t, is.Equal(exitError.ExitCode(), 2))
204+
assert.Equal(t, string(out), "received terminated\n")
205+
})
206+
182207
t.Run("the plugin does not get signalled", func(t *testing.T) {
183208
cmd := run("presocket", "test-socket")
184209
command := exec.Command(cmd.Command[0], cmd.Command[1:]...)

0 commit comments

Comments
 (0)