From 9d7fdddb7318dd3e1886a6cb16d660a5bc2de7d6 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 22 Jun 2016 18:36:51 -0400 Subject: [PATCH] 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 Upstream-commit: 31bf9ca0c8cf29c1ba6cdc044e81c574161a0392 Component: engine --- components/engine/api/client/cli.go | 18 +- .../engine/api/client/container/attach.go | 1 - .../engine/api/client/container/commit.go | 1 - .../engine/api/client/container/create.go | 1 - .../engine/api/client/container/diff.go | 5 +- .../engine/api/client/container/logs.go | 1 - .../engine/api/client/container/pause.go | 5 +- .../engine/api/client/container/port.go | 2 - .../engine/api/client/container/rename.go | 2 - components/engine/api/client/container/run.go | 4 +- .../engine/api/client/container/start.go | 1 - .../engine/api/client/container/stop.go | 1 - components/engine/api/client/container/top.go | 1 - .../engine/api/client/container/unpause.go | 2 - .../engine/api/client/container/wait.go | 2 - components/engine/cli/cobraadaptor/adaptor.go | 86 +---- components/engine/cli/flagerrors.go | 21 -- components/engine/cli/flags/common.go | 7 +- components/engine/cmd/docker/docker.go | 93 ++++- components/engine/cmd/docker/docker_test.go | 6 +- components/engine/cmd/dockerd/daemon_test.go | 327 +++++------------- .../engine/cmd/dockerd/daemon_unix_test.go | 227 ++++-------- components/engine/cmd/dockerd/docker.go | 6 +- .../engine/cmd/dockerd/service_unsupported.go | 7 + .../engine/cmd/dockerd/service_windows.go | 18 +- components/engine/daemon/config.go | 9 +- components/engine/daemon/config_test.go | 62 ++-- components/engine/daemon/config_windows.go | 17 +- 28 files changed, 316 insertions(+), 617 deletions(-) delete mode 100644 components/engine/cli/flagerrors.go diff --git a/components/engine/api/client/cli.go b/components/engine/api/client/cli.go index 005c1942bc..f7a1ba0745 100644 --- a/components/engine/api/client/cli.go +++ b/components/engine/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 diff --git a/components/engine/api/client/container/attach.go b/components/engine/api/client/container/attach.go index 0d466d98ef..a08065cc5c 100644 --- a/components/engine/api/client/container/attach.go +++ b/components/engine/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") diff --git a/components/engine/api/client/container/commit.go b/components/engine/api/client/container/commit.go index eb23b168d7..c6e5c1b582 100644 --- a/components/engine/api/client/container/commit.go +++ b/components/engine/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) diff --git a/components/engine/api/client/container/create.go b/components/engine/api/client/container/create.go index 680d5ad7df..ef9b8cd7f6 100644 --- a/components/engine/api/client/container/create.go +++ b/components/engine/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) diff --git a/components/engine/api/client/container/diff.go b/components/engine/api/client/container/diff.go index 5fdaeadd38..88b2e4e1cd 100644 --- a/components/engine/api/client/container/diff.go +++ b/components/engine/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 { diff --git a/components/engine/api/client/container/logs.go b/components/engine/api/client/container/logs.go index 62119e3cbe..593ec60d29 100644 --- a/components/engine/api/client/container/logs.go +++ b/components/engine/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") diff --git a/components/engine/api/client/container/pause.go b/components/engine/api/client/container/pause.go index 41fcd98edd..46315ee2c1 100644 --- a/components/engine/api/client/container/pause.go +++ b/components/engine/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 { diff --git a/components/engine/api/client/container/port.go b/components/engine/api/client/container/port.go index 4164b30cbf..fe3317a697 100644 --- a/components/engine/api/client/container/port.go +++ b/components/engine/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 } diff --git a/components/engine/api/client/container/rename.go b/components/engine/api/client/container/rename.go index af10102688..04685167b3 100644 --- a/components/engine/api/client/container/rename.go +++ b/components/engine/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 } diff --git a/components/engine/api/client/container/run.go b/components/engine/api/client/container/run.go index 70755b433d..ea72063e2c 100644 --- a/components/engine/api/client/container/run.go +++ b/components/engine/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, } } diff --git a/components/engine/api/client/container/start.go b/components/engine/api/client/container/start.go index 1520b04621..e044985e60 100644 --- a/components/engine/api/client/container/start.go +++ b/components/engine/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") diff --git a/components/engine/api/client/container/stop.go b/components/engine/api/client/container/stop.go index 3e2f27c298..5e8a23f09a 100644 --- a/components/engine/api/client/container/stop.go +++ b/components/engine/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") diff --git a/components/engine/api/client/container/top.go b/components/engine/api/client/container/top.go index e31a903adc..3ade5fbd6d 100644 --- a/components/engine/api/client/container/top.go +++ b/components/engine/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) diff --git a/components/engine/api/client/container/unpause.go b/components/engine/api/client/container/unpause.go index f8c9c0a673..8ce7c3f711 100644 --- a/components/engine/api/client/container/unpause.go +++ b/components/engine/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 } diff --git a/components/engine/api/client/container/wait.go b/components/engine/api/client/container/wait.go index 4b6be886e8..6389a2c907 100644 --- a/components/engine/api/client/container/wait.go +++ b/components/engine/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 } diff --git a/components/engine/cli/cobraadaptor/adaptor.go b/components/engine/cli/cobraadaptor/adaptor.go index d7747351c6..67263e1577 100644 --- a/components/engine/cli/cobraadaptor/adaptor.go +++ b/components/engine/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}} diff --git a/components/engine/cli/flagerrors.go b/components/engine/cli/flagerrors.go deleted file mode 100644 index aab8a98845..0000000000 --- a/components/engine/cli/flagerrors.go +++ /dev/null @@ -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) -} diff --git a/components/engine/cli/flags/common.go b/components/engine/cli/flags/common.go index a3579bff6d..2318b9d975 100644 --- a/components/engine/cli/flags/common.go +++ b/components/engine/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 diff --git a/components/engine/cmd/docker/docker.go b/components/engine/cmd/docker/docker.go index bd04a3a1bd..0ae3906abb 100644 --- a/components/engine/cmd/docker/docker.go +++ b/components/engine/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() } diff --git a/components/engine/cmd/docker/docker_test.go b/components/engine/cmd/docker/docker_test.go index a1e84f1396..72d2311521 100644 --- a/components/engine/cmd/docker/docker_test.go +++ b/components/engine/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()) } diff --git a/components/engine/cmd/dockerd/daemon_test.go b/components/engine/cmd/dockerd/daemon_test.go index f404bf7fc5..01b453b501 100644 --- a/components/engine/cmd/dockerd/daemon_test.go +++ b/components/engine/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 TestLoadDaemonCliConfigWithoutOverriding(t *testing.T) { - c := &daemon.Config{} - common := &cliflags.CommonFlags{ - Debug: true, +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 +} - 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) - } +func TestLoadDaemonCliConfigWithoutOverriding(t *testing.T) { + opts := defaultOptions("") + opts.common.Debug = true + + 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", - }, - } + opts := defaultOptions("") + opts.common.TLSOptions.CAFile = "/tmp/ca.pem" + opts.common.TLS = 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) - } - if loadedConfig.CommonTLSOptions.CAFile != "/tmp/ca.pem" { - t.Fatalf("expected /tmp/ca.pem, got %s: %q", loadedConfig.CommonTLSOptions.CAFile, loadedConfig) - } + 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) + tempFile := tempfile.NewTempFile(t, "config", `{"labels": ["l3=foo"]}`) + defer tempFile.Remove() + configFile := tempFile.Name() - f.Write([]byte(`{"labels": ["l3=foo"]}`)) - f.Close() + opts := defaultOptions(configFile) + flags := opts.flags - var labels []string + assert.NilError(t, flags.Set(flagDaemonConfigFile, configFile)) + assert.NilError(t, flags.Set("label", "l1=bar")) + assert.NilError(t, flags.Set("label", "l2=baz")) - flags := mflag.NewFlagSet("test", mflag.ContinueOnError) - flags.String([]string{daemonConfigFileFlag}, "", "") - flags.Var(opts.NewNamedListOptsRef("labels", &labels, opts.ValidateLabel), []string{"-label"}, "") - - 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) - } - - _, 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", - }, - } + tempFile := tempfile.NewTempFile(t, "config", `{"tlsverify": true}`) + defer tempFile.Remove() - f, err := ioutil.TempFile("", "docker-config-") - if err != nil { - t.Fatal(err) - } - configFile := f.Name() - defer os.Remove(configFile) + opts := defaultOptions(tempFile.Name()) + opts.common.TLSOptions.CAFile = "/tmp/ca.pem" - f.Write([]byte(`{"tlsverify": true}`)) - 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) - } - - 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) + opts := defaultOptions(tempFile.Name()) + opts.common.TLSOptions.CAFile = "/tmp/ca.pem" - 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) - } - - 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", - }, - } + tempFile := tempfile.NewTempFile(t, "config", `{}`) + defer tempFile.Remove() - f, err := ioutil.TempFile("", "docker-config-") - if err != nil { - t.Fatal(err) - } - configFile := f.Name() - defer os.Remove(configFile) + opts := defaultOptions(tempFile.Name()) + opts.common.TLSOptions.CAFile = "/tmp/ca.pem" - f.Write([]byte(`{}`)) - f.Close() - - 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{} + tempFile := tempfile.NewTempFile(t, "config", `{"log-level": "warn"}`) + 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(`{"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()) - } + 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{} + content := `{"tlscacert": "/etc/certs/ca.pem", "log-driver": "syslog"}` + tempFile := tempfile.NewTempFile(t, "config", content) + defer tempFile.Remove() - 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) - } + 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) + content := `{ + "registry-mirrors": ["https://mirrors.docker.com"], + "insecure-registries": ["https://insecure.docker.com"], + }` + tempFile := tempfile.NewTempFile(t, "config", content) + defer tempFile.Remove() - f, err := ioutil.TempFile("", "docker-config-") - if err != nil { - t.Fatal(err) - } - configFile := f.Name() - defer os.Remove(configFile) + opts := defaultOptions(tempFile.Name()) + loadedConfig, err := loadDaemonCliConfig(opts) + assert.NilError(t, err) + assert.NotNil(t, loadedConfig) - 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)) - } + assert.Equal(t, len(loadedConfig.Mirrors), 1) + assert.Equal(t, len(loadedConfig.InsecureRegistries), 1) } diff --git a/components/engine/cmd/dockerd/daemon_unix_test.go b/components/engine/cmd/dockerd/daemon_unix_test.go index 2d8bebda08..f2ef011cbd 100644 --- a/components/engine/cmd/dockerd/daemon_unix_test.go +++ b/components/engine/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", - } + content := `{"log-opts": {"max-size": "1k"}}` + tempFile := tempfile.NewTempFile(t, "config", content) + defer tempFile.Remove() - f, err := ioutil.TempFile("", "docker-config-") - if err != nil { - t.Fatal(err) - } + opts := defaultOptions(tempFile.Name()) + opts.common.Debug = true + opts.common.LogLevel = "info" + assert.NilError(t, opts.flags.Set("selinux-enabled", "true")) - configFile := f.Name() - f.Write([]byte(`{"log-opts": {"max-size": "1k"}}`)) - f.Close() + loadedConfig, err := loadDaemonCliConfig(opts) + assert.NilError(t, err) + assert.NotNil(t, loadedConfig) - 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) - } + 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"}, "", "") + content := `{"bip": "127.0.0.2", "ip": "127.0.0.1"}` + tempFile := tempfile.NewTempFile(t, "config", content) + defer tempFile.Remove() - f, err := ioutil.TempFile("", "docker-config-") - if err != nil { - t.Fatal(err) - } + opts := defaultOptions(tempFile.Name()) + loadedConfig, err := loadDaemonCliConfig(opts) + assert.NilError(t, err) + assert.NotNil(t, loadedConfig) - configFile := f.Name() - f.Write([]byte(`{"bip": "127.0.0.2", "ip": "127.0.0.1"}`)) - f.Close() - - 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, "") + content := `{ "userland-proxy": false }` + tempFile := tempfile.NewTempFile(t, "config", content) + defer tempFile.Remove() - f, err := ioutil.TempFile("", "docker-config-") - if err != nil { - t.Fatal(err) - } + opts := defaultOptions(tempFile.Name()) + loadedConfig, err := loadDaemonCliConfig(opts) + assert.NilError(t, err) + assert.NotNil(t, loadedConfig) + assert.NotNil(t, loadedConfig.ClusterOpts) - if err := flags.ParseFlags([]string{}, false); err != nil { - t.Fatal(err) - } - - 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") - } - - 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, "") + tempFile := tempfile.NewTempFile(t, "config", `{}`) + defer tempFile.Remove() - f, err := ioutil.TempFile("", "docker-config-") - if err != nil { - t.Fatal(err) - } + opts := defaultOptions(tempFile.Name()) + loadedConfig, err := loadDaemonCliConfig(opts) + assert.NilError(t, err) + assert.NotNil(t, loadedConfig) + assert.NotNil(t, loadedConfig.ClusterOpts) - 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") - } - - if !loadedConfig.EnableUserlandProxy { - t.Fatal("expected userland proxy to be enabled, got disabled") - } + assert.Equal(t, loadedConfig.EnableUserlandProxy, true) } func TestLoadDaemonConfigWithLegacyRegistryOptions(t *testing.T) { diff --git a/components/engine/cmd/dockerd/docker.go b/components/engine/cmd/dockerd/docker.go index cf55c0fd84..85f63e000a 100644 --- a/components/engine/cmd/dockerd/docker.go +++ b/components/engine/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 } diff --git a/components/engine/cmd/dockerd/service_unsupported.go b/components/engine/cmd/dockerd/service_unsupported.go index dd53802a5e..204ab2b116 100644 --- a/components/engine/cmd/dockerd/service_unsupported.go +++ b/components/engine/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) { +} diff --git a/components/engine/cmd/dockerd/service_windows.go b/components/engine/cmd/dockerd/service_windows.go index e78dad20c5..022aef578b 100644 --- a/components/engine/cmd/dockerd/service_windows.go +++ b/components/engine/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 diff --git a/components/engine/daemon/config.go b/components/engine/daemon/config.go index 2f41cabb58..133aedad21 100644 --- a/components/engine/daemon/config.go +++ b/components/engine/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 diff --git a/components/engine/daemon/config_test.go b/components/engine/daemon/config_test.go index 0375c1ae21..3e9271aece 100644 --- a/components/engine/daemon/config_test.go +++ b/components/engine/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") diff --git a/components/engine/daemon/config_windows.go b/components/engine/daemon/config_windows.go index 061f7e737c..6740743a59 100644 --- a/components/engine/daemon/config_windows.go +++ b/components/engine/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