diff --git a/daemon/mgr/container.go b/daemon/mgr/container.go index 4000bbc9f..4da718ee6 100644 --- a/daemon/mgr/container.go +++ b/daemon/mgr/container.go @@ -1036,42 +1036,11 @@ func (mgr *ContainerManager) Update(ctx context.Context, name string, config *ty } // Update Env - if len(config.Env) > 0 { - newEnvMap, err := opts.ParseEnv(config.Env) - if err != nil { - return errors.Wrapf(err, "failed to parse new env") - } - - oldEnvMap, err := opts.ParseEnv(c.Config.Env) - if err != nil { - return errors.Wrapf(err, "failed to parse old env") - } - - for k, v := range newEnvMap { - // key should not be empty - if k == "" { - continue - } - - // add or change an env - if v != "" { - oldEnvMap[k] = v - continue - } - - // value is empty, we need delete the env - if _, exists := oldEnvMap[k]; exists { - delete(oldEnvMap, k) - } - } - - newEnvSlice := []string{} - for k, v := range oldEnvMap { - newEnvSlice = append(newEnvSlice, fmt.Sprintf("%s=%s", k, v)) - } - - c.Config.Env = newEnvSlice + newEnvSlice, err := mergeEnvSlice(config.Env, c.Config.Env) + if err != nil { + return err } + c.Config.Env = newEnvSlice // If container is not running, update container metadata struct is enough, // resources will be updated when the container is started again, diff --git a/daemon/mgr/container_types.go b/daemon/mgr/container_types.go index 985da48ef..5b74967db 100644 --- a/daemon/mgr/container_types.go +++ b/daemon/mgr/container_types.go @@ -287,11 +287,13 @@ func (c *Container) merge(getconfig func() (v1.ImageConfig, error)) error { } c.Config.Entrypoint = config.Entrypoint } - if c.Config.Env == nil { - c.Config.Env = config.Env - } else { - c.Config.Env = append(c.Config.Env, config.Env...) + + // ContainerConfig.Env is new, and the ImageConfig.Env is old + newEnvSlice, err := mergeEnvSlice(c.Config.Env, config.Env) + if err != nil { + return err } + c.Config.Env = newEnvSlice if c.Config.WorkingDir == "" { c.Config.WorkingDir = config.WorkingDir } diff --git a/daemon/mgr/container_utils.go b/daemon/mgr/container_utils.go index a8df7ac77..e394f8866 100644 --- a/daemon/mgr/container_utils.go +++ b/daemon/mgr/container_utils.go @@ -6,6 +6,7 @@ import ( "strconv" "strings" + "github.com/alibaba/pouch/apis/opts" "github.com/alibaba/pouch/apis/types" networktypes "github.com/alibaba/pouch/network/types" "github.com/alibaba/pouch/pkg/errtypes" @@ -256,3 +257,45 @@ func amendContainerSettings(config *types.ContainerConfig, hostConfig *types.Hos r.MemorySwap = 2 * r.Memory } } + +func mergeEnvSlice(newEnv, oldEnv []string) ([]string, error) { + // if newEnv is empty, return old env slice + if len(newEnv) == 0 { + return oldEnv, nil + } + + newEnvMap, err := opts.ParseEnv(newEnv) + if err != nil { + return nil, errors.Wrapf(err, "failed to parse new env") + } + + oldEnvMap, err := opts.ParseEnv(oldEnv) + if err != nil { + return nil, errors.Wrapf(err, "failed to parse old env") + } + + for k, v := range newEnvMap { + // key should not be empty + if k == "" { + continue + } + + // add or change an env + if v != "" { + oldEnvMap[k] = v + continue + } + + // value is empty, we need delete the env + if _, exists := oldEnvMap[k]; exists { + delete(oldEnvMap, k) + } + } + + newEnvSlice := []string{} + for k, v := range oldEnvMap { + newEnvSlice = append(newEnvSlice, fmt.Sprintf("%s=%s", k, v)) + } + + return newEnvSlice, nil +} diff --git a/daemon/mgr/container_utils_test.go b/daemon/mgr/container_utils_test.go index 3a11d81ec..0e1e803d5 100644 --- a/daemon/mgr/container_utils_test.go +++ b/daemon/mgr/container_utils_test.go @@ -187,3 +187,84 @@ func Test_parsePSOutput(t *testing.T) { }) } } + +func Test_mergeEnvSlice(t *testing.T) { + type args struct { + newEnv []string + oldEnv []string + } + tests := []struct { + name string + args args + want []string + wantErr bool + }{ + // TODO: Add test cases. + { + name: "test1", + args: args{ + newEnv: []string{"a=b"}, + oldEnv: []string{"c=d"}, + }, + want: []string{"c=d", "a=b"}, + wantErr: false, + }, + { + name: "test2", + args: args{ + newEnv: []string{"test=false"}, + oldEnv: []string{"test=true"}, + }, + want: []string{"test=false"}, + wantErr: false, + }, + { + name: "test3", + args: args{ + newEnv: []string{"wrong-format"}, + oldEnv: []string{"c=d"}, + }, + want: nil, + wantErr: true, + }, + { + name: "test4", + args: args{ + newEnv: []string{}, + oldEnv: []string{"c=d"}, + }, + want: []string{"c=d"}, + wantErr: false, + }, + { + name: "test5", + args: args{ + newEnv: []string{"a=b"}, + oldEnv: []string{}, + }, + want: []string{"a=b"}, + wantErr: false, + }, + { + name: "test6", + args: args{ + newEnv: []string{"a="}, + oldEnv: []string{"a=b"}, + }, + want: []string{}, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := mergeEnvSlice(tt.args.newEnv, tt.args.oldEnv) + if (err != nil) != tt.wantErr { + t.Errorf("mergeEnvSlice() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("mergeEnvSlice() = %v, want %v", got, tt.want) + } + }) + } +}