Skip to content

Commit c6de8ad

Browse files
committed
Fix service update --env-add issue
This fix tries to address the issue in 25404 where updating environmental variable in `service update --env-add` will not work. The issue is because `--env-add` will only append the env, not update if the same env already exist. This fix tracks the env variable with a map and update if the variable is the same. An integration test has been added. This fix fixes 25404. Signed-off-by: Yong Tang <[email protected]>
1 parent b2b41b2 commit c6de8ad

File tree

2 files changed

+30
-3
lines changed

2 files changed

+30
-3
lines changed

api/client/service/update.go

+13-1
Original file line numberDiff line numberDiff line change
@@ -295,10 +295,22 @@ func updateLabels(flags *pflag.FlagSet, field *map[string]string) {
295295
}
296296

297297
func updateEnvironment(flags *pflag.FlagSet, field *[]string) {
298+
envSet := map[string]string{}
299+
for _, v := range *field {
300+
envSet[envKey(v)] = v
301+
}
298302
if flags.Changed(flagEnvAdd) {
299303
value := flags.Lookup(flagEnvAdd).Value.(*opts.ListOpts)
300-
*field = append(*field, value.GetAll()...)
304+
for _, v := range value.GetAll() {
305+
envSet[envKey(v)] = v
306+
}
301307
}
308+
309+
*field = []string{}
310+
for _, v := range envSet {
311+
*field = append(*field, v)
312+
}
313+
302314
toRemove := buildToRemoveSet(flags, flagEnvRemove)
303315
*field = removeItems(*field, toRemove, envKey)
304316
}

api/client/service/update_test.go

+17-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package service
22

33
import (
4+
"sort"
45
"testing"
56

67
"github.com/docker/docker/pkg/testutil/assert"
@@ -68,8 +69,10 @@ func TestUpdateEnvironment(t *testing.T) {
6869

6970
updateEnvironment(flags, &envs)
7071
assert.Equal(t, len(envs), 2)
71-
assert.Equal(t, envs[0], "tokeep=value")
72-
assert.Equal(t, envs[1], "toadd=newenv")
72+
// Order has been removed in updateEnvironment (map)
73+
sort.Strings(envs)
74+
assert.Equal(t, envs[0], "toadd=newenv")
75+
assert.Equal(t, envs[1], "tokeep=value")
7376
}
7477

7578
func TestUpdateEnvironmentWithDuplicateValues(t *testing.T) {
@@ -84,6 +87,18 @@ func TestUpdateEnvironmentWithDuplicateValues(t *testing.T) {
8487
assert.Equal(t, len(envs), 0)
8588
}
8689

90+
func TestUpdateEnvironmentWithDuplicateKeys(t *testing.T) {
91+
// Test case for #25404
92+
flags := newUpdateCommand(nil).Flags()
93+
flags.Set("env-add", "A=b")
94+
95+
envs := []string{"A=c"}
96+
97+
updateEnvironment(flags, &envs)
98+
assert.Equal(t, len(envs), 1)
99+
assert.Equal(t, envs[0], "A=b")
100+
}
101+
87102
func TestUpdateMounts(t *testing.T) {
88103
flags := newUpdateCommand(nil).Flags()
89104
flags.Set("mount-add", "type=volume,target=/toadd")

0 commit comments

Comments
 (0)