@@ -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

 }

@@ -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, "env", "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,29 +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   = "constraint-rm"

+	flagConstraintAdd      = "constraint-add"

 	flagEndpointMode       = "endpoint-mode"

+	flagEnv                = "env"

+	flagEnvRemove          = "env-rm"

+	flagEnvAdd             = "env-add"

 	flagLabel              = "label"

+	flagLabelRemove        = "label-rm"

+	flagLabelAdd           = "label-add"

 	flagLimitCPU           = "limit-cpu"

 	flagLimitMemory        = "limit-memory"

 	flagMode               = "mode"

 	flagMount              = "mount"

+	flagMountRemove        = "mount-rm"

+	flagMountAdd           = "mount-add"

 	flagName               = "name"

 	flagNetwork            = "network"

+	flagNetworkRemove      = "network-rm"

+	flagNetworkAdd         = "network-add"

 	flagPublish            = "publish"

+	flagPublishRemove      = "publish-rm"

+	flagPublishAdd         = "publish-add"

 	flagReplicas           = "replicas"

 	flagReserveCPU         = "reserve-cpu"

 	flagReserveMemory      = "reserve-memory"

@@ -2,6 +2,7 @@ package service

 import (

 	"fmt"

+	"strings"

 	"time"

 	"golang.org/x/net/context"

@@ -34,9 +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.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()

@@ -85,13 +103,6 @@ func updateService(flags *pflag.FlagSet, spec *swarm.ServiceSpec) error {

 		}

 	}

-	updateListOpts := func(flag string, field *[]string) {

-		if flags.Changed(flag) {

-			value := flags.Lookup(flag).Value.(*opts.ListOpts)

-			*field = value.GetAll()

-		}

-	}

-

 	updateInt64Value := func(flag string, field *int64) {

 		if flags.Changed(flag) {

 			*field = flags.Lookup(flag).Value.(int64Value).Value()

@@ -136,7 +147,7 @@ func updateService(flags *pflag.FlagSet, spec *swarm.ServiceSpec) error {

 	updateLabels(flags, &spec.Labels)

 	updateString("image", &cspec.Image)

 	updateStringToSlice(flags, "args", &cspec.Args)

-	updateListOpts("env", &cspec.Env)

+	updateEnvironment(flags, &cspec.Env)

 	updateString("workdir", &cspec.Dir)

 	updateString(flagUser, &cspec.User)

 	updateMounts(flags, &cspec.Mounts)

@@ -169,7 +180,12 @@ func updateService(flags *pflag.FlagSet, spec *swarm.ServiceSpec) error {

 		updateDurationOpt((flagRestartWindow), task.RestartPolicy.Window)

 	}

-	// TODO: The constraints field is fixed in #23773

+	if anyChanged(flags, flagConstraintAdd, flagConstraintRemove) {

+		if task.Placement == nil {

+			task.Placement = &swarm.Placement{}

+		}

+		updatePlacement(flags, task.Placement)

+	}

 	if err := updateReplicas(flags, &spec.Mode); err != nil {

 		return err

@@ -189,7 +205,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{}

 		}

@@ -209,65 +225,154 @@ func updateStringToSlice(flags *pflag.FlagSet, flag string, field *[]string) err

 	return err

 }

+func anyChanged(flags *pflag.FlagSet, fields ...string) bool {

+	for _, flag := range fields {

+		if flags.Changed(flag) {

+			return true

+		}

+	}

+	return false

+}

+

+func updatePlacement(flags *pflag.FlagSet, placement *swarm.Placement) {

+	field, _ := flags.GetStringSlice(flagConstraintAdd)

+	placement.Constraints = append(placement.Constraints, field...)

+

+	toRemove := buildToRemoveSet(flags, flagConstraintRemove)

+	placement.Constraints = removeItems(placement.Constraints, toRemove, itemKey)

+}

+

 func updateLabels(flags *pflag.FlagSet, field *map[string]string) {

-	if !flags.Changed(flagLabel) {

-		return

+	if flags.Changed(flagLabelAdd) {

+		if field == nil {

+			*field = map[string]string{}

+		}

+

+		values := flags.Lookup(flagLabelAdd).Value.(*opts.ListOpts).GetAll()

+		for key, value := range runconfigopts.ConvertKVStringsToMap(values) {

+			(*field)[key] = value

+		}

 	}

-	values := flags.Lookup(flagLabel).Value.(*opts.ListOpts).GetAll()

+	if field != nil && flags.Changed(flagLabelRemove) {

+		toRemove := flags.Lookup(flagLabelRemove).Value.(*opts.ListOpts).GetAll()

+		for _, label := range toRemove {

+			delete(*field, label)

+		}

+	}

+}

-	localLabels := map[string]string{}

-	for key, value := range runconfigopts.ConvertKVStringsToMap(values) {

-		localLabels[key] = value

+func updateEnvironment(flags *pflag.FlagSet, field *[]string) {

+	if flags.Changed(flagEnvAdd) {

+		value := flags.Lookup(flagEnvAdd).Value.(*opts.ListOpts)

+		*field = append(*field, value.GetAll()...)

 	}

-	*field = localLabels

+	toRemove := buildToRemoveSet(flags, flagEnvRemove)

+	*field = removeItems(*field, toRemove, envKey)

 }

-func anyChanged(flags *pflag.FlagSet, fields ...string) bool {

-	for _, flag := range fields {

-		if flags.Changed(flag) {

-			return true

+func envKey(value string) string {

+	kv := strings.SplitN(value, "=", 2)

+	return kv[0]

+}

+

+func itemKey(value string) string {

+	return value

+}

+

+func buildToRemoveSet(flags *pflag.FlagSet, flag string) map[string]struct{} {

+	var empty struct{}

+	toRemove := make(map[string]struct{})

+

+	if !flags.Changed(flag) {

+		return toRemove

+	}

+

+	toRemoveSlice := flags.Lookup(flag).Value.(*opts.ListOpts).GetAll()

+	for _, key := range toRemoveSlice {

+		toRemove[key] = empty

+	}

+	return toRemove

+}

+

+func removeItems(

+	seq []string,

+	toRemove map[string]struct{},

+	keyFunc func(string) string,

+) []string {

+	newSeq := []string{}

+	for _, item := range seq {

+		if _, exists := toRemove[keyFunc(item)]; !exists {

+			newSeq = append(newSeq, item)

 		}

 	}

-	return false

+	return newSeq

 }

-// TODO: should this override by destination path, or does swarm handle that?

 func updateMounts(flags *pflag.FlagSet, mounts *[]swarm.Mount) {

-	if !flags.Changed(flagMount) {

-		return

+	if flags.Changed(flagMountAdd) {

+		values := flags.Lookup(flagMountAdd).Value.(*MountOpt).Value()

+		*mounts = append(*mounts, values...)

 	}

+	toRemove := buildToRemoveSet(flags, flagMountRemove)

-	*mounts = flags.Lookup(flagMount).Value.(*MountOpt).Value()

+	newMounts := []swarm.Mount{}

+	for _, mount := range *mounts {

+		if _, exists := toRemove[mount.Target]; !exists {

+			newMounts = append(newMounts, mount)

+		}

+	}

+	*mounts = newMounts

 }

-// TODO: should this override by name, or does swarm handle that?

 func updatePorts(flags *pflag.FlagSet, portConfig *[]swarm.PortConfig) {

-	if !flags.Changed(flagPublish) {

-		return

-	}

+	if flags.Changed(flagPublishAdd) {

+		values := flags.Lookup(flagPublishAdd).Value.(*opts.ListOpts).GetAll()

+		ports, portBindings, _ := nat.ParsePortSpecs(values)

-	values := flags.Lookup(flagPublish).Value.(*opts.ListOpts).GetAll()

-	ports, portBindings, _ := nat.ParsePortSpecs(values)

+		for port := range ports {

+			*portConfig = append(*portConfig, convertPortToPortConfig(port, portBindings)...)

+		}

+	}

-	var localPortConfig []swarm.PortConfig

-	for port := range ports {

-		localPortConfig = append(localPortConfig, convertPortToPortConfig(port, portBindings)...)

+	if !flags.Changed(flagPublishRemove) {

+		return

 	}

-	*portConfig = localPortConfig

+	toRemove := flags.Lookup(flagPublishRemove).Value.(*opts.ListOpts).GetAll()

+	newPorts := []swarm.PortConfig{}

+portLoop:

+	for _, port := range *portConfig {

+		for _, rawTargetPort := range toRemove {

+			targetPort := nat.Port(rawTargetPort)

+			if equalPort(targetPort, port) {

+				continue portLoop

+			}

+		}

+		newPorts = append(newPorts, port)

+	}

+	*portConfig = newPorts

+}

+

+func equalPort(targetPort nat.Port, port swarm.PortConfig) bool {

+	return (string(port.Protocol) == targetPort.Proto() &&

+		port.TargetPort == uint32(targetPort.Int()))

 }

 func updateNetworks(flags *pflag.FlagSet, attachments *[]swarm.NetworkAttachmentConfig) {

-	if !flags.Changed(flagNetwork) {

-		return

+	if flags.Changed(flagNetworkAdd) {

+		networks, _ := flags.GetStringSlice(flagNetworkAdd)

+		for _, network := range networks {

+			*attachments = append(*attachments, swarm.NetworkAttachmentConfig{Target: network})

+		}

 	}

-	networks, _ := flags.GetStringSlice(flagNetwork)

-

-	var localAttachments []swarm.NetworkAttachmentConfig

-	for _, network := range networks {

-		localAttachments = append(localAttachments, swarm.NetworkAttachmentConfig{Target: network})

+	toRemove := buildToRemoveSet(flags, flagNetworkRemove)

+	newNetworks := []swarm.NetworkAttachmentConfig{}

+	for _, network := range *attachments {

+		if _, exists := toRemove[network.Target]; !exists {

+			newNetworks = append(newNetworks, network)

+		}

 	}

-	*attachments = localAttachments

+	*attachments = newNetworks

 }

 func updateReplicas(flags *pflag.FlagSet, serviceMode *swarm.ServiceMode) error {

@@ -18,3 +18,116 @@ func TestUpdateServiceArgs(t *testing.T) {

 	updateService(flags, spec)

 	assert.EqualStringSlice(t, cspec.Args, []string{"the", "new args"})

 }

+

+func TestUpdateLabels(t *testing.T) {

+	flags := newUpdateCommand(nil).Flags()

+	flags.Set("label-add", "toadd=newlabel")

+	flags.Set("label-rm", "toremove")

+

+	labels := map[string]string{

+		"toremove": "thelabeltoremove",

+		"tokeep":   "value",

+	}

+

+	updateLabels(flags, &labels)

+	assert.Equal(t, len(labels), 2)

+	assert.Equal(t, labels["tokeep"], "value")

+	assert.Equal(t, labels["toadd"], "newlabel")

+}

+

+func TestUpdateLabelsRemoveALabelThatDoesNotExist(t *testing.T) {

+	flags := newUpdateCommand(nil).Flags()

+	flags.Set("label-rm", "dne")

+

+	labels := map[string]string{"foo": "theoldlabel"}

+	updateLabels(flags, &labels)

+	assert.Equal(t, len(labels), 1)

+}

+

+func TestUpdatePlacement(t *testing.T) {

+	flags := newUpdateCommand(nil).Flags()

+	flags.Set("constraint-add", "node=toadd")

+	flags.Set("constraint-rm", "node!=toremove")

+

+	placement := &swarm.Placement{

+		Constraints: []string{"node!=toremove", "container=tokeep"},

+	}

+

+	updatePlacement(flags, placement)

+	assert.Equal(t, len(placement.Constraints), 2)

+	assert.Equal(t, placement.Constraints[0], "container=tokeep")

+	assert.Equal(t, placement.Constraints[1], "node=toadd")

+}

+

+func TestUpdateEnvironment(t *testing.T) {

+	flags := newUpdateCommand(nil).Flags()

+	flags.Set("env-add", "toadd=newenv")

+	flags.Set("env-rm", "toremove")

+

+	envs := []string{"toremove=theenvtoremove", "tokeep=value"}

+

+	updateEnvironment(flags, &envs)

+	assert.Equal(t, len(envs), 2)

+	assert.Equal(t, envs[0], "tokeep=value")

+	assert.Equal(t, envs[1], "toadd=newenv")

+}

+

+func TestUpdateEnvironmentWithDuplicateValues(t *testing.T) {

+	flags := newUpdateCommand(nil).Flags()

+	flags.Set("env-add", "foo=newenv")

+	flags.Set("env-add", "foo=dupe")

+	flags.Set("env-rm", "foo")

+

+	envs := []string{"foo=value"}

+

+	updateEnvironment(flags, &envs)

+	assert.Equal(t, len(envs), 0)

+}

+

+func TestUpdateMounts(t *testing.T) {

+	flags := newUpdateCommand(nil).Flags()

+	flags.Set("mount-add", "type=volume,target=/toadd")

+	flags.Set("mount-rm", "/toremove")

+

+	mounts := []swarm.Mount{

+		{Target: "/toremove", Type: swarm.MountType("BIND")},

+		{Target: "/tokeep", Type: swarm.MountType("BIND")},

+	}

+

+	updateMounts(flags, &mounts)

+	assert.Equal(t, len(mounts), 2)

+	assert.Equal(t, mounts[0].Target, "/tokeep")

+	assert.Equal(t, mounts[1].Target, "/toadd")

+}

+

+func TestUpdateNetworks(t *testing.T) {

+	flags := newUpdateCommand(nil).Flags()

+	flags.Set("network-add", "toadd")

+	flags.Set("network-rm", "toremove")

+

+	attachments := []swarm.NetworkAttachmentConfig{

+		{Target: "toremove", Aliases: []string{"foo"}},

+		{Target: "tokeep"},

+	}

+

+	updateNetworks(flags, &attachments)

+	assert.Equal(t, len(attachments), 2)

+	assert.Equal(t, attachments[0].Target, "tokeep")

+	assert.Equal(t, attachments[1].Target, "toadd")

+}

+

+func TestUpdatePorts(t *testing.T) {

+	flags := newUpdateCommand(nil).Flags()

+	flags.Set("publish-add", "1000:1000")

+	flags.Set("publish-rm", "333/udp")

+

+	portConfigs := []swarm.PortConfig{

+		{TargetPort: 333, Protocol: swarm.PortConfigProtocol("udp")},

+		{TargetPort: 555},

+	}

+

+	updatePorts(flags, &portConfigs)

+	assert.Equal(t, len(portConfigs), 2)

+	assert.Equal(t, portConfigs[0].TargetPort, uint32(555))

+	assert.Equal(t, portConfigs[1].TargetPort, uint32(1000))

+}

@@ -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", 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