Merge pull request #23678 from dnephin/share-more-node-update-code

Some cleanup for new CLI commands

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
-
-	cmd := &cobra.Command{
+	return &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 {
-	for _, id := range args {
-		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)
+func runAccept(dockerCli *client.DockerCli, nodes []string) error {
+	accept := func(node *swarm.Node) {
+		node.Spec.Membership = swarm.NodeMembershipAccepted
 	}
-
-	return nil
+	success := func(nodeID string) {
+		fmt.Fprintf(dockerCli.Out(), "Node %s accepted in the swarm.\n", nodeID)
+	}
+	return updateNodes(dockerCli, nodes, accept, success)
 }

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
-
-	cmd := &cobra.Command{
+	return &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 {
-	for _, id := range args {
-		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)
+func runDemote(dockerCli *client.DockerCli, nodes []string) error {
+	demote := func(node *swarm.Node) {
+		node.Spec.Role = swarm.NodeRoleWorker
 	}
-
-	return nil
+	success := func(nodeID string) {
+		fmt.Fprintf(dockerCli.Out(), "Manager %s demoted in the swarm.\n", nodeID)
+	}
+	return updateNodes(dockerCli, nodes, demote, success)
 }

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
-
-	cmd := &cobra.Command{
+	return &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 {
-	for _, id := range args {
-		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)
+func runPromote(dockerCli *client.DockerCli, nodes []string) error {
+	promote := func(node *swarm.Node) {
+		node.Spec.Role = swarm.NodeRoleManager
 	}
-
-	return nil
+	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)
 }

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,90 +13,71 @@ 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 err
-			}
-			fmt.Fprintln(dockerCli.Out(), args[0])
-			return nil
+			return runUpdate(dockerCli, cmd.Flags(), args[0])
 		},
 	}
 
-	flags = cmd.Flags()
-	flags.StringVar(&opts.role, "role", "", "Role of the node (worker/manager)")
-	flags.StringVar(&opts.membership, "membership", "", "Membership of the node (accepted/rejected)")
-	flags.StringVar(&opts.availability, "availability", "", "Availability of the node (active/pause/drain)")
+	flags := cmd.Flags()
+	flags.StringVar(&opts.role, flagRole, "", "Role of the node (worker/manager)")
+	flags.StringVar(&opts.membership, flagMembership, "", "Membership of the node (accepted/rejected)")
+	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()
 
-	node, err := client.NodeInspect(ctx, nodeID)
-	if err != nil {
-		return err
-	}
+	for _, nodeID := range nodes {
+		node, err := client.NodeInspect(ctx, nodeID)
+		if err != nil {
+			return err
+		}
 
-	mergeNode(&node)
-	err = client.NodeUpdate(ctx, node.ID, node.Version, node.Spec)
-	if err != nil {
-		return err
+		mergeNode(&node)
+		err = client.NodeUpdate(ctx, node.ID, node.Version, node.Spec)
+		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)
-			}
-		}
+		spec := &node.Spec
 
-		mergeMembership := func(flag string, field *swarm.NodeMembership) {
-			if flags.Changed(flag) {
-				str, _ := flags.GetString(flag)
-				*field = swarm.NodeMembership(str)
-			}
+		if flags.Changed(flagRole) {
+			str, _ := flags.GetString(flagRole)
+			spec.Role = swarm.NodeRole(str)
 		}
-
-		mergeAvailability := func(flag string, field *swarm.NodeAvailability) {
-			if flags.Changed(flag) {
-				str, _ := flags.GetString(flag)
-				*field = swarm.NodeAvailability(str)
-			}
+		if flags.Changed(flagMembership) {
+			str, _ := flags.GetString(flagMembership)
+			spec.Membership = swarm.NodeMembership(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
-				}
-			}
+		if flags.Changed(flagAvailability) {
+			str, _ := flags.GetString(flagAvailability)
+			spec.Availability = swarm.NodeAvailability(str)
 		}
-
-		spec := &node.Spec
-		mergeString("name", &spec.Name)
-		// TODO: setting labels is not working
-		mergeLabels("label", &spec.Labels)
-		mergeRole("role", &spec.Role)
-		mergeMembership("membership", &spec.Membership)
-		mergeAvailability("availability", &spec.Availability)
 	}
 }
+
+const (
+	flagRole         = "role"
+	flagMembership   = "membership"
+	flagAvailability = "availability"
+)

api/client/service/opts.go

@@ -438,14 +438,14 @@ func addServiceFlags(cmd *cobra.Command, opts *serviceOptions) {
 
 	flags.VarP(&opts.env, "env", "e", "Set environment variables")
 	flags.StringVarP(&opts.workdir, "workdir", "w", "", "Working directory inside the container")
-	flags.StringVarP(&opts.user, "user", "u", "", "Username or UID")
+	flags.StringVarP(&opts.user, flagUser, "u", "", "Username or UID")
 	flags.VarP(&opts.mounts, flagMount, "m", "Attach a mount to the service")
 
 	flags.Var(&opts.resources.limitCPU, flagLimitCPU, "Limit CPUs")
 	flags.Var(&opts.resources.limitMemBytes, flagLimitMemory, "Limit Memory")
 	flags.Var(&opts.resources.resCPU, flagReserveCPU, "Reserve CPUs")
 	flags.Var(&opts.resources.resMemBytes, flagReserveMemory, "Reserve Memory")
-	flags.Var(&opts.stopGrace, "stop-grace-period", "Time to wait before force killing a container")
+	flags.Var(&opts.stopGrace, flagStopGracePeriod, "Time to wait before force killing a container")
 
 	flags.StringVar(&opts.mode, flagMode, "replicated", "Service mode (replicated or global)")
 	flags.Var(&opts.replicas, flagReplicas, "Number of tasks")
@@ -467,22 +467,24 @@ func addServiceFlags(cmd *cobra.Command, opts *serviceOptions) {
 
 const (
 	flagConstraint         = "constraint"
-	flagName               = "name"
+	flagEndpointMode       = "endpoint-mode"
 	flagLabel              = "label"
 	flagLimitCPU           = "limit-cpu"
 	flagLimitMemory        = "limit-memory"
-	flagReserveCPU         = "reserve-cpu"
-	flagReserveMemory      = "reserve-memory"
-	flagMount              = "mount"
 	flagMode               = "mode"
-	flagReplicas           = "replicas"
-	flagPublish            = "publish"
+	flagMount              = "mount"
+	flagName               = "name"
 	flagNetwork            = "network"
+	flagPublish            = "publish"
+	flagReplicas           = "replicas"
+	flagReserveCPU         = "reserve-cpu"
+	flagReserveMemory      = "reserve-memory"
 	flagRestartCondition   = "restart-condition"
 	flagRestartDelay       = "restart-delay"
 	flagRestartMaxAttempts = "restart-max-attempts"
 	flagRestartWindow      = "restart-window"
-	flagEndpointMode       = "endpoint-mode"
-	flagUpdateParallelism  = "update-parallelism"
+	flagStopGracePeriod    = "stop-grace-period"
 	flagUpdateDelay        = "update-delay"
+	flagUpdateParallelism  = "update-parallelism"
+	flagUser               = "user"
 )

api/client/service/update.go

@@ -18,18 +18,17 @@ import (
 
 func newUpdateCommand(dockerCli *client.DockerCli) *cobra.Command {
 	opts := newServiceOptions()
-	var flags *pflag.FlagSet
 
 	cmd := &cobra.Command{
 		Use:   "update [OPTIONS] SERVICE",
 		Short: "Update a service",
 		Args:  cli.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return runUpdate(dockerCli, flags, args[0])
+			return runUpdate(dockerCli, cmd.Flags(), args[0])
 		},
 	}
 
-	flags = cmd.Flags()
+	flags := cmd.Flags()
 	flags.String("image", "", "Service image tag")
 	flags.StringSlice("command", []string{}, "Service command")
 	flags.StringSlice("arg", []string{}, "Service command args")
@@ -112,6 +111,14 @@ func updateService(spec *swarm.ServiceSpec, flags *pflag.FlagSet) error {
 
 	cspec := &spec.TaskTemplate.ContainerSpec
 	task := &spec.TaskTemplate
+
+	taskResources := func() *swarm.ResourceRequirements {
+		if task.Resources == nil {
+			task.Resources = &swarm.ResourceRequirements{}
+		}
+		return task.Resources
+	}
+
 	updateString(flagName, &spec.Name)
 	updateLabels(flags, &spec.Labels)
 	updateString("image", &cspec.Image)
@@ -119,30 +126,24 @@ func updateService(spec *swarm.ServiceSpec, flags *pflag.FlagSet) error {
 	updateSlice("arg", &cspec.Command)
 	updateListOpts("env", &cspec.Env)
 	updateString("workdir", &cspec.Dir)
-	updateString("user", &cspec.User)
+	updateString(flagUser, &cspec.User)
 	updateMounts(flags, &cspec.Mounts)
 
 	if flags.Changed(flagLimitCPU) || flags.Changed(flagLimitMemory) {
-		if task.Resources == nil {
-			task.Resources = &swarm.ResourceRequirements{}
-		}
-		task.Resources.Limits = &swarm.Resources{}
+		taskResources().Limits = &swarm.Resources{}
 		updateInt64Value(flagLimitCPU, &task.Resources.Limits.NanoCPUs)
 		updateInt64Value(flagLimitMemory, &task.Resources.Limits.MemoryBytes)
 
 	}
 	if flags.Changed(flagReserveCPU) || flags.Changed(flagReserveMemory) {
-		if task.Resources == nil {
-			task.Resources = &swarm.ResourceRequirements{}
-		}
-		task.Resources.Reservations = &swarm.Resources{}
+		taskResources().Reservations = &swarm.Resources{}
 		updateInt64Value(flagReserveCPU, &task.Resources.Reservations.NanoCPUs)
 		updateInt64Value(flagReserveMemory, &task.Resources.Reservations.MemoryBytes)
 	}
 
-	updateDurationOpt("stop-grace-period", cspec.StopGracePeriod)
+	updateDurationOpt(flagStopGracePeriod, cspec.StopGracePeriod)
 
-	if flags.Changed(flagRestartCondition) || flags.Changed(flagRestartDelay) || flags.Changed(flagRestartMaxAttempts) || flags.Changed(flagRestartWindow) {
+	if anyChanged(flags, flagRestartCondition, flagRestartDelay, flagRestartMaxAttempts, flagRestartWindow) {
 		if task.RestartPolicy == nil {
 			task.RestartPolicy = &swarm.RestartPolicy{}
 		}
@@ -165,7 +166,7 @@ func updateService(spec *swarm.ServiceSpec, flags *pflag.FlagSet) error {
 		return err
 	}
 
-	if flags.Changed(flagUpdateParallelism) || flags.Changed(flagUpdateDelay) {
+	if anyChanged(flags, flagUpdateParallelism, flagUpdateDelay) {
 		if spec.UpdateConfig == nil {
 			spec.UpdateConfig = &swarm.UpdateConfig{}
 		}
@@ -202,6 +203,15 @@ func updateLabels(flags *pflag.FlagSet, field *map[string]string) {
 	*field = localLabels
 }
 
+func anyChanged(flags *pflag.FlagSet, fields ...string) bool {
+	for _, flag := range fields {
+		if flags.Changed(flag) {
+			return true
+		}
+	}
+	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) {

api/client/swarm/init.go

@@ -20,7 +20,6 @@ type initOptions struct {
 }
 
 func newInitCommand(dockerCli *client.DockerCli) *cobra.Command {
-	var flags *pflag.FlagSet
 	opts := initOptions{
 		listenAddr: NewListenAddrOption(),
 		autoAccept: NewAutoAcceptOption(),
@@ -31,12 +30,12 @@ func newInitCommand(dockerCli *client.DockerCli) *cobra.Command {
 		Short: "Initialize a Swarm",
 		Args:  cli.NoArgs,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return runInit(dockerCli, flags, opts)
+			return runInit(dockerCli, cmd.Flags(), opts)
 		},
 	}
 
-	flags = cmd.Flags()
-	flags.Var(&opts.listenAddr, flagListenAddr, "Listen address")
+	flags := cmd.Flags()
+	flags.Var(&opts.listenAddr, "listen-addr", "Listen address")
 	flags.Var(&opts.autoAccept, flagAutoAccept, "Auto acceptance policy (worker, manager, or none)")
 	flags.StringVar(&opts.secret, flagSecret, "", "Set secret value needed to accept nodes into cluster")
 	flags.BoolVar(&opts.forceNewCluster, "force-new-cluster", false, "Force create a new cluster from current state.")
@@ -52,7 +51,7 @@ func runInit(dockerCli *client.DockerCli, flags *pflag.FlagSet, opts initOptions
 		ForceNewCluster: opts.forceNewCluster,
 	}
 
-	if flags.Changed("secret") {
+	if flags.Changed(flagSecret) {
 		req.Spec.AcceptancePolicy.Policies = opts.autoAccept.Policies(&opts.secret)
 	} else {
 		req.Spec.AcceptancePolicy.Policies = opts.autoAccept.Policies(nil)

api/client/swarm/join.go

@@ -36,7 +36,7 @@ func newJoinCommand(dockerCli *client.DockerCli) *cobra.Command {
 	flags := cmd.Flags()
 	flags.Var(&opts.listenAddr, flagListenAddr, "Listen address")
 	flags.BoolVar(&opts.manager, "manager", false, "Try joining as a manager.")
-	flags.StringVar(&opts.secret, "secret", "", "Secret for node acceptance")
+	flags.StringVar(&opts.secret, flagSecret, "", "Secret for node acceptance")
 	flags.StringVar(&opts.CACertHash, "ca-hash", "", "Hash of the Root Certificate Authority certificate used for trusted join")
 	return cmd
 }

api/client/swarm/leave.go

@@ -39,6 +39,6 @@ func runLeave(dockerCli *client.DockerCli, opts leaveOptions) error {
 		return err
 	}
 
-	fmt.Fprintln(dockerCli.Out(), "Node left the default swarm.")
+	fmt.Fprintln(dockerCli.Out(), "Node left the swarm.")
 	return nil
 }

api/client/swarm/update.go

@@ -23,23 +23,22 @@ type updateOptions struct {
 
 func newUpdateCommand(dockerCli *client.DockerCli) *cobra.Command {
 	opts := updateOptions{autoAccept: NewAutoAcceptOption()}
-	var flags *pflag.FlagSet
 
 	cmd := &cobra.Command{
 		Use:   "update",
 		Short: "Update the Swarm",
 		Args:  cli.NoArgs,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return runUpdate(dockerCli, flags, opts)
+			return runUpdate(dockerCli, cmd.Flags(), opts)
 		},
 	}
 
-	flags = cmd.Flags()
-	flags.Var(&opts.autoAccept, "auto-accept", "Auto acceptance policy (worker, manager or none)")
-	flags.StringVar(&opts.secret, "secret", "", "Set secret value needed to accept nodes into cluster")
-	flags.Int64Var(&opts.taskHistoryLimit, "task-history-limit", 10, "Task history retention limit")
-	flags.DurationVar(&opts.dispatcherHeartbeat, "dispatcher-heartbeat", time.Duration(5*time.Second), "Dispatcher heartbeat period")
-	flags.DurationVar(&opts.nodeCertExpiry, "cert-expiry", time.Duration(90*24*time.Hour), "Validity period for node certificates")
+	flags := cmd.Flags()
+	flags.Var(&opts.autoAccept, flagAutoAccept, "Auto acceptance policy (worker, manager or none)")
+	flags.StringVar(&opts.secret, flagSecret, "", "Set secret value needed to accept nodes into cluster")
+	flags.Int64Var(&opts.taskHistoryLimit, flagTaskHistoryLimit, 10, "Task history retention limit")
+	flags.DurationVar(&opts.dispatcherHeartbeat, flagDispatcherHeartbeat, time.Duration(5*time.Second), "Dispatcher heartbeat period")
+	flags.DurationVar(&opts.nodeCertExpiry, flagCertExpiry, time.Duration(90*24*time.Hour), "Validity period for node certificates")
 	return cmd
 }
 
@@ -69,14 +68,14 @@ func runUpdate(dockerCli *client.DockerCli, flags *pflag.FlagSet, opts updateOpt
 func mergeSwarm(swarm *swarm.Swarm, flags *pflag.FlagSet) error {
 	spec := &swarm.Spec
 
-	if flags.Changed("auto-accept") {
-		value := flags.Lookup("auto-accept").Value.(*AutoAcceptOption)
+	if flags.Changed(flagAutoAccept) {
+		value := flags.Lookup(flagAutoAccept).Value.(*AutoAcceptOption)
 		spec.AcceptancePolicy.Policies = value.Policies(nil)
 	}
 
 	var psecret *string
-	if flags.Changed("secret") {
-		secret, _ := flags.GetString("secret")
+	if flags.Changed(flagSecret) {
+		secret, _ := flags.GetString(flagSecret)
 		psecret = &secret
 	}
 
@@ -84,18 +83,18 @@ func mergeSwarm(swarm *swarm.Swarm, flags *pflag.FlagSet) error {
 		spec.AcceptancePolicy.Policies[i].Secret = psecret
 	}
 
-	if flags.Changed("task-history-limit") {
-		spec.Orchestration.TaskHistoryRetentionLimit, _ = flags.GetInt64("task-history-limit")
+	if flags.Changed(flagTaskHistoryLimit) {
+		spec.Orchestration.TaskHistoryRetentionLimit, _ = flags.GetInt64(flagTaskHistoryLimit)
 	}
 
-	if flags.Changed("dispatcher-heartbeat") {
-		if v, err := flags.GetDuration("dispatcher-heartbeat"); err == nil {
+	if flags.Changed(flagDispatcherHeartbeat) {
+		if v, err := flags.GetDuration(flagDispatcherHeartbeat); err == nil {
 			spec.Dispatcher.HeartbeatPeriod = uint64(v.Nanoseconds())
 		}
 	}
 
-	if flags.Changed("cert-expiry") {
-		if v, err := flags.GetDuration("cert-expiry"); err == nil {
+	if flags.Changed(flagCertExpiry) {
+		if v, err := flags.GetDuration(flagCertExpiry); err == nil {
 			spec.CAConfig.NodeCertExpiry = v
 		}
 	}