Skip to content

Commit 4f6b62c

Browse files
committed
allow clusterapi provider to process managed labels
This change enables the provider to read the managed propagated labels from MachineSet and MachineDeployment resources when creating node templates.
1 parent 7e8cf0b commit 4f6b62c

File tree

5 files changed

+192
-22
lines changed

5 files changed

+192
-22
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: 75 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -53,25 +53,27 @@ 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
63-
nodeInfo 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+
nodeInfo map[string]string
64+
managedLabels map[string]string
6465
}
6566

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

@@ -93,6 +95,7 @@ func (b *testConfigBuilder) Build() *TestConfig {
9395
b.annotations,
9496
b.capacity,
9597
b.nodeInfo,
98+
b.managedLabels,
9699
)[0],
97100
)[0]
98101
}
@@ -114,6 +117,7 @@ func (b *testConfigBuilder) BuildMultiple(configCount int) []*TestConfig {
114117
b.annotations,
115118
b.capacity,
116119
b.nodeInfo,
120+
b.managedLabels,
117121
)...,
118122
)
119123
}
@@ -186,6 +190,19 @@ func (b *testConfigBuilder) WithNodeInfo(n map[string]string) *testConfigBuilder
186190
return b
187191
}
188192

193+
func (b *testConfigBuilder) WithManagedLabels(l map[string]string) *testConfigBuilder {
194+
if l == nil {
195+
// explicitly setting managed labels to nil
196+
b.managedLabels = nil
197+
} else {
198+
if b.managedLabels == nil {
199+
b.managedLabels = map[string]string{}
200+
}
201+
maps.Insert(b.managedLabels, maps.All(l))
202+
}
203+
return b
204+
}
205+
189206
// TestConfig contains clusterspecific information about a single test configuration.
190207
type TestConfig struct {
191208
spec *TestSpec
@@ -235,6 +252,9 @@ func createTestConfigs(specs ...TestSpec) []*TestConfig {
235252
"kind": machineTemplateKind,
236253
"name": "TestMachineTemplate",
237254
},
255+
"metadata": map[string]interface{}{
256+
"labels": map[string]interface{}{},
257+
},
238258
},
239259
},
240260
},
@@ -244,6 +264,12 @@ func createTestConfigs(specs ...TestSpec) []*TestConfig {
244264

245265
config.machineSet.SetAnnotations(make(map[string]string))
246266

267+
if spec.managedLabels != nil {
268+
if err := unstructured.SetNestedStringMap(config.machineSet.Object, spec.managedLabels, "spec", "template", "spec", "metadata", "labels"); err != nil {
269+
panic(err)
270+
}
271+
}
272+
247273
if !spec.rootIsMachineDeployment {
248274
config.machineSet.SetAnnotations(spec.annotations)
249275
} else {
@@ -273,6 +299,9 @@ func createTestConfigs(specs ...TestSpec) []*TestConfig {
273299
"kind": machineTemplateKind,
274300
"name": "TestMachineTemplate",
275301
},
302+
"metadata": map[string]interface{}{
303+
"labels": map[string]interface{}{},
304+
},
276305
},
277306
},
278307
},
@@ -293,6 +322,12 @@ func createTestConfigs(specs ...TestSpec) []*TestConfig {
293322
},
294323
}
295324
config.machineSet.SetOwnerReferences(ownerRefs)
325+
326+
if spec.managedLabels != nil {
327+
if err := unstructured.SetNestedStringMap(config.machineDeployment.Object, spec.managedLabels, "spec", "template", "spec", "metadata", "labels"); err != nil {
328+
panic(err)
329+
}
330+
}
296331
}
297332
config.machineSet.SetLabels(machineSetLabels)
298333
if err := unstructured.SetNestedStringMap(config.machineSet.Object, machineSetLabels, "spec", "selector", "matchLabels"); err != nil {
@@ -352,6 +387,7 @@ type TestSpec struct {
352387
annotations map[string]string
353388
capacity map[string]string
354389
nodeInfo map[string]string
390+
managedLabels map[string]string
355391
machineDeploymentName string
356392
machineSetName string
357393
machinePoolName string
@@ -361,20 +397,42 @@ type TestSpec struct {
361397
rootIsMachineDeployment bool
362398
}
363399

364-
func createTestSpecs(namespace, clusterName, namePrefix string, scalableResourceCount, nodeCount int, isMachineDeployment bool, annotations map[string]string, capacity map[string]string, nodeInfo map[string]string) []TestSpec {
400+
func createTestSpecs(
401+
namespace string,
402+
clusterName string,
403+
namePrefix string,
404+
scalableResourceCount int,
405+
nodeCount int,
406+
isMachineDeployment bool,
407+
annotations map[string]string,
408+
capacity map[string]string,
409+
nodeInfo map[string]string,
410+
managedLabels map[string]string,
411+
) []TestSpec {
365412
var specs []TestSpec
366413

367414
for i := 0; i < scalableResourceCount; i++ {
368-
specs = append(specs, createTestSpec(namespace, clusterName, fmt.Sprintf("%s-%d", namePrefix, i), nodeCount, isMachineDeployment, annotations, capacity, nodeInfo))
415+
specs = append(specs, createTestSpec(namespace, clusterName, fmt.Sprintf("%s-%d", namePrefix, i), nodeCount, isMachineDeployment, annotations, capacity, nodeInfo, managedLabels))
369416
}
370417

371418
return specs
372419
}
373420

374-
func createTestSpec(namespace, clusterName, name string, nodeCount int, isMachineDeployment bool, annotations map[string]string, capacity map[string]string, nodeInfo map[string]string) TestSpec {
421+
func createTestSpec(
422+
namespace string,
423+
clusterName string,
424+
name string,
425+
nodeCount int,
426+
isMachineDeployment bool,
427+
annotations map[string]string,
428+
capacity map[string]string,
429+
nodeInfo map[string]string,
430+
managedLabels map[string]string,
431+
) TestSpec {
375432
return TestSpec{
376433
annotations: annotations,
377434
capacity: capacity,
435+
managedLabels: managedLabels,
378436
machineDeploymentName: name,
379437
machineSetName: name,
380438
clusterName: clusterName,

cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import (
3636
"k8s.io/apimachinery/pkg/runtime/schema"
3737
"k8s.io/apimachinery/pkg/types"
3838
"k8s.io/apimachinery/pkg/util/validation"
39+
"k8s.io/autoscaler/cluster-autoscaler/cloudprovider"
3940
klog "k8s.io/klog/v2"
4041
"k8s.io/utils/ptr"
4142
)
@@ -200,20 +201,29 @@ func (r unstructuredScalableResource) MarkMachineForDeletion(machine *unstructur
200201
}
201202

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

219229
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
@@ -59,6 +59,14 @@ const (
5959
scaleUpFromZeroDefaultArchEnvVar = "CAPI_SCALE_ZERO_DEFAULT_ARCH"
6060
// GpuDeviceType is used if DRA device is GPU
6161
GpuDeviceType = "gpu"
62+
63+
// Cluster API constants, copied from cluster-api/api/core/v1beta1/machine_types.go
64+
// nodeRoleLabelPrefix is one of the CAPI managed Node label prefixes.
65+
nodeRoleLabelPrefix = "node-role.kubernetes.io"
66+
// nodeRestrictionLabelDomain is one of the CAPI managed Node label domains.
67+
nodeRestrictionLabelDomain = "node-restriction.kubernetes.io"
68+
// managedNodeLabelDomain is one of the CAPI managed Node label domains.
69+
managedNodeLabelDomain = "node.cluster.x-k8s.io"
6270
)
6371

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

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)