diff --git a/cli/command/service/update.go b/cli/command/service/update.go index a36ea2e4f1df..78230e228121 100644 --- a/cli/command/service/update.go +++ b/cli/command/service/update.go @@ -640,6 +640,12 @@ func updatePlacementPreferences(flags *pflag.FlagSet, placement *swarm.Placement } func updateContainerLabels(flags *pflag.FlagSet, field *map[string]string) { + if *field != nil && flags.Changed(flagContainerLabelRemove) { + toRemove := flags.Lookup(flagContainerLabelRemove).Value.(*opts.ListOpts).GetAll() + for _, label := range toRemove { + delete(*field, label) + } + } if flags.Changed(flagContainerLabelAdd) { if *field == nil { *field = map[string]string{} @@ -650,16 +656,15 @@ func updateContainerLabels(flags *pflag.FlagSet, field *map[string]string) { (*field)[key] = value } } +} - if *field != nil && flags.Changed(flagContainerLabelRemove) { - toRemove := flags.Lookup(flagContainerLabelRemove).Value.(*opts.ListOpts).GetAll() +func updateLabels(flags *pflag.FlagSet, field *map[string]string) { + if *field != nil && flags.Changed(flagLabelRemove) { + toRemove := flags.Lookup(flagLabelRemove).Value.(*opts.ListOpts).GetAll() for _, label := range toRemove { delete(*field, label) } } -} - -func updateLabels(flags *pflag.FlagSet, field *map[string]string) { if flags.Changed(flagLabelAdd) { if *field == nil { *field = map[string]string{} @@ -670,13 +675,6 @@ func updateLabels(flags *pflag.FlagSet, field *map[string]string) { (*field)[key] = value } } - - if *field != nil && flags.Changed(flagLabelRemove) { - toRemove := flags.Lookup(flagLabelRemove).Value.(*opts.ListOpts).GetAll() - for _, label := range toRemove { - delete(*field, label) - } - } } func updateSysCtls(flags *pflag.FlagSet, field *map[string]string) { @@ -699,6 +697,9 @@ func updateSysCtls(flags *pflag.FlagSet, field *map[string]string) { } func updateEnvironment(flags *pflag.FlagSet, field *[]string) { + toRemove := buildToRemoveSet(flags, flagEnvRemove) + *field = removeItems(*field, toRemove, envKey) + if flags.Changed(flagEnvAdd) { envSet := map[string]string{} for _, v := range *field { @@ -715,9 +716,6 @@ func updateEnvironment(flags *pflag.FlagSet, field *[]string) { *field = append(*field, v) } } - - toRemove := buildToRemoveSet(flags, flagEnvRemove) - *field = removeItems(*field, toRemove, envKey) } func getUpdatedSecrets(apiClient client.SecretAPIClient, flags *pflag.FlagSet, secrets []*swarm.SecretReference) ([]*swarm.SecretReference, error) { diff --git a/cli/command/service/update_test.go b/cli/command/service/update_test.go index a785d35f3db1..7a2511815bb3 100644 --- a/cli/command/service/update_test.go +++ b/cli/command/service/update_test.go @@ -34,27 +34,58 @@ func TestUpdateServiceArgs(t *testing.T) { func TestUpdateLabels(t *testing.T) { flags := newUpdateCommand(nil).Flags() - flags.Set("label-add", "toadd=newlabel") - flags.Set("label-rm", "toremove") + flags.Set("label-add", "add-beats-remove=value") + flags.Set("label-add", "to-add=value") + flags.Set("label-add", "to-update=new-value") + flags.Set("label-add", "to-replace=new-value") + flags.Set("label-rm", "add-beats-remove") + flags.Set("label-rm", "to-remove") + flags.Set("label-rm", "to-replace") + flags.Set("label-rm", "no-such-label") labels := map[string]string{ - "toremove": "thelabeltoremove", - "tokeep": "value", + "to-keep": "value", + "to-remove": "value", + "to-replace": "value", + "to-update": "value", } updateLabels(flags, &labels) - assert.Check(t, is.Len(labels, 2)) - assert.Check(t, is.Equal("value", labels["tokeep"])) - assert.Check(t, is.Equal("newlabel", labels["toadd"])) + assert.DeepEqual(t, labels, map[string]string{ + "add-beats-remove": "value", + "to-add": "value", + "to-keep": "value", + "to-replace": "new-value", + "to-update": "new-value", + }) } -func TestUpdateLabelsRemoveALabelThatDoesNotExist(t *testing.T) { +func TestUpdateContainerLabels(t *testing.T) { flags := newUpdateCommand(nil).Flags() - flags.Set("label-rm", "dne") + flags.Set("container-label-add", "add-beats-remove=value") + flags.Set("container-label-add", "to-add=value") + flags.Set("container-label-add", "to-update=new-value") + flags.Set("container-label-add", "to-replace=new-value") + flags.Set("container-label-rm", "add-beats-remove") + flags.Set("container-label-rm", "to-remove") + flags.Set("container-label-rm", "to-replace") + flags.Set("container-label-rm", "no-such-label") - labels := map[string]string{"foo": "theoldlabel"} - updateLabels(flags, &labels) - assert.Check(t, is.Len(labels, 1)) + labels := map[string]string{ + "to-keep": "value", + "to-remove": "value", + "to-replace": "value", + "to-update": "value", + } + + updateContainerLabels(flags, &labels) + assert.DeepEqual(t, labels, map[string]string{ + "add-beats-remove": "value", + "to-add": "value", + "to-keep": "value", + "to-replace": "new-value", + "to-update": "new-value", + }) } func TestUpdatePlacementConstraints(t *testing.T) { @@ -115,14 +146,15 @@ func TestUpdateEnvironment(t *testing.T) { 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") + flags.Set("env-add", "foo=first") + flags.Set("env-add", "foo=second") envs := []string{"foo=value"} updateEnvironment(flags, &envs) - assert.Check(t, is.Len(envs, 0)) + assert.Check(t, is.Len(envs, 1)) + assert.Equal(t, envs[0], "foo=second") } func TestUpdateEnvironmentWithDuplicateKeys(t *testing.T) {