Преглед изворни кода

Remove redundant error messages

For operations on multi containers, we printed error for each
failed container, then printed an extra message for container
names, it seems redundant.

Addresses comments:
https://github.com/docker/docker/pull/15078#discussion_r47988449

Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>
Qiang Huang пре 9 година
родитељ
комит
e0dc4f27f6

+ 5 - 5
api/client/kill.go

@@ -2,6 +2,7 @@ package client
 
 
 import (
 import (
 	"fmt"
 	"fmt"
+	"strings"
 
 
 	Cli "github.com/docker/docker/cli"
 	Cli "github.com/docker/docker/cli"
 	flag "github.com/docker/docker/pkg/mflag"
 	flag "github.com/docker/docker/pkg/mflag"
@@ -17,17 +18,16 @@ func (cli *DockerCli) CmdKill(args ...string) error {
 
 
 	cmd.ParseFlags(args, true)
 	cmd.ParseFlags(args, true)
 
 
-	var errNames []string
+	var errs []string
 	for _, name := range cmd.Args() {
 	for _, name := range cmd.Args() {
 		if err := cli.client.ContainerKill(name, *signal); err != nil {
 		if err := cli.client.ContainerKill(name, *signal); err != nil {
-			fmt.Fprintf(cli.err, "%s\n", err)
-			errNames = append(errNames, name)
+			errs = append(errs, fmt.Sprintf("Failed to kill container (%s): %s", name, err))
 		} else {
 		} else {
 			fmt.Fprintf(cli.out, "%s\n", name)
 			fmt.Fprintf(cli.out, "%s\n", name)
 		}
 		}
 	}
 	}
-	if len(errNames) > 0 {
-		return fmt.Errorf("Error: failed to kill containers: %v", errNames)
+	if len(errs) > 0 {
+		return fmt.Errorf("%s", strings.Join(errs, "\n"))
 	}
 	}
 	return nil
 	return nil
 }
 }

+ 5 - 5
api/client/pause.go

@@ -2,6 +2,7 @@ package client
 
 
 import (
 import (
 	"fmt"
 	"fmt"
+	"strings"
 
 
 	Cli "github.com/docker/docker/cli"
 	Cli "github.com/docker/docker/cli"
 	flag "github.com/docker/docker/pkg/mflag"
 	flag "github.com/docker/docker/pkg/mflag"
@@ -16,17 +17,16 @@ func (cli *DockerCli) CmdPause(args ...string) error {
 
 
 	cmd.ParseFlags(args, true)
 	cmd.ParseFlags(args, true)
 
 
-	var errNames []string
+	var errs []string
 	for _, name := range cmd.Args() {
 	for _, name := range cmd.Args() {
 		if err := cli.client.ContainerPause(name); err != nil {
 		if err := cli.client.ContainerPause(name); err != nil {
-			fmt.Fprintf(cli.err, "%s\n", err)
-			errNames = append(errNames, name)
+			errs = append(errs, fmt.Sprintf("Failed to pause container (%s): %s", name, err))
 		} else {
 		} else {
 			fmt.Fprintf(cli.out, "%s\n", name)
 			fmt.Fprintf(cli.out, "%s\n", name)
 		}
 		}
 	}
 	}
-	if len(errNames) > 0 {
-		return fmt.Errorf("Error: failed to pause containers: %v", errNames)
+	if len(errs) > 0 {
+		return fmt.Errorf("%s", strings.Join(errs, "\n"))
 	}
 	}
 	return nil
 	return nil
 }
 }

+ 5 - 5
api/client/restart.go

@@ -2,6 +2,7 @@ package client
 
 
 import (
 import (
 	"fmt"
 	"fmt"
+	"strings"
 
 
 	Cli "github.com/docker/docker/cli"
 	Cli "github.com/docker/docker/cli"
 	flag "github.com/docker/docker/pkg/mflag"
 	flag "github.com/docker/docker/pkg/mflag"
@@ -17,17 +18,16 @@ func (cli *DockerCli) CmdRestart(args ...string) error {
 
 
 	cmd.ParseFlags(args, true)
 	cmd.ParseFlags(args, true)
 
 
-	var errNames []string
+	var errs []string
 	for _, name := range cmd.Args() {
 	for _, name := range cmd.Args() {
 		if err := cli.client.ContainerRestart(name, *nSeconds); err != nil {
 		if err := cli.client.ContainerRestart(name, *nSeconds); err != nil {
-			fmt.Fprintf(cli.err, "%s\n", err)
-			errNames = append(errNames, name)
+			errs = append(errs, fmt.Sprintf("Failed to kill container (%s): %s", name, err))
 		} else {
 		} else {
 			fmt.Fprintf(cli.out, "%s\n", name)
 			fmt.Fprintf(cli.out, "%s\n", name)
 		}
 		}
 	}
 	}
-	if len(errNames) > 0 {
-		return fmt.Errorf("Error: failed to restart containers: %v", errNames)
+	if len(errs) > 0 {
+		return fmt.Errorf("%s", strings.Join(errs, "\n"))
 	}
 	}
 	return nil
 	return nil
 }
 }

+ 4 - 5
api/client/rm.go

@@ -21,7 +21,7 @@ func (cli *DockerCli) CmdRm(args ...string) error {
 
 
 	cmd.ParseFlags(args, true)
 	cmd.ParseFlags(args, true)
 
 
-	var errNames []string
+	var errs []string
 	for _, name := range cmd.Args() {
 	for _, name := range cmd.Args() {
 		if name == "" {
 		if name == "" {
 			return fmt.Errorf("Container name cannot be empty")
 			return fmt.Errorf("Container name cannot be empty")
@@ -36,14 +36,13 @@ func (cli *DockerCli) CmdRm(args ...string) error {
 		}
 		}
 
 
 		if err := cli.client.ContainerRemove(options); err != nil {
 		if err := cli.client.ContainerRemove(options); err != nil {
-			fmt.Fprintf(cli.err, "%s\n", err)
-			errNames = append(errNames, name)
+			errs = append(errs, fmt.Sprintf("Failed to remove container (%s): %s", name, err))
 		} else {
 		} else {
 			fmt.Fprintf(cli.out, "%s\n", name)
 			fmt.Fprintf(cli.out, "%s\n", name)
 		}
 		}
 	}
 	}
-	if len(errNames) > 0 {
-		return fmt.Errorf("Error: failed to remove containers: %v", errNames)
+	if len(errs) > 0 {
+		return fmt.Errorf("%s", strings.Join(errs, "\n"))
 	}
 	}
 	return nil
 	return nil
 }
 }

+ 5 - 5
api/client/rmi.go

@@ -3,6 +3,7 @@ package client
 import (
 import (
 	"fmt"
 	"fmt"
 	"net/url"
 	"net/url"
+	"strings"
 
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types"
 	Cli "github.com/docker/docker/cli"
 	Cli "github.com/docker/docker/cli"
@@ -28,7 +29,7 @@ func (cli *DockerCli) CmdRmi(args ...string) error {
 		v.Set("noprune", "1")
 		v.Set("noprune", "1")
 	}
 	}
 
 
-	var errNames []string
+	var errs []string
 	for _, name := range cmd.Args() {
 	for _, name := range cmd.Args() {
 		options := types.ImageRemoveOptions{
 		options := types.ImageRemoveOptions{
 			ImageID:       name,
 			ImageID:       name,
@@ -38,8 +39,7 @@ func (cli *DockerCli) CmdRmi(args ...string) error {
 
 
 		dels, err := cli.client.ImageRemove(options)
 		dels, err := cli.client.ImageRemove(options)
 		if err != nil {
 		if err != nil {
-			fmt.Fprintf(cli.err, "%s\n", err)
-			errNames = append(errNames, name)
+			errs = append(errs, fmt.Sprintf("Failed to remove image (%s): %s", name, err))
 		} else {
 		} else {
 			for _, del := range dels {
 			for _, del := range dels {
 				if del.Deleted != "" {
 				if del.Deleted != "" {
@@ -50,8 +50,8 @@ func (cli *DockerCli) CmdRmi(args ...string) error {
 			}
 			}
 		}
 		}
 	}
 	}
-	if len(errNames) > 0 {
-		return fmt.Errorf("Error: failed to remove images: %v", errNames)
+	if len(errs) > 0 {
+		return fmt.Errorf("%s", strings.Join(errs, "\n"))
 	}
 	}
 	return nil
 	return nil
 }
 }

+ 5 - 5
api/client/stop.go

@@ -2,6 +2,7 @@ package client
 
 
 import (
 import (
 	"fmt"
 	"fmt"
+	"strings"
 
 
 	Cli "github.com/docker/docker/cli"
 	Cli "github.com/docker/docker/cli"
 	flag "github.com/docker/docker/pkg/mflag"
 	flag "github.com/docker/docker/pkg/mflag"
@@ -19,17 +20,16 @@ func (cli *DockerCli) CmdStop(args ...string) error {
 
 
 	cmd.ParseFlags(args, true)
 	cmd.ParseFlags(args, true)
 
 
-	var errNames []string
+	var errs []string
 	for _, name := range cmd.Args() {
 	for _, name := range cmd.Args() {
 		if err := cli.client.ContainerStop(name, *nSeconds); err != nil {
 		if err := cli.client.ContainerStop(name, *nSeconds); err != nil {
-			fmt.Fprintf(cli.err, "%s\n", err)
-			errNames = append(errNames, name)
+			errs = append(errs, fmt.Sprintf("Failed to stop container (%s): %s", name, err))
 		} else {
 		} else {
 			fmt.Fprintf(cli.out, "%s\n", name)
 			fmt.Fprintf(cli.out, "%s\n", name)
 		}
 		}
 	}
 	}
-	if len(errNames) > 0 {
-		return fmt.Errorf("Error: failed to stop containers: %v", errNames)
+	if len(errs) > 0 {
+		return fmt.Errorf("%s", strings.Join(errs, "\n"))
 	}
 	}
 	return nil
 	return nil
 }
 }

+ 5 - 5
api/client/unpause.go

@@ -2,6 +2,7 @@ package client
 
 
 import (
 import (
 	"fmt"
 	"fmt"
+	"strings"
 
 
 	Cli "github.com/docker/docker/cli"
 	Cli "github.com/docker/docker/cli"
 	flag "github.com/docker/docker/pkg/mflag"
 	flag "github.com/docker/docker/pkg/mflag"
@@ -16,17 +17,16 @@ func (cli *DockerCli) CmdUnpause(args ...string) error {
 
 
 	cmd.ParseFlags(args, true)
 	cmd.ParseFlags(args, true)
 
 
-	var errNames []string
+	var errs []string
 	for _, name := range cmd.Args() {
 	for _, name := range cmd.Args() {
 		if err := cli.client.ContainerUnpause(name); err != nil {
 		if err := cli.client.ContainerUnpause(name); err != nil {
-			fmt.Fprintf(cli.err, "%s\n", err)
-			errNames = append(errNames, name)
+			errs = append(errs, fmt.Sprintf("Failed to unpause container (%s): %s", name, err))
 		} else {
 		} else {
 			fmt.Fprintf(cli.out, "%s\n", name)
 			fmt.Fprintf(cli.out, "%s\n", name)
 		}
 		}
 	}
 	}
-	if len(errNames) > 0 {
-		return fmt.Errorf("Error: failed to unpause containers: %v", errNames)
+	if len(errs) > 0 {
+		return fmt.Errorf("%s", strings.Join(errs, "\n"))
 	}
 	}
 	return nil
 	return nil
 }
 }

+ 5 - 5
api/client/update.go

@@ -2,6 +2,7 @@ package client
 
 
 import (
 import (
 	"fmt"
 	"fmt"
+	"strings"
 
 
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/container"
 	Cli "github.com/docker/docker/cli"
 	Cli "github.com/docker/docker/cli"
@@ -86,18 +87,17 @@ func (cli *DockerCli) CmdUpdate(args ...string) error {
 	}
 	}
 
 
 	names := cmd.Args()
 	names := cmd.Args()
-	var errNames []string
+	var errs []string
 	for _, name := range names {
 	for _, name := range names {
 		if err := cli.client.ContainerUpdate(name, hostConfig); err != nil {
 		if err := cli.client.ContainerUpdate(name, hostConfig); err != nil {
-			fmt.Fprintf(cli.err, "%s\n", err)
-			errNames = append(errNames, name)
+			errs = append(errs, fmt.Sprintf("Failed to update container (%s): %s", name, err))
 		} else {
 		} else {
 			fmt.Fprintf(cli.out, "%s\n", name)
 			fmt.Fprintf(cli.out, "%s\n", name)
 		}
 		}
 	}
 	}
 
 
-	if len(errNames) > 0 {
-		return fmt.Errorf("Error: failed to update resources of containers: %v", errNames)
+	if len(errs) > 0 {
+		return fmt.Errorf("%s", strings.Join(errs, "\n"))
 	}
 	}
 
 
 	return nil
 	return nil

+ 5 - 5
api/client/wait.go

@@ -2,6 +2,7 @@ package client
 
 
 import (
 import (
 	"fmt"
 	"fmt"
+	"strings"
 
 
 	Cli "github.com/docker/docker/cli"
 	Cli "github.com/docker/docker/cli"
 	flag "github.com/docker/docker/pkg/mflag"
 	flag "github.com/docker/docker/pkg/mflag"
@@ -18,18 +19,17 @@ func (cli *DockerCli) CmdWait(args ...string) error {
 
 
 	cmd.ParseFlags(args, true)
 	cmd.ParseFlags(args, true)
 
 
-	var errNames []string
+	var errs []string
 	for _, name := range cmd.Args() {
 	for _, name := range cmd.Args() {
 		status, err := cli.client.ContainerWait(name)
 		status, err := cli.client.ContainerWait(name)
 		if err != nil {
 		if err != nil {
-			fmt.Fprintf(cli.err, "%s\n", err)
-			errNames = append(errNames, name)
+			errs = append(errs, fmt.Sprintf("Failed to wait container (%s): %s", name, err))
 		} else {
 		} else {
 			fmt.Fprintf(cli.out, "%d\n", status)
 			fmt.Fprintf(cli.out, "%d\n", status)
 		}
 		}
 	}
 	}
-	if len(errNames) > 0 {
-		return fmt.Errorf("Error: failed to wait containers: %v", errNames)
+	if len(errs) > 0 {
+		return fmt.Errorf("%s", strings.Join(errs, "\n"))
 	}
 	}
 	return nil
 	return nil
 }
 }

+ 2 - 2
integration-cli/docker_cli_rm_test.go

@@ -73,8 +73,8 @@ func (s *DockerSuite) TestRmContainerOrphaning(c *check.C) {
 func (s *DockerSuite) TestRmInvalidContainer(c *check.C) {
 func (s *DockerSuite) TestRmInvalidContainer(c *check.C) {
 	if out, _, err := dockerCmdWithError("rm", "unknown"); err == nil {
 	if out, _, err := dockerCmdWithError("rm", "unknown"); err == nil {
 		c.Fatal("Expected error on rm unknown container, got none")
 		c.Fatal("Expected error on rm unknown container, got none")
-	} else if !strings.Contains(out, "failed to remove containers") {
-		c.Fatalf("Expected output to contain 'failed to remove containers', got %q", out)
+	} else if !strings.Contains(out, "Failed to remove container") {
+		c.Fatalf("Expected output to contain 'Failed to remove container', got %q", out)
 	}
 	}
 }
 }