Remove dead code from node update.

Also share mode code between update commands
and use flag constants

Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit cacaeab9db51f42c33872c6cce1adc3e55f1c9c5)

api/client/node/accept.go

@@ -7,34 +7,25 @@ import (
 	"github.com/docker/docker/cli"
 	"github.com/docker/engine-api/types/swarm"
 	"github.com/spf13/cobra"
-	"github.com/spf13/pflag"
 )
 
 func newAcceptCommand(dockerCli *client.DockerCli) *cobra.Command {
-	var flags *pflag.FlagSet
+	return &cobra.Command{
-
-	cmd := &cobra.Command{
 		Use:   "accept NODE [NODE...]",
 		Short: "Accept a node in the swarm",
 		Args:  cli.RequiresMinArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return runAccept(dockerCli, flags, args)
+			return runAccept(dockerCli, args)
 		},
 	}
-
-	flags = cmd.Flags()
-	return cmd
 }
 
-func runAccept(dockerCli *client.DockerCli, flags *pflag.FlagSet, args []string) error {
+func runAccept(dockerCli *client.DockerCli, nodes []string) error {
-	for _, id := range args {
+	accept := func(node *swarm.Node) {
-		if err := runUpdate(dockerCli, id, func(node *swarm.Node) {
 		node.Spec.Membership = swarm.NodeMembershipAccepted
-		}); err != nil {
-			return err
 	}
-		fmt.Fprintf(dockerCli.Out(), "Node %s accepted in the swarm.\n", id)
+	success := func(nodeID string) {
+		fmt.Fprintf(dockerCli.Out(), "Node %s accepted in the swarm.\n", nodeID)
 	}
-
+	return updateNodes(dockerCli, nodes, accept, success)
-	return nil
 }

api/client/node/demote.go

@@ -7,34 +7,25 @@ import (
 	"github.com/docker/docker/cli"
 	"github.com/docker/engine-api/types/swarm"
 	"github.com/spf13/cobra"
-	"github.com/spf13/pflag"
 )
 
 func newDemoteCommand(dockerCli *client.DockerCli) *cobra.Command {
-	var flags *pflag.FlagSet
+	return &cobra.Command{
-
-	cmd := &cobra.Command{
 		Use:   "demote NODE [NODE...]",
 		Short: "Demote a node from manager in the swarm",
 		Args:  cli.RequiresMinArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return runDemote(dockerCli, flags, args)
+			return runDemote(dockerCli, args)
 		},
 	}
-
-	flags = cmd.Flags()
-	return cmd
 }
 
-func runDemote(dockerCli *client.DockerCli, flags *pflag.FlagSet, args []string) error {
+func runDemote(dockerCli *client.DockerCli, nodes []string) error {
-	for _, id := range args {
+	demote := func(node *swarm.Node) {
-		if err := runUpdate(dockerCli, id, func(node *swarm.Node) {
 		node.Spec.Role = swarm.NodeRoleWorker
-		}); err != nil {
-			return err
 	}
-		fmt.Fprintf(dockerCli.Out(), "Manager %s demoted in the swarm.\n", id)
+	success := func(nodeID string) {
+		fmt.Fprintf(dockerCli.Out(), "Manager %s demoted in the swarm.\n", nodeID)
 	}
-
+	return updateNodes(dockerCli, nodes, demote, success)
-	return nil
 }

api/client/node/promote.go

@@ -7,34 +7,25 @@ import (
 	"github.com/docker/docker/cli"
 	"github.com/docker/engine-api/types/swarm"
 	"github.com/spf13/cobra"
-	"github.com/spf13/pflag"
 )
 
 func newPromoteCommand(dockerCli *client.DockerCli) *cobra.Command {
-	var flags *pflag.FlagSet
+	return &cobra.Command{
-
-	cmd := &cobra.Command{
 		Use:   "promote NODE [NODE...]",
 		Short: "Promote a node to a manager in the swarm",
 		Args:  cli.RequiresMinArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return runPromote(dockerCli, flags, args)
+			return runPromote(dockerCli, args)
 		},
 	}
-
-	flags = cmd.Flags()
-	return cmd
 }
 
-func runPromote(dockerCli *client.DockerCli, flags *pflag.FlagSet, args []string) error {
+func runPromote(dockerCli *client.DockerCli, nodes []string) error {
-	for _, id := range args {
+	promote := func(node *swarm.Node) {
-		if err := runUpdate(dockerCli, id, func(node *swarm.Node) {
 		node.Spec.Role = swarm.NodeRoleManager
-		}); err != nil {
-			return err
 	}
-		fmt.Fprintf(dockerCli.Out(), "Node %s promoted to a manager in the swarm.\n", id)
+	success := func(nodeID string) {
+		fmt.Fprintf(dockerCli.Out(), "Node %s promoted to a manager in the swarm.\n", nodeID)
 	}
-
+	return updateNodes(dockerCli, nodes, promote, success)
-	return nil
 }

api/client/node/update.go

@@ -5,7 +5,6 @@ import (
 
 	"github.com/docker/docker/api/client"
 	"github.com/docker/docker/cli"
-	runconfigopts "github.com/docker/docker/runconfig/opts"
 	"github.com/docker/engine-api/types/swarm"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
@@ -14,32 +13,35 @@ import (
 
 func newUpdateCommand(dockerCli *client.DockerCli) *cobra.Command {
 	var opts nodeOptions
-	var flags *pflag.FlagSet
 
 	cmd := &cobra.Command{
 		Use:   "update [OPTIONS] NODE",
 		Short: "Update a node",
 		Args:  cli.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			if err := runUpdate(dockerCli, args[0], mergeNodeUpdate(flags)); err != nil {
+			return runUpdate(dockerCli, cmd.Flags(), args[0])
-				return err
-			}
-			fmt.Fprintln(dockerCli.Out(), args[0])
-			return nil
 		},
 	}
 
-	flags = cmd.Flags()
+	flags := cmd.Flags()
-	flags.StringVar(&opts.role, "role", "", "Role of the node (worker/manager)")
+	flags.StringVar(&opts.role, flagRole, "", "Role of the node (worker/manager)")
-	flags.StringVar(&opts.membership, "membership", "", "Membership of the node (accepted/rejected)")
+	flags.StringVar(&opts.membership, flagMembership, "", "Membership of the node (accepted/rejected)")
-	flags.StringVar(&opts.availability, "availability", "", "Availability of the node (active/pause/drain)")
+	flags.StringVar(&opts.availability, flagAvailability, "", "Availability of the node (active/pause/drain)")
 	return cmd
 }
 
-func runUpdate(dockerCli *client.DockerCli, nodeID string, mergeNode func(node *swarm.Node)) error {
+func runUpdate(dockerCli *client.DockerCli, flags *pflag.FlagSet, nodeID string) error {
+	success := func(_ string) {
+		fmt.Fprintln(dockerCli.Out(), nodeID)
+	}
+	return updateNodes(dockerCli, []string{nodeID}, mergeNodeUpdate(flags), success)
+}
+
+func updateNodes(dockerCli *client.DockerCli, nodes []string, mergeNode func(node *swarm.Node), success func(nodeID string)) error {
 	client := dockerCli.Client()
 	ctx := context.Background()
 
+	for _, nodeID := range nodes {
 		node, err := client.NodeInspect(ctx, nodeID)
 		if err != nil {
 			return err
@@ -50,54 +52,32 @@ func runUpdate(dockerCli *client.DockerCli, nodeID string, mergeNode func(node *
 		if err != nil {
 			return err
 		}
-
+		success(nodeID)
+	}
 	return nil
 }
 
 func mergeNodeUpdate(flags *pflag.FlagSet) func(*swarm.Node) {
 	return func(node *swarm.Node) {
-		mergeString := func(flag string, field *string) {
-			if flags.Changed(flag) {
-				*field, _ = flags.GetString(flag)
-			}
-		}
-
-		mergeRole := func(flag string, field *swarm.NodeRole) {
-			if flags.Changed(flag) {
-				str, _ := flags.GetString(flag)
-				*field = swarm.NodeRole(str)
-			}
-		}
-
-		mergeMembership := func(flag string, field *swarm.NodeMembership) {
-			if flags.Changed(flag) {
-				str, _ := flags.GetString(flag)
-				*field = swarm.NodeMembership(str)
-			}
-		}
-
-		mergeAvailability := func(flag string, field *swarm.NodeAvailability) {
-			if flags.Changed(flag) {
-				str, _ := flags.GetString(flag)
-				*field = swarm.NodeAvailability(str)
-			}
-		}
-
-		mergeLabels := func(flag string, field *map[string]string) {
-			if flags.Changed(flag) {
-				values, _ := flags.GetStringSlice(flag)
-				for key, value := range runconfigopts.ConvertKVStringsToMap(values) {
-					(*field)[key] = value
-				}
-			}
-		}
-
 		spec := &node.Spec
-		mergeString("name", &spec.Name)
+
-		// TODO: setting labels is not working
+		if flags.Changed(flagRole) {
-		mergeLabels("label", &spec.Labels)
+			str, _ := flags.GetString(flagRole)
-		mergeRole("role", &spec.Role)
+			spec.Role = swarm.NodeRole(str)
-		mergeMembership("membership", &spec.Membership)
+		}
-		mergeAvailability("availability", &spec.Availability)
+		if flags.Changed(flagMembership) {
+			str, _ := flags.GetString(flagMembership)
+			spec.Membership = swarm.NodeMembership(str)
+		}
+		if flags.Changed(flagAvailability) {
+			str, _ := flags.GetString(flagAvailability)
+			spec.Availability = swarm.NodeAvailability(str)
 		}
 	}
+}
+
+const (
+	flagRole         = "role"
+	flagMembership   = "membership"
+	flagAvailability = "availability"
+)

api/client/service/update.go

@@ -212,7 +212,6 @@ func anyChanged(flags *pflag.FlagSet, fields ...string) bool {
 	return false
 }
 
-
 // TODO: should this override by destination path, or does swarm handle that?
 func updateMounts(flags *pflag.FlagSet, mounts *[]swarm.Mount) {
 	if !flags.Changed(flagMount) {