Convert docker root command to use pflag and cobra

Fix the daemon proxy for cobra commands.

Signed-off-by: Daniel Nephin <dnephin@docker.com>

api/client/cli.go

@@ -14,7 +14,7 @@ import (
 	"github.com/docker/docker/cliconfig/configfile"
 	"github.com/docker/docker/cliconfig/credentials"
 	"github.com/docker/docker/dockerversion"
-	"github.com/docker/docker/opts"
+	dopts "github.com/docker/docker/opts"
 	"github.com/docker/docker/pkg/term"
 	"github.com/docker/engine-api/client"
 	"github.com/docker/go-connections/sockets"
@@ -53,15 +53,6 @@ type DockerCli struct {
 	outState *term.State
 }
 
-// Initialize calls the init function that will setup the configuration for the client
-// such as the TLS, tcp and other parameters used to run the client.
-func (cli *DockerCli) Initialize() error {
-	if cli.init == nil {
-		return nil
-	}
-	return cli.init()
-}
-
 // Client returns the APIClient
 func (cli *DockerCli) Client() client.APIClient {
 	return cli.client
@@ -155,39 +146,39 @@ func (cli *DockerCli) restoreTerminal(in io.Closer) error {
 	return nil
 }
 
+// Initialize the dockerCli runs initialization that must happen after command
+// line flags are parsed.
+func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions) error {
+	cli.configFile = LoadDefaultConfigFile(cli.err)
+
+	client, err := NewAPIClientFromFlags(opts.Common, cli.configFile)
+	if err != nil {
+		return err
+	}
+
+	cli.client = client
+
+	if cli.in != nil {
+		cli.inFd, cli.isTerminalIn = term.GetFdInfo(cli.in)
+	}
+	if cli.out != nil {
+		cli.outFd, cli.isTerminalOut = term.GetFdInfo(cli.out)
+	}
+	return nil
+}
+
 // NewDockerCli returns a DockerCli instance with IO output and error streams set by in, out and err.
 // The key file, protocol (i.e. unix) and address are passed in as strings, along with the tls.Config. If the tls.Config
 // is set the client scheme will be set to https.
 // The client will be given a 32-second timeout (see https://github.com/docker/docker/pull/8035).
-func NewDockerCli(in io.ReadCloser, out, err io.Writer, clientFlags *cliflags.ClientFlags) *DockerCli {
+func NewDockerCli(in io.ReadCloser, out, err io.Writer, clientOpts *cliflags.ClientOptions) *DockerCli {
 	cli := &DockerCli{
-		in:      in,
+		in:  in,
-		out:     out,
+		out: out,
-		err:     err,
+		err: err,
-		keyFile: clientFlags.Common.TrustKey,
+		// TODO: just pass trustKey, not the entire opts struct
+		keyFile: clientOpts.Common.TrustKey,
 	}
-
-	cli.init = func() error {
-		clientFlags.PostParse()
-		cli.configFile = LoadDefaultConfigFile(err)
-
-		client, err := NewAPIClientFromFlags(clientFlags, cli.configFile)
-		if err != nil {
-			return err
-		}
-
-		cli.client = client
-
-		if cli.in != nil {
-			cli.inFd, cli.isTerminalIn = term.GetFdInfo(cli.in)
-		}
-		if cli.out != nil {
-			cli.outFd, cli.isTerminalOut = term.GetFdInfo(cli.out)
-		}
-
-		return nil
-	}
-
 	return cli
 }
 
@@ -205,8 +196,8 @@ func LoadDefaultConfigFile(err io.Writer) *configfile.ConfigFile {
 }
 
 // NewAPIClientFromFlags creates a new APIClient from command line flags
-func NewAPIClientFromFlags(clientFlags *cliflags.ClientFlags, configFile *configfile.ConfigFile) (client.APIClient, error) {
+func NewAPIClientFromFlags(opts *cliflags.CommonOptions, configFile *configfile.ConfigFile) (client.APIClient, error) {
-	host, err := getServerHost(clientFlags.Common.Hosts, clientFlags.Common.TLSOptions)
+	host, err := getServerHost(opts.Hosts, opts.TLSOptions)
 	if err != nil {
 		return &client.Client{}, err
 	}
@@ -222,7 +213,7 @@ func NewAPIClientFromFlags(clientFlags *cliflags.ClientFlags, configFile *config
 		verStr = tmpStr
 	}
 
-	httpClient, err := newHTTPClient(host, clientFlags.Common.TLSOptions)
+	httpClient, err := newHTTPClient(host, opts.TLSOptions)
 	if err != nil {
 		return &client.Client{}, err
 	}
@@ -240,7 +231,7 @@ func getServerHost(hosts []string, tlsOptions *tlsconfig.Options) (host string,
 		return "", errors.New("Please specify only one -H")
 	}
 
-	host, err = opts.ParseHost(tlsOptions != nil, host)
+	host, err = dopts.ParseHost(tlsOptions != nil, host)
 	return
 }
 

cli/cobraadaptor/adaptor.go

@@ -14,33 +14,18 @@ import (
 	"github.com/docker/docker/api/client/system"
 	"github.com/docker/docker/api/client/volume"
 	"github.com/docker/docker/cli"
-	cliflags "github.com/docker/docker/cli/flags"
-	"github.com/docker/docker/pkg/term"
 	"github.com/spf13/cobra"
 )
 
-// CobraAdaptor is an adaptor for supporting spf13/cobra commands in the
+// SetupRootCommand sets default usage, help, and error handling for the
-// docker/cli framework
+// root command.
-type CobraAdaptor struct {
+// TODO: move to cmd/docker/docker?
-	rootCmd   *cobra.Command
+// TODO: split into common setup and client setup
-	dockerCli *client.DockerCli
+func SetupRootCommand(rootCmd *cobra.Command, dockerCli *client.DockerCli) {
-}
-
-// NewCobraAdaptor returns a new handler
-func NewCobraAdaptor(clientFlags *cliflags.ClientFlags) CobraAdaptor {
-	stdin, stdout, stderr := term.StdStreams()
-	dockerCli := client.NewDockerCli(stdin, stdout, stderr, clientFlags)
-
-	var rootCmd = &cobra.Command{
-		Use:           "docker [OPTIONS]",
-		Short:         "A self-sufficient runtime for containers",
-		SilenceUsage:  true,
-		SilenceErrors: true,
-	}
 	rootCmd.SetUsageTemplate(usageTemplate)
 	rootCmd.SetHelpTemplate(helpTemplate)
 	rootCmd.SetFlagErrorFunc(cli.FlagErrorFunc)
-	rootCmd.SetOutput(stdout)
+	rootCmd.SetOutput(dockerCli.Out())
 	rootCmd.AddCommand(
 		node.NewNodeCommand(dockerCli),
 		service.NewServiceCommand(dockerCli),
@@ -94,44 +79,6 @@ func NewCobraAdaptor(clientFlags *cliflags.ClientFlags) CobraAdaptor {
 
 	rootCmd.PersistentFlags().BoolP("help", "h", false, "Print usage")
 	rootCmd.PersistentFlags().MarkShorthandDeprecated("help", "please use --help")
-
-	return CobraAdaptor{
-		rootCmd:   rootCmd,
-		dockerCli: dockerCli,
-	}
-}
-
-// Usage returns the list of commands and their short usage string for
-// all top level cobra commands.
-func (c CobraAdaptor) Usage() []cli.Command {
-	cmds := []cli.Command{}
-	for _, cmd := range c.rootCmd.Commands() {
-		if cmd.Name() != "" {
-			cmds = append(cmds, cli.Command{Name: cmd.Name(), Description: cmd.Short})
-		}
-	}
-	return cmds
-}
-
-func (c CobraAdaptor) run(cmd string, args []string) error {
-	if err := c.dockerCli.Initialize(); err != nil {
-		return err
-	}
-	// Prepend the command name to support normal cobra command delegation
-	c.rootCmd.SetArgs(append([]string{cmd}, args...))
-	return c.rootCmd.Execute()
-}
-
-// Command returns a cli command handler if one exists
-func (c CobraAdaptor) Command(name string) func(...string) error {
-	for _, cmd := range c.rootCmd.Commands() {
-		if cmd.Name() == name {
-			return func(args ...string) error {
-				return c.run(name, args)
-			}
-		}
-	}
-	return nil
 }
 
 // GetRootCommand returns the root command. Required to generate the man pages

cli/flags/client.go

@@ -1,14 +1,13 @@
 package flags
 
-import (
+// ClientOptions are the options used to configure the client cli
-	"github.com/spf13/pflag"
+type ClientOptions struct {
-)
-
-// ClientFlags represents flags for the docker client.
-type ClientFlags struct {
-	FlagSet   *pflag.FlagSet
 	Common    *CommonOptions
-	PostParse func()
-
 	ConfigDir string
+	Version   bool
+}
+
+// NewClientOptions returns a new ClientOptions
+func NewClientOptions() *ClientOptions {
+	return &ClientOptions{Common: NewCommonOptions()}
 }

cmd/docker/daemon.go

@@ -1,18 +0,0 @@
-package main
-
-const daemonBinary = "dockerd"
-
-// DaemonProxy acts as a cli.Handler to proxy calls to the daemon binary
-type DaemonProxy struct{}
-
-// NewDaemonProxy returns a new handler
-func NewDaemonProxy() DaemonProxy {
-	return DaemonProxy{}
-}
-
-// Command returns a cli command handler if one exists
-func (p DaemonProxy) Command(name string) func(...string) error {
-	return map[string]func(...string) error{
-		"daemon": p.CmdDaemon,
-	}[name]
-}

cmd/docker/daemon_none.go

@@ -4,12 +4,22 @@ package main
 
 import (
 	"fmt"
+	"github.com/spf13/cobra"
 	"runtime"
 	"strings"
 )
 
-// CmdDaemon reports on an error on windows, because there is no exec
+func newDaemonCommand() *cobra.Command {
-func (p DaemonProxy) CmdDaemon(args ...string) error {
+	return &cobra.Command{
+		Use:    "daemon",
+		Hidden: true,
+		RunE: func(cmd *cobra.Command, args []string) error {
+			return runDaemon()
+		},
+	}
+}
+
+func runDaemon() error {
 	return fmt.Errorf(
 		"`docker daemon` is not supported on %s. Please run `dockerd` directly",
 		strings.Title(runtime.GOOS))

cmd/docker/daemon_unix.go

@@ -3,23 +3,37 @@
 package main
 
 import (
+	"fmt"
+
 	"os"
 	"os/exec"
 	"path/filepath"
 	"syscall"
+
+	"github.com/spf13/cobra"
 )
 
-// CmdDaemon execs dockerd with the same flags
+const daemonBinary = "dockerd"
-func (p DaemonProxy) CmdDaemon(args ...string) error {
-	// Special case for handling `docker help daemon`. When pkg/mflag is removed
-	// we can support this on the daemon side, but that is not possible with
-	// pkg/mflag because it uses os.Exit(1) instead of returning an error on
-	// unexpected args.
-	if len(args) == 0 || args[0] != "--help" {
-		// Use os.Args[1:] so that "global" args are passed to dockerd
-		args = stripDaemonArg(os.Args[1:])
-	}
 
+func newDaemonCommand() *cobra.Command {
+	cmd := &cobra.Command{
+		Use:    "daemon",
+		Hidden: true,
+		RunE: func(cmd *cobra.Command, args []string) error {
+			return runDaemon()
+		},
+	}
+	cmd.SetHelpFunc(helpFunc)
+	return cmd
+}
+
+// CmdDaemon execs dockerd with the same flags
+func runDaemon() error {
+	// Use os.Args[1:] so that "global" args are passed to dockerd
+	return execDaemon(stripDaemonArg(os.Args[1:]))
+}
+
+func execDaemon(args []string) error {
 	binaryPath, err := findDaemonBinary()
 	if err != nil {
 		return err
@@ -31,6 +45,12 @@ func (p DaemonProxy) CmdDaemon(args ...string) error {
 		os.Environ())
 }
 
+func helpFunc(cmd *cobra.Command, args []string) {
+	if err := execDaemon([]string{"--help"}); err != nil {
+		fmt.Fprintf(os.Stderr, "%s\n", err.Error())
+	}
+}
+
 // findDaemonBinary looks for the path to the dockerd binary starting with
 // the directory of the current executable (if one exists) and followed by $PATH
 func findDaemonBinary() (string, error) {

cmd/docker/docker.go

@@ -12,64 +12,52 @@ import (
 	cliflags "github.com/docker/docker/cli/flags"
 	"github.com/docker/docker/cliconfig"
 	"github.com/docker/docker/dockerversion"
-	flag "github.com/docker/docker/pkg/mflag"
 	"github.com/docker/docker/pkg/term"
 	"github.com/docker/docker/utils"
+	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 )
 
-var (
+func newDockerCommand(dockerCli *client.DockerCli, opts *cliflags.ClientOptions) *cobra.Command {
-	commonFlags = cliflags.InitCommonFlags()
+	cmd := &cobra.Command{
-	clientFlags = initClientFlags(commonFlags)
+		Use:           "docker [OPTIONS] COMMAND [arg...]",
-	flHelp      = flag.Bool([]string{"h", "-help"}, false, "Print usage")
+		Short:         "A self-sufficient runtime for containers.",
-	flVersion   = flag.Bool([]string{"v", "-version"}, false, "Print version information and quit")
+		SilenceUsage:  true,
-)
+		SilenceErrors: true,
+		Args:          cli.NoArgs,
+		RunE: func(cmd *cobra.Command, args []string) error {
+			if opts.Version {
+				showVersion()
+				return nil
+			}
+			fmt.Fprintf(dockerCli.Err(), "\n"+cmd.UsageString())
+			return nil
+		},
+		PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
+			dockerPreRun(cmd.Flags(), opts)
+			return dockerCli.Initialize(opts)
+		},
+	}
+	cobraadaptor.SetupRootCommand(cmd, dockerCli)
+
+	flags := cmd.Flags()
+	flags.BoolVarP(&opts.Version, "version", "v", false, "Print version information and quit")
+	flags.StringVar(&opts.ConfigDir, "config", cliconfig.ConfigDir(), "Location of client config files")
+	opts.Common.InstallFlags(flags)
+
+	return cmd
+}
 
 func main() {
 	// Set terminal emulation based on platform as required.
 	stdin, stdout, stderr := term.StdStreams()
-
 	logrus.SetOutput(stderr)
 
-	flag.Merge(flag.CommandLine, clientFlags.FlagSet, commonFlags.FlagSet)
+	opts := cliflags.NewClientOptions()
+	dockerCli := client.NewDockerCli(stdin, stdout, stderr, opts)
+	cmd := newDockerCommand(dockerCli, opts)
 
-	cobraAdaptor := cobraadaptor.NewCobraAdaptor(clientFlags)
+	if err := cmd.Execute(); err != nil {
-
-	flag.Usage = func() {
-		fmt.Fprint(stdout, "Usage: docker [OPTIONS] COMMAND [arg...]\n       docker [ --help | -v | --version ]\n\n")
-		fmt.Fprint(stdout, "A self-sufficient runtime for containers.\n\nOptions:\n")
-
-		flag.CommandLine.SetOutput(stdout)
-		flag.PrintDefaults()
-
-		help := "\nCommands:\n"
-
-		dockerCommands := append(cli.DockerCommandUsage, cobraAdaptor.Usage()...)
-		for _, cmd := range sortCommands(dockerCommands) {
-			help += fmt.Sprintf("    %-10.10s%s\n", cmd.Name, cmd.Description)
-		}
-
-		help += "\nRun 'docker COMMAND --help' for more information on a command."
-		fmt.Fprintf(stdout, "%s\n", help)
-	}
-
-	flag.Parse()
-
-	if *flVersion {
-		showVersion()
-		return
-	}
-
-	if *flHelp {
-		// if global flag --help is present, regardless of what other options and commands there are,
-		// just print the usage.
-		flag.Usage()
-		return
-	}
-
-	clientCli := client.NewDockerCli(stdin, stdout, stderr, clientFlags)
-
-	c := cli.New(clientCli, NewDaemonProxy(), cobraAdaptor)
-	if err := c.Run(flag.Args()...); err != nil {
 		if sterr, ok := err.(cli.StatusError); ok {
 			if sterr.Status != "" {
 				fmt.Fprintln(stderr, sterr.Status)
@@ -94,26 +82,22 @@ func showVersion() {
 	}
 }
 
-func initClientFlags(commonFlags *cliflags.CommonFlags) *cliflags.ClientFlags {
+func dockerPreRun(flags *pflag.FlagSet, opts *cliflags.ClientOptions) {
-	clientFlags := &cliflags.ClientFlags{FlagSet: new(flag.FlagSet), Common: commonFlags}
+	opts.Common.SetDefaultOptions(flags)
-	client := clientFlags.FlagSet
+	cliflags.SetDaemonLogLevel(opts.Common.LogLevel)
-	client.StringVar(&clientFlags.ConfigDir, []string{"-config"}, cliconfig.ConfigDir(), "Location of client config files")
 
-	clientFlags.PostParse = func() {
+	// TODO: remove this, set a default in New, and pass it in opts
-		clientFlags.Common.PostParse()
+	if opts.ConfigDir != "" {
-		cliflags.SetDaemonLogLevel(commonOpts.LogLevel)
+		cliconfig.SetConfigDir(opts.ConfigDir)
-
+	}
-		if clientFlags.ConfigDir != "" {
+
-			cliconfig.SetConfigDir(clientFlags.ConfigDir)
+	if opts.Common.TrustKey == "" {
-		}
+		opts.Common.TrustKey = filepath.Join(
-
+			cliconfig.ConfigDir(),
-		if clientFlags.Common.TrustKey == "" {
+			cliflags.DefaultTrustKeyFile)
-			clientFlags.Common.TrustKey = filepath.Join(cliconfig.ConfigDir(), cliflags.DefaultTrustKeyFile)
+	}
-		}
+
-
+	if opts.Common.Debug {
-		if clientFlags.Common.Debug {
+		utils.EnableDebug()
-			utils.EnableDebug()
-		}
 	}
-	return clientFlags
 }

cmd/docker/docker_test.go

@@ -6,13 +6,21 @@ import (
 
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/utils"
+
+	"github.com/docker/docker/api/client"
+	cliflags "github.com/docker/docker/cli/flags"
 )
 
 func TestClientDebugEnabled(t *testing.T) {
 	defer utils.DisableDebug()
 
-	clientFlags.Common.FlagSet.Parse([]string{"-D"})
+	opts := cliflags.NewClientOptions()
-	clientFlags.PostParse()
+	cmd := newDockerCommand(&client.DockerCli{}, opts)
+
+	opts.Common.Debug = true
+	if err := cmd.PersistentPreRunE(cmd, []string{}); err != nil {
+		t.Fatalf("Unexpected error: %s", err.Error())
+	}
 
 	if os.Getenv("DEBUG") != "1" {
 		t.Fatal("expected debug enabled, got false")