Skip to content

Commit

Permalink
UPSTREAM: <carry>: Fix unstructured taint parsing in Cluster API prov…
Browse files Browse the repository at this point in the history
…ider

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.
JoelSpeed authored and elmiko committed Oct 14, 2024
1 parent 43224a9 commit 1409aa5
Showing 3 changed files with 110 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -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",
},
},
},
},
},
Original file line number Diff line number Diff line change
@@ -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 {
Original file line number Diff line number Diff line change
@@ -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(),

0 comments on commit 1409aa5

Please sign in to comment.