Browse code

Add remove flags for service update

with unit tests

Signed-off-by: Daniel Nephin <dnephin@docker.com>

Daniel Nephin authored on 2016/06/21 01:57:57
Showing 4 changed files
... ...
@@ -461,7 +461,7 @@ func addServiceFlags(cmd *cobra.Command, opts *serviceOptions) {
461 461
 	flags.StringVar(&opts.name, flagName, "", "Service name")
462 462
 	flags.VarP(&opts.labels, flagLabel, "l", "Service labels")
463 463
 
464
-	flags.VarP(&opts.env, "env", "e", "Set environment variables")
464
+	flags.VarP(&opts.env, flagEnv, "e", "Set environment variables")
465 465
 	flags.StringVarP(&opts.workdir, "workdir", "w", "", "Working directory inside the container")
466 466
 	flags.StringVarP(&opts.user, flagUser, "u", "", "Username or UID")
467 467
 	flags.Var(&opts.mounts, flagMount, "Attach a mount to the service")
... ...
@@ -494,14 +494,20 @@ func addServiceFlags(cmd *cobra.Command, opts *serviceOptions) {
494 494
 const (
495 495
 	flagConstraint         = "constraint"
496 496
 	flagEndpointMode       = "endpoint-mode"
497
+	flagEnv                = "env"
498
+	flagEnvRemove          = "remove-env"
497 499
 	flagLabel              = "label"
500
+	flagLabelRemove        = "remove-label"
498 501
 	flagLimitCPU           = "limit-cpu"
499 502
 	flagLimitMemory        = "limit-memory"
500 503
 	flagMode               = "mode"
501 504
 	flagMount              = "mount"
505
+	flagMountRemove        = "remove-mount"
502 506
 	flagName               = "name"
503 507
 	flagNetwork            = "network"
508
+	flagNetworkRemove      = "remove-network"
504 509
 	flagPublish            = "publish"
510
+	flagPublishRemove      = "remove-publish"
505 511
 	flagReplicas           = "replicas"
506 512
 	flagReserveCPU         = "reserve-cpu"
507 513
 	flagReserveMemory      = "reserve-memory"
... ...
@@ -2,6 +2,7 @@ package service
2 2
 
3 3
 import (
4 4
 	"fmt"
5
+	"strings"
5 6
 	"time"
6 7
 
7 8
 	"golang.org/x/net/context"
... ...
@@ -34,6 +35,11 @@ func newUpdateCommand(dockerCli *client.DockerCli) *cobra.Command {
34 34
 	flags.String("image", "", "Service image tag")
35 35
 	flags.String("args", "", "Service command args")
36 36
 	addServiceFlags(cmd, opts)
37
+	flags.StringSlice(flagEnvRemove, []string{}, "Remove an environment variable")
38
+	flags.StringSlice(flagLabelRemove, []string{}, "The key of a label to remove")
39
+	flags.StringSlice(flagMountRemove, []string{}, "The mount target for a mount to remove")
40
+	flags.StringSlice(flagPublishRemove, []string{}, "The target port to remove")
41
+	flags.StringSlice(flagNetworkRemove, []string{}, "The name of a network to remove")
37 42
 	return cmd
38 43
 }
39 44
 
... ...
@@ -136,7 +142,7 @@ func updateService(flags *pflag.FlagSet, spec *swarm.ServiceSpec) error {
136 136
 	updateLabels(flags, &spec.Labels)
137 137
 	updateString("image", &cspec.Image)
138 138
 	updateStringToSlice(flags, "args", &cspec.Args)
139
-	updateListOpts("env", &cspec.Env)
139
+	updateEnvironment(flags, &cspec.Env)
140 140
 	updateString("workdir", &cspec.Dir)
141 141
 	updateString(flagUser, &cspec.User)
142 142
 	updateMounts(flags, &cspec.Mounts)
... ...
@@ -169,7 +175,10 @@ func updateService(flags *pflag.FlagSet, spec *swarm.ServiceSpec) error {
169 169
 		updateDurationOpt((flagRestartWindow), task.RestartPolicy.Window)
170 170
 	}
171 171
 
172
-	// TODO: The constraints field is fixed in #23773
172
+	if flags.Changed(flagConstraint) {
173
+		task.Placement = &swarm.Placement{}
174
+		updateSlice(flagConstraint, &task.Placement.Constraints)
175
+	}
173 176
 
174 177
 	if err := updateReplicas(flags, &spec.Mode); err != nil {
175 178
 		return err
... ...
@@ -209,65 +218,116 @@ func updateStringToSlice(flags *pflag.FlagSet, flag string, field *[]string) err
209 209
 	return err
210 210
 }
211 211
 
212
-func updateLabels(flags *pflag.FlagSet, field *map[string]string) {
213
-	if !flags.Changed(flagLabel) {
214
-		return
212
+func anyChanged(flags *pflag.FlagSet, fields ...string) bool {
213
+	for _, flag := range fields {
214
+		if flags.Changed(flag) {
215
+			return true
216
+		}
215 217
 	}
218
+	return false
219
+}
220
+
221
+func updateLabels(flags *pflag.FlagSet, field *map[string]string) {
222
+	if flags.Changed(flagLabel) {
223
+		if field == nil {
224
+			*field = map[string]string{}
225
+		}
216 226
 
217
-	values := flags.Lookup(flagLabel).Value.(*opts.ListOpts).GetAll()
227
+		values := flags.Lookup(flagLabel).Value.(*opts.ListOpts).GetAll()
228
+		for key, value := range runconfigopts.ConvertKVStringsToMap(values) {
229
+			(*field)[key] = value
230
+		}
231
+	}
218 232
 
219
-	localLabels := map[string]string{}
220
-	for key, value := range runconfigopts.ConvertKVStringsToMap(values) {
221
-		localLabels[key] = value
233
+	if field != nil && flags.Changed(flagLabelRemove) {
234
+		toRemove, _ := flags.GetStringSlice(flagLabelRemove)
235
+		for _, label := range toRemove {
236
+			delete(*field, label)
237
+		}
222 238
 	}
223
-	*field = localLabels
224 239
 }
225 240
 
226
-func anyChanged(flags *pflag.FlagSet, fields ...string) bool {
227
-	for _, flag := range fields {
228
-		if flags.Changed(flag) {
229
-			return true
241
+func updateEnvironment(flags *pflag.FlagSet, field *[]string) {
242
+	if flags.Changed(flagEnv) {
243
+		value := flags.Lookup(flagEnv).Value.(*opts.ListOpts)
244
+		*field = append(*field, value.GetAll()...)
245
+	}
246
+	if flags.Changed(flagEnvRemove) {
247
+		toRemove, _ := flags.GetStringSlice(flagEnvRemove)
248
+		for _, envRemove := range toRemove {
249
+			for i, env := range *field {
250
+				key := envKey(env)
251
+				if key == envRemove {
252
+					*field = append((*field)[:i], (*field)[i+1:]...)
253
+				}
254
+			}
230 255
 		}
231 256
 	}
232
-	return false
233 257
 }
234 258
 
235
-// TODO: should this override by destination path, or does swarm handle that?
259
+func envKey(value string) string {
260
+	kv := strings.SplitN(value, "=", 2)
261
+	return kv[0]
262
+}
263
+
236 264
 func updateMounts(flags *pflag.FlagSet, mounts *[]swarm.Mount) {
237
-	if !flags.Changed(flagMount) {
238
-		return
265
+	if flags.Changed(flagMount) {
266
+		values := flags.Lookup(flagMount).Value.(*MountOpt).Value()
267
+		*mounts = append(*mounts, values...)
268
+	}
269
+	if flags.Changed(flagMountRemove) {
270
+		toRemove, _ := flags.GetStringSlice(flagMountRemove)
271
+		for _, mountTarget := range toRemove {
272
+			for i, mount := range *mounts {
273
+				if mount.Target == mountTarget {
274
+					*mounts = append((*mounts)[:i], (*mounts)[i+1:]...)
275
+				}
276
+			}
277
+		}
239 278
 	}
240
-
241
-	*mounts = flags.Lookup(flagMount).Value.(*MountOpt).Value()
242 279
 }
243 280
 
244
-// TODO: should this override by name, or does swarm handle that?
245 281
 func updatePorts(flags *pflag.FlagSet, portConfig *[]swarm.PortConfig) {
246
-	if !flags.Changed(flagPublish) {
247
-		return
248
-	}
282
+	if flags.Changed(flagPublish) {
283
+		values := flags.Lookup(flagPublish).Value.(*opts.ListOpts).GetAll()
284
+		ports, portBindings, _ := nat.ParsePortSpecs(values)
249 285
 
250
-	values := flags.Lookup(flagPublish).Value.(*opts.ListOpts).GetAll()
251
-	ports, portBindings, _ := nat.ParsePortSpecs(values)
286
+		for port := range ports {
287
+			*portConfig = append(*portConfig, convertPortToPortConfig(port, portBindings)...)
288
+		}
289
+	}
252 290
 
253
-	var localPortConfig []swarm.PortConfig
254
-	for port := range ports {
255
-		localPortConfig = append(localPortConfig, convertPortToPortConfig(port, portBindings)...)
291
+	if flags.Changed(flagPublishRemove) {
292
+		toRemove, _ := flags.GetStringSlice(flagPublishRemove)
293
+		for _, rawTargetPort := range toRemove {
294
+			targetPort := nat.Port(rawTargetPort)
295
+			for i, port := range *portConfig {
296
+				if string(port.Protocol) == targetPort.Proto() &&
297
+					port.TargetPort == uint32(targetPort.Int()) {
298
+					*portConfig = append((*portConfig)[:i], (*portConfig)[i+1:]...)
299
+				}
300
+			}
301
+		}
256 302
 	}
257
-	*portConfig = localPortConfig
258 303
 }
259 304
 
260 305
 func updateNetworks(flags *pflag.FlagSet, attachments *[]swarm.NetworkAttachmentConfig) {
261
-	if !flags.Changed(flagNetwork) {
262
-		return
306
+	if flags.Changed(flagNetwork) {
307
+		networks, _ := flags.GetStringSlice(flagNetwork)
308
+		for _, network := range networks {
309
+			*attachments = append(*attachments, swarm.NetworkAttachmentConfig{Target: network})
310
+		}
263 311
 	}
264
-	networks, _ := flags.GetStringSlice(flagNetwork)
265
-
266
-	var localAttachments []swarm.NetworkAttachmentConfig
267
-	for _, network := range networks {
268
-		localAttachments = append(localAttachments, swarm.NetworkAttachmentConfig{Target: network})
312
+	if flags.Changed(flagNetworkRemove) {
313
+		toRemove, _ := flags.GetStringSlice(flagNetworkRemove)
314
+		for _, networkTarget := range toRemove {
315
+			for i, network := range *attachments {
316
+				if network.Target == networkTarget {
317
+					*attachments = append((*attachments)[:i], (*attachments)[i+1:]...)
318
+				}
319
+			}
320
+		}
269 321
 	}
270
-	*attachments = localAttachments
271 322
 }
272 323
 
273 324
 func updateReplicas(flags *pflag.FlagSet, serviceMode *swarm.ServiceMode) error {
... ...
@@ -18,3 +18,83 @@ func TestUpdateServiceArgs(t *testing.T) {
18 18
 	updateService(flags, spec)
19 19
 	assert.EqualStringSlice(t, cspec.Args, []string{"the", "new args"})
20 20
 }
21
+
22
+func TestUpdateLabels(t *testing.T) {
23
+	flags := newUpdateCommand(nil).Flags()
24
+	flags.Set("label", "toadd=newlabel")
25
+	flags.Set("remove-label", "toremove")
26
+
27
+	labels := map[string]string{
28
+		"toremove": "thelabeltoremove",
29
+		"tokeep":   "value",
30
+	}
31
+
32
+	updateLabels(flags, &labels)
33
+	assert.Equal(t, len(labels), 2)
34
+	assert.Equal(t, labels["tokeep"], "value")
35
+	assert.Equal(t, labels["toadd"], "newlabel")
36
+}
37
+
38
+func TestUpdateEnvironment(t *testing.T) {
39
+	flags := newUpdateCommand(nil).Flags()
40
+	flags.Set("env", "toadd=newenv")
41
+	flags.Set("remove-env", "toremove")
42
+
43
+	envs := []string{
44
+		"toremove=theenvtoremove",
45
+		"tokeep=value",
46
+	}
47
+
48
+	updateEnvironment(flags, &envs)
49
+	assert.Equal(t, len(envs), 2)
50
+	assert.Equal(t, envs[0], "tokeep=value")
51
+	assert.Equal(t, envs[1], "toadd=newenv")
52
+}
53
+
54
+func TestUpdateMounts(t *testing.T) {
55
+	flags := newUpdateCommand(nil).Flags()
56
+	flags.Set("mount", "type=volume,target=/toadd")
57
+	flags.Set("remove-mount", "/toremove")
58
+
59
+	mounts := []swarm.Mount{
60
+		{Target: "/toremove", Type: swarm.MountType("BIND")},
61
+		{Target: "/tokeep", Type: swarm.MountType("BIND")},
62
+	}
63
+
64
+	updateMounts(flags, &mounts)
65
+	assert.Equal(t, len(mounts), 2)
66
+	assert.Equal(t, mounts[0].Target, "/tokeep")
67
+	assert.Equal(t, mounts[1].Target, "/toadd")
68
+}
69
+
70
+func TestUpdateNetworks(t *testing.T) {
71
+	flags := newUpdateCommand(nil).Flags()
72
+	flags.Set("network", "toadd")
73
+	flags.Set("remove-network", "toremove")
74
+
75
+	attachments := []swarm.NetworkAttachmentConfig{
76
+		{Target: "toremove", Aliases: []string{"foo"}},
77
+		{Target: "tokeep"},
78
+	}
79
+
80
+	updateNetworks(flags, &attachments)
81
+	assert.Equal(t, len(attachments), 2)
82
+	assert.Equal(t, attachments[0].Target, "tokeep")
83
+	assert.Equal(t, attachments[1].Target, "toadd")
84
+}
85
+
86
+func TestUpdatePorts(t *testing.T) {
87
+	flags := newUpdateCommand(nil).Flags()
88
+	flags.Set("publish", "1000:1000")
89
+	flags.Set("remove-publish", "333/udp")
90
+
91
+	portConfigs := []swarm.PortConfig{
92
+		{TargetPort: 333, Protocol: swarm.PortConfigProtocol("udp")},
93
+		{TargetPort: 555},
94
+	}
95
+
96
+	updatePorts(flags, &portConfigs)
97
+	assert.Equal(t, len(portConfigs), 2)
98
+	assert.Equal(t, portConfigs[0].TargetPort, uint32(555))
99
+	assert.Equal(t, portConfigs[1].TargetPort, uint32(1000))
100
+}
... ...
@@ -22,7 +22,7 @@ func (s *DockerSwarmSuite) TestServiceUpdatePort(c *check.C) {
22 22
 	waitAndAssert(c, defaultReconciliationTimeout, d.checkActiveContainerCount, checker.Equals, 1)
23 23
 
24 24
 	// Update the service: changed the port mapping from 8080:8081 to 8082:8083.
25
-	_, err = d.Cmd("service", "update", "-p", "8082:8083", serviceName)
25
+	_, err = d.Cmd("service", "update", "-p", "8082:8083", "--remove-publish", "8081", serviceName)
26 26
 	c.Assert(err, checker.IsNil)
27 27
 
28 28
 	// Inspect the service and verify port mapping