diff --git a/cli-plugins/socket/socket.go b/cli-plugins/socket/socket.go index b9b297fa06db..9724c4e7f0e9 100644 --- a/cli-plugins/socket/socket.go +++ b/cli-plugins/socket/socket.go @@ -90,6 +90,16 @@ func (pl *PluginServer) Addr() net.Addr { return pl.l.Addr() } +// HasConnections reports whether any plugin has connected to the server. +func (pl *PluginServer) HasConnections() bool { + if pl == nil { + return false + } + pl.mu.Lock() + defer pl.mu.Unlock() + return len(pl.conns) > 0 +} + // Close ensures that the server is no longer accepting new connections and // closes all existing connections. Existing connections will receive [io.EOF]. // diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go index 0efcc70118fe..d6592ef2b555 100644 --- a/cmd/docker/docker.go +++ b/cmd/docker/docker.go @@ -368,10 +368,22 @@ func tryPluginRun(ctx context.Context, dockerCli command.Cli, cmd *cobra.Command // // Repeated invocations result in EINVAL or EBADF, // but that is fine for our purposes. + hasSocketConnections := srv != nil && srv.HasConnections() if srv != nil { _ = srv.Close() } + // Plugins using the CLI plugin socket are notified by closing the + // server connections above. Older plugins do not connect to the socket, + // so forward the user's termination signal directly to let them shut down + // gracefully instead of leaving them running in the background. + if !force && !hasSocketConnections { + var signalErr errCtxSignalTerminated + if errors.As(context.Cause(ctx), &signalErr) && signalErr.signal != os.Interrupt && plugincmd.Process != nil { + _ = plugincmd.Process.Signal(signalErr.signal) + } + } + // force the process to terminate if it hasn't already if force { // Close forceExitCh before Kill so the channel is guaranteed @@ -387,11 +399,11 @@ func tryPluginRun(ctx context.Context, dockerCli command.Cli, cmd *cobra.Command retries := 0 // catch the first signal through context cancellation <-ctx.Done() - tryTerminatePlugin(false) - - // register subsequent signals + // Register subsequent signals before attempting graceful termination, so + // quick repeated signals are still observed while cleanup is in progress. signals := make(chan os.Signal, exitLimit) signal.Notify(signals, platformsignals.TerminationSignals...) + tryTerminatePlugin(false) force := false for range signals { diff --git a/e2e/cli-plugins/plugins/presocket/main.go b/e2e/cli-plugins/plugins/presocket/main.go index d1a4b3501175..cd12d49d7918 100644 --- a/e2e/cli-plugins/plugins/presocket/main.go +++ b/e2e/cli-plugins/plugins/presocket/main.go @@ -55,6 +55,19 @@ func RootCmd(dockerCli command.Cli) *cobra.Command { }, }) + cmd.AddCommand(&cobra.Command{ + Use: "test-no-socket-exit-on-signal", + Short: "test command that exits when it receives a termination signal", + RunE: func(cmd *cobra.Command, args []string) error { + signalCh := make(chan os.Signal, 1) + signal.Notify(signalCh, syscall.SIGINT, syscall.SIGTERM) + sig := <-signalCh + _, _ = fmt.Fprintln(dockerCli.Out(), "received", sig) + os.Exit(2) + return nil + }, + }) + cmd.AddCommand(&cobra.Command{ Use: "test-socket", Short: "test command that runs until it receives a SIGINT", diff --git a/e2e/cli-plugins/socket_test.go b/e2e/cli-plugins/socket_test.go index 2bb93dd8f2a6..1a4e983144f7 100644 --- a/e2e/cli-plugins/socket_test.go +++ b/e2e/cli-plugins/socket_test.go @@ -179,6 +179,31 @@ func TestPluginSocketCommunication(t *testing.T) { }) t.Run("detached", func(t *testing.T) { + t.Run("the plugin receives SIGTERM sent directly to the CLI", func(t *testing.T) { + cmd := run("presocket", "test-no-socket-exit-on-signal") + command := exec.Command(cmd.Command[0], cmd.Command[1:]...) + t.Log(strings.Join(command.Args, " ")) + command.SysProcAttr = &syscall.SysProcAttr{ + Setpgid: true, + } + + go func() { + <-time.After(time.Second) + // Signal the CLI process directly, not the process group. When a + // plugin is detached and cannot be canceled over the plugin socket, + // the CLI should forward the termination signal to the plugin. + err := syscall.Kill(command.Process.Pid, syscall.SIGTERM) + assert.NilError(t, err, "failed to signal CLI process") + }() + out, err := command.CombinedOutput() + + var exitError *exec.ExitError + assert.Assert(t, errors.As(err, &exitError)) + assert.Check(t, exitError.Exited()) + assert.Check(t, is.Equal(exitError.ExitCode(), 2)) + assert.Equal(t, string(out), "received terminated\n") + }) + t.Run("the plugin does not get signalled", func(t *testing.T) { cmd := run("presocket", "test-socket") command := exec.Command(cmd.Command[0], cmd.Command[1:]...)