From 0e3f902c2d4061f4176d642e68a71735b379ecbd Mon Sep 17 00:00:00 2001
From: Aaron Francis Fernandes <79958509+aaronfern@users.noreply.github.com>
Date: Thu, 4 Jul 2024 14:20:45 +0530
Subject: [PATCH] Updated NodeGroupAutoscalingOptions to now include all
 options (#310) (#314)

---
 .../cloudprovider/mcm/mcm_cloud_provider.go   | 24 ++++----------
 .../mcm/mcm_cloud_provider_test.go            | 32 +++++++++----------
 2 files changed, 23 insertions(+), 33 deletions(-)

diff --git a/cluster-autoscaler/cloudprovider/mcm/mcm_cloud_provider.go b/cluster-autoscaler/cloudprovider/mcm/mcm_cloud_provider.go
index 68ae50797380..5d8fc293b919 100644
--- a/cluster-autoscaler/cloudprovider/mcm/mcm_cloud_provider.go
+++ b/cluster-autoscaler/cloudprovider/mcm/mcm_cloud_provider.go
@@ -478,48 +478,38 @@ func (machinedeployment *MachineDeployment) Nodes() ([]cloudprovider.Instance, e
 // NodeGroup. Returning a nil will result in using default options.
 // Implementation optional.
 func (machinedeployment *MachineDeployment) GetOptions(defaults config.NodeGroupAutoscalingOptions) (*config.NodeGroupAutoscalingOptions, error) {
+	options := defaults
 	mcdAnnotations, err := machinedeployment.mcmManager.GetMachineDeploymentAnnotations(machinedeployment.Name)
 	if err != nil {
 		return nil, err
 	}
 
-	scaleDownUtilThresholdValue := defaults.ScaleDownUtilizationThreshold
 	if _, ok := mcdAnnotations[ScaleDownUtilizationThresholdAnnotation]; ok {
 		if floatVal, err := strconv.ParseFloat(mcdAnnotations[ScaleDownUtilizationThresholdAnnotation], 64); err == nil {
-			scaleDownUtilThresholdValue = floatVal
+			options.ScaleDownUtilizationThreshold = floatVal
 		}
 	}
-	scaleDownGPUUtilThresholdValue := defaults.ScaleDownGpuUtilizationThreshold
 	if _, ok := mcdAnnotations[ScaleDownGpuUtilizationThresholdAnnotation]; ok {
 		if floatVal, err := strconv.ParseFloat(mcdAnnotations[ScaleDownGpuUtilizationThresholdAnnotation], 64); err == nil {
-			scaleDownGPUUtilThresholdValue = floatVal
+			options.ScaleDownGpuUtilizationThreshold = floatVal
 		}
 	}
-	scaleDownUnneededDuration := defaults.ScaleDownUnneededTime
 	if _, ok := mcdAnnotations[ScaleDownUnneededTimeAnnotation]; ok {
 		if durationVal, err := time.ParseDuration(mcdAnnotations[ScaleDownUnneededTimeAnnotation]); err == nil {
-			scaleDownUnneededDuration = durationVal
+			options.ScaleDownUnneededTime = durationVal
 		}
 	}
-	scaleDownUnreadyDuration := defaults.ScaleDownUnreadyTime
 	if _, ok := mcdAnnotations[ScaleDownUnreadyTimeAnnotation]; ok {
 		if durationVal, err := time.ParseDuration(mcdAnnotations[ScaleDownUnreadyTimeAnnotation]); err == nil {
-			scaleDownUnreadyDuration = durationVal
+			options.ScaleDownUnreadyTime = durationVal
 		}
 	}
-	maxNodeProvisionDuration := defaults.MaxNodeProvisionTime
 	if _, ok := mcdAnnotations[MaxNodeProvisionTimeAnnotation]; ok {
 		if durationVal, err := time.ParseDuration(mcdAnnotations[MaxNodeProvisionTimeAnnotation]); err == nil {
-			maxNodeProvisionDuration = durationVal
+			options.MaxNodeProvisionTime = durationVal
 		}
 	}
-	return &config.NodeGroupAutoscalingOptions{
-		ScaleDownUtilizationThreshold:    scaleDownUtilThresholdValue,
-		ScaleDownGpuUtilizationThreshold: scaleDownGPUUtilThresholdValue,
-		ScaleDownUnneededTime:            scaleDownUnneededDuration,
-		ScaleDownUnreadyTime:             scaleDownUnreadyDuration,
-		MaxNodeProvisionTime:             maxNodeProvisionDuration,
-	}, nil
+	return &options, nil
 }
 
 // TemplateNodeInfo returns a node template for this node group.
diff --git a/cluster-autoscaler/cloudprovider/mcm/mcm_cloud_provider_test.go b/cluster-autoscaler/cloudprovider/mcm/mcm_cloud_provider_test.go
index f8242a0e27cb..1ef72a061afa 100644
--- a/cluster-autoscaler/cloudprovider/mcm/mcm_cloud_provider_test.go
+++ b/cluster-autoscaler/cloudprovider/mcm/mcm_cloud_provider_test.go
@@ -590,6 +590,16 @@ func TestNodes(t *testing.T) {
 }
 
 func TestGetOptions(t *testing.T) {
+	ngAutoScalingOpDefaults := config.NodeGroupAutoscalingOptions{
+		ScaleDownUtilizationThreshold:    0.5,
+		ScaleDownGpuUtilizationThreshold: 0.5,
+		ScaleDownUnneededTime:            1 * time.Minute,
+		ScaleDownUnreadyTime:             1 * time.Minute,
+		MaxNodeProvisionTime:             1 * time.Minute,
+		IgnoreDaemonSetsUtilization:      true,
+		ZeroOrMaxNodeScaling:             true,
+	}
+
 	type expect struct {
 		ngOptions *config.NodeGroupAutoscalingOptions
 		err       error
@@ -616,14 +626,8 @@ func TestGetOptions(t *testing.T) {
 				nodeGroups:         []string{nodeGroup1},
 			},
 			expect{
-				ngOptions: &config.NodeGroupAutoscalingOptions{
-					ScaleDownUtilizationThreshold:    0.5,
-					ScaleDownGpuUtilizationThreshold: 0.5,
-					ScaleDownUnneededTime:            1 * time.Minute,
-					ScaleDownUnreadyTime:             1 * time.Minute,
-					MaxNodeProvisionTime:             1 * time.Minute,
-				},
-				err: nil,
+				ngOptions: &ngAutoScalingOpDefaults,
+				err:       nil,
 			},
 		},
 		{
@@ -651,6 +655,8 @@ func TestGetOptions(t *testing.T) {
 					ScaleDownUnneededTime:            5 * time.Minute,
 					ScaleDownUnreadyTime:             5 * time.Minute,
 					MaxNodeProvisionTime:             5 * time.Minute,
+					IgnoreDaemonSetsUtilization:      ngAutoScalingOpDefaults.IgnoreDaemonSetsUtilization,
+					ZeroOrMaxNodeScaling:             ngAutoScalingOpDefaults.ZeroOrMaxNodeScaling,
 				},
 				err: nil,
 			},
@@ -678,6 +684,8 @@ func TestGetOptions(t *testing.T) {
 					ScaleDownUnneededTime:            5 * time.Minute,
 					ScaleDownUnreadyTime:             1 * time.Minute,
 					MaxNodeProvisionTime:             2 * time.Minute,
+					IgnoreDaemonSetsUtilization:      ngAutoScalingOpDefaults.IgnoreDaemonSetsUtilization,
+					ZeroOrMaxNodeScaling:             ngAutoScalingOpDefaults.ZeroOrMaxNodeScaling,
 				},
 				err: nil,
 			},
@@ -699,14 +707,6 @@ func TestGetOptions(t *testing.T) {
 			md, err := buildMachineDeploymentFromSpec(entry.setup.nodeGroups[0], m)
 			g.Expect(err).To(BeNil())
 
-			ngAutoScalingOpDefaults := config.NodeGroupAutoscalingOptions{
-				ScaleDownUtilizationThreshold:    0.5,
-				ScaleDownGpuUtilizationThreshold: 0.5,
-				ScaleDownUnneededTime:            1 * time.Minute,
-				ScaleDownUnreadyTime:             1 * time.Minute,
-				MaxNodeProvisionTime:             1 * time.Minute,
-			}
-
 			options, err := md.GetOptions(ngAutoScalingOpDefaults)
 
 			if entry.expect.err != nil {