From 04cc850698915e4478a358d939fa79c0248c231c Mon Sep 17 00:00:00 2001 From: michael mccune Date: Wed, 13 Mar 2024 10:57:48 -0400 Subject: [PATCH] UPSTREAM: : Fix unstructured taint parsing in Cluster API provider Co-authored-by: Joel Speed --- .../clusterapi/clusterapi_controller_test.go | 14 +++++++ .../clusterapi/clusterapi_unstructured.go | 24 ++++++++--- .../clusterapi_unstructured_test.go | 42 +++++++++++++++++++ 3 files changed, 74 insertions(+), 6 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go index 278626e3e9b9..77a4273853f3 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go @@ -339,6 +339,13 @@ func createTestConfigs(specs ...testSpec) []*testConfig { "kind": machineTemplateKind, "name": "TestMachineTemplate", }, + "taints": []interface{}{ + map[string]interface{}{ + "key": "test", + "value": "test", + "effect": "NoSchedule", + }, + }, }, }, }, @@ -377,6 +384,13 @@ func createTestConfigs(specs ...testSpec) []*testConfig { "kind": machineTemplateKind, "name": "TestMachineTemplate", }, + "taints": []interface{}{ + map[string]interface{}{ + "key": "test", + "value": "test", + "effect": "NoSchedule", + }, + }, }, }, }, diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go index 8170c6d043bb..a50bb16da942 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go @@ -184,18 +184,30 @@ func (r unstructuredScalableResource) Taints() []apiv1.Taint { if !found || err != nil { return nil } - ret := make([]apiv1.Taint, len(taints)) - for i, t := range taints { - if v, ok := t.(apiv1.Taint); ok { - ret[i] = v + ret := make([]apiv1.Taint, 0) + for _, t := range taints { + if t := unstructuredToTaint(t); t != nil { + ret = append(ret, *t) } else { - // if we cannot convert the interface to a Taint, return early with zero value - return nil + klog.Warning("Unable to convert of type %T data to taint: %+v", t, t) } } return ret } +func unstructuredToTaint(unstructuredTaintInterface interface{}) *corev1.Taint { + unstructuredTaint := unstructuredTaintInterface.(map[string]interface{}) + if unstructuredTaint == nil { + return nil + } + + taint := &corev1.Taint{} + taint.Key = unstructuredTaint["key"].(string) + 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 { diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured_test.go index b565a08d98d8..16e30364d712 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured_test.go @@ -19,9 +19,12 @@ package clusterapi import ( "context" "fmt" + "reflect" "testing" "time" + "github.com/stretchr/testify/assert" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -282,11 +285,47 @@ func TestSetSizeAndReplicas(t *testing.T) { }) } +func TestTaints(t *testing.T) { + initialReplicas := 1 + + expectedTaints := []v1.Taint{{Key: "test", Effect: v1.TaintEffectNoSchedule, Value: "test"}} + + 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) + } + } + + 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 TestAnnotations(t *testing.T) { cpuQuantity := resource.MustParse("2") memQuantity := resource.MustParse("1024") gpuQuantity := resource.MustParse("1") maxPodsQuantity := resource.MustParse("42") + expectedTaints := []v1.Taint{{Key: "test", Effect: v1.TaintEffectNoSchedule, Value: "test"}} annotations := map[string]string{ cpuKey: cpuQuantity.String(), memoryKey: memQuantity.String(), @@ -331,6 +370,9 @@ func TestAnnotations(t *testing.T) { } else if maxPodsQuantity.Cmp(maxPods) != 0 { t.Errorf("expected %v, got %v", maxPodsQuantity, maxPods) } + + taints := sr.Taints() + assert.Equal(t, expectedTaints, taints) } t.Run("MachineSet", func(t *testing.T) {