cli-plugins: terminate plugin when CLI exits

Previously, long lived CLI plugin processes weren't
properly handled
(see: #4402)
resulting in plugin processes being left behind
running, after the CLI process exits.

This commit changes the plugin handling code to open
an abstract unix socket before running the plugin and
passing it to the plugin process, and changes the
signal handling on the CLI side to close this socket
which tells the plugin that it should exit.

This implementation makes use of sockets instead of
simply setting PDEATHSIG on the plugin process
so that it will work on both BSDs, assorted UNIXes
and Windows.

Signed-off-by: Laura Brehm <[email protected]>

cli-plugins/plugin/plugin.go

@@ -2,7 +2,10 @@ package plugin
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
+	"io"
+	"net"
 	"os"
 	"sync"
 
@@ -14,6 +17,11 @@ import (
 	"github.com/spf13/cobra"
 )
 
+// CliPluginCloseSocketEnvKey is used to pass the plugin being
+// executed the abstract socket name it should listen on to know
+// when the CLI has exited.
+const CliPluginCloseSocketEnvKey = "CLI_PLUGIN_EXIT_SOCKET"
+
 // PersistentPreRunE must be called by any plugin command (or
 // subcommand) which uses the cobra `PersistentPreRun*` hook. Plugins
 // which do not make use of `PersistentPreRun*` do not need to call
@@ -24,12 +32,44 @@ import (
 // called.
 var PersistentPreRunE func(*cobra.Command, []string) error
 
+func closeOnParentSocketClose() error {
+	socketAddr, ok := os.LookupEnv(CliPluginCloseSocketEnvKey)
+	if !ok {
+		return fmt.Errorf("no CLI_PLUGIN_EXIT_SOCKET variable to listen on")
+	}
+	addr, err := net.ResolveUnixAddr("unix", socketAddr)
+	if err != nil {
+		return err
+	}
+	udsSock, err := net.DialUnix("unix", nil, addr)
+	if err != nil {
+		return err
+	}
+
+	go func() {
+		for {
+			b := make([]byte, 1)
+			_, err := udsSock.Read(b)
+			if errors.Is(err, io.EOF) {
+				// TODO(laurazard): remove this, debugging purposes only
+				println("exited due to parent closing socket")
+				os.Exit(0)
+			}
+		}
+	}()
+
+	return nil
+}
+
 // RunPlugin executes the specified plugin command
 func RunPlugin(dockerCli *command.DockerCli, plugin *cobra.Command, meta manager.Metadata) error {
 	tcmd := newPluginCommand(dockerCli, plugin, meta)
 
 	var persistentPreRunOnce sync.Once
 	PersistentPreRunE = func(_ *cobra.Command, _ []string) error {
+		if err := closeOnParentSocketClose(); err != nil {
+			return err
+		}
 		var err error
 		persistentPreRunOnce.Do(func() {
 			var opts []command.InitializeOpt

cmd/docker/docker.go

@@ -2,23 +2,26 @@ package main
 
 import (
 	"fmt"
+	"net"
 	"os"
 	"os/exec"
+	"os/signal"
 	"strings"
 	"syscall"
 
 	"github.com/docker/cli/cli"
 	pluginmanager "github.com/docker/cli/cli-plugins/manager"
+	"github.com/docker/cli/cli-plugins/plugin"
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/commands"
 	cliflags "github.com/docker/cli/cli/flags"
 	"github.com/docker/cli/cli/version"
-	"github.com/docker/cli/cmd/docker/internal/appcontext"
 	"github.com/docker/docker/api/types/versions"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
+	"golang.org/x/sys/unix"
 )
 
 func newDockerCommand(dockerCli *command.DockerCli) *cli.TopLevelCommand {
@@ -194,9 +197,48 @@ func tryPluginRun(dockerCli command.Cli, cmd *cobra.Command, subcommand string,
 	}
 	plugincmd.Env = append(envs, plugincmd.Env...)
 
+	// TODO(laurazard): this will break if we run this plugin more than once,
+	// as we will collide with the already-in-use socket name. We should retry
+	// a couple of times and add a counter or some other unique element to them
+	// socket name
+	pluginCloseSocketAddr := net.UnixAddr{Name: "@docker_cli_" + subcommand, Net: "unix"}
+	listener, err := net.ListenUnix("unix", &pluginCloseSocketAddr)
+	if err != nil {
+		return errors.New("bork")
+	}
+	defer listener.Close()
+	plugincmd.Env = append(envs, plugin.CliPluginCloseSocketEnvKey+"="+pluginCloseSocketAddr.Name)
+
 	go func() {
-		// override SIGTERM handler so we let the plugin shut down first
-		<-appcontext.Context().Done()
+		signals := make(chan os.Signal, 2048)
+		signal.Notify(signals, unix.SIGTERM, unix.SIGINT)
+
+		const exitLimit = 3
+		retries := 0
+
+		var conn *net.UnixConn
+		go func() {
+			conn, err = listener.AcceptUnix()
+			if err != nil {
+				// TODO(laurazard): change me, halt execution or fallback to
+				// previous behaviour if we couldn't set up socket connection
+				println("got a weird error!", err.Error())
+			}
+		}()
+
+		go func() {
+			for {
+				<-signals
+				if err := conn.Close(); err != nil {
+					logrus.Error("failed to signal plugin to close: " + err.Error())
+				}
+				retries++
+				if retries >= exitLimit {
+					logrus.Errorf("got %d SIGTERM/SIGINTs, exiting without cleanup", retries)
+					os.Exit(1)
+				}
+			}
+		}()
 	}()
 
 	if err := plugincmd.Run(); err != nil {