Merge pull request #1737 from ijc/plugins-using-PersistentPreRunE

Fix regression for CLI plugins using PersistentPreRunE

cli-plugins/examples/helloworld/main.go

@@ -45,12 +45,21 @@ func main() {
 		}
 
 		var (
-			who, context string
-			debug        bool
+			who, context  string
+			preRun, debug bool
 		)
 		cmd := &cobra.Command{
 			Use:   "helloworld",
 			Short: "A basic Hello World plugin for tests",
+			PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
+				if err := plugin.PersistentPreRunE(cmd, args); err != nil {
+					return err
+				}
+				if preRun {
+					fmt.Fprintf(dockerCli.Err(), "Plugin PersistentPreRunE called")
+				}
+				return nil
+			},
 			RunE: func(cmd *cobra.Command, args []string) error {
 				if debug {
 					fmt.Fprintf(dockerCli.Err(), "Plugin debug mode enabled")
@@ -79,6 +88,7 @@ func main() {
 
 		flags := cmd.Flags()
 		flags.StringVar(&who, "who", "", "Who are we addressing?")
+		flags.BoolVar(&preRun, "pre-run", false, "Log from prerun hook")
 		// These are intended to deliberately clash with the CLIs own top
 		// level arguments.
 		flags.BoolVarP(&debug, "debug", "D", false, "Enable debug")

cli-plugins/plugin/plugin.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
+	"sync"
 
 	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli-plugins/manager"
@@ -13,14 +14,26 @@ import (
 	"github.com/spf13/cobra"
 )
 
+// 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
+// this (although it remains safe to do so). Plugins are recommended
+// to use `PersistenPreRunE` to enable the error to be
+// returned. Should not be called outside of a command's
+// PersistentPreRunE hook and must not be run unless Run has been
+// called.
+var PersistentPreRunE func(*cobra.Command, []string) error
+
 func runPlugin(dockerCli *command.DockerCli, plugin *cobra.Command, meta manager.Metadata) error {
 	tcmd := newPluginCommand(dockerCli, plugin, meta)
 
-	// Doing this here avoids also calling it for the metadata
-	// command which needlessly initializes the client and tries
-	// to connect to the daemon.
-	plugin.PersistentPreRunE = func(_ *cobra.Command, _ []string) error {
-		return tcmd.Initialize(withPluginClientConn(plugin.Name()))
+	var persistentPreRunOnce sync.Once
+	PersistentPreRunE = func(_ *cobra.Command, _ []string) error {
+		var err error
+		persistentPreRunOnce.Do(func() {
+			err = tcmd.Initialize(withPluginClientConn(plugin.Name()))
+		})
+		return err
 	}
 
 	cmd, _, err := tcmd.HandleGlobalFlags()
@@ -98,6 +111,7 @@ func newPluginCommand(dockerCli *command.DockerCli, plugin *cobra.Command, meta
 		Short:                 fullname + " is a Docker CLI plugin",
 		SilenceUsage:          true,
 		SilenceErrors:         true,
+		PersistentPreRunE:     PersistentPreRunE,
 		TraverseChildren:      true,
 		DisableFlagsInUseLine: true,
 	}
@@ -122,6 +136,10 @@ func newMetadataSubcommand(plugin *cobra.Command, meta manager.Metadata) *cobra.
 	cmd := &cobra.Command{
 		Use:    manager.MetadataSubcommandName,
 		Hidden: true,
+		// Suppress the global/parent PersistentPreRunE, which
+		// needlessly initializes the client and tries to
+		// connect to the daemon.
+		PersistentPreRun: func(cmd *cobra.Command, args []string) {},
 		RunE: func(cmd *cobra.Command, args []string) error {
 			enc := json.NewEncoder(os.Stdout)
 			enc.SetEscapeHTML(false)

e2e/cli-plugins/run_test.go

@@ -177,10 +177,10 @@ func TestCliInitialized(t *testing.T) {
 	run, _, cleanup := prepare(t)
 	defer cleanup()
 
-	res := icmd.RunCmd(run("helloworld", "apiversion"))
+	res := icmd.RunCmd(run("helloworld", "--pre-run", "apiversion"))
 	res.Assert(t, icmd.Success)
 	assert.Assert(t, res.Stdout() != "")
-	assert.Assert(t, is.Equal(res.Stderr(), ""))
+	assert.Assert(t, is.Equal(res.Stderr(), "Plugin PersistentPreRunE called"))
 }
 
 // TestPluginErrorCode tests when the plugin return with a given exit status.

e2e/cli-plugins/testdata/docker-help-helloworld.golden

@@ -6,6 +6,7 @@ A basic Hello World plugin for tests
 Options:
   -c, --context string   Is it Christmas?
   -D, --debug            Enable debug
+      --pre-run          Log from prerun hook
       --who string       Who are we addressing?
 
 Commands: