Add templateName to kube-env to ensure that correct value is cached

cluster-autoscaler/cloudprovider/gce/kube_env.go

@@ -29,42 +29,45 @@ const (
 )
 
 // KubeEnv stores kube-env information from InstanceTemplate
-type KubeEnv map[string]string
+type KubeEnv struct {
+	templateName string
+	env          map[string]string
+}
 
 // ExtractKubeEnv extracts kube-env from InstanceTemplate
 func ExtractKubeEnv(template *gce.InstanceTemplate) (KubeEnv, error) {
 	if template == nil {
-		return nil, errors.New("instance template is nil")
+		return KubeEnv{}, errors.New("instance template is nil")
 	}
 	if template.Properties == nil || template.Properties.Metadata == nil {
-		return nil, fmt.Errorf("instance template %s has no metadata", template.Name)
+		return KubeEnv{}, fmt.Errorf("instance template %s has no metadata", template.Name)
 	}
 	for _, item := range template.Properties.Metadata.Items {
 		if item.Key == kubeEnvKey {
 			if item.Value == nil {
-				return nil, fmt.Errorf("no kube-env content in metadata")
+				return KubeEnv{}, fmt.Errorf("no kube-env content in metadata")
 			}
-			return ParseKubeEnv(*item.Value)
+			return ParseKubeEnv(template.Name, *item.Value)
 		}
 	}
-	return nil, nil
+	return KubeEnv{templateName: template.Name}, nil
 }
 
 // ParseKubeEnv parses kube-env from its string representation
-func ParseKubeEnv(kubeEnvValue string) (KubeEnv, error) {
-	kubeEnv := make(map[string]string)
-	err := yaml.Unmarshal([]byte(kubeEnvValue), &kubeEnv)
+func ParseKubeEnv(templateName, kubeEnvValue string) (KubeEnv, error) {
+	env := make(map[string]string)
+	err := yaml.Unmarshal([]byte(kubeEnvValue), &env)
 	if err != nil {
-		return nil, fmt.Errorf("error unmarshalling kubeEnv: %v", err)
+		return KubeEnv{}, fmt.Errorf("error unmarshalling kubeEnv: %v", err)
 	}
-	return kubeEnv, nil
+	return KubeEnv{templateName: templateName, env: env}, nil
 }
 
 // Var extracts variable from KubeEnv
 func (ke KubeEnv) Var(name string) (string, bool) {
-	if ke == nil {
+	if ke.env == nil {
 		return "", false
 	}
-	val, found := ke[name]
+	val, found := ke.env[name]
 	return val, found
 }

cluster-autoscaler/cloudprovider/gce/mig_info_provider.go

@@ -289,22 +289,21 @@ func (c *cachingMigInfoProvider) GetMigInstanceTemplate(migRef GceRef) (*gce.Ins
 func (c *cachingMigInfoProvider) GetMigKubeEnv(migRef GceRef) (KubeEnv, error) {
 	templateName, err := c.GetMigInstanceTemplateName(migRef)
 	if err != nil {
-		return nil, err
+		return KubeEnv{}, err
 	}
 
-	template, templateFound := c.cache.GetMigInstanceTemplate(migRef)
 	kubeEnv, kubeEnvFound := c.cache.GetMigKubeEnv(migRef)
-	if templateFound && template.Name == templateName && kubeEnvFound {
+	if kubeEnvFound && kubeEnv.templateName == templateName {
 		return kubeEnv, nil
 	}
 
-	template, err = c.GetMigInstanceTemplate(migRef)
+	template, err := c.GetMigInstanceTemplate(migRef)
 	if err != nil {
-		return nil, err
+		return KubeEnv{}, err
 	}
 	kubeEnv, err = ExtractKubeEnv(template)
 	if err != nil {
-		return nil, err
+		return KubeEnv{}, err
 	}
 	c.cache.SetMigKubeEnv(migRef, kubeEnv)
 	return kubeEnv, nil

cluster-autoscaler/cloudprovider/gce/templates.go

@@ -213,7 +213,7 @@ func (t *GceTemplateBuilder) BuildNodeFromTemplate(mig Mig, migOsInfo MigOsInfo,
 	}
 	var nodeAllocatable apiv1.ResourceList
 
-	if kubeEnv != nil {
+	if kubeEnv.env != nil {
 		// Extract labels
 		kubeEnvLabels, err := extractLabelsFromKubeEnv(kubeEnv)
 		if err != nil {

cluster-autoscaler/cloudprovider/gce/templates_test.go

@@ -481,7 +481,7 @@ func TestBuildAllocatableFromKubeEnv(t *testing.T) {
 		assert.NoError(t, err)
 		tb := GceTemplateBuilder{}
 		var allocatable apiv1.ResourceList
-		kubeEnv, err := ParseKubeEnv(tc.kubeEnvValue)
+		kubeEnv, err := ParseKubeEnv("test", tc.kubeEnvValue)
 		if err == nil {
 			allocatable, err = tb.BuildAllocatableFromKubeEnv(capacity, kubeEnv, ParseEvictionHardOrGetDefault(nil))
 		}
@@ -671,7 +671,7 @@ func TestExtractAutoscalingOptionsFromKubeEnv(t *testing.T) {
 	for _, c := range cases {
 		t.Run(c.desc, func(t *testing.T) {
 			var value map[string]string
-			kubeEnv, err := ParseKubeEnv(c.kubeEnvValue)
+			kubeEnv, err := ParseKubeEnv("test", c.kubeEnvValue)
 			if err == nil {
 				value, err = extractAutoscalingOptionsFromKubeEnv(kubeEnv)
 			}
@@ -739,7 +739,7 @@ func TestExtractAutoscalerVarFromKubeEnv(t *testing.T) {
 		t.Run(c.desc, func(t *testing.T) {
 			var value string
 			var found bool
-			kubeEnv, err := ParseKubeEnv(c.kubeEnvValue)
+			kubeEnv, err := ParseKubeEnv("test", c.kubeEnvValue)
 			if err == nil {
 				value, found, err = extractAutoscalerVarFromKubeEnv(kubeEnv, c.name)
 			}
@@ -794,7 +794,7 @@ func TestExtractLabelsFromKubeEnv(t *testing.T) {
 	for _, c := range cases {
 		t.Run(c.desc, func(t *testing.T) {
 			var labels map[string]string
-			kubeEnv, err := ParseKubeEnv(c.kubeEnvValue)
+			kubeEnv, err := ParseKubeEnv("test", c.kubeEnvValue)
 			if err == nil {
 				labels, err = extractLabelsFromKubeEnv(kubeEnv)
 			}
@@ -891,7 +891,7 @@ func TestExtractTaintsFromKubeEnv(t *testing.T) {
 	for _, c := range cases {
 		t.Run(c.desc, func(t *testing.T) {
 			var taints []apiv1.Taint
-			kubeEnv, err := ParseKubeEnv(c.kubeEnvValue)
+			kubeEnv, err := ParseKubeEnv("test", c.kubeEnvValue)
 			if err == nil {
 				taints, err = extractTaintsFromKubeEnv(kubeEnv)
 			}
@@ -989,7 +989,7 @@ func TestExtractKubeReservedFromKubeEnv(t *testing.T) {
 
 	for _, tc := range testCases {
 		var reserved string
-		kubeEnv, err := ParseKubeEnv(tc.kubeEnvValue)
+		kubeEnv, err := ParseKubeEnv("test", tc.kubeEnvValue)
 		if err == nil {
 			reserved, err = extractKubeReservedFromKubeEnv(kubeEnv)
 		}
@@ -1077,7 +1077,7 @@ func TestExtractOperatingSystemFromKubeEnv(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			kubeEnv, err := ParseKubeEnv(tc.kubeEnvValue)
+			kubeEnv, err := ParseKubeEnv("test", tc.kubeEnvValue)
 			assert.NoError(t, err)
 			actualOperatingSystem := extractOperatingSystemFromKubeEnv(kubeEnv)
 			assert.Equal(t, tc.expectedOperatingSystem, actualOperatingSystem)
@@ -1204,7 +1204,7 @@ func TestExtractOperatingSystemDistributionFromKubeEnv(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			kubeEnv, err := ParseKubeEnv(tc.kubeEnvValue)
+			kubeEnv, err := ParseKubeEnv("test", tc.kubeEnvValue)
 			assert.NoError(t, err)
 			actualOperatingSystem := extractOperatingSystemDistributionFromKubeEnv(kubeEnv)
 			assert.Equal(t, tc.expectedOperatingSystemDistribution, actualOperatingSystem)
@@ -1307,7 +1307,7 @@ func TestExtractExtendedResourcesFromKubeEnv(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			var extendedResources apiv1.ResourceList
-			kubeEnv, err := ParseKubeEnv(tc.kubeEnvValue)
+			kubeEnv, err := ParseKubeEnv("test", tc.kubeEnvValue)
 			if err == nil {
 				extendedResources, err = extractExtendedResourcesFromKubeEnv(kubeEnv)
 			}
@@ -1417,7 +1417,7 @@ func TestExtractSystemArchitectureFromKubeEnv(t *testing.T) {
 	} {
 		t.Run(tn, func(t *testing.T) {
 			var gotArch SystemArchitecture
-			kubeEnv, gotErr := ParseKubeEnv(tc.kubeEnvValue)
+			kubeEnv, gotErr := ParseKubeEnv("test", tc.kubeEnvValue)
 			if gotErr == nil {
 				gotArch, gotErr = extractSystemArchitectureFromKubeEnv(kubeEnv)
 			}