From 894266c1bbdfeb53bf278f3cb762945bac69e592 Mon Sep 17 00:00:00 2001 From: Zhang Wei Date: Wed, 3 Feb 2016 14:29:15 +0800 Subject: [PATCH] Remove redundant error message Currently some commands including `kill`, `pause`, `restart`, `rm`, `rmi`, `stop`, `unpause`, `udpate`, `wait` will print a lot of error message on client side, with a lot of redundant messages, this commit is trying to remove the unuseful and redundant information for user. Signed-off-by: Zhang Wei --- api/client/kill.go | 2 +- api/client/pause.go | 2 +- api/client/restart.go | 2 +- api/client/rm.go | 2 +- api/client/rmi.go | 2 +- api/client/stop.go | 2 +- api/client/unpause.go | 2 +- api/client/update.go | 2 +- api/client/wait.go | 2 +- daemon/delete.go | 6 ++--- daemon/kill.go | 2 +- daemon/pause.go | 4 ++-- daemon/stop.go | 2 +- daemon/unpause.go | 4 ++-- daemon/update.go | 11 +++++---- errors/daemon.go | 34 +++++++++++++++++++++++---- integration-cli/docker_cli_rm_test.go | 9 +++---- 17 files changed, 58 insertions(+), 32 deletions(-) diff --git a/api/client/kill.go b/api/client/kill.go index 5e8e5b0753..8220f57681 100644 --- a/api/client/kill.go +++ b/api/client/kill.go @@ -21,7 +21,7 @@ func (cli *DockerCli) CmdKill(args ...string) error { var errs []string for _, name := range cmd.Args() { if err := cli.client.ContainerKill(name, *signal); err != nil { - errs = append(errs, fmt.Sprintf("Failed to kill container (%s): %s", name, err)) + errs = append(errs, err.Error()) } else { fmt.Fprintf(cli.out, "%s\n", name) } diff --git a/api/client/pause.go b/api/client/pause.go index dab4eb2e05..88a1ecc7d6 100644 --- a/api/client/pause.go +++ b/api/client/pause.go @@ -20,7 +20,7 @@ func (cli *DockerCli) CmdPause(args ...string) error { var errs []string for _, name := range cmd.Args() { if err := cli.client.ContainerPause(name); err != nil { - errs = append(errs, fmt.Sprintf("Failed to pause container (%s): %s", name, err)) + errs = append(errs, err.Error()) } else { fmt.Fprintf(cli.out, "%s\n", name) } diff --git a/api/client/restart.go b/api/client/restart.go index 245aca540e..27b9d668f3 100644 --- a/api/client/restart.go +++ b/api/client/restart.go @@ -21,7 +21,7 @@ func (cli *DockerCli) CmdRestart(args ...string) error { var errs []string for _, name := range cmd.Args() { if err := cli.client.ContainerRestart(name, *nSeconds); err != nil { - errs = append(errs, fmt.Sprintf("Failed to kill container (%s): %s", name, err)) + errs = append(errs, err.Error()) } else { fmt.Fprintf(cli.out, "%s\n", name) } diff --git a/api/client/rm.go b/api/client/rm.go index f4e3c58233..0846306ce2 100644 --- a/api/client/rm.go +++ b/api/client/rm.go @@ -48,7 +48,7 @@ func (cli *DockerCli) removeContainer(containerID string, removeVolumes, removeL Force: force, } if err := cli.client.ContainerRemove(options); err != nil { - return fmt.Errorf("Failed to remove container (%s): %v", containerID, err) + return err } return nil } diff --git a/api/client/rmi.go b/api/client/rmi.go index a190ab38cc..f58c68a495 100644 --- a/api/client/rmi.go +++ b/api/client/rmi.go @@ -39,7 +39,7 @@ func (cli *DockerCli) CmdRmi(args ...string) error { dels, err := cli.client.ImageRemove(options) if err != nil { - errs = append(errs, fmt.Sprintf("Failed to remove image (%s): %s", name, err)) + errs = append(errs, err.Error()) } else { for _, del := range dels { if del.Deleted != "" { diff --git a/api/client/stop.go b/api/client/stop.go index 9d429ea0ac..5ece077fc2 100644 --- a/api/client/stop.go +++ b/api/client/stop.go @@ -23,7 +23,7 @@ func (cli *DockerCli) CmdStop(args ...string) error { var errs []string for _, name := range cmd.Args() { if err := cli.client.ContainerStop(name, *nSeconds); err != nil { - errs = append(errs, fmt.Sprintf("Failed to stop container (%s): %s", name, err)) + errs = append(errs, err.Error()) } else { fmt.Fprintf(cli.out, "%s\n", name) } diff --git a/api/client/unpause.go b/api/client/unpause.go index 9ec3310df9..b1a864dce5 100644 --- a/api/client/unpause.go +++ b/api/client/unpause.go @@ -20,7 +20,7 @@ func (cli *DockerCli) CmdUnpause(args ...string) error { var errs []string for _, name := range cmd.Args() { if err := cli.client.ContainerUnpause(name); err != nil { - errs = append(errs, fmt.Sprintf("Failed to unpause container (%s): %s", name, err)) + errs = append(errs, err.Error()) } else { fmt.Fprintf(cli.out, "%s\n", name) } diff --git a/api/client/update.go b/api/client/update.go index 7083048859..b33ed476a6 100644 --- a/api/client/update.go +++ b/api/client/update.go @@ -90,7 +90,7 @@ func (cli *DockerCli) CmdUpdate(args ...string) error { var errs []string for _, name := range names { if err := cli.client.ContainerUpdate(name, updateConfig); err != nil { - errs = append(errs, fmt.Sprintf("Failed to update container (%s): %s", name, err)) + errs = append(errs, err.Error()) } else { fmt.Fprintf(cli.out, "%s\n", name) } diff --git a/api/client/wait.go b/api/client/wait.go index d77a523e48..0e9f0b3962 100644 --- a/api/client/wait.go +++ b/api/client/wait.go @@ -23,7 +23,7 @@ func (cli *DockerCli) CmdWait(args ...string) error { for _, name := range cmd.Args() { status, err := cli.client.ContainerWait(name) if err != nil { - errs = append(errs, fmt.Sprintf("Failed to wait container (%s): %s", name, err)) + errs = append(errs, err.Error()) } else { fmt.Fprintf(cli.out, "%d\n", status) } diff --git a/daemon/delete.go b/daemon/delete.go index a086aed616..6c56fd7ba1 100644 --- a/daemon/delete.go +++ b/daemon/delete.go @@ -30,7 +30,7 @@ func (daemon *Daemon) ContainerRm(name string, config *types.ContainerRmConfig) // do not fail when the removal is in progress started by other request. return nil } - return derr.ErrorCodeRmState.WithArgs(err) + return derr.ErrorCodeRmState.WithArgs(container.ID, err) } defer container.ResetRemovalInProgress() @@ -84,10 +84,10 @@ func (daemon *Daemon) rmLink(container *container.Container, name string) error func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemove bool) (err error) { if container.IsRunning() { if !forceRemove { - return derr.ErrorCodeRmRunning + return derr.ErrorCodeRmRunning.WithArgs(container.ID) } if err := daemon.Kill(container); err != nil { - return derr.ErrorCodeRmFailed.WithArgs(err) + return derr.ErrorCodeRmFailed.WithArgs(container.ID, err) } } diff --git a/daemon/kill.go b/daemon/kill.go index 642d488246..2bb695699c 100644 --- a/daemon/kill.go +++ b/daemon/kill.go @@ -62,7 +62,7 @@ func (daemon *Daemon) killWithSignal(container *container.Container, sig int) er } if err := daemon.kill(container, sig); err != nil { - return err + return derr.ErrorCodeCantKill.WithArgs(container.ID, err) } attributes := map[string]string{ diff --git a/daemon/pause.go b/daemon/pause.go index bcf2280e80..e94c8b868f 100644 --- a/daemon/pause.go +++ b/daemon/pause.go @@ -13,7 +13,7 @@ func (daemon *Daemon) ContainerPause(name string) error { } if err := daemon.containerPause(container); err != nil { - return derr.ErrorCodePauseError.WithArgs(name, err) + return err } return nil @@ -36,7 +36,7 @@ func (daemon *Daemon) containerPause(container *container.Container) error { } if err := daemon.execDriver.Pause(container.Command); err != nil { - return err + return derr.ErrorCodeCantPause.WithArgs(container.ID, err) } container.Paused = true daemon.LogContainerEvent(container, "pause") diff --git a/daemon/stop.go b/daemon/stop.go index 60f51410dd..55e3787751 100644 --- a/daemon/stop.go +++ b/daemon/stop.go @@ -20,7 +20,7 @@ func (daemon *Daemon) ContainerStop(name string, seconds int) error { return err } if !container.IsRunning() { - return derr.ErrorCodeStopped + return derr.ErrorCodeStopped.WithArgs(name) } if err := daemon.containerStop(container, seconds); err != nil { return derr.ErrorCodeCantStop.WithArgs(name, err) diff --git a/daemon/unpause.go b/daemon/unpause.go index dcf7336a28..ace52593d7 100644 --- a/daemon/unpause.go +++ b/daemon/unpause.go @@ -13,7 +13,7 @@ func (daemon *Daemon) ContainerUnpause(name string) error { } if err := daemon.containerUnpause(container); err != nil { - return derr.ErrorCodeCantUnpause.WithArgs(name, err) + return err } return nil @@ -35,7 +35,7 @@ func (daemon *Daemon) containerUnpause(container *container.Container) error { } if err := daemon.execDriver.Unpause(container.Command); err != nil { - return err + return derr.ErrorCodeCantUnpause.WithArgs(container.ID, err) } container.Paused = false diff --git a/daemon/update.go b/daemon/update.go index 600d022452..181b399c96 100644 --- a/daemon/update.go +++ b/daemon/update.go @@ -3,6 +3,7 @@ package daemon import ( "fmt" + derr "github.com/docker/docker/errors" "github.com/docker/engine-api/types/container" ) @@ -44,15 +45,17 @@ func (daemon *Daemon) update(name string, hostConfig *container.HostConfig) erro } if container.RemovalInProgress || container.Dead { - return fmt.Errorf("Container is marked for removal and cannot be \"update\".") + errMsg := fmt.Errorf("Container is marked for removal and cannot be \"update\".") + return derr.ErrorCodeCantUpdate.WithArgs(container.ID, errMsg) } if container.IsRunning() && hostConfig.KernelMemory != 0 { - return fmt.Errorf("Can not update kernel memory to a running container, please stop it first.") + errMsg := fmt.Errorf("Can not update kernel memory to a running container, please stop it first.") + return derr.ErrorCodeCantUpdate.WithArgs(container.ID, errMsg) } if err := container.UpdateContainer(hostConfig); err != nil { - return err + return derr.ErrorCodeCantUpdate.WithArgs(container.ID, err.Error()) } // If container is not running, update hostConfig struct is enough, @@ -61,7 +64,7 @@ func (daemon *Daemon) update(name string, hostConfig *container.HostConfig) erro // to the real world. if container.IsRunning() { if err := daemon.execDriver.Update(container.Command); err != nil { - return err + return derr.ErrorCodeCantUpdate.WithArgs(container.ID, err.Error()) } } diff --git a/errors/daemon.go b/errors/daemon.go index 78c4422ab1..9e2e5527fc 100644 --- a/errors/daemon.go +++ b/errors/daemon.go @@ -478,6 +478,15 @@ var ( HTTPStatusCode: http.StatusInternalServerError, }) + // ErrorCodeCantPause is generated when there's an error while trying + // to pause a container. + ErrorCodeCantPause = errcode.Register(errGroup, errcode.ErrorDescriptor{ + Value: "CANTPAUSE", + Message: "Cannot pause container %s: %s", + Description: "An error occurred while trying to pause the specified container", + HTTPStatusCode: http.StatusInternalServerError, + }) + // ErrorCodeCantUnpause is generated when there's an error while trying // to unpause a container. ErrorCodeCantUnpause = errcode.Register(errGroup, errcode.ErrorDescriptor{ @@ -487,6 +496,23 @@ var ( HTTPStatusCode: http.StatusInternalServerError, }) + // ErrorCodeCantKill is generated when there's an error while trying + // to kill a container. + ErrorCodeCantKill = errcode.Register(errGroup, errcode.ErrorDescriptor{ + Value: "CANTKILL", + Message: "Cannot kill container %s: %s", + Description: "An error occurred while trying to kill the specified container", + HTTPStatusCode: http.StatusInternalServerError, + }) + + // ErrorCodeCantUpdate is generated when there's an error while trying + // to update a container. + ErrorCodeCantUpdate = errcode.Register(errGroup, errcode.ErrorDescriptor{ + Value: "CANTUPDATE", + Message: "Cannot update container %s: %s", + Description: "An error occurred while trying to update the specified container", + HTTPStatusCode: http.StatusInternalServerError, + }) // ErrorCodePSError is generated when trying to run 'ps'. ErrorCodePSError = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "PSError", @@ -525,7 +551,7 @@ var ( // that is already stopped. ErrorCodeStopped = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "STOPPED", - Message: "Container already stopped", + Message: "Container %s is already stopped", Description: "An attempt was made to stop a container, but the container is already stopped", HTTPStatusCode: http.StatusNotModified, }) @@ -818,7 +844,7 @@ var ( // but its still running. ErrorCodeRmRunning = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "RMRUNNING", - Message: "Conflict, You cannot remove a running container. Stop the container before attempting removal or use -f", + Message: "You cannot remove a running container %s. Stop the container before attempting removal or use -f", Description: "An attempt was made to delete a container but the container is still running, try to either stop it first or use '-f'", HTTPStatusCode: http.StatusConflict, }) @@ -827,7 +853,7 @@ var ( // but it failed for some reason. ErrorCodeRmFailed = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "RMFAILED", - Message: "Could not kill running container, cannot remove - %v", + Message: "Could not kill running container %s, cannot remove - %v", Description: "An error occurred while trying to delete a running container", HTTPStatusCode: http.StatusInternalServerError, }) @@ -845,7 +871,7 @@ var ( // but couldn't set its state to RemovalInProgress. ErrorCodeRmState = errcode.Register(errGroup, errcode.ErrorDescriptor{ Value: "RMSTATE", - Message: "Failed to set container state to RemovalInProgress: %s", + Message: "Failed to set container %s state to RemovalInProgress: %s", Description: "An attempt to delete a container was made, but there as an error trying to set its state to 'RemovalInProgress'", HTTPStatusCode: http.StatusInternalServerError, }) diff --git a/integration-cli/docker_cli_rm_test.go b/integration-cli/docker_cli_rm_test.go index ea2ca64035..6e800b69ca 100644 --- a/integration-cli/docker_cli_rm_test.go +++ b/integration-cli/docker_cli_rm_test.go @@ -2,7 +2,6 @@ package main import ( "os" - "strings" "github.com/docker/docker/pkg/integration/checker" "github.com/go-check/check" @@ -71,11 +70,9 @@ func (s *DockerSuite) TestRmContainerOrphaning(c *check.C) { } func (s *DockerSuite) TestRmInvalidContainer(c *check.C) { - if out, _, err := dockerCmdWithError("rm", "unknown"); err == nil { - c.Fatal("Expected error on rm unknown container, got none") - } else if !strings.Contains(out, "Failed to remove container") { - c.Fatalf("Expected output to contain 'Failed to remove container', got %q", out) - } + out, _, err := dockerCmdWithError("rm", "unknown") + c.Assert(err, checker.NotNil, check.Commentf("Expected error on rm unknown container, got none")) + c.Assert(out, checker.Contains, "No such container") } func createRunningContainer(c *check.C, name string) {