UPSTREAM: <carry>: Fix unstructured taint parsing in Cluster API provider

This change corrects the behavior for parsing taints from the
unstructured scalable resource. This is required on OpenShift as our
implementation is slightly different from the upstream.

Author: JoelSpeed

cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go

@@ -347,6 +347,17 @@ func createTestConfigs(specs ...testSpec) []*testConfig {
 								"kind":       machineTemplateKind,
 								"name":       "TestMachineTemplate",
 							},
+							"taints": []interface{}{
+								map[string]interface{}{
+									"key":    "test",
+									"value":  "test",
+									"effect": "NoSchedule",
+								},
+								map[string]interface{}{
+									"key":    "test-no-value",
+									"effect": "NoSchedule",
+								},
+							},
 						},
 					},
 				},
@@ -385,6 +396,17 @@ func createTestConfigs(specs ...testSpec) []*testConfig {
 									"kind":       machineTemplateKind,
 									"name":       "TestMachineTemplate",
 								},
+								"taints": []interface{}{
+									map[string]interface{}{
+										"key":    "test",
+										"value":  "test",
+										"effect": "NoSchedule",
+									},
+									map[string]interface{}{
+										"key":    "test-no-value",
+										"effect": "NoSchedule",
+									},
+								},
 							},
 						},
 					},

cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go

@@ -212,10 +212,10 @@ func (r unstructuredScalableResource) Taints() []apiv1.Taint {
 	}
 	if found {
 		for _, t := range newtaints {
-			if v, ok := t.(apiv1.Taint); ok {
-				taints = append(taints, v)
+			if t := unstructuredToTaint(t); t != nil {
+				taints = append(taints, *t)
 			} else {
-				klog.Warning("Unable to convert data to taint: %v", t)
+				klog.Warningf("Unable to convert of type %T data to taint: %+v", t, t)
 				continue
 			}
 		}
@@ -236,6 +236,22 @@ func (r unstructuredScalableResource) Taints() []apiv1.Taint {
 	return taints
 }
 
+func unstructuredToTaint(unstructuredTaintInterface interface{}) *corev1.Taint {
+	unstructuredTaint := unstructuredTaintInterface.(map[string]interface{})
+	if unstructuredTaint == nil {
+		return nil
+	}
+
+	taint := &corev1.Taint{}
+	taint.Key = unstructuredTaint["key"].(string)
+	// value is optional and could be nil if not present
+	if unstructuredTaint["value"] != nil {
+		taint.Value = unstructuredTaint["value"].(string)
+	}
+	taint.Effect = corev1.TaintEffect(unstructuredTaint["effect"].(string))
+	return taint
+}
+
 // A node group can scale from zero if it can inform about the CPU and memory
 // capacity of the nodes within the group.
 func (r unstructuredScalableResource) CanScaleFromZero() bool {

cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured_test.go

@@ -19,6 +19,7 @@ package clusterapi
 import (
 	"context"
 	"fmt"
+	"reflect"
 	"testing"
 	"time"
 
@@ -222,6 +223,65 @@ func TestReplicas(t *testing.T) {
 	})
 }
 
+func TestTaints(t *testing.T) {
+	initialReplicas := 1
+
+	expectedTaints := []v1.Taint{
+		{Key: "test", Effect: v1.TaintEffectNoSchedule, Value: "test"},
+		{Key: "test-no-value", Effect: v1.TaintEffectNoSchedule},
+	}
+	expectedTaintsWithAnnotations := []v1.Taint{
+		{Key: "test", Effect: v1.TaintEffectNoSchedule, Value: "test"},
+		{Key: "test-no-value", Effect: v1.TaintEffectNoSchedule},
+		{Key: "key1", Effect: v1.TaintEffectNoSchedule, Value: "value1"},
+		{Key: "key2", Effect: v1.TaintEffectNoExecute, Value: "value2"},
+	}
+	taintAnnotation := "key1=value1:NoSchedule,key2=value2:NoExecute"
+
+	test := func(t *testing.T, testConfig *testConfig) {
+		controller, stop := mustCreateTestController(t, testConfig)
+		defer stop()
+
+		testResource := testConfig.machineSet
+		if testConfig.machineDeployment != nil {
+			testResource = testConfig.machineDeployment
+		}
+
+		sr, err := newUnstructuredScalableResource(controller, testResource)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		taints := sr.Taints()
+
+		if !reflect.DeepEqual(taints, expectedTaints) {
+			t.Errorf("expected %v, got: %v", expectedTaints, taints)
+		}
+
+		srAnnotations := sr.unstructured.GetAnnotations()
+		if srAnnotations == nil {
+			srAnnotations = make(map[string]string)
+		}
+
+		srAnnotations[taintsKey] = taintAnnotation
+		sr.unstructured.SetAnnotations(srAnnotations)
+
+		taints = sr.Taints()
+
+		if !reflect.DeepEqual(taints, expectedTaintsWithAnnotations) {
+			t.Errorf("expected %v, got: %v", expectedTaintsWithAnnotations, taints)
+		}
+	}
+
+	t.Run("MachineSet", func(t *testing.T) {
+		test(t, createMachineSetTestConfig(RandomString(6), RandomString(6), RandomString(6), initialReplicas, nil, nil))
+	})
+
+	t.Run("MachineDeployment", func(t *testing.T) {
+		test(t, createMachineDeploymentTestConfig(RandomString(6), RandomString(6), RandomString(6), initialReplicas, nil, nil))
+	})
+}
+
 func TestSetSizeAndReplicas(t *testing.T) {
 	initialReplicas := 1
 	updatedReplicas := 5
@@ -297,7 +357,15 @@ func TestAnnotations(t *testing.T) {
 	memQuantity := resource.MustParse("1024")
 	gpuQuantity := resource.MustParse("1")
 	maxPodsQuantity := resource.MustParse("42")
-	expectedTaints := []v1.Taint{{Key: "key1", Effect: v1.TaintEffectNoSchedule, Value: "value1"}, {Key: "key2", Effect: v1.TaintEffectNoExecute, Value: "value2"}}
+
+	// Note, the first two taints comes from the spec of the machine template, the rest from the annotations.
+	expectedTaints := []v1.Taint{
+		{Key: "test", Effect: v1.TaintEffectNoSchedule, Value: "test"},
+		{Key: "test-no-value", Effect: v1.TaintEffectNoSchedule},
+		{Key: "key1", Effect: v1.TaintEffectNoSchedule, Value: "value1"},
+		{Key: "key2", Effect: v1.TaintEffectNoExecute, Value: "value2"},
+	}
+
 	annotations := map[string]string{
 		cpuKey:          cpuQuantity.String(),
 		memoryKey:       memQuantity.String(),