Change the add/update flags to include 'add'

Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit 4c6faa434071b87a55256e86020cb78495e9951d)
Signed-off-by: Tibor Vass <tibor@docker.com>

api/client/service/create.go

@@ -28,7 +28,15 @@ func newCreateCommand(dockerCli *client.DockerCli) *cobra.Command {
 	flags := cmd.Flags()
 	flags.StringVar(&opts.mode, flagMode, "replicated", "Service mode (replicated or global)")
 	addServiceFlags(cmd, opts)
-	cmd.Flags().SetInterspersed(false)
+
+	flags.VarP(&opts.labels, flagLabel, "l", "Service labels")
+	flags.VarP(&opts.env, flagEnv, "e", "Set environment variables")
+	flags.Var(&opts.mounts, flagMount, "Attach a mount to the service")
+	flags.StringSliceVar(&opts.constraints, flagConstraint, []string{}, "Placement constraints")
+	flags.StringSliceVar(&opts.networks, flagNetwork, []string{}, "Network attachments")
+	flags.VarP(&opts.endpoint.ports, flagPublish, "p", "Publish a port as a node port")
+
+	flags.SetInterspersed(false)
 	return cmd
 }
 

api/client/service/opts.go

@@ -459,12 +459,9 @@ func (opts *serviceOptions) ToService() (swarm.ServiceSpec, error) {
 func addServiceFlags(cmd *cobra.Command, opts *serviceOptions) {
 	flags := cmd.Flags()
 	flags.StringVar(&opts.name, flagName, "", "Service name")
-	flags.VarP(&opts.labels, flagLabel, "l", "Service labels")
 
-	flags.VarP(&opts.env, flagEnv, "e", "Set environment variables")
 	flags.StringVarP(&opts.workdir, "workdir", "w", "", "Working directory inside the container")
 	flags.StringVarP(&opts.user, flagUser, "u", "", "Username or UID")
-	flags.Var(&opts.mounts, flagMount, "Attach a mount to the service")
 
 	flags.Var(&opts.resources.limitCPU, flagLimitCPU, "Limit CPUs")
 	flags.Var(&opts.resources.limitMemBytes, flagLimitMemory, "Limit Memory")
@@ -479,36 +476,38 @@ func addServiceFlags(cmd *cobra.Command, opts *serviceOptions) {
 	flags.Var(&opts.restartPolicy.maxAttempts, flagRestartMaxAttempts, "Maximum number of restarts before giving up")
 	flags.Var(&opts.restartPolicy.window, flagRestartWindow, "Window used to evaluate the restart policy")
 
-	flags.StringSliceVar(&opts.constraints, flagConstraint, []string{}, "Placement constraints")
-
 	flags.Uint64Var(&opts.update.parallelism, flagUpdateParallelism, 0, "Maximum number of tasks updated simultaneously")
 	flags.DurationVar(&opts.update.delay, flagUpdateDelay, time.Duration(0), "Delay between updates")
 
-	flags.StringSliceVar(&opts.networks, flagNetwork, []string{}, "Network attachments")
 	flags.StringVar(&opts.endpoint.mode, flagEndpointMode, "", "Endpoint mode (vip or dnsrr)")
-	flags.VarP(&opts.endpoint.ports, flagPublish, "p", "Publish a port as a node port")
 
 	flags.BoolVar(&opts.registryAuth, flagRegistryAuth, false, "Send registry authentication details to Swarm agents")
 }
 
 const (
 	flagConstraint         = "constraint"
-	flagConstraintRemove   = "remove-constraint"
+	flagConstraintRemove   = "constraint-rm"
+	flagConstraintAdd      = "constraint-add"
 	flagEndpointMode       = "endpoint-mode"
 	flagEnv                = "env"
-	flagEnvRemove          = "remove-env"
+	flagEnvRemove          = "env-rm"
+	flagEnvAdd             = "env-add"
 	flagLabel              = "label"
-	flagLabelRemove        = "remove-label"
+	flagLabelRemove        = "label-rm"
+	flagLabelAdd           = "label-add"
 	flagLimitCPU           = "limit-cpu"
 	flagLimitMemory        = "limit-memory"
 	flagMode               = "mode"
 	flagMount              = "mount"
-	flagMountRemove        = "remove-mount"
+	flagMountRemove        = "mount-rm"
+	flagMountAdd           = "mount-add"
 	flagName               = "name"
 	flagNetwork            = "network"
-	flagNetworkRemove      = "remove-network"
+	flagNetworkRemove      = "network-rm"
+	flagNetworkAdd         = "network-add"
 	flagPublish            = "publish"
-	flagPublishRemove      = "remove-publish"
+	flagPublishRemove      = "publish-rm"
+	flagPublishAdd         = "publish-add"
 	flagReplicas           = "replicas"
 	flagReserveCPU         = "reserve-cpu"
 	flagReserveMemory      = "reserve-memory"

api/client/service/update.go

@@ -35,15 +35,26 @@ func newUpdateCommand(dockerCli *client.DockerCli) *cobra.Command {
 	flags.String("image", "", "Service image tag")
 	flags.String("args", "", "Service command args")
 	addServiceFlags(cmd, opts)
-	flags.StringSlice(flagEnvRemove, []string{}, "Remove an environment variable")
-	flags.StringSlice(flagLabelRemove, []string{}, "Remove a label by its key")
-	flags.StringSlice(flagMountRemove, []string{}, "Remove a mount by its target path")
-	flags.StringSlice(flagPublishRemove, []string{}, "Remove a published port by its target port")
-	flags.StringSlice(flagNetworkRemove, []string{}, "Remove a network by name")
-	flags.StringSlice(flagConstraintRemove, []string{}, "Remove a constraint")
+
+	flags.Var(newListOptsVar(), flagEnvRemove, "Remove an environment variable")
+	flags.Var(newListOptsVar(), flagLabelRemove, "Remove a label by its key")
+	flags.Var(newListOptsVar(), flagMountRemove, "Remove a mount by its target path")
+	flags.Var(newListOptsVar(), flagPublishRemove, "Remove a published port by its target port")
+	flags.Var(newListOptsVar(), flagNetworkRemove, "Remove a network by name")
+	flags.Var(newListOptsVar(), flagConstraintRemove, "Remove a constraint")
+	flags.Var(&opts.labels, flagLabelAdd, "Add or update service labels")
+	flags.Var(&opts.env, flagEnvAdd, "Add or update environment variables")
+	flags.Var(&opts.mounts, flagMountAdd, "Add or update a mount on a service")
+	flags.StringSliceVar(&opts.constraints, flagConstraintAdd, []string{}, "Add or update placement constraints")
+	flags.StringSliceVar(&opts.networks, flagNetworkAdd, []string{}, "Add or update network attachments")
+	flags.Var(&opts.endpoint.ports, flagPublishAdd, "Add or update a published port")
 	return cmd
 }
 
+func newListOptsVar() *opts.ListOpts {
+	return opts.NewListOptsRef(&[]string{}, nil)
+}
+
 func runUpdate(dockerCli *client.DockerCli, flags *pflag.FlagSet, serviceID string) error {
 	apiClient := dockerCli.Client()
 	ctx := context.Background()
@@ -176,7 +187,7 @@ func updateService(flags *pflag.FlagSet, spec *swarm.ServiceSpec) error {
 		updateDurationOpt((flagRestartWindow), task.RestartPolicy.Window)
 	}
 
-	if flags.Changed(flagConstraint) {
+	if anyChanged(flags, flagConstraintAdd, flagConstraintRemove) {
 		if task.Placement == nil {
 			task.Placement = &swarm.Placement{}
 		}
@@ -201,7 +212,7 @@ func updateService(flags *pflag.FlagSet, spec *swarm.ServiceSpec) error {
 		spec.EndpointSpec.Mode = swarm.ResolutionMode(value)
 	}
 
-	if flags.Changed(flagPublish) {
+	if anyChanged(flags, flagPublishAdd, flagPublishRemove) {
 		if spec.EndpointSpec == nil {
 			spec.EndpointSpec = &swarm.EndpointSpec{}
 		}
@@ -231,7 +242,7 @@ func anyChanged(flags *pflag.FlagSet, fields ...string) bool {
 }
 
 func updatePlacement(flags *pflag.FlagSet, placement *swarm.Placement) {
-	field, _ := flags.GetStringSlice(flagConstraint)
+	field, _ := flags.GetStringSlice(flagConstraintAdd)
 	constraints := &placement.Constraints
 	placement.Constraints = append(placement.Constraints, field...)
 
@@ -244,19 +255,19 @@ func updatePlacement(flags *pflag.FlagSet, placement *swarm.Placement) {
 }
 
 func updateLabels(flags *pflag.FlagSet, field *map[string]string) {
-	if flags.Changed(flagLabel) {
+	if flags.Changed(flagLabelAdd) {
 		if field == nil {
 			*field = map[string]string{}
 		}
 
-		values := flags.Lookup(flagLabel).Value.(*opts.ListOpts).GetAll()
+		values := flags.Lookup(flagLabelAdd).Value.(*opts.ListOpts).GetAll()
 		for key, value := range runconfigopts.ConvertKVStringsToMap(values) {
 			(*field)[key] = value
 		}
 	}
 
 	if field != nil && flags.Changed(flagLabelRemove) {
-		toRemove, _ := flags.GetStringSlice(flagLabelRemove)
+		toRemove := flags.Lookup(flagLabelRemove).Value.(*opts.ListOpts).GetAll()
 		for _, label := range toRemove {
 			delete(*field, label)
 		}
@@ -264,8 +275,8 @@ func updateLabels(flags *pflag.FlagSet, field *map[string]string) {
 }
 
 func updateEnvironment(flags *pflag.FlagSet, field *[]string) {
-	if flags.Changed(flagEnv) {
-		value := flags.Lookup(flagEnv).Value.(*opts.ListOpts)
+	if flags.Changed(flagEnvAdd) {
+		value := flags.Lookup(flagEnvAdd).Value.(*opts.ListOpts)
 		*field = append(*field, value.GetAll()...)
 	}
 	toRemove := buildToRemoveSet(flags, flagEnvRemove)
@@ -290,7 +301,7 @@ func buildToRemoveSet(flags *pflag.FlagSet, flag string) map[string]struct{} {
 		return toRemove
 	}
 
-	toRemoveSlice, _ := flags.GetStringSlice(flag)
+	toRemoveSlice := flags.Lookup(flag).Value.(*opts.ListOpts).GetAll()
 	for _, key := range toRemoveSlice {
 		toRemove[key] = empty
 	}
@@ -298,8 +309,8 @@ func buildToRemoveSet(flags *pflag.FlagSet, flag string) map[string]struct{} {
 }
 
 func updateMounts(flags *pflag.FlagSet, mounts *[]swarm.Mount) {
-	if flags.Changed(flagMount) {
-		values := flags.Lookup(flagMount).Value.(*MountOpt).Value()
+	if flags.Changed(flagMountAdd) {
+		values := flags.Lookup(flagMountAdd).Value.(*MountOpt).Value()
 		*mounts = append(*mounts, values...)
 	}
 	toRemove := buildToRemoveSet(flags, flagMountRemove)
@@ -311,8 +322,8 @@ func updateMounts(flags *pflag.FlagSet, mounts *[]swarm.Mount) {
 }
 
 func updatePorts(flags *pflag.FlagSet, portConfig *[]swarm.PortConfig) {
-	if flags.Changed(flagPublish) {
-		values := flags.Lookup(flagPublish).Value.(*opts.ListOpts).GetAll()
+	if flags.Changed(flagPublishAdd) {
+		values := flags.Lookup(flagPublishAdd).Value.(*opts.ListOpts).GetAll()
 		ports, portBindings, _ := nat.ParsePortSpecs(values)
 
 		for port := range ports {
@@ -321,13 +332,14 @@ func updatePorts(flags *pflag.FlagSet, portConfig *[]swarm.PortConfig) {
 	}
 
 	if flags.Changed(flagPublishRemove) {
-		toRemove, _ := flags.GetStringSlice(flagPublishRemove)
+		toRemove := flags.Lookup(flagPublishRemove).Value.(*opts.ListOpts).GetAll()
 		for _, rawTargetPort := range toRemove {
 			targetPort := nat.Port(rawTargetPort)
 			for i, port := range *portConfig {
 				if string(port.Protocol) == targetPort.Proto() &&
 					port.TargetPort == uint32(targetPort.Int()) {
 					*portConfig = append((*portConfig)[:i], (*portConfig)[i+1:]...)
+					break
 				}
 			}
 		}
@@ -335,8 +347,8 @@ func updatePorts(flags *pflag.FlagSet, portConfig *[]swarm.PortConfig) {
 }
 
 func updateNetworks(flags *pflag.FlagSet, attachments *[]swarm.NetworkAttachmentConfig) {
-	if flags.Changed(flagNetwork) {
-		networks, _ := flags.GetStringSlice(flagNetwork)
+	if flags.Changed(flagNetworkAdd) {
+		networks, _ := flags.GetStringSlice(flagNetworkAdd)
 		for _, network := range networks {
 			*attachments = append(*attachments, swarm.NetworkAttachmentConfig{Target: network})
 		}

api/client/service/update_test.go

@@ -21,8 +21,8 @@ func TestUpdateServiceArgs(t *testing.T) {
 
 func TestUpdateLabels(t *testing.T) {
 	flags := newUpdateCommand(nil).Flags()
-	flags.Set("label", "toadd=newlabel")
-	flags.Set("remove-label", "toremove")
+	flags.Set("label-add", "toadd=newlabel")
+	flags.Set("label-rm", "toremove")
 
 	labels := map[string]string{
 		"toremove": "thelabeltoremove",
@@ -37,8 +37,8 @@ func TestUpdateLabels(t *testing.T) {
 
 func TestUpdatePlacement(t *testing.T) {
 	flags := newUpdateCommand(nil).Flags()
-	flags.Set("constraint", "node=toadd")
-	flags.Set("remove-constraint", "node!=toremove")
+	flags.Set("constraint-add", "node=toadd")
+	flags.Set("constraint-rm", "node!=toremove")
 
 	placement := &swarm.Placement{
 		Constraints: []string{"node!=toremove", "container=tokeep"},
@@ -52,8 +52,8 @@ func TestUpdatePlacement(t *testing.T) {
 
 func TestUpdateEnvironment(t *testing.T) {
 	flags := newUpdateCommand(nil).Flags()
-	flags.Set("env", "toadd=newenv")
-	flags.Set("remove-env", "toremove")
+	flags.Set("env-add", "toadd=newenv")
+	flags.Set("env-rm", "toremove")
 
 	envs := []string{"toremove=theenvtoremove", "tokeep=value"}
 
@@ -65,8 +65,8 @@ func TestUpdateEnvironment(t *testing.T) {
 
 func TestUpdateMounts(t *testing.T) {
 	flags := newUpdateCommand(nil).Flags()
-	flags.Set("mount", "type=volume,target=/toadd")
-	flags.Set("remove-mount", "/toremove")
+	flags.Set("mount-add", "type=volume,target=/toadd")
+	flags.Set("mount-rm", "/toremove")
 
 	mounts := []swarm.Mount{
 		{Target: "/toremove", Type: swarm.MountType("BIND")},
@@ -81,8 +81,8 @@ func TestUpdateMounts(t *testing.T) {
 
 func TestUpdateNetworks(t *testing.T) {
 	flags := newUpdateCommand(nil).Flags()
-	flags.Set("network", "toadd")
-	flags.Set("remove-network", "toremove")
+	flags.Set("network-add", "toadd")
+	flags.Set("network-rm", "toremove")
 
 	attachments := []swarm.NetworkAttachmentConfig{
 		{Target: "toremove", Aliases: []string{"foo"}},
@@ -97,8 +97,8 @@ func TestUpdateNetworks(t *testing.T) {
 
 func TestUpdatePorts(t *testing.T) {
 	flags := newUpdateCommand(nil).Flags()
-	flags.Set("publish", "1000:1000")
-	flags.Set("remove-publish", "333/udp")
+	flags.Set("publish-add", "1000:1000")
+	flags.Set("publish-rm", "333/udp")
 
 	portConfigs := []swarm.PortConfig{
 		{TargetPort: 333, Protocol: swarm.PortConfigProtocol("udp")},

integration-cli/docker_cli_service_update_test.go

@@ -22,7 +22,7 @@ func (s *DockerSwarmSuite) TestServiceUpdatePort(c *check.C) {
 	waitAndAssert(c, defaultReconciliationTimeout, d.checkActiveContainerCount, checker.Equals, 1)
 
 	// Update the service: changed the port mapping from 8080:8081 to 8082:8083.
-	_, err = d.Cmd("service", "update", "-p", "8082:8083", "--remove-publish", "8081", serviceName)
+	_, err = d.Cmd("service", "update", "--publish-add", "8082:8083", "--publish-rm", "8081", serviceName)
 	c.Assert(err, checker.IsNil)
 
 	// Inspect the service and verify port mapping