Skip to content

Commit 4214fd5

Browse files
jackfrancisjackfrancis
committed
[cluster-autoscaler-1.34] allow clusterapi provider to process managed labels
1 parent a76e813 commit 4214fd5

File tree

5 files changed

+170
-21
lines changed

5 files changed

+170
-21
lines changed

‎cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1494,6 +1494,7 @@ func TestNodeGroupTemplateNodeInfo(t *testing.T) {
14941494

14951495
type testCaseConfig struct {
14961496
nodeLabels map[string]string
1497+
managedLabels map[string]string
14971498
includeNodes bool
14981499
expectedErr error
14991500
expectedCapacity map[corev1.ResourceName]int64
@@ -1618,6 +1619,37 @@ func TestNodeGroupTemplateNodeInfo(t *testing.T) {
16181619
},
16191620
},
16201621
},
1622+
{
1623+
name: "When the NodeGroup can scale from zero, and the scalable resource contains managed labels",
1624+
nodeGroupAnnotations: map[string]string{
1625+
memoryKey: "2048Mi",
1626+
cpuKey: "2",
1627+
gpuTypeKey: gpuapis.ResourceNvidiaGPU,
1628+
gpuCountKey: "1",
1629+
},
1630+
config: testCaseConfig{
1631+
expectedErr: nil,
1632+
nodeLabels: map[string]string{
1633+
"kubernetes.io/os": "linux",
1634+
"kubernetes.io/arch": "amd64",
1635+
},
1636+
managedLabels: map[string]string{
1637+
"node-role.kubernetes.io/test": "test",
1638+
},
1639+
expectedCapacity: map[corev1.ResourceName]int64{
1640+
corev1.ResourceCPU: 2,
1641+
corev1.ResourceMemory: 2048 * 1024 * 1024,
1642+
corev1.ResourcePods: 110,
1643+
gpuapis.ResourceNvidiaGPU: 1,
1644+
},
1645+
expectedNodeLabels: map[string]string{
1646+
"kubernetes.io/os": "linux",
1647+
"kubernetes.io/arch": "amd64",
1648+
"kubernetes.io/hostname": "random value",
1649+
"node-role.kubernetes.io/test": "test",
1650+
},
1651+
},
1652+
},
16211653
}
16221654

16231655
test := func(t *testing.T, testConfig *TestConfig, config testCaseConfig) {
@@ -1704,6 +1736,7 @@ func TestNodeGroupTemplateNodeInfo(t *testing.T) {
17041736
WithNamespace(testNamespace).
17051737
WithNodeCount(10).
17061738
WithAnnotations(cloudprovider.JoinStringMaps(enableScaleAnnotations, tc.nodeGroupAnnotations)).
1739+
WithManagedLabels(tc.config.managedLabels).
17071740
Build()
17081741
test(t, testConfig, tc.config)
17091742
})
@@ -1714,6 +1747,7 @@ func TestNodeGroupTemplateNodeInfo(t *testing.T) {
17141747
WithNamespace(testNamespace).
17151748
WithNodeCount(10).
17161749
WithAnnotations(cloudprovider.JoinStringMaps(enableScaleAnnotations, tc.nodeGroupAnnotations)).
1750+
WithManagedLabels(tc.config.managedLabels).
17171751
Build()
17181752
test(t, testConfig, tc.config)
17191753
})

‎cluster-autoscaler/cloudprovider/clusterapi/clusterapi_test_framework.go‎

Lines changed: 53 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -53,24 +53,26 @@ const (
5353
)
5454

5555
type testConfigBuilder struct {
56-
scalableType scalableTestType
57-
namespace string
58-
clusterName string
59-
namePrefix string
60-
nodeCount int
61-
annotations map[string]string
62-
capacity map[string]string
56+
scalableType scalableTestType
57+
namespace string
58+
clusterName string
59+
namePrefix string
60+
nodeCount int
61+
annotations map[string]string
62+
capacity map[string]string
63+
managedLabels map[string]string
6364
}
6465

6566
// NewTestConfigBuilder returns a builder for dynamically constructing mock ClusterAPI resources for testing.
6667
func NewTestConfigBuilder() *testConfigBuilder {
6768
return &testConfigBuilder{
68-
namespace: RandomString(6),
69-
clusterName: RandomString(6),
70-
namePrefix: RandomString(6),
71-
nodeCount: 0,
72-
annotations: nil,
73-
capacity: nil,
69+
namespace: RandomString(6),
70+
clusterName: RandomString(6),
71+
namePrefix: RandomString(6),
72+
nodeCount: 0,
73+
annotations: nil,
74+
capacity: nil,
75+
managedLabels: nil,
7476
}
7577
}
7678

@@ -91,6 +93,7 @@ func (b *testConfigBuilder) Build() *TestConfig {
9193
isMachineDeployment,
9294
b.annotations,
9395
b.capacity,
96+
b.managedLabels,
9497
)[0],
9598
)[0]
9699
}
@@ -111,6 +114,7 @@ func (b *testConfigBuilder) BuildMultiple(configCount int) []*TestConfig {
111114
isMachineDeployment,
112115
b.annotations,
113116
b.capacity,
117+
b.managedLabels,
114118
)...,
115119
)
116120
}
@@ -171,6 +175,19 @@ func (b *testConfigBuilder) WithCapacity(c map[string]string) *testConfigBuilder
171175
return b
172176
}
173177

178+
func (b *testConfigBuilder) WithManagedLabels(l map[string]string) *testConfigBuilder {
179+
if l == nil {
180+
// explicitly setting managed labels to nil
181+
b.managedLabels = nil
182+
} else {
183+
if b.managedLabels == nil {
184+
b.managedLabels = map[string]string{}
185+
}
186+
maps.Insert(b.managedLabels, maps.All(l))
187+
}
188+
return b
189+
}
190+
174191
// TestConfig contains clusterspecific information about a single test configuration.
175192
type TestConfig struct {
176193
spec *TestSpec
@@ -220,6 +237,9 @@ func createTestConfigs(specs ...TestSpec) []*TestConfig {
220237
"kind": machineTemplateKind,
221238
"name": "TestMachineTemplate",
222239
},
240+
"metadata": map[string]interface{}{
241+
"labels": map[string]interface{}{},
242+
},
223243
},
224244
},
225245
},
@@ -229,6 +249,12 @@ func createTestConfigs(specs ...TestSpec) []*TestConfig {
229249

230250
config.machineSet.SetAnnotations(make(map[string]string))
231251

252+
if spec.managedLabels != nil {
253+
if err := unstructured.SetNestedStringMap(config.machineSet.Object, spec.managedLabels, "spec", "template", "spec", "metadata", "labels"); err != nil {
254+
panic(err)
255+
}
256+
}
257+
232258
if !spec.rootIsMachineDeployment {
233259
config.machineSet.SetAnnotations(spec.annotations)
234260
} else {
@@ -258,6 +284,9 @@ func createTestConfigs(specs ...TestSpec) []*TestConfig {
258284
"kind": machineTemplateKind,
259285
"name": "TestMachineTemplate",
260286
},
287+
"metadata": map[string]interface{}{
288+
"labels": map[string]interface{}{},
289+
},
261290
},
262291
},
263292
},
@@ -278,6 +307,12 @@ func createTestConfigs(specs ...TestSpec) []*TestConfig {
278307
},
279308
}
280309
config.machineSet.SetOwnerReferences(ownerRefs)
310+
311+
if spec.managedLabels != nil {
312+
if err := unstructured.SetNestedStringMap(config.machineDeployment.Object, spec.managedLabels, "spec", "template", "spec", "metadata", "labels"); err != nil {
313+
panic(err)
314+
}
315+
}
281316
}
282317
config.machineSet.SetLabels(machineSetLabels)
283318
if err := unstructured.SetNestedStringMap(config.machineSet.Object, machineSetLabels, "spec", "selector", "matchLabels"); err != nil {
@@ -324,6 +359,7 @@ func createTestConfigs(specs ...TestSpec) []*TestConfig {
324359
type TestSpec struct {
325360
annotations map[string]string
326361
capacity map[string]string
362+
managedLabels map[string]string
327363
machineDeploymentName string
328364
machineSetName string
329365
machinePoolName string
@@ -333,20 +369,21 @@ type TestSpec struct {
333369
rootIsMachineDeployment bool
334370
}
335371

336-
func createTestSpecs(namespace, clusterName, namePrefix string, scalableResourceCount, nodeCount int, isMachineDeployment bool, annotations map[string]string, capacity map[string]string) []TestSpec {
372+
func createTestSpecs(namespace, clusterName, namePrefix string, scalableResourceCount, nodeCount int, isMachineDeployment bool, annotations map[string]string, capacity map[string]string, managedLabels map[string]string) []TestSpec {
337373
var specs []TestSpec
338374

339375
for i := 0; i < scalableResourceCount; i++ {
340-
specs = append(specs, createTestSpec(namespace, clusterName, fmt.Sprintf("%s-%d", namePrefix, i), nodeCount, isMachineDeployment, annotations, capacity))
376+
specs = append(specs, createTestSpec(namespace, clusterName, fmt.Sprintf("%s-%d", namePrefix, i), nodeCount, isMachineDeployment, annotations, capacity, managedLabels))
341377
}
342378

343379
return specs
344380
}
345381

346-
func createTestSpec(namespace, clusterName, name string, nodeCount int, isMachineDeployment bool, annotations map[string]string, capacity map[string]string) TestSpec {
382+
func createTestSpec(namespace, clusterName, name string, nodeCount int, isMachineDeployment bool, annotations map[string]string, capacity map[string]string, managedLabels map[string]string) TestSpec {
347383
return TestSpec{
348384
annotations: annotations,
349385
capacity: capacity,
386+
managedLabels: managedLabels,
350387
machineDeploymentName: name,
351388
machineSetName: name,
352389
clusterName: clusterName,

‎cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import (
3535
"k8s.io/apimachinery/pkg/runtime/schema"
3636
"k8s.io/apimachinery/pkg/types"
3737
"k8s.io/apimachinery/pkg/util/validation"
38+
"k8s.io/autoscaler/cluster-autoscaler/cloudprovider"
3839
klog "k8s.io/klog/v2"
3940
"k8s.io/utils/ptr"
4041
)
@@ -197,20 +198,29 @@ func (r unstructuredScalableResource) MarkMachineForDeletion(machine *unstructur
197198
}
198199

199200
func (r unstructuredScalableResource) Labels() map[string]string {
201+
allLabels := map[string]string{}
202+
203+
// get the managed labels from the scalable resource, if they exist.
204+
if labels, found, err := unstructured.NestedStringMap(r.unstructured.UnstructuredContent(), "spec", "template", "spec", "metadata", "labels"); found && err == nil {
205+
managedLabels := getManagedNodeLabelsFromLabels(labels)
206+
allLabels = cloudprovider.JoinStringMaps(allLabels, managedLabels)
207+
}
208+
209+
// annotation labels are supplied as an override to other values, we process them last.
200210
annotations := r.unstructured.GetAnnotations()
201211
// annotation value of the form "key1=value1,key2=value2"
202212
if val, found := annotations[labelsKey]; found {
203213
labels := strings.Split(val, ",")
204-
kv := make(map[string]string, len(labels))
214+
annotationLabels := make(map[string]string, len(labels))
205215
for _, label := range labels {
206216
split := strings.SplitN(label, "=", 2)
207217
if len(split) == 2 {
208-
kv[split[0]] = split[1]
218+
annotationLabels[split[0]] = split[1]
209219
}
210220
}
211-
return kv
221+
allLabels = cloudprovider.JoinStringMaps(allLabels, annotationLabels)
212222
}
213-
return nil
223+
return allLabels
214224
}
215225

216226
func (r unstructuredScalableResource) Taints() []apiv1.Taint {

‎cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils.go‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,14 @@ const (
5757
scaleUpFromZeroDefaultArchEnvVar = "CAPI_SCALE_ZERO_DEFAULT_ARCH"
5858
// GpuDeviceType is used if DRA device is GPU
5959
GpuDeviceType = "gpu"
60+
61+
// Cluster API constants, copied from cluster-api/api/core/v1beta1/machine_types.go
62+
// nodeRoleLabelPrefix is one of the CAPI managed Node label prefixes.
63+
nodeRoleLabelPrefix = "node-role.kubernetes.io"
64+
// nodeRestrictionLabelDomain is one of the CAPI managed Node label domains.
65+
nodeRestrictionLabelDomain = "node-restriction.kubernetes.io"
66+
// managedNodeLabelDomain is one of the CAPI managed Node label domains.
67+
managedNodeLabelDomain = "node.cluster.x-k8s.io"
6068
)
6169

6270
var (
@@ -405,3 +413,33 @@ func GetDefaultScaleFromZeroArchitecture() SystemArchitecture {
405413
})
406414
return *systemArchitecture
407415
}
416+
417+
// getManagedNodeLabelsFromLabels returns a map of labels that will be propagated
418+
// to nodes based on the Cluster API metadata propagation rules.
419+
func getManagedNodeLabelsFromLabels(labels map[string]string) map[string]string {
420+
// TODO elmiko, add a user configuration to inject a string with their `--additional-sync-machine-labels` string.
421+
// ref: https://cluster-api.sigs.k8s.io/reference/api/metadata-propagation#machine
422+
managedLabels := map[string]string{}
423+
for key, value := range labels {
424+
if isManagedLabel(key) {
425+
managedLabels[key] = value
426+
}
427+
428+
}
429+
430+
return managedLabels
431+
}
432+
433+
func isManagedLabel(key string) bool {
434+
dnsSubdomainOrName := strings.Split(key, "/")[0]
435+
if dnsSubdomainOrName == nodeRoleLabelPrefix {
436+
return true
437+
}
438+
if dnsSubdomainOrName == nodeRestrictionLabelDomain || strings.HasSuffix(dnsSubdomainOrName, "."+nodeRestrictionLabelDomain) {
439+
return true
440+
}
441+
if dnsSubdomainOrName == managedNodeLabelDomain || strings.HasSuffix(dnsSubdomainOrName, "."+managedNodeLabelDomain) {
442+
return true
443+
}
444+
return false
445+
}

‎cluster-autoscaler/cloudprovider/clusterapi/clusterapi_utils_test.go‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,13 @@ package clusterapi
1818

1919
import (
2020
"fmt"
21-
"github.com/google/go-cmp/cmp"
2221
"reflect"
2322
"strings"
2423
"sync"
2524
"testing"
2625

26+
"github.com/google/go-cmp/cmp"
27+
2728
"k8s.io/apimachinery/pkg/api/resource"
2829
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2930
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -33,6 +34,11 @@ import (
3334
const (
3435
uuid1 = "ec21c5fb-a3d5-a45f-887b-6b49aa8fc218"
3536
uuid2 = "ec23ebb0-bc60-443f-d139-046ec5046283"
37+
38+
managedLabel1 = "node-restriction.kubernetes.io/some-thing"
39+
managedLabel2 = "prefixed.node-restriction.kubernetes.io/some-other-thing"
40+
managedLabel3 = "node.cluster.x-k8s.io/another-thing"
41+
managedLabel4 = "prefixed.node.cluster.x-k8s.io/another-thing"
3642
)
3743

3844
func TestUtilParseScalingBounds(t *testing.T) {
@@ -937,3 +943,27 @@ func TestGetSystemArchitectureFromEnvOrDefault(t *testing.T) {
937943
})
938944
}
939945
}
946+
947+
func TestGetManagedNodeLabelsFromLabels(t *testing.T) {
948+
actualLabels := map[string]string{
949+
// these labels should propagate
950+
managedLabel1: "1",
951+
managedLabel2: "2",
952+
managedLabel3: "3",
953+
managedLabel4: "4",
954+
// the following should NOT propagate
955+
"my.special.label/should-not-propagate": "bar",
956+
"prefixed.node-role.kubernetes.io/no-propagate": "special-role",
957+
}
958+
959+
observedLabels := getManagedNodeLabelsFromLabels(actualLabels)
960+
if len(observedLabels) != 4 {
961+
t.Errorf("expected observedLabels length to be 4, actual: %d", len(observedLabels))
962+
}
963+
expectedLabels := []string{managedLabel1, managedLabel2, managedLabel3, managedLabel4}
964+
for _, l := range expectedLabels {
965+
if _, ok := observedLabels[l]; !ok {
966+
t.Errorf("expected observedLabels to contain %q", l)
967+
}
968+
}
969+
}

0 commit comments

Comments
 (0)