From 12ff9c15dcbe3224b4cbb474413f8846d82f42d5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Radek=20Ma=C5=88=C3=A1k?= <rmanak@redhat.com>
Date: Tue, 20 Sep 2022 14:01:43 +0200
Subject: [PATCH] Expose autoscaler "--balancing-ignore-label" flag through CRD

---
 install/01_clusterautoscaler.crd.yaml               | 13 ++++++++++++-
 pkg/apis/autoscaling/v1/clusterautoscaler_types.go  |  8 +++++++-
 pkg/apis/autoscaling/v1/zz_generated.deepcopy.go    |  5 +++++
 .../clusterautoscaler/clusterautoscaler.go          |  5 +++++
 .../clusterautoscaler/clusterautoscaler_test.go     |  4 ++++
 5 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/install/01_clusterautoscaler.crd.yaml b/install/01_clusterautoscaler.crd.yaml
index 324efe35e..70b94a91e 100644
--- a/install/01_clusterautoscaler.crd.yaml
+++ b/install/01_clusterautoscaler.crd.yaml
@@ -41,10 +41,21 @@ spec:
             properties:
               balanceSimilarNodeGroups:
                 description: BalanceSimilarNodeGroups enables/disables the `--balance-similar-node-groups`
-                  cluster-autocaler feature. This feature will automatically identify
+                  cluster-autoscaler feature. This feature will automatically identify
                   node groups with the same instance type and the same set of labels
                   and try to keep the respective sizes of those node groups balanced.
                 type: boolean
+              balancingIgnoredLabels:
+                description: BalancingIgnoredLabels sets "--balancing-ignore-label
+                  <label name>" flag on cluster-autoscaler for each listed label.
+                  This option specifies labels that cluster autoscaler should ignore
+                  when considering node group similarity. For example, if you have
+                  nodes with "topology.ebs.csi.aws.com/zone" label, you can add name
+                  of this label here to prevent cluster autoscaler from spliting nodes
+                  into different node groups based on its value.
+                items:
+                  type: string
+                type: array
               ignoreDaemonsetsUtilization:
                 description: Enables/Disables `--ignore-daemonsets-utilization` CA
                   feature flag. Should CA ignore DaemonSet pods when calculating resource
diff --git a/pkg/apis/autoscaling/v1/clusterautoscaler_types.go b/pkg/apis/autoscaling/v1/clusterautoscaler_types.go
index a36ecdbfc..cc6a7ffac 100644
--- a/pkg/apis/autoscaling/v1/clusterautoscaler_types.go
+++ b/pkg/apis/autoscaling/v1/clusterautoscaler_types.go
@@ -29,12 +29,18 @@ type ClusterAutoscalerSpec struct {
 	PodPriorityThreshold *int32 `json:"podPriorityThreshold,omitempty"`
 
 	// BalanceSimilarNodeGroups enables/disables the
-	// `--balance-similar-node-groups` cluster-autocaler feature.
+	// `--balance-similar-node-groups` cluster-autoscaler feature.
 	// This feature will automatically identify node groups with
 	// the same instance type and the same set of labels and try
 	// to keep the respective sizes of those node groups balanced.
 	BalanceSimilarNodeGroups *bool `json:"balanceSimilarNodeGroups,omitempty"`
 
+	// BalancingIgnoredLabels sets "--balancing-ignore-label <label name>" flag on cluster-autoscaler for each listed label.
+	// This option specifies labels that cluster autoscaler should ignore when considering node group similarity.
+	// For example, if you have nodes with "topology.ebs.csi.aws.com/zone" label, you can add name of this label here
+	// to prevent cluster autoscaler from spliting nodes into different node groups based on its value.
+	BalancingIgnoredLabels []string `json:"balancingIgnoredLabels,omitempty"`
+
 	// Enables/Disables `--ignore-daemonsets-utilization` CA feature flag. Should CA ignore DaemonSet pods when calculating resource utilization for scaling down. false by default
 	IgnoreDaemonsetsUtilization *bool `json:"ignoreDaemonsetsUtilization,omitempty"`
 
diff --git a/pkg/apis/autoscaling/v1/zz_generated.deepcopy.go b/pkg/apis/autoscaling/v1/zz_generated.deepcopy.go
index f889f49fb..49086f562 100644
--- a/pkg/apis/autoscaling/v1/zz_generated.deepcopy.go
+++ b/pkg/apis/autoscaling/v1/zz_generated.deepcopy.go
@@ -113,6 +113,11 @@ func (in *ClusterAutoscalerSpec) DeepCopyInto(out *ClusterAutoscalerSpec) {
 		*out = new(bool)
 		**out = **in
 	}
+	if in.BalancingIgnoredLabels != nil {
+		in, out := &in.BalancingIgnoredLabels, &out.BalancingIgnoredLabels
+		*out = make([]string, len(*in))
+		copy(*out, *in)
+	}
 	if in.IgnoreDaemonsetsUtilization != nil {
 		in, out := &in.IgnoreDaemonsetsUtilization, &out.IgnoreDaemonsetsUtilization
 		*out = new(bool)
diff --git a/pkg/controller/clusterautoscaler/clusterautoscaler.go b/pkg/controller/clusterautoscaler/clusterautoscaler.go
index dd37db953..9dffdc2be 100644
--- a/pkg/controller/clusterautoscaler/clusterautoscaler.go
+++ b/pkg/controller/clusterautoscaler/clusterautoscaler.go
@@ -59,6 +59,7 @@ const (
 	GPUTotalArg                      AutoscalerArg = "--gpu-total"
 	VerbosityArg                     AutoscalerArg = "--v"
 	BalanceSimilarNodeGroupsArg      AutoscalerArg = "--balance-similar-node-groups"
+	BalancingIgnoreLabelArg          AutoscalerArg = "--balancing-ignore-label"
 	IgnoreDaemonsetsUtilization      AutoscalerArg = "--ignore-daemonsets-utilization"
 	SkipNodesWithLocalStorage        AutoscalerArg = "--skip-nodes-with-local-storage"
 	LeaderElectLeaseDurationArg      AutoscalerArg = "--leader-elect-lease-duration"
@@ -116,6 +117,10 @@ func AutoscalerArgs(ca *v1.ClusterAutoscaler, cfg *Config) []string {
 		args = append(args, SkipNodesWithLocalStorage.Value(*ca.Spec.SkipNodesWithLocalStorage))
 	}
 
+	for _, ignoredLabel := range ca.Spec.BalancingIgnoredLabels {
+		args = append(args, BalancingIgnoreLabelArg.Value(ignoredLabel))
+	}
+
 	// Prefer log level set from ClousterAutoscaler resource
 	if ca.Spec.LogVerbosity != nil {
 		args = append(args, VerbosityArg.Value(*ca.Spec.LogVerbosity))
diff --git a/pkg/controller/clusterautoscaler/clusterautoscaler_test.go b/pkg/controller/clusterautoscaler/clusterautoscaler_test.go
index b6b80e552..4c65179c7 100644
--- a/pkg/controller/clusterautoscaler/clusterautoscaler_test.go
+++ b/pkg/controller/clusterautoscaler/clusterautoscaler_test.go
@@ -174,6 +174,7 @@ func TestAutoscalerArgs(t *testing.T) {
 		"--balance-similar-node-groups",
 		"--ignore-daemonsets-utilization",
 		"--skip-nodes-with-local-storage",
+		"--balancing-ignore-label",
 	}
 
 	for _, e := range expectedMissing {
@@ -191,6 +192,7 @@ func TestAutoscalerArgEnabled(t *testing.T) {
 	ca.Spec.IgnoreDaemonsetsUtilization = pointer.BoolPtr(true)
 	ca.Spec.SkipNodesWithLocalStorage = pointer.BoolPtr(true)
 	ca.Spec.MaxNodeProvisionTime = MaxNodeProvisionTime
+	ca.Spec.BalancingIgnoredLabels = []string{"test/ignoredLabel", "test/anotherIgnoredLabel"}
 
 	args := AutoscalerArgs(ca, &Config{CloudProvider: TestCloudProvider, Namespace: TestNamespace})
 
@@ -199,6 +201,8 @@ func TestAutoscalerArgEnabled(t *testing.T) {
 		fmt.Sprintf("--ignore-daemonsets-utilization=true"),
 		fmt.Sprintf("--skip-nodes-with-local-storage=true"),
 		fmt.Sprintf("--max-node-provision-time=%s", MaxNodeProvisionTime),
+		fmt.Sprintf("--balancing-ignore-label=test/ignoredLabel"),
+		fmt.Sprintf("--balancing-ignore-label=test/anotherIgnoredLabel"),
 	}
 
 	for _, e := range expected {