Ensure plugins can use PersistentPreRunE again.

I got a bit carried away in d4ced2ef77 ("allow plugins to have argument
which match a top-level flag.") and broke the ability of a plugin to use the
`PersistentPreRun(E)` hook on its top-level command (by unconditionally
overwriting it) and also broke the plugin framework if a plugin's subcommand
used those hooks (because they would shadow the root one). This could result in
either `dockerCli.Client()` returning `nil` or whatever initialisation the
plugin hoped to do not occuring.

This change revert the relevant bits and reinstates the requirement that a
plugin calls `plugin.PersistentPreRunE` if it uses that hook itself.

It is at least a bit nicer now since we avoid the need for the global struct
since the interesting state is now encapsulated in `tcmd` (and the closure).

In principal this could be done even more simply (by calling `tcmd.Initialize`
statically between `tcmd.HandleGlobalFlags` and `cmd.Execute`) however this has
the downside of _always_ initialising the cli (and therefore dialing the
daemon) even for the `docker-cli-plugin-metadata` command but also for the
`help foo` and `foo --help` commands (Cobra short-circuits the hooks in this
case).

Signed-off-by: Ian Campbell <ijc@docker.com>

cli-plugins/examples/helloworld/main.go

@@ -51,7 +51,10 @@ func main() {
 		cmd := &cobra.Command{
 			Use:   "helloworld",
 			Short: "A basic Hello World plugin for tests",
-			PersistentPreRunE: func(_ *cobra.Command, _ []string) error {
+			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")
 				}

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
+	var persistentPreRunOnce sync.Once
-	// command which needlessly initializes the client and tries
+	PersistentPreRunE = func(_ *cobra.Command, _ []string) error {
-	// to connect to the daemon.
+		var err error
-	plugin.PersistentPreRunE = func(_ *cobra.Command, _ []string) error {
+		persistentPreRunOnce.Do(func() {
-		return tcmd.Initialize(withPluginClientConn(plugin.Name()))
+			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)