From 0024935f641ab596732f9162d3eab795bbc2ae06 Mon Sep 17 00:00:00 2001 From: Doug Davis Date: Sat, 9 May 2015 11:33:06 -0700 Subject: [PATCH] Use stderr instead of logrus for CLI error messages Signed-off-by: Doug Davis --- api/client/build.go | 3 +-- api/client/cli.go | 8 +++++++ api/client/exec.go | 4 ++-- api/client/run.go | 6 ++--- api/client/start.go | 6 ++--- api/client/version.go | 3 +-- docker/docker.go | 25 +++++++++++++------ integration-cli/docker_cli_build_test.go | 6 ++--- integration-cli/docker_cli_daemon_test.go | 4 ++-- integration-cli/docker_cli_help_test.go | 29 +++++++++++++++++++++++ integration-cli/docker_cli_run_test.go | 2 +- 11 files changed, 71 insertions(+), 25 deletions(-) diff --git a/api/client/build.go b/api/client/build.go index 1611f527f1..4ac147ee9a 100644 --- a/api/client/build.go +++ b/api/client/build.go @@ -17,7 +17,6 @@ import ( "strconv" "strings" - "github.com/Sirupsen/logrus" "github.com/docker/docker/api" "github.com/docker/docker/graph" "github.com/docker/docker/pkg/archive" @@ -192,7 +191,7 @@ func (cli *DockerCli) CmdBuild(args ...string) error { // windows: show error message about modified file permissions // FIXME: this is not a valid warning when the daemon is running windows. should be removed once docker engine for windows can build. if runtime.GOOS == "windows" { - logrus.Warn(`SECURITY WARNING: You are building a Docker image from Windows against a Linux Docker host. All files and directories added to build context will have '-rwxr-xr-x' permissions. It is recommended to double check and reset permissions for sensitive files and directories.`) + fmt.Fprintln(cli.err, `SECURITY WARNING: You are building a Docker image from Windows against a Linux Docker host. All files and directories added to build context will have '-rwxr-xr-x' permissions. It is recommended to double check and reset permissions for sensitive files and directories.`) } var body io.Reader diff --git a/api/client/cli.go b/api/client/cli.go index 7d9610078c..b649537c97 100644 --- a/api/client/cli.go +++ b/api/client/cli.go @@ -63,6 +63,14 @@ var funcMap = template.FuncMap{ }, } +func (cli *DockerCli) Out() io.Writer { + return cli.out +} + +func (cli *DockerCli) Err() io.Writer { + return cli.err +} + func (cli *DockerCli) getMethod(args ...string) (func(...string) error, bool) { camelArgs := make([]string, len(args)) for i, s := range args { diff --git a/api/client/exec.go b/api/client/exec.go index 4bf53eaec2..f247ec5217 100644 --- a/api/client/exec.go +++ b/api/client/exec.go @@ -71,7 +71,7 @@ func (cli *DockerCli) CmdExec(args ...string) error { defer func() { logrus.Debugf("End of CmdExec(), Waiting for hijack to finish.") if _, ok := <-hijacked; ok { - logrus.Errorf("Hijack did not finish (chan still open)") + fmt.Fprintln(cli.err, "Hijack did not finish (chan still open)") } }() @@ -109,7 +109,7 @@ func (cli *DockerCli) CmdExec(args ...string) error { if execConfig.Tty && cli.isTerminalIn { if err := cli.monitorTtySize(execID, true); err != nil { - logrus.Errorf("Error monitoring TTY size: %s", err) + fmt.Fprintf(cli.err, "Error monitoring TTY size: %s\n", err) } } diff --git a/api/client/run.go b/api/client/run.go index ff0e534c67..92f242ce14 100644 --- a/api/client/run.go +++ b/api/client/run.go @@ -133,7 +133,7 @@ func (cli *DockerCli) CmdRun(args ...string) error { defer func() { logrus.Debugf("End of CmdRun(), Waiting for hijack to finish.") if _, ok := <-hijacked; ok { - logrus.Errorf("Hijack did not finish (chan still open)") + fmt.Fprintln(cli.err, "Hijack did not finish (chan still open)") } }() if config.AttachStdin || config.AttachStdout || config.AttachStderr { @@ -183,7 +183,7 @@ func (cli *DockerCli) CmdRun(args ...string) error { defer func() { if *flAutoRemove { if _, _, err = readBody(cli.call("DELETE", "/containers/"+createResponse.ID+"?v=1", nil, nil)); err != nil { - logrus.Errorf("Error deleting container: %s", err) + fmt.Fprintf(cli.err, "Error deleting container: %s\n", err) } } }() @@ -195,7 +195,7 @@ func (cli *DockerCli) CmdRun(args ...string) error { if (config.AttachStdin || config.AttachStdout || config.AttachStderr) && config.Tty && cli.isTerminalOut { if err := cli.monitorTtySize(createResponse.ID, false); err != nil { - logrus.Errorf("Error monitoring TTY size: %s", err) + fmt.Fprintf(cli.err, "Error monitoring TTY size: %s\n", err) } } diff --git a/api/client/start.go b/api/client/start.go index 55f307f522..40f84d7cf3 100644 --- a/api/client/start.go +++ b/api/client/start.go @@ -30,7 +30,7 @@ func (cli *DockerCli) forwardAllSignals(cid string) chan os.Signal { } } if sig == "" { - logrus.Errorf("Unsupported signal: %v. Discarding.", s) + fmt.Fprintf(cli.err, "Unsupported signal: %v. Discarding.\n", s) } if _, _, err := readBody(cli.call("POST", fmt.Sprintf("/containers/%s/kill?signal=%s", cid, sig), nil, nil)); err != nil { logrus.Debugf("Error sending signal: %s", err) @@ -96,7 +96,7 @@ func (cli *DockerCli) CmdStart(args ...string) error { defer func() { logrus.Debugf("CmdStart() returned, defer waiting for hijack to finish.") if _, ok := <-hijacked; ok { - logrus.Errorf("Hijack did not finish (chan still open)") + fmt.Fprintln(cli.err, "Hijack did not finish (chan still open)") } cli.in.Close() }() @@ -149,7 +149,7 @@ func (cli *DockerCli) CmdStart(args ...string) error { if *openStdin || *attach { if tty && cli.isTerminalOut { if err := cli.monitorTtySize(cmd.Arg(0), false); err != nil { - logrus.Errorf("Error monitoring TTY size: %s", err) + fmt.Fprintf(cli.err, "Error monitoring TTY size: %s\n", err) } } if attchErr := <-cErr; attchErr != nil { diff --git a/api/client/version.go b/api/client/version.go index 2fb6f8a8d5..4e06a6c8ad 100644 --- a/api/client/version.go +++ b/api/client/version.go @@ -5,7 +5,6 @@ import ( "fmt" "runtime" - "github.com/Sirupsen/logrus" "github.com/docker/docker/api" "github.com/docker/docker/api/types" "github.com/docker/docker/autogen/dockerversion" @@ -40,7 +39,7 @@ func (cli *DockerCli) CmdVersion(args ...string) error { var v types.Version if err := json.NewDecoder(stream).Decode(&v); err != nil { - logrus.Errorf("Error reading remote version: %s", err) + fmt.Fprintf(cli.err, "Error reading remote version: %s\n", err) return err } diff --git a/docker/docker.go b/docker/docker.go index 698991e053..05f5dcc043 100644 --- a/docker/docker.go +++ b/docker/docker.go @@ -46,7 +46,8 @@ func main() { if *flLogLevel != "" { lvl, err := logrus.ParseLevel(*flLogLevel) if err != nil { - logrus.Fatalf("Unable to parse logging level: %s", *flLogLevel) + fmt.Fprintf(os.Stderr, "Unable to parse logging level: %s\n", *flLogLevel) + os.Exit(1) } setLogLevel(lvl) } else { @@ -73,7 +74,12 @@ func main() { } defaultHost, err := opts.ValidateHost(defaultHost) if err != nil { - logrus.Fatal(err) + if *flDaemon { + logrus.Fatal(err) + } else { + fmt.Fprint(os.Stderr, err) + } + os.Exit(1) } flHosts = append(flHosts, defaultHost) } @@ -90,7 +96,8 @@ func main() { } if len(flHosts) > 1 { - logrus.Fatal("Please specify only one -H") + fmt.Fprintf(os.Stderr, "Please specify only one -H") + os.Exit(0) } protoAddrParts := strings.SplitN(flHosts[0], "://", 2) @@ -111,7 +118,8 @@ func main() { certPool := x509.NewCertPool() file, err := ioutil.ReadFile(*flCa) if err != nil { - logrus.Fatalf("Couldn't read ca cert %s: %s", *flCa, err) + fmt.Fprintf(os.Stderr, "Couldn't read ca cert %s: %s\n", *flCa, err) + os.Exit(1) } certPool.AppendCertsFromPEM(file) tlsConfig.RootCAs = certPool @@ -126,7 +134,8 @@ func main() { *flTls = true cert, err := tls.LoadX509KeyPair(*flCert, *flKey) if err != nil { - logrus.Fatalf("Couldn't load X509 key pair: %q. Make sure the key is encrypted", err) + fmt.Fprintf(os.Stderr, "Couldn't load X509 key pair: %q. Make sure the key is encrypted\n", err) + os.Exit(1) } tlsConfig.Certificates = []tls.Certificate{cert} } @@ -143,11 +152,13 @@ func main() { if err := cli.Cmd(flag.Args()...); err != nil { if sterr, ok := err.(client.StatusError); ok { if sterr.Status != "" { - logrus.Println(sterr.Status) + fmt.Fprintln(cli.Err(), sterr.Status) + os.Exit(1) } os.Exit(sterr.StatusCode) } - logrus.Fatal(err) + fmt.Fprintln(cli.Err(), err) + os.Exit(1) } } diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index 945eb787e3..73aef80d80 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -4808,7 +4808,7 @@ func (s *DockerSuite) TestBuildRenamedDockerfile(c *check.C) { c.Fatalf("test5 was supposed to fail to find passwd") } - if expected := fmt.Sprintf("The Dockerfile (%s) must be within the build context (.)", strings.Replace(nonDockerfileFile, `\`, `\\`, -1)); !strings.Contains(out, expected) { + if expected := fmt.Sprintf("The Dockerfile (%s) must be within the build context (.)", nonDockerfileFile); !strings.Contains(out, expected) { c.Fatalf("wrong error messsage:%v\nexpected to contain=%v", out, expected) } @@ -5404,7 +5404,7 @@ func (s *DockerSuite) TestBuildBadCmdFlag(c *check.C) { c.Fatal("Build should have failed") } - exp := `"Unknown flag: boo"` + exp := "\nUnknown flag: boo\n" if !strings.Contains(out, exp) { c.Fatalf("Bad output\nGot:%s\n\nExpected to contain:%s\n", out, exp) } @@ -5421,7 +5421,7 @@ func (s *DockerSuite) TestBuildRUNErrMsg(c *check.C) { c.Fatal("Should have failed to build") } - exp := "The command '/bin/sh -c badEXE a1 \\\\& a2\\ta3' returned a non-zero code: 127" + exp := `The command '/bin/sh -c badEXE a1 \& a2 a3' returned a non-zero code: 127` if !strings.Contains(out, exp) { c.Fatalf("RUN doesn't have the correct output:\nGot:%s\nExpected:%s", out, exp) } diff --git a/integration-cli/docker_cli_daemon_test.go b/integration-cli/docker_cli_daemon_test.go index 3058076bd1..8e563390fb 100644 --- a/integration-cli/docker_cli_daemon_test.go +++ b/integration-cli/docker_cli_daemon_test.go @@ -907,8 +907,8 @@ func (s *DockerDaemonSuite) TestDaemonLoggingDriverNoneLogsError(c *check.C) { if err == nil { c.Fatalf("Logs should fail with \"none\" driver") } - if !strings.Contains(out, `\"logs\" command is supported only for \"json-file\" logging driver`) { - c.Fatalf("There should be error about non-json-file driver, got %s", out) + if !strings.Contains(out, `"logs" command is supported only for "json-file" logging driver`) { + c.Fatalf("There should be error about non-json-file driver, got: %s", out) } } diff --git a/integration-cli/docker_cli_help_test.go b/integration-cli/docker_cli_help_test.go index d6903e4fb9..86b0b3bbc7 100644 --- a/integration-cli/docker_cli_help_test.go +++ b/integration-cli/docker_cli_help_test.go @@ -152,3 +152,32 @@ func (s *DockerSuite) TestHelpTextVerify(c *check.C) { } } + +func (s *DockerSuite) TestHelpErrorStderr(c *check.C) { + // If we had a generic CLI test file this one shoudl go in there + + cmd := exec.Command(dockerBinary, "boogie") + out, ec, err := runCommandWithOutput(cmd) + if err == nil || ec == 0 { + c.Fatalf("Boogie command should have failed") + } + + expected := "docker: 'boogie' is not a docker command. See 'docker --help'.\n" + if out != expected { + c.Fatalf("Bad output from boogie\nGot:%s\nExpected:%s", out, expected) + } + + cmd = exec.Command(dockerBinary, "rename", "foo", "bar") + out, ec, err = runCommandWithOutput(cmd) + if err == nil || ec == 0 { + c.Fatalf("Rename should have failed") + } + + expected = `Error response from daemon: no such id: foo +Error: failed to rename container named foo +` + if out != expected { + c.Fatalf("Bad output from rename\nGot:%s\nExpected:%s", out, expected) + } + +} diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index 7de38918d0..4a12a92baa 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -2051,7 +2051,7 @@ func (s *DockerSuite) TestRunWithBadDevice(c *check.C) { if err == nil { c.Fatal("Run should fail with bad device") } - expected := `\"/etc\": not a device node` + expected := `"/etc": not a device node` if !strings.Contains(out, expected) { c.Fatalf("Output should contain %q, actual out: %q", expected, out) }