diff --git a/cli/cobra.go b/cli/cobra.go index 5e20c96003..f924e67b27 100644 --- a/cli/cobra.go +++ b/cli/cobra.go @@ -28,7 +28,10 @@ func FlagErrorFunc(cmd *cobra.Command, err error) error { if cmd.HasSubCommands() { usage = "\n\n" + cmd.UsageString() } - return fmt.Errorf("%s\nSee '%s --help'.%s", err, cmd.CommandPath(), usage) + return StatusError{ + Status: fmt.Sprintf("%s\nSee '%s --help'.%s", err, cmd.CommandPath(), usage), + StatusCode: 125, + } } var usageTemplate = `Usage: {{if not .HasSubCommands}}{{.UseLine}}{{end}}{{if .HasSubCommands}}{{ .CommandPath}} COMMAND{{end}} diff --git a/cmd/docker/daemon_none.go b/cmd/docker/daemon_none.go index c57896ed71..65f9f37be2 100644 --- a/cmd/docker/daemon_none.go +++ b/cmd/docker/daemon_none.go @@ -4,9 +4,10 @@ package main import ( "fmt" - "github.com/spf13/cobra" "runtime" "strings" + + "github.com/spf13/cobra" ) func newDaemonCommand() *cobra.Command { diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go index 8d7861847f..38907970d3 100644 --- a/cmd/docker/docker.go +++ b/cmd/docker/docker.go @@ -19,13 +19,15 @@ import ( func newDockerCommand(dockerCli *client.DockerCli) *cobra.Command { opts := cliflags.NewClientOptions() + var flags *pflag.FlagSet + cmd := &cobra.Command{ Use: "docker [OPTIONS] COMMAND [arg...]", Short: "A self-sufficient runtime for containers.", SilenceUsage: true, SilenceErrors: true, TraverseChildren: true, - Args: cli.NoArgs, + Args: noArgs, RunE: func(cmd *cobra.Command, args []string) error { if opts.Version { showVersion() @@ -35,13 +37,15 @@ func newDockerCommand(dockerCli *client.DockerCli) *cobra.Command { return nil }, PersistentPreRunE: func(cmd *cobra.Command, args []string) error { - dockerPreRun(cmd.Flags(), opts) + // flags must be the top-level command flags, not cmd.Flags() + opts.Common.SetDefaultOptions(flags) + dockerPreRun(opts) return dockerCli.Initialize(opts) }, } cli.SetupRootCommand(cmd) - flags := cmd.Flags() + 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) @@ -53,6 +57,14 @@ func newDockerCommand(dockerCli *client.DockerCli) *cobra.Command { return cmd } +func noArgs(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + return nil + } + return fmt.Errorf( + "docker: '%s' is not a docker command.\nSee 'docker --help'%s", args[0], ".") +} + func main() { // Set terminal emulation based on platform as required. stdin, stdout, stderr := term.StdStreams() @@ -86,8 +98,7 @@ func showVersion() { } } -func dockerPreRun(flags *pflag.FlagSet, opts *cliflags.ClientOptions) { - opts.Common.SetDefaultOptions(flags) +func dockerPreRun(opts *cliflags.ClientOptions) { cliflags.SetDaemonLogLevel(opts.Common.LogLevel) if opts.ConfigDir != "" { diff --git a/cmd/dockerd/docker.go b/cmd/dockerd/docker.go index 0cd19c6478..3399597aef 100644 --- a/cmd/dockerd/docker.go +++ b/cmd/dockerd/docker.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "os" "github.com/Sirupsen/logrus" "github.com/docker/docker/cli" @@ -58,9 +59,11 @@ func runDaemon(opts daemonOptions) error { return nil } + daemonCli := NewDaemonCli() + // On Windows, this may be launching as a service or with an option to // register the service. - stop, err := initService() + stop, err := initService(daemonCli) if err != nil { logrus.Fatal(err) } @@ -69,7 +72,7 @@ func runDaemon(opts daemonOptions) error { return nil } - err = NewDaemonCli().start(opts) + err = daemonCli.start(opts) notifyShutdown(err) return err } @@ -94,6 +97,7 @@ func main() { cmd := newDaemonCommand() cmd.SetOutput(stdout) if err := cmd.Execute(); err != nil { - logrus.Fatal(err) + fmt.Fprintf(stderr, "%s\n", err) + os.Exit(1) } } diff --git a/cmd/dockerd/service_unsupported.go b/cmd/dockerd/service_unsupported.go index 204ab2b116..64ad7fcaa0 100644 --- a/cmd/dockerd/service_unsupported.go +++ b/cmd/dockerd/service_unsupported.go @@ -6,7 +6,7 @@ import ( "github.com/spf13/pflag" ) -func initService() (bool, error) { +func initService(daemonCli *DaemonCli) (bool, error) { return false, nil } diff --git a/cmd/dockerd/service_windows.go b/cmd/dockerd/service_windows.go index 022aef578b..3761188e26 100644 --- a/cmd/dockerd/service_windows.go +++ b/cmd/dockerd/service_windows.go @@ -3,7 +3,6 @@ package main import ( "bytes" "errors" - "flag" "fmt" "io/ioutil" "os" @@ -53,8 +52,9 @@ func installServiceFlags(flags *pflag.FlagSet) { } type handler struct { - tosvc chan bool - fromsvc chan error + tosvc chan bool + fromsvc chan error + daemonCli *DaemonCli } type etwHook struct { @@ -211,7 +211,7 @@ func unregisterService() error { return nil } -func initService() (bool, error) { +func initService(daemonCli *DaemonCli) (bool, error) { if *flUnregisterService { if *flRegisterService { return true, errors.New("--register-service and --unregister-service cannot be used together") @@ -233,8 +233,9 @@ func initService() (bool, error) { } h := &handler{ - tosvc: make(chan bool), - fromsvc: make(chan error), + tosvc: make(chan bool), + fromsvc: make(chan error), + daemonCli: daemonCli, } var log *eventlog.Log @@ -269,7 +270,7 @@ func initService() (bool, error) { func (h *handler) started() error { // This must be delayed until daemonCli initializes Config.Root - err := initPanicFile(filepath.Join(daemonCli.Config.Root, "panic.log")) + err := initPanicFile(filepath.Join(h.daemonCli.Config.Root, "panic.log")) if err != nil { return err } @@ -306,12 +307,12 @@ Loop: case c := <-r: switch c.Cmd { case svc.Cmd(windows.SERVICE_CONTROL_PARAMCHANGE): - daemonCli.reloadConfig() + h.daemonCli.reloadConfig() case svc.Interrogate: s <- c.CurrentStatus case svc.Stop, svc.Shutdown: s <- svc.Status{State: svc.StopPending, Accepts: 0} - daemonCli.stop() + h.daemonCli.stop() } } } diff --git a/daemon/config_unix.go b/daemon/config_unix.go index 69049de2b7..10b5a0da28 100644 --- a/daemon/config_unix.go +++ b/daemon/config_unix.go @@ -66,6 +66,7 @@ func (config *Config) InstallFlags(flags *pflag.FlagSet) { // Then platform-specific install flags flags.BoolVar(&config.EnableSelinuxSupport, "selinux-enabled", false, "Enable selinux support") + flags.StringVarP(&config.SocketGroup, "group", "G", "docker", "Group for the unix socket") flags.Var(runconfigopts.NewUlimitOpt(&config.Ulimits), "default-ulimit", "Default ulimits for containers") flags.BoolVar(&config.bridgeConfig.EnableIPTables, "iptables", true, "Enable addition of iptables rules") flags.BoolVar(&config.bridgeConfig.EnableIPForward, "ip-forward", true, "Enable net.ipv4.ip_forward") diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index 1c85021245..b7e8f5bf5d 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -1555,12 +1555,12 @@ func (s *DockerSuite) TestBuildWithInaccessibleFilesInContext(c *check.C) { c.Fatalf("failed to chmod file to 700: %s", err) } - buildCmd := exec.Command("su", "unprivilegeduser", "-c", fmt.Sprintf("%s build -t %s .", dockerBinary, name)) - buildCmd.Dir = ctx.Dir - if out, _, err := runCommandWithOutput(buildCmd); err != nil { - c.Fatalf("build should have worked: %s %s", err, out) - } - + result := icmd.RunCmd(icmd.Cmd{ + Dir: ctx.Dir, + Command: []string{"su", "unprivilegeduser", "-c", + fmt.Sprintf("%s build -t %s .", dockerBinary, name)}, + }) + result.Assert(c, icmd.Expected{}) } } diff --git a/integration-cli/docker_cli_cp_test.go b/integration-cli/docker_cli_cp_test.go index e3602c637c..174bf53ab8 100644 --- a/integration-cli/docker_cli_cp_test.go +++ b/integration-cli/docker_cli_cp_test.go @@ -11,6 +11,7 @@ import ( "strings" "github.com/docker/docker/pkg/integration/checker" + icmd "github.com/docker/docker/pkg/integration/cmd" "github.com/go-check/check" ) @@ -399,10 +400,9 @@ func (s *DockerSuite) TestCpUnprivilegedUser(c *check.C) { c.Assert(os.Chmod(tmpdir, 0777), checker.IsNil) - path := cpTestName - - _, _, err = runCommandWithOutput(exec.Command("su", "unprivilegeduser", "-c", dockerBinary+" cp "+containerID+":"+path+" "+tmpdir)) - c.Assert(err, checker.IsNil, check.Commentf("couldn't copy with unprivileged user: %s:%s", containerID, path)) + result := icmd.RunCommand("su", "unprivilegeduser", "-c", + fmt.Sprintf("%s cp %s:%s %s", dockerBinary, containerID, cpTestName, tmpdir)) + result.Assert(c, icmd.Expected{}) } func (s *DockerSuite) TestCpSpecialFiles(c *check.C) { diff --git a/integration-cli/docker_cli_daemon_test.go b/integration-cli/docker_cli_daemon_test.go index ec73beac22..0f28cfbe9d 100644 --- a/integration-cli/docker_cli_daemon_test.go +++ b/integration-cli/docker_cli_daemon_test.go @@ -23,6 +23,7 @@ import ( "github.com/docker/docker/pkg/integration/checker" icmd "github.com/docker/docker/pkg/integration/cmd" "github.com/docker/docker/pkg/mount" + "github.com/docker/docker/pkg/testutil/tempfile" "github.com/docker/go-units" "github.com/docker/libnetwork/iptables" "github.com/docker/libtrust" @@ -352,10 +353,8 @@ func (s *DockerDaemonSuite) TestDaemonIptablesCreate(c *check.C) { func (s *DockerSuite) TestDaemonIPv6Enabled(c *check.C) { testRequires(c, IPv6) - if err := setupV6(); err != nil { - c.Fatal("Could not set up host for IPv6 tests") - } - + setupV6(c) + defer teardownV6(c) d := NewDaemon(c) if err := d.StartWithBusybox("--ipv6"); err != nil { @@ -412,11 +411,6 @@ func (s *DockerSuite) TestDaemonIPv6Enabled(c *check.C) { if ip := net.ParseIP(out); ip != nil { c.Fatalf("Container should not have a global IPv6 address: %v", out) } - - if err := teardownV6(); err != nil { - c.Fatal("Could not perform teardown for IPv6 tests") - } - } // TestDaemonIPv6FixedCIDR checks that when the daemon is started with --ipv6=true and a fixed CIDR @@ -424,16 +418,16 @@ func (s *DockerSuite) TestDaemonIPv6Enabled(c *check.C) { func (s *DockerDaemonSuite) TestDaemonIPv6FixedCIDR(c *check.C) { // IPv6 setup is messing with local bridge address. testRequires(c, SameHostDaemon) - err := setupV6() - c.Assert(err, checker.IsNil, check.Commentf("Could not set up host for IPv6 tests")) + setupV6(c) + defer teardownV6(c) - err = s.d.StartWithBusybox("--ipv6", "--fixed-cidr-v6='2001:db8:2::/64'", "--default-gateway-v6='2001:db8:2::100'") + err := s.d.StartWithBusybox("--ipv6", "--fixed-cidr-v6=2001:db8:2::/64", "--default-gateway-v6=2001:db8:2::100") c.Assert(err, checker.IsNil, check.Commentf("Could not start daemon with busybox: %v", err)) out, err := s.d.Cmd("run", "-itd", "--name=ipv6test", "busybox:latest") c.Assert(err, checker.IsNil, check.Commentf("Could not run container: %s, %v", out, err)) - out, err = s.d.Cmd("inspect", "--format", "'{{.NetworkSettings.Networks.bridge.GlobalIPv6Address}}'", "ipv6test") + out, err = s.d.Cmd("inspect", "--format", "{{.NetworkSettings.Networks.bridge.GlobalIPv6Address}}", "ipv6test") out = strings.Trim(out, " \r\n'") c.Assert(err, checker.IsNil, check.Commentf(out)) @@ -441,13 +435,10 @@ func (s *DockerDaemonSuite) TestDaemonIPv6FixedCIDR(c *check.C) { ip := net.ParseIP(out) c.Assert(ip, checker.NotNil, check.Commentf("Container should have a global IPv6 address")) - out, err = s.d.Cmd("inspect", "--format", "'{{.NetworkSettings.Networks.bridge.IPv6Gateway}}'", "ipv6test") + out, err = s.d.Cmd("inspect", "--format", "{{.NetworkSettings.Networks.bridge.IPv6Gateway}}", "ipv6test") c.Assert(err, checker.IsNil, check.Commentf(out)) c.Assert(strings.Trim(out, " \r\n'"), checker.Equals, "2001:db8:2::100", check.Commentf("Container should have a global IPv6 gateway")) - - err = teardownV6() - c.Assert(err, checker.IsNil, check.Commentf("Could not perform teardown for IPv6 tests")) } // TestDaemonIPv6FixedCIDRAndMac checks that when the daemon is started with ipv6 fixed CIDR @@ -455,21 +446,18 @@ func (s *DockerDaemonSuite) TestDaemonIPv6FixedCIDR(c *check.C) { func (s *DockerDaemonSuite) TestDaemonIPv6FixedCIDRAndMac(c *check.C) { // IPv6 setup is messing with local bridge address. testRequires(c, SameHostDaemon) - err := setupV6() - c.Assert(err, checker.IsNil) + setupV6(c) + defer teardownV6(c) - err = s.d.StartWithBusybox("--ipv6", "--fixed-cidr-v6='2001:db8:1::/64'") + err := s.d.StartWithBusybox("--ipv6", "--fixed-cidr-v6=2001:db8:1::/64") c.Assert(err, checker.IsNil) out, err := s.d.Cmd("run", "-itd", "--name=ipv6test", "--mac-address", "AA:BB:CC:DD:EE:FF", "busybox") c.Assert(err, checker.IsNil) - out, err = s.d.Cmd("inspect", "--format", "'{{.NetworkSettings.Networks.bridge.GlobalIPv6Address}}'", "ipv6test") + out, err = s.d.Cmd("inspect", "--format", "{{.NetworkSettings.Networks.bridge.GlobalIPv6Address}}", "ipv6test") c.Assert(err, checker.IsNil) c.Assert(strings.Trim(out, " \r\n'"), checker.Equals, "2001:db8:1::aabb:ccdd:eeff") - - err = teardownV6() - c.Assert(err, checker.IsNil) } func (s *DockerDaemonSuite) TestDaemonLogLevelWrong(c *check.C) { @@ -1724,13 +1712,15 @@ func (s *DockerDaemonSuite) TestDaemonNoTlsCliTlsVerifyWithEnv(c *check.C) { } -func setupV6() error { +func setupV6(c *check.C) { // Hack to get the right IPv6 address on docker0, which has already been created - return exec.Command("ip", "addr", "add", "fe80::1/64", "dev", "docker0").Run() + result := icmd.RunCommand("ip", "addr", "add", "fe80::1/64", "dev", "docker0") + result.Assert(c, icmd.Expected{}) } -func teardownV6() error { - return exec.Command("ip", "addr", "del", "fe80::1/64", "dev", "docker0").Run() +func teardownV6(c *check.C) { + result := icmd.RunCommand("ip", "addr", "del", "fe80::1/64", "dev", "docker0") + result.Assert(c, icmd.Expected{}) } func (s *DockerDaemonSuite) TestDaemonRestartWithContainerWithRestartPolicyAlways(c *check.C) { @@ -2362,31 +2352,26 @@ func (s *DockerSuite) TestDaemonDiscoveryBackendConfigReload(c *check.C) { testRequires(c, SameHostDaemon, DaemonIsLinux) // daemon config file - daemonConfig := `{ "debug" : false }` - configFilePath := "test.json" - - configFile, err := os.Create(configFilePath) - c.Assert(err, checker.IsNil) - fmt.Fprintf(configFile, "%s", daemonConfig) + tmpfile := tempfile.NewTempFile(c, "config-test", `{ "debug" : false }`) + defer tmpfile.Remove() d := NewDaemon(c) - err = d.Start(fmt.Sprintf("--config-file=%s", configFilePath)) + // --log-level needs to be set so that d.Start() doesn't add --debug causing + // a conflict with the config + err := d.Start("--config-file", tmpfile.Name(), "--log-level=info") c.Assert(err, checker.IsNil) defer d.Stop() // daemon config file - daemonConfig = `{ + daemonConfig := `{ "cluster-store": "consul://consuladdr:consulport/some/path", "cluster-advertise": "192.168.56.100:0", "debug" : false }` - configFile.Close() - os.Remove(configFilePath) - - configFile, err = os.Create(configFilePath) + os.Remove(tmpfile.Name()) + configFile, err := os.Create(tmpfile.Name()) c.Assert(err, checker.IsNil) - defer os.Remove(configFilePath) fmt.Fprintf(configFile, "%s", daemonConfig) configFile.Close() @@ -2396,6 +2381,7 @@ func (s *DockerSuite) TestDaemonDiscoveryBackendConfigReload(c *check.C) { out, err := d.Cmd("info") c.Assert(err, checker.IsNil) + c.Assert(out, checker.Contains, fmt.Sprintf("Cluster Store: consul://consuladdr:consulport/some/path")) c.Assert(out, checker.Contains, fmt.Sprintf("Cluster Advertise: 192.168.56.100:0")) } diff --git a/integration-cli/docker_cli_help_test.go b/integration-cli/docker_cli_help_test.go index a4dca18ef0..2d6610c71f 100644 --- a/integration-cli/docker_cli_help_test.go +++ b/integration-cli/docker_cli_help_test.go @@ -9,6 +9,7 @@ import ( "github.com/docker/docker/pkg/homedir" "github.com/docker/docker/pkg/integration/checker" + icmd "github.com/docker/docker/pkg/integration/cmd" "github.com/go-check/check" ) @@ -56,12 +57,7 @@ func (s *DockerSuite) TestHelpTextVerify(c *check.C) { out, _, err := runCommandWithOutput(helpCmd) c.Assert(err, checker.IsNil, check.Commentf(out)) lines := strings.Split(out, "\n") - foundTooLongLine := false for _, line := range lines { - if !foundTooLongLine && len(line) > 80 { - c.Logf("Line is too long:\n%s", line) - foundTooLongLine = true - } // All lines should not end with a space c.Assert(line, checker.Not(checker.HasSuffix), " ", check.Commentf("Line should not end with a space")) @@ -229,14 +225,6 @@ func testCommand(cmd string, newEnvs []string, scanForHome bool, home string) er // Check each line for lots of stuff lines := strings.Split(out, "\n") for _, line := range lines { - if len(line) > 107 { - return fmt.Errorf("Help for %q is too long:\n%s\n", cmd, line) - } - - if scanForHome && strings.Contains(line, `"`+home) { - return fmt.Errorf("Help for %q should use ~ instead of %q on:\n%s\n", - cmd, home, line) - } i := strings.Index(line, "~") if i >= 0 && i != len(line)-1 && line[i+1] != '/' { return fmt.Errorf("Help for %q should not have used ~:\n%s", cmd, line) @@ -290,10 +278,6 @@ func testCommand(cmd string, newEnvs []string, scanForHome bool, home string) er } if _, ok := noShortUsage[cmd]; !ok { - // For each command run it w/o any args. It will either return - // valid output or print a short-usage - var dCmd *exec.Cmd - // skipNoArgs are ones that we don't want to try w/o // any args. Either because it'll hang the test or // lead to incorrect test result (like false negative). @@ -305,33 +289,31 @@ func testCommand(cmd string, newEnvs []string, scanForHome bool, home string) er "load": {}, } - ec := 0 + var result *icmd.Result if _, ok := skipNoArgs[cmd]; !ok { - args = strings.Split(cmd, " ") - dCmd = exec.Command(dockerBinary, args...) - out, stderr, ec, err = runCommandWithStdoutStderr(dCmd) + result = dockerCmdWithResult(strings.Split(cmd, " ")...) } // If its ok w/o any args then try again with an arg - if ec == 0 { - args = strings.Split(cmd+" badArg", " ") - dCmd = exec.Command(dockerBinary, args...) - out, stderr, ec, err = runCommandWithStdoutStderr(dCmd) + if result == nil || result.ExitCode == 0 { + result = dockerCmdWithResult(strings.Split(cmd+" badArg", " ")...) } - if len(out) != 0 || len(stderr) == 0 || ec == 0 || err == nil { - return fmt.Errorf("Bad output from %q\nstdout:%q\nstderr:%q\nec:%d\nerr:%q\n", args, out, stderr, ec, err) + if err := result.Compare(icmd.Expected{ + Out: icmd.None, + Err: "\nUsage:", + ExitCode: 1, + }); err != nil { + return err } - // Should have just short usage - if !strings.Contains(stderr, "\nUsage:") { - return fmt.Errorf("Missing short usage on %q\n:%#v", args, stderr) - } - // But shouldn't have full usage + + stderr := result.Stderr() + // Shouldn't have full usage if strings.Contains(stderr, "--help=false") { - return fmt.Errorf("Should not have full usage on %q\n", args) + return fmt.Errorf("Should not have full usage on %q:%v", result.Cmd.Args, stderr) } if strings.HasSuffix(stderr, "\n\n") { - return fmt.Errorf("Should not have a blank line on %q\n%v", args, stderr) + return fmt.Errorf("Should not have a blank line on %q\n%v", result.Cmd.Args, stderr) } } diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index 8e659e551a..07c8a4381e 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -334,11 +334,8 @@ func (s *DockerSuite) TestUserDefinedNetworkAlias(c *check.C) { // Issue 9677. func (s *DockerSuite) TestRunWithDaemonFlags(c *check.C) { out, _, err := dockerCmdWithError("--exec-opt", "foo=bar", "run", "-i", "busybox", "true") - if err != nil { - if !strings.Contains(out, "flag provided but not defined: --exec-opt") { // no daemon (client-only) - c.Fatal(err, out) - } - } + c.Assert(err, checker.NotNil) + c.Assert(out, checker.Contains, "unknown flag: --exec-opt") } // Regression test for #4979 @@ -2663,15 +2660,11 @@ func (s *DockerSuite) TestRunTLSverify(c *check.C) { // Regardless of whether we specify true or false we need to // test to make sure tls is turned on if --tlsverify is specified at all - out, code, err := dockerCmdWithError("--tlsverify=false", "ps") - if err == nil || code == 0 || !strings.Contains(out, "trying to connect") { - c.Fatalf("Should have failed: \net:%v\nout:%v\nerr:%v", code, out, err) - } + result := dockerCmdWithResult("--tlsverify=false", "ps") + result.Assert(c, icmd.Expected{ExitCode: 1, Err: "trying to connect"}) - out, code, err = dockerCmdWithError("--tlsverify=true", "ps") - if err == nil || code == 0 || !strings.Contains(out, "cert") { - c.Fatalf("Should have failed: \net:%v\nout:%v\nerr:%v", code, out, err) - } + result = dockerCmdWithResult("--tlsverify=true", "ps") + result.Assert(c, icmd.Expected{ExitCode: 1, Err: "cert"}) } func (s *DockerSuite) TestRunPortFromDockerRangeInUse(c *check.C) {