Browse Source

Update unit tests for new cobra root command.

Cleanup cobra integration
Update windows files for cobra and pflags
Cleanup SetupRootcmd, and remove unnecessary SetFlagErrorFunc.
Use cobra command traversal

Signed-off-by: Daniel Nephin <dnephin@docker.com>
Daniel Nephin 9 years ago
parent
commit
31bf9ca0c8

+ 10 - 8
api/client/cli.go

@@ -6,6 +6,7 @@ import (
 	"io"
 	"net/http"
 	"os"
+	"path/filepath"
 	"runtime"
 
 	"github.com/docker/docker/api"
@@ -164,22 +165,23 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions) error {
 	if cli.out != nil {
 		cli.outFd, cli.isTerminalOut = term.GetFdInfo(cli.out)
 	}
+
+	if opts.Common.TrustKey == "" {
+		cli.keyFile = filepath.Join(cliconfig.ConfigDir(), cliflags.DefaultTrustKeyFile)
+	} else {
+		cli.keyFile = opts.Common.TrustKey
+	}
+
 	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, clientOpts *cliflags.ClientOptions) *DockerCli {
-	cli := &DockerCli{
+func NewDockerCli(in io.ReadCloser, out, err io.Writer) *DockerCli {
+	return &DockerCli{
 		in:  in,
 		out: out,
 		err: err,
-		// TODO: just pass trustKey, not the entire opts struct
-		keyFile: clientOpts.Common.TrustKey,
 	}
-	return cli
 }
 
 // LoadDefaultConfigFile attempts to load the default config file and returns

+ 0 - 1
api/client/container/attach.go

@@ -36,7 +36,6 @@ func NewAttachCommand(dockerCli *client.DockerCli) *cobra.Command {
 			return runAttach(dockerCli, &opts)
 		},
 	}
-	cmd.SetFlagErrorFunc(flagErrorFunc)
 
 	flags := cmd.Flags()
 	flags.BoolVar(&opts.noStdin, "no-stdin", false, "Do not attach STDIN")

+ 0 - 1
api/client/container/commit.go

@@ -38,7 +38,6 @@ func NewCommitCommand(dockerCli *client.DockerCli) *cobra.Command {
 			return runCommit(dockerCli, &opts)
 		},
 	}
-	cmd.SetFlagErrorFunc(flagErrorFunc)
 
 	flags := cmd.Flags()
 	flags.SetInterspersed(false)

+ 0 - 1
api/client/container/create.go

@@ -43,7 +43,6 @@ func NewCreateCommand(dockerCli *client.DockerCli) *cobra.Command {
 			return runCreate(dockerCli, cmd.Flags(), &opts, copts)
 		},
 	}
-	cmd.SetFlagErrorFunc(flagErrorFunc)
 
 	flags := cmd.Flags()
 	flags.SetInterspersed(false)

+ 1 - 4
api/client/container/diff.go

@@ -19,7 +19,7 @@ type diffOptions struct {
 func NewDiffCommand(dockerCli *client.DockerCli) *cobra.Command {
 	var opts diffOptions
 
-	cmd := &cobra.Command{
+	return &cobra.Command{
 		Use:   "diff CONTAINER",
 		Short: "Inspect changes on a container's filesystem",
 		Args:  cli.ExactArgs(1),
@@ -28,9 +28,6 @@ func NewDiffCommand(dockerCli *client.DockerCli) *cobra.Command {
 			return runDiff(dockerCli, &opts)
 		},
 	}
-	cmd.SetFlagErrorFunc(flagErrorFunc)
-
-	return cmd
 }
 
 func runDiff(dockerCli *client.DockerCli, opts *diffOptions) error {

+ 0 - 1
api/client/container/logs.go

@@ -41,7 +41,6 @@ func NewLogsCommand(dockerCli *client.DockerCli) *cobra.Command {
 			return runLogs(dockerCli, &opts)
 		},
 	}
-	cmd.SetFlagErrorFunc(flagErrorFunc)
 
 	flags := cmd.Flags()
 	flags.BoolVarP(&opts.follow, "follow", "f", false, "Follow log output")

+ 1 - 4
api/client/container/pause.go

@@ -19,7 +19,7 @@ type pauseOptions struct {
 func NewPauseCommand(dockerCli *client.DockerCli) *cobra.Command {
 	var opts pauseOptions
 
-	cmd := &cobra.Command{
+	return &cobra.Command{
 		Use:   "pause CONTAINER [CONTAINER...]",
 		Short: "Pause all processes within one or more containers",
 		Args:  cli.RequiresMinArgs(1),
@@ -28,9 +28,6 @@ func NewPauseCommand(dockerCli *client.DockerCli) *cobra.Command {
 			return runPause(dockerCli, &opts)
 		},
 	}
-	cmd.SetFlagErrorFunc(flagErrorFunc)
-
-	return cmd
 }
 
 func runPause(dockerCli *client.DockerCli, opts *pauseOptions) error {

+ 0 - 2
api/client/container/port.go

@@ -34,8 +34,6 @@ func NewPortCommand(dockerCli *client.DockerCli) *cobra.Command {
 			return runPort(dockerCli, &opts)
 		},
 	}
-	cmd.SetFlagErrorFunc(flagErrorFunc)
-
 	return cmd
 }
 

+ 0 - 2
api/client/container/rename.go

@@ -30,8 +30,6 @@ func NewRenameCommand(dockerCli *client.DockerCli) *cobra.Command {
 			return runRename(dockerCli, &opts)
 		},
 	}
-	cmd.SetFlagErrorFunc(flagErrorFunc)
-
 	return cmd
 }
 

+ 2 - 2
api/client/container/run.go

@@ -14,6 +14,7 @@ import (
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/api/client"
 	"github.com/docker/docker/cli"
+	"github.com/docker/docker/cli/cobraadaptor"
 	opttypes "github.com/docker/docker/opts"
 	"github.com/docker/docker/pkg/promise"
 	"github.com/docker/docker/pkg/signal"
@@ -48,7 +49,6 @@ func NewRunCommand(dockerCli *client.DockerCli) *cobra.Command {
 			return runRun(dockerCli, cmd.Flags(), &opts, copts)
 		},
 	}
-	cmd.SetFlagErrorFunc(flagErrorFunc)
 
 	flags := cmd.Flags()
 	flags.SetInterspersed(false)
@@ -70,7 +70,7 @@ func NewRunCommand(dockerCli *client.DockerCli) *cobra.Command {
 
 func flagErrorFunc(cmd *cobra.Command, err error) error {
 	return cli.StatusError{
-		Status:     cli.FlagErrorFunc(cmd, err).Error(),
+		Status:     cobraadaptor.FlagErrorFunc(cmd, err).Error(),
 		StatusCode: 125,
 	}
 }

+ 0 - 1
api/client/container/start.go

@@ -37,7 +37,6 @@ func NewStartCommand(dockerCli *client.DockerCli) *cobra.Command {
 			return runStart(dockerCli, &opts)
 		},
 	}
-	cmd.SetFlagErrorFunc(flagErrorFunc)
 
 	flags := cmd.Flags()
 	flags.BoolVarP(&opts.attach, "attach", "a", false, "Attach STDOUT/STDERR and forward signals")

+ 0 - 1
api/client/container/stop.go

@@ -31,7 +31,6 @@ func NewStopCommand(dockerCli *client.DockerCli) *cobra.Command {
 			return runStop(dockerCli, &opts)
 		},
 	}
-	cmd.SetFlagErrorFunc(flagErrorFunc)
 
 	flags := cmd.Flags()
 	flags.IntVarP(&opts.time, "time", "t", 10, "Seconds to wait for stop before killing it")

+ 0 - 1
api/client/container/top.go

@@ -32,7 +32,6 @@ func NewTopCommand(dockerCli *client.DockerCli) *cobra.Command {
 			return runTop(dockerCli, &opts)
 		},
 	}
-	cmd.SetFlagErrorFunc(flagErrorFunc)
 
 	flags := cmd.Flags()
 	flags.SetInterspersed(false)

+ 0 - 2
api/client/container/unpause.go

@@ -28,8 +28,6 @@ func NewUnpauseCommand(dockerCli *client.DockerCli) *cobra.Command {
 			return runUnpause(dockerCli, &opts)
 		},
 	}
-	cmd.SetFlagErrorFunc(flagErrorFunc)
-
 	return cmd
 }
 

+ 0 - 2
api/client/container/wait.go

@@ -28,8 +28,6 @@ func NewWaitCommand(dockerCli *client.DockerCli) *cobra.Command {
 			return runWait(dockerCli, &opts)
 		},
 	}
-	cmd.SetFlagErrorFunc(flagErrorFunc)
-
 	return cmd
 }
 

+ 18 - 68
cli/cobraadaptor/adaptor.go

@@ -1,81 +1,17 @@
 package cobraadaptor
 
 import (
-	"github.com/docker/docker/api/client"
-	"github.com/docker/docker/api/client/container"
-	"github.com/docker/docker/api/client/image"
-	"github.com/docker/docker/api/client/network"
-	"github.com/docker/docker/api/client/node"
-	"github.com/docker/docker/api/client/plugin"
-	"github.com/docker/docker/api/client/registry"
-	"github.com/docker/docker/api/client/service"
-	"github.com/docker/docker/api/client/stack"
-	"github.com/docker/docker/api/client/swarm"
-	"github.com/docker/docker/api/client/system"
-	"github.com/docker/docker/api/client/volume"
-	"github.com/docker/docker/cli"
+	"fmt"
+
 	"github.com/spf13/cobra"
 )
 
 // SetupRootCommand sets default usage, help, and error handling for the
 // root command.
-// TODO: move to cmd/docker/docker?
-// TODO: split into common setup and client setup
-func SetupRootCommand(rootCmd *cobra.Command, dockerCli *client.DockerCli) {
+func SetupRootCommand(rootCmd *cobra.Command) {
 	rootCmd.SetUsageTemplate(usageTemplate)
 	rootCmd.SetHelpTemplate(helpTemplate)
-	rootCmd.SetFlagErrorFunc(cli.FlagErrorFunc)
-	rootCmd.SetOutput(dockerCli.Out())
-	rootCmd.AddCommand(
-		node.NewNodeCommand(dockerCli),
-		service.NewServiceCommand(dockerCli),
-		stack.NewStackCommand(dockerCli),
-		stack.NewTopLevelDeployCommand(dockerCli),
-		swarm.NewSwarmCommand(dockerCli),
-		container.NewAttachCommand(dockerCli),
-		container.NewCommitCommand(dockerCli),
-		container.NewCopyCommand(dockerCli),
-		container.NewCreateCommand(dockerCli),
-		container.NewDiffCommand(dockerCli),
-		container.NewExecCommand(dockerCli),
-		container.NewExportCommand(dockerCli),
-		container.NewKillCommand(dockerCli),
-		container.NewLogsCommand(dockerCli),
-		container.NewPauseCommand(dockerCli),
-		container.NewPortCommand(dockerCli),
-		container.NewPsCommand(dockerCli),
-		container.NewRenameCommand(dockerCli),
-		container.NewRestartCommand(dockerCli),
-		container.NewRmCommand(dockerCli),
-		container.NewRunCommand(dockerCli),
-		container.NewStartCommand(dockerCli),
-		container.NewStatsCommand(dockerCli),
-		container.NewStopCommand(dockerCli),
-		container.NewTopCommand(dockerCli),
-		container.NewUnpauseCommand(dockerCli),
-		container.NewUpdateCommand(dockerCli),
-		container.NewWaitCommand(dockerCli),
-		image.NewBuildCommand(dockerCli),
-		image.NewHistoryCommand(dockerCli),
-		image.NewImagesCommand(dockerCli),
-		image.NewLoadCommand(dockerCli),
-		image.NewRemoveCommand(dockerCli),
-		image.NewSaveCommand(dockerCli),
-		image.NewPullCommand(dockerCli),
-		image.NewPushCommand(dockerCli),
-		image.NewSearchCommand(dockerCli),
-		image.NewImportCommand(dockerCli),
-		image.NewTagCommand(dockerCli),
-		network.NewNetworkCommand(dockerCli),
-		system.NewEventsCommand(dockerCli),
-		system.NewInspectCommand(dockerCli),
-		registry.NewLoginCommand(dockerCli),
-		registry.NewLogoutCommand(dockerCli),
-		system.NewVersionCommand(dockerCli),
-		volume.NewVolumeCommand(dockerCli),
-		system.NewInfoCommand(dockerCli),
-	)
-	plugin.NewPluginCommand(rootCmd, dockerCli)
+	rootCmd.SetFlagErrorFunc(FlagErrorFunc)
 
 	rootCmd.PersistentFlags().BoolP("help", "h", false, "Print usage")
 	rootCmd.PersistentFlags().MarkShorthandDeprecated("help", "please use --help")
@@ -87,6 +23,20 @@ func (c CobraAdaptor) GetRootCommand() *cobra.Command {
 	return c.rootCmd
 }
 
+// FlagErrorFunc prints an error messages which matches the format of the
+// docker/docker/cli error messages
+func FlagErrorFunc(cmd *cobra.Command, err error) error {
+	if err == nil {
+		return err
+	}
+
+	usage := ""
+	if cmd.HasSubCommands() {
+		usage = "\n\n" + cmd.UsageString()
+	}
+	return fmt.Errorf("%s\nSee '%s --help'.%s", err, cmd.CommandPath(), usage)
+}
+
 var usageTemplate = `Usage:	{{if not .HasSubCommands}}{{.UseLine}}{{end}}{{if .HasSubCommands}}{{ .CommandPath}} COMMAND{{end}}
 
 {{ .Short | trim }}{{if gt .Aliases 0}}

+ 0 - 21
cli/flagerrors.go

@@ -1,21 +0,0 @@
-package cli
-
-import (
-	"fmt"
-
-	"github.com/spf13/cobra"
-)
-
-// FlagErrorFunc prints an error messages which matches the format of the
-// docker/docker/cli error messages
-func FlagErrorFunc(cmd *cobra.Command, err error) error {
-	if err == nil {
-		return err
-	}
-
-	usage := ""
-	if cmd.HasSubCommands() {
-		usage = "\n\n" + cmd.UsageString()
-	}
-	return fmt.Errorf("%s\nSee '%s --help'.%s", err, cmd.CommandPath(), usage)
-}

+ 3 - 4
cli/flags/common.go

@@ -43,9 +43,7 @@ type CommonOptions struct {
 
 // NewCommonOptions returns a new CommonOptions
 func NewCommonOptions() *CommonOptions {
-	return &CommonOptions{
-		TLSOptions: &tlsconfig.Options{},
-	}
+	return &CommonOptions{}
 }
 
 // InstallFlags adds flags for the common options on the FlagSet
@@ -61,13 +59,14 @@ func (commonOpts *CommonOptions) InstallFlags(flags *pflag.FlagSet) {
 
 	// TODO use flag flags.String("identity"}, "i", "", "Path to libtrust key file")
 
+	commonOpts.TLSOptions = &tlsconfig.Options{}
 	tlsOptions := commonOpts.TLSOptions
 	flags.StringVar(&tlsOptions.CAFile, "tlscacert", filepath.Join(dockerCertPath, DefaultCaFile), "Trust certs signed only by this CA")
 	flags.StringVar(&tlsOptions.CertFile, "tlscert", filepath.Join(dockerCertPath, DefaultCertFile), "Path to TLS certificate file")
 	flags.StringVar(&tlsOptions.KeyFile, "tlskey", filepath.Join(dockerCertPath, DefaultKeyFile), "Path to TLS key file")
 
 	hostOpt := opts.NewNamedListOptsRef("hosts", &commonOpts.Hosts, opts.ValidateHost)
-	flags.VarP(hostOpt, "-host", "H", "Daemon socket(s) to connect to")
+	flags.VarP(hostOpt, "host", "H", "Daemon socket(s) to connect to")
 }
 
 // SetDefaultOptions sets default values for options after flag parsing is

+ 75 - 18
cmd/docker/docker.go

@@ -3,10 +3,20 @@ package main
 import (
 	"fmt"
 	"os"
-	"path/filepath"
 
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/api/client"
+	"github.com/docker/docker/api/client/container"
+	"github.com/docker/docker/api/client/image"
+	"github.com/docker/docker/api/client/network"
+	"github.com/docker/docker/api/client/node"
+	"github.com/docker/docker/api/client/plugin"
+	"github.com/docker/docker/api/client/registry"
+	"github.com/docker/docker/api/client/service"
+	"github.com/docker/docker/api/client/stack"
+	"github.com/docker/docker/api/client/swarm"
+	"github.com/docker/docker/api/client/system"
+	"github.com/docker/docker/api/client/volume"
 	"github.com/docker/docker/cli"
 	"github.com/docker/docker/cli/cobraadaptor"
 	cliflags "github.com/docker/docker/cli/flags"
@@ -18,13 +28,15 @@ import (
 	"github.com/spf13/pflag"
 )
 
-func newDockerCommand(dockerCli *client.DockerCli, opts *cliflags.ClientOptions) *cobra.Command {
+func newDockerCommand(dockerCli *client.DockerCli) *cobra.Command {
+	opts := cliflags.NewClientOptions()
 	cmd := &cobra.Command{
-		Use:           "docker [OPTIONS] COMMAND [arg...]",
-		Short:         "A self-sufficient runtime for containers.",
-		SilenceUsage:  true,
-		SilenceErrors: true,
-		Args:          cli.NoArgs,
+		Use:              "docker [OPTIONS] COMMAND [arg...]",
+		Short:            "A self-sufficient runtime for containers.",
+		SilenceUsage:     true,
+		SilenceErrors:    true,
+		TraverseChildren: true,
+		Args:             cli.NoArgs,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			if opts.Version {
 				showVersion()
@@ -38,13 +50,66 @@ func newDockerCommand(dockerCli *client.DockerCli, opts *cliflags.ClientOptions)
 			return dockerCli.Initialize(opts)
 		},
 	}
-	cobraadaptor.SetupRootCommand(cmd, dockerCli)
+	cobraadaptor.SetupRootCommand(cmd)
 
 	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)
 
+	cmd.SetOutput(dockerCli.Out())
+	cmd.AddCommand(
+		newDaemonCommand(),
+		node.NewNodeCommand(dockerCli),
+		service.NewServiceCommand(dockerCli),
+		stack.NewStackCommand(dockerCli),
+		stack.NewTopLevelDeployCommand(dockerCli),
+		swarm.NewSwarmCommand(dockerCli),
+		container.NewAttachCommand(dockerCli),
+		container.NewCommitCommand(dockerCli),
+		container.NewCopyCommand(dockerCli),
+		container.NewCreateCommand(dockerCli),
+		container.NewDiffCommand(dockerCli),
+		container.NewExecCommand(dockerCli),
+		container.NewExportCommand(dockerCli),
+		container.NewKillCommand(dockerCli),
+		container.NewLogsCommand(dockerCli),
+		container.NewPauseCommand(dockerCli),
+		container.NewPortCommand(dockerCli),
+		container.NewPsCommand(dockerCli),
+		container.NewRenameCommand(dockerCli),
+		container.NewRestartCommand(dockerCli),
+		container.NewRmCommand(dockerCli),
+		container.NewRunCommand(dockerCli),
+		container.NewStartCommand(dockerCli),
+		container.NewStatsCommand(dockerCli),
+		container.NewStopCommand(dockerCli),
+		container.NewTopCommand(dockerCli),
+		container.NewUnpauseCommand(dockerCli),
+		container.NewUpdateCommand(dockerCli),
+		container.NewWaitCommand(dockerCli),
+		image.NewBuildCommand(dockerCli),
+		image.NewHistoryCommand(dockerCli),
+		image.NewImagesCommand(dockerCli),
+		image.NewLoadCommand(dockerCli),
+		image.NewRemoveCommand(dockerCli),
+		image.NewSaveCommand(dockerCli),
+		image.NewPullCommand(dockerCli),
+		image.NewPushCommand(dockerCli),
+		image.NewSearchCommand(dockerCli),
+		image.NewImportCommand(dockerCli),
+		image.NewTagCommand(dockerCli),
+		network.NewNetworkCommand(dockerCli),
+		system.NewEventsCommand(dockerCli),
+		system.NewInspectCommand(dockerCli),
+		registry.NewLoginCommand(dockerCli),
+		registry.NewLogoutCommand(dockerCli),
+		system.NewVersionCommand(dockerCli),
+		volume.NewVolumeCommand(dockerCli),
+		system.NewInfoCommand(dockerCli),
+	)
+	plugin.NewPluginCommand(cmd, dockerCli)
+
 	return cmd
 }
 
@@ -53,9 +118,8 @@ func main() {
 	stdin, stdout, stderr := term.StdStreams()
 	logrus.SetOutput(stderr)
 
-	opts := cliflags.NewClientOptions()
-	dockerCli := client.NewDockerCli(stdin, stdout, stderr, opts)
-	cmd := newDockerCommand(dockerCli, opts)
+	dockerCli := client.NewDockerCli(stdin, stdout, stderr)
+	cmd := newDockerCommand(dockerCli)
 
 	if err := cmd.Execute(); err != nil {
 		if sterr, ok := err.(cli.StatusError); ok {
@@ -86,17 +150,10 @@ func dockerPreRun(flags *pflag.FlagSet, opts *cliflags.ClientOptions) {
 	opts.Common.SetDefaultOptions(flags)
 	cliflags.SetDaemonLogLevel(opts.Common.LogLevel)
 
-	// TODO: remove this, set a default in New, and pass it in opts
 	if opts.ConfigDir != "" {
 		cliconfig.SetConfigDir(opts.ConfigDir)
 	}
 
-	if opts.Common.TrustKey == "" {
-		opts.Common.TrustKey = filepath.Join(
-			cliconfig.ConfigDir(),
-			cliflags.DefaultTrustKeyFile)
-	}
-
 	if opts.Common.Debug {
 		utils.EnableDebug()
 	}

+ 2 - 4
cmd/docker/docker_test.go

@@ -8,16 +8,14 @@ import (
 	"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()
 
-	opts := cliflags.NewClientOptions()
-	cmd := newDockerCommand(&client.DockerCli{}, opts)
+	cmd := newDockerCommand(&client.DockerCli{})
+	cmd.Flags().Set("debug", "true")
 
-	opts.Common.Debug = true
 	if err := cmd.PersistentPreRunE(cmd, []string{}); err != nil {
 		t.Fatalf("Unexpected error: %s", err.Error())
 	}

+ 96 - 241
cmd/dockerd/daemon_test.go

@@ -1,290 +1,145 @@
 package main
 
 import (
-	"io/ioutil"
-	"os"
-	"strings"
 	"testing"
 
 	"github.com/Sirupsen/logrus"
 	cliflags "github.com/docker/docker/cli/flags"
 	"github.com/docker/docker/daemon"
-	"github.com/docker/docker/opts"
-	"github.com/docker/docker/pkg/mflag"
-	"github.com/docker/go-connections/tlsconfig"
+	"github.com/docker/docker/pkg/testutil/assert"
+	"github.com/docker/docker/pkg/testutil/tempfile"
+	"github.com/spf13/pflag"
 )
 
+func defaultOptions(configFile string) daemonOptions {
+	opts := daemonOptions{
+		daemonConfig: &daemon.Config{},
+		flags:        &pflag.FlagSet{},
+		common:       cliflags.NewCommonOptions(),
+	}
+	opts.common.InstallFlags(opts.flags)
+	opts.daemonConfig.InstallFlags(opts.flags)
+	opts.flags.StringVar(&opts.configFile, flagDaemonConfigFile, defaultDaemonConfigFile, "")
+	opts.configFile = configFile
+	return opts
+}
+
 func TestLoadDaemonCliConfigWithoutOverriding(t *testing.T) {
-	c := &daemon.Config{}
-	common := &cliflags.CommonFlags{
-		Debug: true,
-	}
+	opts := defaultOptions("")
+	opts.common.Debug = true
 
-	flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
-	loadedConfig, err := loadDaemonCliConfig(c, flags, common, "/tmp/fooobarbaz")
-	if err != nil {
-		t.Fatal(err)
-	}
-	if loadedConfig == nil {
-		t.Fatalf("expected configuration %v, got nil", c)
-	}
+	loadedConfig, err := loadDaemonCliConfig(opts)
+	assert.NilError(t, err)
+	assert.NotNil(t, loadedConfig)
 	if !loadedConfig.Debug {
 		t.Fatalf("expected debug to be copied from the common flags, got false")
 	}
 }
 
 func TestLoadDaemonCliConfigWithTLS(t *testing.T) {
-	c := &daemon.Config{}
-	common := &cliflags.CommonFlags{
-		TLS: true,
-		TLSOptions: &tlsconfig.Options{
-			CAFile: "/tmp/ca.pem",
-		},
-	}
-
-	flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
-	loadedConfig, err := loadDaemonCliConfig(c, flags, common, "/tmp/fooobarbaz")
-	if err != nil {
-		t.Fatal(err)
-	}
-	if loadedConfig == nil {
-		t.Fatalf("expected configuration %v, got nil", c)
-	}
-	if loadedConfig.CommonTLSOptions.CAFile != "/tmp/ca.pem" {
-		t.Fatalf("expected /tmp/ca.pem, got %s: %q", loadedConfig.CommonTLSOptions.CAFile, loadedConfig)
-	}
+	opts := defaultOptions("")
+	opts.common.TLSOptions.CAFile = "/tmp/ca.pem"
+	opts.common.TLS = true
+
+	loadedConfig, err := loadDaemonCliConfig(opts)
+	assert.NilError(t, err)
+	assert.NotNil(t, loadedConfig)
+	assert.Equal(t, loadedConfig.CommonTLSOptions.CAFile, "/tmp/ca.pem")
 }
 
 func TestLoadDaemonCliConfigWithConflicts(t *testing.T) {
-	c := &daemon.Config{}
-	common := &cliflags.CommonFlags{}
-	f, err := ioutil.TempFile("", "docker-config-")
-	if err != nil {
-		t.Fatal(err)
-	}
-	configFile := f.Name()
-	defer os.Remove(configFile)
-
-	f.Write([]byte(`{"labels": ["l3=foo"]}`))
-	f.Close()
-
-	var labels []string
+	tempFile := tempfile.NewTempFile(t, "config", `{"labels": ["l3=foo"]}`)
+	defer tempFile.Remove()
+	configFile := tempFile.Name()
 
-	flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
-	flags.String([]string{daemonConfigFileFlag}, "", "")
-	flags.Var(opts.NewNamedListOptsRef("labels", &labels, opts.ValidateLabel), []string{"-label"}, "")
+	opts := defaultOptions(configFile)
+	flags := opts.flags
 
-	flags.Set(daemonConfigFileFlag, configFile)
-	if err := flags.Set("-label", "l1=bar"); err != nil {
-		t.Fatal(err)
-	}
-	if err := flags.Set("-label", "l2=baz"); err != nil {
-		t.Fatal(err)
-	}
+	assert.NilError(t, flags.Set(flagDaemonConfigFile, configFile))
+	assert.NilError(t, flags.Set("label", "l1=bar"))
+	assert.NilError(t, flags.Set("label", "l2=baz"))
 
-	_, err = loadDaemonCliConfig(c, flags, common, configFile)
-	if err == nil {
-		t.Fatalf("expected configuration error, got nil")
-	}
-	if !strings.Contains(err.Error(), "labels") {
-		t.Fatalf("expected labels conflict, got %v", err)
-	}
+	_, err := loadDaemonCliConfig(opts)
+	assert.Error(t, err, "as a flag and in the configuration file: labels")
 }
 
 func TestLoadDaemonCliConfigWithTLSVerify(t *testing.T) {
-	c := &daemon.Config{}
-	common := &cliflags.CommonFlags{
-		TLSOptions: &tlsconfig.Options{
-			CAFile: "/tmp/ca.pem",
-		},
-	}
-
-	f, err := ioutil.TempFile("", "docker-config-")
-	if err != nil {
-		t.Fatal(err)
-	}
-	configFile := f.Name()
-	defer os.Remove(configFile)
-
-	f.Write([]byte(`{"tlsverify": true}`))
-	f.Close()
+	tempFile := tempfile.NewTempFile(t, "config", `{"tlsverify": true}`)
+	defer tempFile.Remove()
 
-	flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
-	flags.Bool([]string{"-tlsverify"}, false, "")
-	loadedConfig, err := loadDaemonCliConfig(c, flags, common, configFile)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if loadedConfig == nil {
-		t.Fatalf("expected configuration %v, got nil", c)
-	}
+	opts := defaultOptions(tempFile.Name())
+	opts.common.TLSOptions.CAFile = "/tmp/ca.pem"
 
-	if !loadedConfig.TLS {
-		t.Fatalf("expected TLS enabled, got %q", loadedConfig)
-	}
+	loadedConfig, err := loadDaemonCliConfig(opts)
+	assert.NilError(t, err)
+	assert.NotNil(t, loadedConfig)
+	assert.Equal(t, loadedConfig.TLS, true)
 }
 
 func TestLoadDaemonCliConfigWithExplicitTLSVerifyFalse(t *testing.T) {
-	c := &daemon.Config{}
-	common := &cliflags.CommonFlags{
-		TLSOptions: &tlsconfig.Options{
-			CAFile: "/tmp/ca.pem",
-		},
-	}
+	tempFile := tempfile.NewTempFile(t, "config", `{"tlsverify": false}`)
+	defer tempFile.Remove()
 
-	f, err := ioutil.TempFile("", "docker-config-")
-	if err != nil {
-		t.Fatal(err)
-	}
-	configFile := f.Name()
-	defer os.Remove(configFile)
-
-	f.Write([]byte(`{"tlsverify": false}`))
-	f.Close()
-
-	flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
-	flags.Bool([]string{"-tlsverify"}, false, "")
-	loadedConfig, err := loadDaemonCliConfig(c, flags, common, configFile)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if loadedConfig == nil {
-		t.Fatalf("expected configuration %v, got nil", c)
-	}
+	opts := defaultOptions(tempFile.Name())
+	opts.common.TLSOptions.CAFile = "/tmp/ca.pem"
 
-	if !loadedConfig.TLS {
-		t.Fatalf("expected TLS enabled, got %q", loadedConfig)
-	}
+	loadedConfig, err := loadDaemonCliConfig(opts)
+	assert.NilError(t, err)
+	assert.NotNil(t, loadedConfig)
+	assert.Equal(t, loadedConfig.TLS, true)
 }
 
 func TestLoadDaemonCliConfigWithoutTLSVerify(t *testing.T) {
-	c := &daemon.Config{}
-	common := &cliflags.CommonFlags{
-		TLSOptions: &tlsconfig.Options{
-			CAFile: "/tmp/ca.pem",
-		},
-	}
-
-	f, err := ioutil.TempFile("", "docker-config-")
-	if err != nil {
-		t.Fatal(err)
-	}
-	configFile := f.Name()
-	defer os.Remove(configFile)
+	tempFile := tempfile.NewTempFile(t, "config", `{}`)
+	defer tempFile.Remove()
 
-	f.Write([]byte(`{}`))
-	f.Close()
+	opts := defaultOptions(tempFile.Name())
+	opts.common.TLSOptions.CAFile = "/tmp/ca.pem"
 
-	flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
-	loadedConfig, err := loadDaemonCliConfig(c, flags, common, configFile)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if loadedConfig == nil {
-		t.Fatalf("expected configuration %v, got nil", c)
-	}
-
-	if loadedConfig.TLS {
-		t.Fatalf("expected TLS disabled, got %q", loadedConfig)
-	}
+	loadedConfig, err := loadDaemonCliConfig(opts)
+	assert.NilError(t, err)
+	assert.NotNil(t, loadedConfig)
+	assert.Equal(t, loadedConfig.TLS, false)
 }
 
 func TestLoadDaemonCliConfigWithLogLevel(t *testing.T) {
-	c := &daemon.Config{}
-	common := &cliflags.CommonFlags{}
-
-	f, err := ioutil.TempFile("", "docker-config-")
-	if err != nil {
-		t.Fatal(err)
-	}
-	configFile := f.Name()
-	defer os.Remove(configFile)
-
-	f.Write([]byte(`{"log-level": "warn"}`))
-	f.Close()
-
-	flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
-	flags.String([]string{"-log-level"}, "", "")
-	loadedConfig, err := loadDaemonCliConfig(c, flags, common, configFile)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if loadedConfig == nil {
-		t.Fatalf("expected configuration %v, got nil", c)
-	}
-	if loadedConfig.LogLevel != "warn" {
-		t.Fatalf("expected warn log level, got %v", loadedConfig.LogLevel)
-	}
-
-	if logrus.GetLevel() != logrus.WarnLevel {
-		t.Fatalf("expected warn log level, got %v", logrus.GetLevel())
-	}
+	tempFile := tempfile.NewTempFile(t, "config", `{"log-level": "warn"}`)
+	defer tempFile.Remove()
+
+	opts := defaultOptions(tempFile.Name())
+	loadedConfig, err := loadDaemonCliConfig(opts)
+	assert.NilError(t, err)
+	assert.NotNil(t, loadedConfig)
+	assert.Equal(t, loadedConfig.LogLevel, "warn")
+	assert.Equal(t, logrus.GetLevel(), logrus.WarnLevel)
 }
 
 func TestLoadDaemonConfigWithEmbeddedOptions(t *testing.T) {
-	c := &daemon.Config{}
-	common := &cliflags.CommonFlags{}
-
-	flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
-	flags.String([]string{"-tlscacert"}, "", "")
-	flags.String([]string{"-log-driver"}, "", "")
-
-	f, err := ioutil.TempFile("", "docker-config-")
-	if err != nil {
-		t.Fatal(err)
-	}
-	configFile := f.Name()
-	defer os.Remove(configFile)
-
-	f.Write([]byte(`{"tlscacert": "/etc/certs/ca.pem", "log-driver": "syslog"}`))
-	f.Close()
-
-	loadedConfig, err := loadDaemonCliConfig(c, flags, common, configFile)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if loadedConfig == nil {
-		t.Fatal("expected configuration, got nil")
-	}
-	if loadedConfig.CommonTLSOptions.CAFile != "/etc/certs/ca.pem" {
-		t.Fatalf("expected CA file path /etc/certs/ca.pem, got %v", loadedConfig.CommonTLSOptions.CAFile)
-	}
-	if loadedConfig.LogConfig.Type != "syslog" {
-		t.Fatalf("expected LogConfig type syslog, got %v", loadedConfig.LogConfig.Type)
-	}
+	content := `{"tlscacert": "/etc/certs/ca.pem", "log-driver": "syslog"}`
+	tempFile := tempfile.NewTempFile(t, "config", content)
+	defer tempFile.Remove()
+
+	opts := defaultOptions(tempFile.Name())
+	loadedConfig, err := loadDaemonCliConfig(opts)
+	assert.NilError(t, err)
+	assert.NotNil(t, loadedConfig)
+	assert.Equal(t, loadedConfig.CommonTLSOptions.CAFile, "/etc/certs/ca.pem")
+	assert.Equal(t, loadedConfig.LogConfig.Type, "syslog")
 }
 
 func TestLoadDaemonConfigWithRegistryOptions(t *testing.T) {
-	c := &daemon.Config{}
-	common := &cliflags.CommonFlags{}
-	flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
-	c.ServiceOptions.InstallCliFlags(flags, absentFromHelp)
-
-	f, err := ioutil.TempFile("", "docker-config-")
-	if err != nil {
-		t.Fatal(err)
-	}
-	configFile := f.Name()
-	defer os.Remove(configFile)
-
-	f.Write([]byte(`{"registry-mirrors": ["https://mirrors.docker.com"], "insecure-registries": ["https://insecure.docker.com"]}`))
-	f.Close()
-
-	loadedConfig, err := loadDaemonCliConfig(c, flags, common, configFile)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if loadedConfig == nil {
-		t.Fatal("expected configuration, got nil")
-	}
-
-	m := loadedConfig.Mirrors
-	if len(m) != 1 {
-		t.Fatalf("expected 1 mirror, got %d", len(m))
-	}
-
-	r := loadedConfig.InsecureRegistries
-	if len(r) != 1 {
-		t.Fatalf("expected 1 insecure registries, got %d", len(r))
-	}
+	content := `{
+		"registry-mirrors": ["https://mirrors.docker.com"],
+		"insecure-registries": ["https://insecure.docker.com"],
+	}`
+	tempFile := tempfile.NewTempFile(t, "config", content)
+	defer tempFile.Remove()
+
+	opts := defaultOptions(tempFile.Name())
+	loadedConfig, err := loadDaemonCliConfig(opts)
+	assert.NilError(t, err)
+	assert.NotNil(t, loadedConfig)
+
+	assert.Equal(t, len(loadedConfig.Mirrors), 1)
+	assert.Equal(t, len(loadedConfig.InsecureRegistries), 1)
 }

+ 61 - 172
cmd/dockerd/daemon_unix_test.go

@@ -7,209 +7,98 @@ import (
 	"os"
 	"testing"
 
-	cliflags "github.com/docker/docker/cli/flags"
 	"github.com/docker/docker/daemon"
-	"github.com/docker/docker/opts"
-	"github.com/docker/docker/pkg/mflag"
+	"github.com/docker/docker/pkg/testutil/assert"
+	"github.com/docker/docker/pkg/testutil/tempfile"
 )
 
 func TestLoadDaemonCliConfigWithDaemonFlags(t *testing.T) {
-	c := &daemon.Config{}
-	common := &cliflags.CommonFlags{
-		Debug:    true,
-		LogLevel: "info",
-	}
-
-	f, err := ioutil.TempFile("", "docker-config-")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	configFile := f.Name()
-	f.Write([]byte(`{"log-opts": {"max-size": "1k"}}`))
-	f.Close()
-
-	flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
-	flags.String([]string{daemonConfigFileFlag}, "", "")
-	flags.BoolVar(&c.EnableSelinuxSupport, []string{"-selinux-enabled"}, true, "")
-	flags.StringVar(&c.LogConfig.Type, []string{"-log-driver"}, "json-file", "")
-	flags.Var(opts.NewNamedMapOpts("log-opts", c.LogConfig.Config, nil), []string{"-log-opt"}, "")
-	flags.Set(daemonConfigFileFlag, configFile)
-
-	loadedConfig, err := loadDaemonCliConfig(c, flags, common, configFile)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if loadedConfig == nil {
-		t.Fatalf("expected configuration %v, got nil", c)
-	}
-	if !loadedConfig.Debug {
-		t.Fatalf("expected debug mode, got false")
-	}
-	if loadedConfig.LogLevel != "info" {
-		t.Fatalf("expected info log level, got %v", loadedConfig.LogLevel)
-	}
-	if !loadedConfig.EnableSelinuxSupport {
-		t.Fatalf("expected enabled selinux support, got disabled")
-	}
-	if loadedConfig.LogConfig.Type != "json-file" {
-		t.Fatalf("expected LogConfig type json-file, got %v", loadedConfig.LogConfig.Type)
-	}
-	if maxSize := loadedConfig.LogConfig.Config["max-size"]; maxSize != "1k" {
-		t.Fatalf("expected log max-size `1k`, got %s", maxSize)
-	}
+	content := `{"log-opts": {"max-size": "1k"}}`
+	tempFile := tempfile.NewTempFile(t, "config", content)
+	defer tempFile.Remove()
+
+	opts := defaultOptions(tempFile.Name())
+	opts.common.Debug = true
+	opts.common.LogLevel = "info"
+	assert.NilError(t, opts.flags.Set("selinux-enabled", "true"))
+
+	loadedConfig, err := loadDaemonCliConfig(opts)
+	assert.NilError(t, err)
+	assert.NotNil(t, loadedConfig)
+
+	assert.Equal(t, loadedConfig.Debug, true)
+	assert.Equal(t, loadedConfig.LogLevel, "info")
+	assert.Equal(t, loadedConfig.EnableSelinuxSupport, true)
+	assert.Equal(t, loadedConfig.LogConfig.Type, "json-file")
+	assert.Equal(t, loadedConfig.LogConfig.Config["max-size"], "1k")
 }
 
 func TestLoadDaemonConfigWithNetwork(t *testing.T) {
-	c := &daemon.Config{}
-	common := &cliflags.CommonFlags{}
-	flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
-	flags.String([]string{"-bip"}, "", "")
-	flags.String([]string{"-ip"}, "", "")
-
-	f, err := ioutil.TempFile("", "docker-config-")
-	if err != nil {
-		t.Fatal(err)
-	}
+	content := `{"bip": "127.0.0.2", "ip": "127.0.0.1"}`
+	tempFile := tempfile.NewTempFile(t, "config", content)
+	defer tempFile.Remove()
 
-	configFile := f.Name()
-	f.Write([]byte(`{"bip": "127.0.0.2", "ip": "127.0.0.1"}`))
-	f.Close()
+	opts := defaultOptions(tempFile.Name())
+	loadedConfig, err := loadDaemonCliConfig(opts)
+	assert.NilError(t, err)
+	assert.NotNil(t, loadedConfig)
 
-	loadedConfig, err := loadDaemonCliConfig(c, flags, common, configFile)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if loadedConfig == nil {
-		t.Fatalf("expected configuration %v, got nil", c)
-	}
-	if loadedConfig.IP != "127.0.0.2" {
-		t.Fatalf("expected IP 127.0.0.2, got %v", loadedConfig.IP)
-	}
-	if loadedConfig.DefaultIP.String() != "127.0.0.1" {
-		t.Fatalf("expected DefaultIP 127.0.0.1, got %s", loadedConfig.DefaultIP)
-	}
+	assert.Equal(t, loadedConfig.IP, "127.0.0.2")
+	assert.Equal(t, loadedConfig.DefaultIP.String(), "127.0.0.1")
 }
 
 func TestLoadDaemonConfigWithMapOptions(t *testing.T) {
-	c := &daemon.Config{}
-	common := &cliflags.CommonFlags{}
-	flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
-
-	flags.Var(opts.NewNamedMapOpts("cluster-store-opts", c.ClusterOpts, nil), []string{"-cluster-store-opt"}, "")
-	flags.Var(opts.NewNamedMapOpts("log-opts", c.LogConfig.Config, nil), []string{"-log-opt"}, "")
-
-	f, err := ioutil.TempFile("", "docker-config-")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	configFile := f.Name()
-	f.Write([]byte(`{
+	content := `{
 		"cluster-store-opts": {"kv.cacertfile": "/var/lib/docker/discovery_certs/ca.pem"},
 		"log-opts": {"tag": "test"}
-}`))
-	f.Close()
+}`
+	tempFile := tempfile.NewTempFile(t, "config", content)
+	defer tempFile.Remove()
 
-	loadedConfig, err := loadDaemonCliConfig(c, flags, common, configFile)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if loadedConfig == nil {
-		t.Fatal("expected configuration, got nil")
-	}
-	if loadedConfig.ClusterOpts == nil {
-		t.Fatal("expected cluster options, got nil")
-	}
+	opts := defaultOptions(tempFile.Name())
+	loadedConfig, err := loadDaemonCliConfig(opts)
+	assert.NilError(t, err)
+	assert.NotNil(t, loadedConfig)
+	assert.NotNil(t, loadedConfig.ClusterOpts)
 
 	expectedPath := "/var/lib/docker/discovery_certs/ca.pem"
-	if caPath := loadedConfig.ClusterOpts["kv.cacertfile"]; caPath != expectedPath {
-		t.Fatalf("expected %s, got %s", expectedPath, caPath)
-	}
-
-	if loadedConfig.LogConfig.Config == nil {
-		t.Fatal("expected log config options, got nil")
-	}
-	if tag := loadedConfig.LogConfig.Config["tag"]; tag != "test" {
-		t.Fatalf("expected log tag `test`, got %s", tag)
-	}
+	assert.Equal(t, loadedConfig.ClusterOpts["kv.cacertfile"], expectedPath)
+	assert.NotNil(t, loadedConfig.LogConfig.Config)
+	assert.Equal(t, loadedConfig.LogConfig.Config["tag"], "test")
 }
 
 func TestLoadDaemonConfigWithTrueDefaultValues(t *testing.T) {
-	c := &daemon.Config{}
-	common := &cliflags.CommonFlags{}
-	flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
-	flags.BoolVar(&c.EnableUserlandProxy, []string{"-userland-proxy"}, true, "")
-
-	f, err := ioutil.TempFile("", "docker-config-")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if err := flags.ParseFlags([]string{}, false); err != nil {
-		t.Fatal(err)
-	}
+	content := `{ "userland-proxy": false }`
+	tempFile := tempfile.NewTempFile(t, "config", content)
+	defer tempFile.Remove()
 
-	configFile := f.Name()
-	f.Write([]byte(`{
-		"userland-proxy": false
-}`))
-	f.Close()
-
-	loadedConfig, err := loadDaemonCliConfig(c, flags, common, configFile)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if loadedConfig == nil {
-		t.Fatal("expected configuration, got nil")
-	}
+	opts := defaultOptions(tempFile.Name())
+	loadedConfig, err := loadDaemonCliConfig(opts)
+	assert.NilError(t, err)
+	assert.NotNil(t, loadedConfig)
+	assert.NotNil(t, loadedConfig.ClusterOpts)
 
-	if loadedConfig.EnableUserlandProxy {
-		t.Fatal("expected userland proxy to be disabled, got enabled")
-	}
+	assert.Equal(t, loadedConfig.EnableUserlandProxy, false)
 
 	// make sure reloading doesn't generate configuration
 	// conflicts after normalizing boolean values.
-	err = daemon.ReloadConfiguration(configFile, flags, func(reloadedConfig *daemon.Config) {
-		if reloadedConfig.EnableUserlandProxy {
-			t.Fatal("expected userland proxy to be disabled, got enabled")
-		}
-	})
-	if err != nil {
-		t.Fatal(err)
+	reload := func(reloadedConfig *daemon.Config) {
+		assert.Equal(t, reloadedConfig.EnableUserlandProxy, false)
 	}
+	assert.NilError(t, daemon.ReloadConfiguration(opts.configFile, opts.flags, reload))
 }
 
 func TestLoadDaemonConfigWithTrueDefaultValuesLeaveDefaults(t *testing.T) {
-	c := &daemon.Config{}
-	common := &cliflags.CommonFlags{}
-	flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
-	flags.BoolVar(&c.EnableUserlandProxy, []string{"-userland-proxy"}, true, "")
-
-	f, err := ioutil.TempFile("", "docker-config-")
-	if err != nil {
-		t.Fatal(err)
-	}
+	tempFile := tempfile.NewTempFile(t, "config", `{}`)
+	defer tempFile.Remove()
 
-	if err := flags.ParseFlags([]string{}, false); err != nil {
-		t.Fatal(err)
-	}
-
-	configFile := f.Name()
-	f.Write([]byte(`{}`))
-	f.Close()
-
-	loadedConfig, err := loadDaemonCliConfig(c, flags, common, configFile)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if loadedConfig == nil {
-		t.Fatal("expected configuration, got nil")
-	}
+	opts := defaultOptions(tempFile.Name())
+	loadedConfig, err := loadDaemonCliConfig(opts)
+	assert.NilError(t, err)
+	assert.NotNil(t, loadedConfig)
+	assert.NotNil(t, loadedConfig.ClusterOpts)
 
-	if !loadedConfig.EnableUserlandProxy {
-		t.Fatal("expected userland proxy to be enabled, got disabled")
-	}
+	assert.Equal(t, loadedConfig.EnableUserlandProxy, true)
 }
 
 func TestLoadDaemonConfigWithLegacyRegistryOptions(t *testing.T) {

+ 3 - 3
cmd/dockerd/docker.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/cli"
+	"github.com/docker/docker/cli/cobraadaptor"
 	cliflags "github.com/docker/docker/cli/flags"
 	"github.com/docker/docker/daemon"
 	"github.com/docker/docker/dockerversion"
@@ -40,15 +41,14 @@ func newDaemonCommand() *cobra.Command {
 			return runDaemon(opts)
 		},
 	}
-	// TODO: SetUsageTemplate, SetHelpTemplate, SetFlagErrorFunc
+	cobraadaptor.SetupRootCommand(cmd)
 
 	flags := cmd.Flags()
-	flags.BoolP("help", "h", false, "Print usage")
-	flags.MarkShorthandDeprecated("help", "please use --help")
 	flags.BoolVarP(&opts.version, "version", "v", false, "Print version information and quit")
 	flags.StringVar(&opts.configFile, flagDaemonConfigFile, defaultDaemonConfigFile, "Daemon configuration file")
 	opts.common.InstallFlags(flags)
 	opts.daemonConfig.InstallFlags(flags)
+	installServiceFlags(flags)
 
 	return cmd
 }

+ 7 - 0
cmd/dockerd/service_unsupported.go

@@ -2,6 +2,13 @@
 
 package main
 
+import (
+	"github.com/spf13/pflag"
+)
+
 func initService() (bool, error) {
 	return false, nil
 }
+
+func installServiceFlags(flags *pflag.FlagSet) {
+}

+ 13 - 5
cmd/dockerd/service_windows.go

@@ -3,6 +3,7 @@ package main
 import (
 	"bytes"
 	"errors"
+	"flag"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -11,7 +12,7 @@ import (
 	"syscall"
 
 	"github.com/Sirupsen/logrus"
-	flag "github.com/docker/docker/pkg/mflag"
+	"github.com/spf13/pflag"
 	"golang.org/x/sys/windows"
 	"golang.org/x/sys/windows/svc"
 	"golang.org/x/sys/windows/svc/debug"
@@ -20,10 +21,10 @@ import (
 )
 
 var (
-	flServiceName       = flag.String([]string{"-service-name"}, "docker", "Set the Windows service name")
-	flRegisterService   = flag.Bool([]string{"-register-service"}, false, "Register the service and exit")
-	flUnregisterService = flag.Bool([]string{"-unregister-service"}, false, "Unregister the service and exit")
-	flRunService        = flag.Bool([]string{"-run-service"}, false, "")
+	flServiceName       *string
+	flRegisterService   *bool
+	flUnregisterService *bool
+	flRunService        *bool
 
 	setStdHandle = syscall.NewLazyDLL("kernel32.dll").NewProc("SetStdHandle")
 	oldStderr    syscall.Handle
@@ -44,6 +45,13 @@ const (
 	eventExtraOffset = 10 // Add this to any event to get a string that supports extended data
 )
 
+func installServiceFlags(flags *pflag.FlagSet) {
+	flServiceName = flags.String("service-name", "docker", "Set the Windows service name")
+	flRegisterService = flags.Bool("register-service", false, "Register the service and exit")
+	flUnregisterService = flags.Bool("unregister-service", false, "Unregister the service and exit")
+	flRunService = flags.Bool("run-service", false, "")
+}
+
 type handler struct {
 	tosvc   chan bool
 	fromsvc chan error

+ 3 - 6
daemon/config.go

@@ -158,7 +158,7 @@ func (config *Config) InstallCommonFlags(flags *pflag.FlagSet) {
 	flags.StringVarP(&config.Pidfile, "pidfile", "p", defaultPidFile, "Path to use for daemon PID file")
 	flags.StringVarP(&config.Root, "graph", "g", defaultGraph, "Root of the Docker runtime")
 	flags.BoolVarP(&config.AutoRestart, "restart", "r", true, "--restart on the daemon has been deprecated in favor of --restart policies on docker run")
-	flags.MarkDeprecated("restart", "Please use a restart policy on ducker run")
+	flags.MarkDeprecated("restart", "Please use a restart policy on docker run")
 	flags.StringVarP(&config.GraphDriver, "storage-driver", "s", "", "Storage driver to use")
 	flags.IntVar(&config.Mtu, "mtu", 0, "Set the containers network MTU")
 	flags.BoolVar(&config.RawLogs, "raw-logs", false, "Full timestamps without ANSI coloring")
@@ -300,7 +300,7 @@ func getConflictFreeConfiguration(configFile string, flags *pflag.FlagSet) (*Con
 		// TODO: Rewrite configuration logic to avoid same issue with other nullable values, like numbers.
 		namedOptions := make(map[string]interface{})
 		for key, value := range configSet {
-			f := flags.Lookup("-" + key)
+			f := flags.Lookup(key)
 			if f == nil { // ignore named flags that don't match
 				namedOptions[key] = value
 				continue
@@ -354,8 +354,7 @@ func findConfigurationConflicts(config map[string]interface{}, flags *pflag.Flag
 	// 1. Search keys from the file that we don't recognize as flags.
 	unknownKeys := make(map[string]interface{})
 	for key, value := range config {
-		flagName := "-" + key
-		if flag := flags.Lookup(flagName); flag == nil {
+		if flag := flags.Lookup(key); flag == nil {
 			unknownKeys[key] = value
 		}
 	}
@@ -396,8 +395,6 @@ func findConfigurationConflicts(config map[string]interface{}, flags *pflag.Flag
 		} else {
 			// search flag name in the json configuration payload
 			for _, name := range []string{f.Name, f.Shorthand} {
-				name = strings.TrimLeft(name, "-")
-
 				if value, ok := config[name]; ok {
 					conflicts = append(conflicts, printConflict(name, f.Value.String(), value))
 					break

+ 22 - 40
daemon/config_test.go

@@ -7,7 +7,8 @@ import (
 	"testing"
 
 	"github.com/docker/docker/opts"
-	"github.com/docker/docker/pkg/mflag"
+	"github.com/docker/docker/pkg/testutil/assert"
+	"github.com/spf13/pflag"
 )
 
 func TestDaemonConfigurationMerge(t *testing.T) {
@@ -87,42 +88,26 @@ func TestParseClusterAdvertiseSettings(t *testing.T) {
 
 func TestFindConfigurationConflicts(t *testing.T) {
 	config := map[string]interface{}{"authorization-plugins": "foobar"}
-	flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
+	flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
 
-	flags.String([]string{"-authorization-plugins"}, "", "")
-	if err := flags.Set("-authorization-plugins", "asdf"); err != nil {
-		t.Fatal(err)
-	}
+	flags.String("authorization-plugins", "", "")
+	assert.NilError(t, flags.Set("authorization-plugins", "asdf"))
 
-	err := findConfigurationConflicts(config, flags)
-	if err == nil {
-		t.Fatal("expected error, got nil")
-	}
-	if !strings.Contains(err.Error(), "authorization-plugins: (from flag: asdf, from file: foobar)") {
-		t.Fatalf("expected authorization-plugins conflict, got %v", err)
-	}
+	assert.Error(t,
+		findConfigurationConflicts(config, flags),
+		"authorization-plugins: (from flag: asdf, from file: foobar)")
 }
 
 func TestFindConfigurationConflictsWithNamedOptions(t *testing.T) {
 	config := map[string]interface{}{"hosts": []string{"qwer"}}
-	flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
+	flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
 
 	var hosts []string
-	flags.Var(opts.NewNamedListOptsRef("hosts", &hosts, opts.ValidateHost), []string{"H", "-host"}, "Daemon socket(s) to connect to")
-	if err := flags.Set("-host", "tcp://127.0.0.1:4444"); err != nil {
-		t.Fatal(err)
-	}
-	if err := flags.Set("H", "unix:///var/run/docker.sock"); err != nil {
-		t.Fatal(err)
-	}
+	flags.VarP(opts.NewNamedListOptsRef("hosts", &hosts, opts.ValidateHost), "host", "H", "Daemon socket(s) to connect to")
+	assert.NilError(t, flags.Set("host", "tcp://127.0.0.1:4444"))
+	assert.NilError(t, flags.Set("host", "unix:///var/run/docker.sock"))
 
-	err := findConfigurationConflicts(config, flags)
-	if err == nil {
-		t.Fatal("expected error, got nil")
-	}
-	if !strings.Contains(err.Error(), "hosts") {
-		t.Fatalf("expected hosts conflict, got %v", err)
-	}
+	assert.Error(t, findConfigurationConflicts(config, flags), "hosts")
 }
 
 func TestDaemonConfigurationMergeConflicts(t *testing.T) {
@@ -135,8 +120,8 @@ func TestDaemonConfigurationMergeConflicts(t *testing.T) {
 	f.Write([]byte(`{"debug": true}`))
 	f.Close()
 
-	flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
-	flags.Bool([]string{"debug"}, false, "")
+	flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
+	flags.Bool("debug", false, "")
 	flags.Set("debug", "false")
 
 	_, err = MergeDaemonConfigurations(&Config{}, flags, configFile)
@@ -158,8 +143,8 @@ func TestDaemonConfigurationMergeConflictsWithInnerStructs(t *testing.T) {
 	f.Write([]byte(`{"tlscacert": "/etc/certificates/ca.pem"}`))
 	f.Close()
 
-	flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
-	flags.String([]string{"tlscacert"}, "", "")
+	flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
+	flags.String("tlscacert", "", "")
 	flags.Set("tlscacert", "~/.docker/ca.pem")
 
 	_, err = MergeDaemonConfigurations(&Config{}, flags, configFile)
@@ -173,9 +158,9 @@ func TestDaemonConfigurationMergeConflictsWithInnerStructs(t *testing.T) {
 
 func TestFindConfigurationConflictsWithUnknownKeys(t *testing.T) {
 	config := map[string]interface{}{"tls-verify": "true"}
-	flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
+	flags := pflag.NewFlagSet("test", pflag.ContinueOnError)
 
-	flags.Bool([]string{"-tlsverify"}, false, "")
+	flags.Bool("tlsverify", false, "")
 	err := findConfigurationConflicts(config, flags)
 	if err == nil {
 		t.Fatal("expected error, got nil")
@@ -188,18 +173,15 @@ func TestFindConfigurationConflictsWithUnknownKeys(t *testing.T) {
 func TestFindConfigurationConflictsWithMergedValues(t *testing.T) {
 	var hosts []string
 	config := map[string]interface{}{"hosts": "tcp://127.0.0.1:2345"}
-	base := mflag.NewFlagSet("base", mflag.ContinueOnError)
-	base.Var(opts.NewNamedListOptsRef("hosts", &hosts, nil), []string{"H", "-host"}, "")
-
-	flags := mflag.NewFlagSet("test", mflag.ContinueOnError)
-	mflag.Merge(flags, base)
+	flags := pflag.NewFlagSet("base", pflag.ContinueOnError)
+	flags.VarP(opts.NewNamedListOptsRef("hosts", &hosts, nil), "host", "H", "")
 
 	err := findConfigurationConflicts(config, flags)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	flags.Set("-host", "unix:///var/run/docker.sock")
+	flags.Set("host", "unix:///var/run/docker.sock")
 	err = findConfigurationConflicts(config, flags)
 	if err == nil {
 		t.Fatal("expected error, got nil")

+ 7 - 10
daemon/config_windows.go

@@ -3,8 +3,8 @@ package daemon
 import (
 	"os"
 
-	flag "github.com/docker/docker/pkg/mflag"
 	"github.com/docker/engine-api/types"
+	"github.com/spf13/pflag"
 )
 
 var (
@@ -28,18 +28,15 @@ type Config struct {
 	// for the Windows daemon.)
 }
 
-// InstallFlags adds command-line options to the top-level flag parser for
-// the current process.
-// Subsequent calls to `flag.Parse` will populate config with values parsed
-// from the command-line.
-func (config *Config) InstallFlags(cmd *flag.FlagSet, usageFn func(string) string) {
+// InstallFlags adds flags to the pflag.FlagSet to configure the daemon
+func (config *Config) InstallFlags(flags *pflag.FlagSet) {
 	// First handle install flags which are consistent cross-platform
-	config.InstallCommonFlags(cmd, usageFn)
+	config.InstallCommonFlags(flags)
 
 	// Then platform-specific install flags.
-	cmd.StringVar(&config.bridgeConfig.FixedCIDR, []string{"-fixed-cidr"}, "", usageFn("IPv4 subnet for fixed IPs"))
-	cmd.StringVar(&config.bridgeConfig.Iface, []string{"b", "-bridge"}, "", "Attach containers to a virtual switch")
-	cmd.StringVar(&config.SocketGroup, []string{"G", "-group"}, "", usageFn("Users or groups that can access the named pipe"))
+	flags.StringVar(&config.bridgeConfig.FixedCIDR, "fixed-cidr", "", "IPv4 subnet for fixed IPs")
+	flags.StringVarP(&config.bridgeConfig.Iface, "bridge", "b", "", "Attach containers to a virtual switch")
+	flags.StringVarP(&config.SocketGroup, "group", "G", "", "Users or groups that can access the named pipe")
 }
 
 // GetRuntime returns the runtime path and arguments for a given