From 5ba203e73a541935128e29a3babf810aac8ee004 Mon Sep 17 00:00:00 2001
From: allencloud <allen.sun@daocloud.io>
Date: Mon, 19 Sep 2016 16:40:44 +0800
Subject: [PATCH] validate service parameter in client side to avoid api call

Signed-off-by: allencloud <allen.sun@daocloud.io>
---
 cli/command/service/scale.go                  | 24 ++++----
 .../docker_cli_service_scale_test.go          | 57 +++++++++++++++++++
 2 files changed, 71 insertions(+), 10 deletions(-)
 create mode 100644 integration-cli/docker_cli_service_scale_test.go

diff --git a/cli/command/service/scale.go b/cli/command/service/scale.go
index 2e2982db43..61b73bc354 100644
--- a/cli/command/service/scale.go
+++ b/cli/command/service/scale.go
@@ -46,9 +46,17 @@ func runScale(dockerCli *command.DockerCli, args []string) error {
 	var errors []string
 	for _, arg := range args {
 		parts := strings.SplitN(arg, "=", 2)
-		serviceID, scale := parts[0], parts[1]
+		serviceID, scaleStr := parts[0], parts[1]
+
+		// validate input arg scale number
+		scale, err := strconv.ParseUint(scaleStr, 10, 64)
+		if err != nil {
+			errors = append(errors, fmt.Sprintf("%s: invalid replicas value %s: %v", serviceID, scaleStr, err))
+			continue
+		}
+
 		if err := runServiceScale(dockerCli, serviceID, scale); err != nil {
-			errors = append(errors, fmt.Sprintf("%s: %s", serviceID, err.Error()))
+			errors = append(errors, fmt.Sprintf("%s: %v", serviceID, err))
 		}
 	}
 
@@ -58,12 +66,11 @@ func runScale(dockerCli *command.DockerCli, args []string) error {
 	return fmt.Errorf(strings.Join(errors, "\n"))
 }
 
-func runServiceScale(dockerCli *command.DockerCli, serviceID string, scale string) error {
+func runServiceScale(dockerCli *command.DockerCli, serviceID string, scale uint64) error {
 	client := dockerCli.Client()
 	ctx := context.Background()
 
 	service, _, err := client.ServiceInspectWithRaw(ctx, serviceID)
-
 	if err != nil {
 		return err
 	}
@@ -72,17 +79,14 @@ func runServiceScale(dockerCli *command.DockerCli, serviceID string, scale strin
 	if serviceMode.Replicated == nil {
 		return fmt.Errorf("scale can only be used with replicated mode")
 	}
-	uintScale, err := strconv.ParseUint(scale, 10, 64)
-	if err != nil {
-		return fmt.Errorf("invalid replicas value %s: %s", scale, err.Error())
-	}
-	serviceMode.Replicated.Replicas = &uintScale
+
+	serviceMode.Replicated.Replicas = &scale
 
 	err = client.ServiceUpdate(ctx, service.ID, service.Version, service.Spec, types.ServiceUpdateOptions{})
 	if err != nil {
 		return err
 	}
 
-	fmt.Fprintf(dockerCli.Out(), "%s scaled to %s\n", serviceID, scale)
+	fmt.Fprintf(dockerCli.Out(), "%s scaled to %d\n", serviceID, scale)
 	return nil
 }
diff --git a/integration-cli/docker_cli_service_scale_test.go b/integration-cli/docker_cli_service_scale_test.go
new file mode 100644
index 0000000000..29cca2358d
--- /dev/null
+++ b/integration-cli/docker_cli_service_scale_test.go
@@ -0,0 +1,57 @@
+// +build !windows
+
+package main
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/docker/docker/pkg/integration/checker"
+	"github.com/go-check/check"
+)
+
+func (s *DockerSwarmSuite) TestServiceScale(c *check.C) {
+	d := s.AddDaemon(c, true, true)
+
+	service1Name := "TestService1"
+	service1Args := append([]string{"service", "create", "--name", service1Name, defaultSleepImage}, sleepCommandForDaemonPlatform()...)
+
+	// global mode
+	service2Name := "TestService2"
+	service2Args := append([]string{"service", "create", "--name", service2Name, "--mode=global", defaultSleepImage}, sleepCommandForDaemonPlatform()...)
+
+	// Create services
+	out, err := d.Cmd(service1Args...)
+	c.Assert(err, checker.IsNil)
+
+	out, err = d.Cmd(service2Args...)
+	c.Assert(err, checker.IsNil)
+
+	out, err = d.Cmd("service", "scale", "TestService1=2")
+	c.Assert(err, checker.IsNil)
+
+	out, err = d.Cmd("service", "scale", "TestService1=foobar")
+	c.Assert(err, checker.NotNil)
+
+	str := fmt.Sprintf("%s: invalid replicas value %s", service1Name, "foobar")
+	if !strings.Contains(out, str) {
+		c.Errorf("got: %s, expected has sub string: %s", out, str)
+	}
+
+	out, err = d.Cmd("service", "scale", "TestService1=-1")
+	c.Assert(err, checker.NotNil)
+
+	str = fmt.Sprintf("%s: invalid replicas value %s", service1Name, "-1")
+	if !strings.Contains(out, str) {
+		c.Errorf("got: %s, expected has sub string: %s", out, str)
+	}
+
+	// TestService2 is a global mode
+	out, err = d.Cmd("service", "scale", "TestService2=2")
+	c.Assert(err, checker.NotNil)
+
+	str = fmt.Sprintf("%s: scale can only be used with replicated mode\n", service2Name)
+	if out != str {
+		c.Errorf("got: %s, expected: %s", out, str)
+	}
+}