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 <[email protected]>
mattyb · Feb 3, 2016 · 894266c · 894266c
1 parent 6f88a3d
commit 894266c
Show file tree

Hide file tree

Showing 17 changed files with 58 additions and 32 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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) {