should not change relative order of envs (#2086)

pkg/manager/member/pd_member_manager.go

@@ -622,7 +622,7 @@ func getNewPDSetForTidbCluster(tc *v1alpha1.TidbCluster, cm *corev1.ConfigMap) (
  },
  })
  }
- pdContainer.Env = util.MergeEnv(basePDSpec.Env(), env)
+ pdContainer.Env = util.AppendEnv(env, basePDSpec.Env())
  podSpec.Volumes = vols
  podSpec.Containers = []corev1.Container{pdContainer}
 

pkg/manager/member/pd_member_manager_test.go

@@ -1081,17 +1081,6 @@ func TestGetNewPDSetForTidbCluster(t *testing.T) {
  },
  },
  testSts: testPDContainerEnv(t, []corev1.EnvVar{
- {
- Name: "DASHBOARD_SESSION_SECRET",
- ValueFrom: &corev1.EnvVarSource{
- SecretKeyRef: &corev1.SecretKeySelector{
- LocalObjectReference: corev1.LocalObjectReference{
- Name: "dashboard-session-secret",
- },
- Key: "encryption_key",
- },
- },
- },
  {
  Name: "NAMESPACE",
  ValueFrom: &corev1.EnvVarSource{
@@ -1115,6 +1104,17 @@ func TestGetNewPDSetForTidbCluster(t *testing.T) {
  {
  Name: "TZ",
  },
+ {
+ Name: "DASHBOARD_SESSION_SECRET",
+ ValueFrom: &corev1.EnvVarSource{
+ SecretKeyRef: &corev1.SecretKeySelector{
+ LocalObjectReference: corev1.LocalObjectReference{
+ Name: "dashboard-session-secret",
+ },
+ Key: "encryption_key",
+ },
+ },
+ },
  }),
  },
  // TODO add more tests

pkg/manager/member/pump_member_manager.go

@@ -362,7 +362,7 @@ func getNewPumpStatefulSet(tc *v1alpha1.TidbCluster, cm *corev1.ConfigMap) (*app
  ContainerPort: 8250,
  }},
  Resources: controller.ContainerResource(tc.Spec.Pump.ResourceRequirements),
- Env: util.MergeEnv(spec.Env(), envs),
+ Env: util.AppendEnv(envs, spec.Env()),
  VolumeMounts: volumeMounts,
  },
  }

pkg/manager/member/tidb_member_manager.go

@@ -654,7 +654,7 @@ func getNewTiDBSetForTidbCluster(tc *v1alpha1.TidbCluster, cm *corev1.ConfigMap)
  },
  VolumeMounts: volMounts,
  Resources: controller.ContainerResource(tc.Spec.TiDB.ResourceRequirements),
- Env: util.MergeEnv(baseTiDBSpec.Env(), envs),
+ Env: util.AppendEnv(envs, baseTiDBSpec.Env()),
  ReadinessProbe: &corev1.Probe{
  Handler: corev1.Handler{
  HTTPGet: &corev1.HTTPGetAction{

pkg/manager/member/tikv_member_manager.go

@@ -454,7 +454,7 @@ func getNewTiKVSetForTidbCluster(tc *v1alpha1.TidbCluster, cm *corev1.ConfigMap)
  },
  })
  }
- tikvContainer.Env = util.MergeEnv(baseTiKVSpec.Env(), env)
+ tikvContainer.Env = util.AppendEnv(env, baseTiKVSpec.Env())
  podSpec.Volumes = vols
  podSpec.SecurityContext = podSecurityContext
  podSpec.InitContainers = initContainers

pkg/util/util.go

@@ -16,7 +16,6 @@ package util
 import (
  "encoding/json"
  "fmt"
- "sort"
  "strconv"
  "strings"
 
@@ -196,19 +195,18 @@ func (e SortEnvByName) Less(i, j int) bool {
  return e[i].Name < e[j].Name
 }
 
-// MergeEnv merges env in `b` to `a` and overrides env that has the same name.
-func MergeEnv(a []corev1.EnvVar, b []corev1.EnvVar) []corev1.EnvVar {
- tmpEnv := make(map[string]corev1.EnvVar)
+// AppendEnv appends envs `b` into `a` ignoring envs whose names already exist
+// in `b`.
+// Note that this will not change relative order of envs.
+func AppendEnv(a []corev1.EnvVar, b []corev1.EnvVar) []corev1.EnvVar {
+ aMap := make(map[string]corev1.EnvVar)
  for _, e := range a {
- tmpEnv[e.Name] = e
+ aMap[e.Name] = e
  }
  for _, e := range b {
- tmpEnv[e.Name] = e
- }
- c := make([]corev1.EnvVar, 0)
- for _, e := range tmpEnv {
- c = append(c, e)
+ if _, ok := aMap[e.Name]; !ok {
+ a = append(a, e)
+ }
  }
- sort.Sort(SortEnvByName(c))
- return c
+ return a
 }

pkg/util/utils_test.go

@@ -128,15 +128,15 @@ func TestGetPodOrdinals(t *testing.T) {
  }
 }
 
-func TestMergeEnv(t *testing.T) {
+func TestAppendEnv(t *testing.T) {
  tests := []struct {
  name string
  a []corev1.EnvVar
  b []corev1.EnvVar
  want []corev1.EnvVar
  }{
  {
- name: "b overrides a with the same name",
+ name: "envs whose names exist are ignored",
  a: []corev1.EnvVar{
  {
  Name: "foo",
@@ -156,27 +156,31 @@ func TestMergeEnv(t *testing.T) {
  Name: "new",
  Value: "bar",
  },
+ {
+ Name: "xxx",
+ Value: "yyy",
+ },
  },
  want: []corev1.EnvVar{
  {
  Name: "foo",
- Value: "barbar",
- },
- {
- Name: "new",
  Value: "bar",
  },
  {
  Name: "xxx",
  Value: "xxx",
  },
+ {
+ Name: "new",
+ Value: "bar",
+ },
  },
  },
  }
 
  for _, tt := range tests {
  t.Run(tt.name, func(t *testing.T) {
- got := MergeEnv(tt.a, tt.b)
+ got := AppendEnv(tt.a, tt.b)
  if diff := cmp.Diff(tt.want, got); diff != "" {
  t.Errorf("unwant (-want, +got): %s", diff)
  }