Skip to content

Commit 9aae5e4

Browse files
neersightedneersighted
authored
Merge pull request #4960 from neersighted/plugin_comments
plugin: update/improve process lifecycle documentation
2 parents 318911b + 542e82c commit 9aae5e4

2 files changed

Lines changed: 41 additions & 22 deletions

File tree

‎cli-plugins/socket/socket.go‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,13 @@ import (
1111
"sync"
1212
)
1313

14-
// EnvKey represents the well-known environment variable used to pass the plugin being
15-
// executed the socket name it should listen on to coordinate with the host CLI.
14+
// EnvKey represents the well-known environment variable used to pass the
15+
// plugin being executed the socket name it should listen on to coordinate with
16+
// the host CLI.
1617
const EnvKey = "DOCKER_CLI_PLUGIN_SOCKET"
1718

18-
// NewPluginServer creates a plugin server that listens on a new Unix domain socket.
19-
// `h` is called for each new connection to the socket in a goroutine.
19+
// NewPluginServer creates a plugin server that listens on a new Unix domain
20+
// socket. h is called for each new connection to the socket in a goroutine.
2021
func NewPluginServer(h func(net.Conn)) (*PluginServer, error) {
2122
l, err := listen("docker_cli_" + randomID())
2223
if err != nil {
@@ -63,7 +64,7 @@ func (pl *PluginServer) accept() error {
6364
defer pl.mu.Unlock()
6465

6566
if pl.closed {
66-
// handle potential race condition between Close and Accept
67+
// Handle potential race between Close and accept.
6768
conn.Close()
6869
return errors.New("plugin server is closed")
6970
}
@@ -74,20 +75,25 @@ func (pl *PluginServer) accept() error {
7475
return nil
7576
}
7677

78+
// Addr returns the [net.Addr] of the underlying [net.Listener].
7779
func (pl *PluginServer) Addr() net.Addr {
7880
return pl.l.Addr()
7981
}
8082

81-
// Close ensures that the server is no longer accepting new connections and closes all existing connections.
82-
// Existing connections will receive [io.EOF].
83+
// Close ensures that the server is no longer accepting new connections and
84+
// closes all existing connections. Existing connections will receive [io.EOF].
85+
//
86+
// The error value is that of the underlying [net.Listner.Close] call.
8387
func (pl *PluginServer) Close() error {
8488
// Remove the listener socket, if it exists on the filesystem.
8589
unlink(pl.l)
8690

87-
// Close connections first to ensure the connections get io.EOF instead of a connection reset.
91+
// Close connections first to ensure the connections get io.EOF instead
92+
// of a connection reset.
8893
pl.closeAllConns()
8994

90-
// Try to ensure that any active connections have a chance to receive io.EOF
95+
// Try to ensure that any active connections have a chance to receive
96+
// io.EOF.
9197
runtime.Gosched()
9298

9399
return pl.l.Close()
@@ -97,7 +103,7 @@ func (pl *PluginServer) closeAllConns() {
97103
pl.mu.Lock()
98104
defer pl.mu.Unlock()
99105

100-
// Prevent new connections from being accepted
106+
// Prevent new connections from being accepted.
101107
pl.closed = true
102108

103109
for _, conn := range pl.conns {

‎cmd/docker/docker.go‎

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -220,33 +220,46 @@ func tryPluginRun(dockerCli command.Cli, cmd *cobra.Command, subcommand string,
220220
return err
221221
}
222222

223-
// Establish the plugin socket, adding it to the environment under a well-known key if successful.
223+
// Establish the plugin socket, adding it to the environment under a
224+
// well-known key if successful.
224225
srv, err := socket.NewPluginServer(nil)
225226
if err == nil {
226-
envs = append(envs, socket.EnvKey+"="+srv.Addr().String())
227+
plugincmd.Env = append(plugincmd.Env, socket.EnvKey+"="+srv.Addr().String())
227228
}
228229

229-
plugincmd.Env = append(envs, plugincmd.Env...)
230+
// Set additional environment variables specified by the caller.
231+
plugincmd.Env = append(plugincmd.Env, envs...)
230232

233+
// Background signal handling logic: block on the signals channel, and
234+
// notify the plugin via the PluginServer (or signal) as appropriate.
231235
const exitLimit = 3
232-
233236
signals := make(chan os.Signal, exitLimit)
234237
signal.Notify(signals, platformsignals.TerminationSignals...)
235-
// signal handling goroutine: listen on signals channel, and if conn is
236-
// non-nil, attempt to close it to let the plugin know to exit. Regardless
237-
// of whether we successfully signal the plugin or not, after 3 SIGINTs,
238-
// we send a SIGKILL to the plugin process and exit
239238
go func() {
240239
retries := 0
241240
for range signals {
241+
// If stdin is a TTY, the kernel will forward
242+
// signals to the subprocess because the shared
243+
// pgid makes the TTY a controlling terminal.
244+
//
245+
// The plugin should have it's own copy of this
246+
// termination logic, and exit after 3 retries
247+
// on it's own.
242248
if dockerCli.Out().IsTerminal() {
243-
// running attached to a terminal, so the plugin will already
244-
// receive signals due to sharing a pgid with the parent CLI
245249
continue
246250
}
247251

248-
srv.Close()
249-
252+
// Terminate the plugin server, which will
253+
// close all connections with plugin
254+
// subprocesses, and signal them to exit.
255+
//
256+
// Repeated invocations will result in EINVAL,
257+
// or EBADF; but that is fine for our purposes.
258+
_ = srv.Close()
259+
260+
// If we're still running after 3 interruptions
261+
// (SIGINT/SIGTERM), send a SIGKILL to the plugin as a
262+
// final attempt to terminate, and exit.
250263
retries++
251264
if retries >= exitLimit {
252265
_, _ = fmt.Fprintf(dockerCli.Err(), "got %d SIGTERM/SIGINTs, forcefully exiting\n", retries)

0 commit comments

Comments
 (0)