From 542e82caeb6fa7890c11777eee5c0069bf8966f4 Mon Sep 17 00:00:00 2001 From: Bjorn Neergaard Date: Thu, 21 Mar 2024 22:34:39 -0600 Subject: [PATCH] plugin: update/improve process lifecycle documentation Signed-off-by: Bjorn Neergaard --- cli-plugins/socket/socket.go | 26 ++++++++++++++++---------- cmd/docker/docker.go | 35 ++++++++++++++++++++++++----------- 2 files changed, 40 insertions(+), 21 deletions(-) diff --git a/cli-plugins/socket/socket.go b/cli-plugins/socket/socket.go index 2ad3a183d7..30c4afec05 100644 --- a/cli-plugins/socket/socket.go +++ b/cli-plugins/socket/socket.go @@ -11,12 +11,13 @@ import ( "sync" ) -// EnvKey represents the well-known environment variable used to pass the plugin being -// executed the socket name it should listen on to coordinate with the host CLI. +// EnvKey represents the well-known environment variable used to pass the +// plugin being executed the socket name it should listen on to coordinate with +// the host CLI. const EnvKey = "DOCKER_CLI_PLUGIN_SOCKET" -// NewPluginServer creates a plugin server that listens on a new Unix domain socket. -// `h` is called for each new connection to the socket in a goroutine. +// NewPluginServer creates a plugin server that listens on a new Unix domain +// socket. h is called for each new connection to the socket in a goroutine. func NewPluginServer(h func(net.Conn)) (*PluginServer, error) { l, err := listen("docker_cli_" + randomID()) if err != nil { @@ -63,7 +64,7 @@ func (pl *PluginServer) accept() error { defer pl.mu.Unlock() if pl.closed { - // handle potential race condition between Close and Accept + // Handle potential race between Close and accept. conn.Close() return errors.New("plugin server is closed") } @@ -74,20 +75,25 @@ func (pl *PluginServer) accept() error { return nil } +// Addr returns the [net.Addr] of the underlying [net.Listener]. func (pl *PluginServer) Addr() net.Addr { return pl.l.Addr() } -// Close ensures that the server is no longer accepting new connections and closes all existing connections. -// Existing connections will receive [io.EOF]. +// Close ensures that the server is no longer accepting new connections and +// closes all existing connections. Existing connections will receive [io.EOF]. +// +// The error value is that of the underlying [net.Listner.Close] call. func (pl *PluginServer) Close() error { // Remove the listener socket, if it exists on the filesystem. unlink(pl.l) - // Close connections first to ensure the connections get io.EOF instead of a connection reset. + // Close connections first to ensure the connections get io.EOF instead + // of a connection reset. pl.closeAllConns() - // Try to ensure that any active connections have a chance to receive io.EOF + // Try to ensure that any active connections have a chance to receive + // io.EOF. runtime.Gosched() return pl.l.Close() @@ -97,7 +103,7 @@ func (pl *PluginServer) closeAllConns() { pl.mu.Lock() defer pl.mu.Unlock() - // Prevent new connections from being accepted + // Prevent new connections from being accepted. pl.closed = true for _, conn := range pl.conns { diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go index 3a46aeb46d..36bb7b87fa 100644 --- a/cmd/docker/docker.go +++ b/cmd/docker/docker.go @@ -220,33 +220,46 @@ func tryPluginRun(dockerCli command.Cli, cmd *cobra.Command, subcommand string, return err } - // Establish the plugin socket, adding it to the environment under a well-known key if successful. + // Establish the plugin socket, adding it to the environment under a + // well-known key if successful. srv, err := socket.NewPluginServer(nil) if err == nil { - envs = append(envs, socket.EnvKey+"="+srv.Addr().String()) + plugincmd.Env = append(plugincmd.Env, socket.EnvKey+"="+srv.Addr().String()) } - plugincmd.Env = append(envs, plugincmd.Env...) + // Set additional environment variables specified by the caller. + plugincmd.Env = append(plugincmd.Env, envs...) + // Background signal handling logic: block on the signals channel, and + // notify the plugin via the PluginServer (or signal) as appropriate. const exitLimit = 3 - signals := make(chan os.Signal, exitLimit) signal.Notify(signals, platformsignals.TerminationSignals...) - // signal handling goroutine: listen on signals channel, and if conn is - // non-nil, attempt to close it to let the plugin know to exit. Regardless - // of whether we successfully signal the plugin or not, after 3 SIGINTs, - // we send a SIGKILL to the plugin process and exit go func() { retries := 0 for range signals { + // If stdin is a TTY, the kernel will forward + // signals to the subprocess because the shared + // pgid makes the TTY a controlling terminal. + // + // The plugin should have it's own copy of this + // termination logic, and exit after 3 retries + // on it's own. if dockerCli.Out().IsTerminal() { - // running attached to a terminal, so the plugin will already - // receive signals due to sharing a pgid with the parent CLI continue } - srv.Close() + // Terminate the plugin server, which will + // close all connections with plugin + // subprocesses, and signal them to exit. + // + // Repeated invocations will result in EINVAL, + // or EBADF; but that is fine for our purposes. + _ = srv.Close() + // If we're still running after 3 interruptions + // (SIGINT/SIGTERM), send a SIGKILL to the plugin as a + // final attempt to terminate, and exit. retries++ if retries >= exitLimit { _, _ = fmt.Fprintf(dockerCli.Err(), "got %d SIGTERM/SIGINTs, forcefully exiting\n", retries)