Merge pull request #25427 from yongtang/25404-service-update-env-add

Fix `service update --env-add` issue

api/client/service/update.go

@@ -295,10 +295,22 @@ func updateLabels(flags *pflag.FlagSet, field *map[string]string) {
 }
 
 func updateEnvironment(flags *pflag.FlagSet, field *[]string) {
+	envSet := map[string]string{}
+	for _, v := range *field {
+		envSet[envKey(v)] = v
+	}
 	if flags.Changed(flagEnvAdd) {
 		value := flags.Lookup(flagEnvAdd).Value.(*opts.ListOpts)
-		*field = append(*field, value.GetAll()...)
+		for _, v := range value.GetAll() {
+			envSet[envKey(v)] = v
+		}
 	}
+
+	*field = []string{}
+	for _, v := range envSet {
+		*field = append(*field, v)
+	}
+
 	toRemove := buildToRemoveSet(flags, flagEnvRemove)
 	*field = removeItems(*field, toRemove, envKey)
 }

api/client/service/update_test.go

@@ -1,6 +1,7 @@
 package service
 
 import (
+	"sort"
 	"testing"
 
 	"github.com/docker/docker/pkg/testutil/assert"
@@ -68,8 +69,10 @@ func TestUpdateEnvironment(t *testing.T) {
 
 	updateEnvironment(flags, &envs)
 	assert.Equal(t, len(envs), 2)
-	assert.Equal(t, envs[0], "tokeep=value")
-	assert.Equal(t, envs[1], "toadd=newenv")
+	// Order has been removed in updateEnvironment (map)
+	sort.Strings(envs)
+	assert.Equal(t, envs[0], "toadd=newenv")
+	assert.Equal(t, envs[1], "tokeep=value")
 }
 
 func TestUpdateEnvironmentWithDuplicateValues(t *testing.T) {
@@ -84,6 +87,18 @@ func TestUpdateEnvironmentWithDuplicateValues(t *testing.T) {
 	assert.Equal(t, len(envs), 0)
 }
 
+func TestUpdateEnvironmentWithDuplicateKeys(t *testing.T) {
+	// Test case for #25404
+	flags := newUpdateCommand(nil).Flags()
+	flags.Set("env-add", "A=b")
+
+	envs := []string{"A=c"}
+
+	updateEnvironment(flags, &envs)
+	assert.Equal(t, len(envs), 1)
+	assert.Equal(t, envs[0], "A=b")
+}
+
 func TestUpdateMounts(t *testing.T) {
 	flags := newUpdateCommand(nil).Flags()
 	flags.Set("mount-add", "type=volume,target=/toadd")