Skip to content

Commit

Permalink
plugins: run plugin with new process group ID
Browse files Browse the repository at this point in the history
Changes were made in docker#4599 to provide
a mechanism for the CLI to notify running plugin processes that they
should exit, in order to improve the general CLI/plugin UX. The current
implementation boils down to:
1. The CLI creates a socket
2. The CLI executes the plugin
3. The plugin connects to the socket
4. (When) the CLI receives a termination signal, it uses the socket to
   notify the plugin that it should exit
5. The plugin's gets notified via the socket, and cancels it's `cmd.Context`,
   which then gets handled appropriately

This change works in most cases and fixes the issue it sets out to solve
(see: docker/compose#11292) however, in the case
where the user has a TTY attached and the plugin is not already handling
received signals, steps 4+ changes:
4. (When) the CLI receives a termination signal, before it can use the
   socket to notify the plugin that it should exit, the plugin process
   also receives a signal due to sharing the pgid with the CLI

Since we now have a proper "job control" mechanism, we can simplify the
scenarios by executing the plugins with their own process group id,
thereby removing the "double notification" issue and making it so that
plugins can handle the same whether attached to a TTY or not.

In order to make this change "plugin-binary" backwards-compatible, in
the case that a plugin does not connect to the socket, the CLI passes
the signal to the plugin process.

Signed-off-by: Laura Brehm <laurabrehm@hey.com>
  • Loading branch information
laurazard committed Jan 9, 2024
1 parent 7d92573 commit 8072c0f
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 1 deletion.
4 changes: 4 additions & 0 deletions cli-plugins/manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"sort"
"strings"
"sync"
"syscall"

"github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/config"
Expand Down Expand Up @@ -232,6 +233,9 @@ func PluginRunCommand(dockerCli command.Cli, name string, rootcmd *cobra.Command
cmd.Stdin = os.Stdin
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
cmd.SysProcAttr = &syscall.SysProcAttr{
Setpgid: true,
}

cmd.Env = os.Environ()
cmd.Env = append(cmd.Env, ReexecEnvvar+"="+os.Args[0])
Expand Down
4 changes: 3 additions & 1 deletion cmd/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,12 +230,14 @@ func tryPluginRun(dockerCli command.Cli, cmd *cobra.Command, subcommand string,
// we send a SIGKILL to the plugin process and exit
go func() {
retries := 0
for range signals {
for s := range signals {
if conn != nil {
if err := conn.Close(); err != nil {
_, _ = fmt.Fprintf(dockerCli.Err(), "failed to signal plugin to close: %v\n", err)
}
conn = nil
} else {
plugincmd.Process.Signal(s)
}
retries++
if retries >= exitLimit {
Expand Down

0 comments on commit 8072c0f

Please sign in to comment.