From 4611aa7720c132fb211a5e12728c9966d04ca403 Mon Sep 17 00:00:00 2001
From: Yury Akudovich <yorik13@gmail.com>
Date: Mon, 4 Mar 2024 16:59:38 +0100
Subject: [PATCH] fix: Allows ScaledObject.MinReplicaCount to be set to 0.

Makes ScaledObject API to be closer to ScaleJob API.
Renames functions to MinReplicaCount and MaxReplicaCount.
Updates doc strings to correspond new names.
Set MinReplicaCount default to 0 as written in the doc:
https://keda.sh/docs/1.5/concepts/scaling-deployments/#overview.
Moves HPA related hacks into hpa.go.

Fixes: 5570.
---
 apis/keda/v1alpha1/scaledobject_types.go | 28 ++++++++++--------------
 controllers/keda/hpa.go                  | 17 +++++++-------
 2 files changed, 20 insertions(+), 25 deletions(-)

diff --git a/apis/keda/v1alpha1/scaledobject_types.go b/apis/keda/v1alpha1/scaledobject_types.go
index 0e8ddf614fe..80b74b98b30 100644
--- a/apis/keda/v1alpha1/scaledobject_types.go
+++ b/apis/keda/v1alpha1/scaledobject_types.go
@@ -77,8 +77,8 @@ const (
 	// Composite metric name used for scalingModifiers composite metric
 	CompositeMetricName string = "composite-metric"
 
-	defaultHPAMinReplicas int32 = 1
-	defaultHPAMaxReplicas int32 = 100
+	defaultMinReplicas int32 = 0
+	defaultMaxReplicas int32 = 100
 )
 
 // ScaledObjectSpec is the spec for a ScaledObject resource
@@ -230,31 +230,27 @@ func (so *ScaledObject) IsUsingModifiers() bool {
 	return so.Spec.Advanced != nil && !reflect.DeepEqual(so.Spec.Advanced.ScalingModifiers, ScalingModifiers{})
 }
 
-// getHPAMinReplicas returns MinReplicas based on definition in ScaledObject or default value if not defined
-func (so *ScaledObject) GetHPAMinReplicas() *int32 {
-	if so.Spec.MinReplicaCount != nil && *so.Spec.MinReplicaCount > 0 {
-		return so.Spec.MinReplicaCount
+// MinReplicaCount returns MinReplicaCount based on definition in ScaledObject or default value if not defined
+func (so *ScaledObject) MinReplicaCount() int32 {
+	if so.Spec.MinReplicaCount != nil {
+		return *so.Spec.MinReplicaCount
 	}
-	tmp := defaultHPAMinReplicas
-	return &tmp
+	return defaultMinReplicas
 }
 
-// getHPAMaxReplicas returns MaxReplicas based on definition in ScaledObject or default value if not defined
-func (so *ScaledObject) GetHPAMaxReplicas() int32 {
+// MaxReplicaCount returns MaxReplicaCount based on definition in ScaledObject or default value if not defined
+func (so *ScaledObject) MaxReplicaCount() int32 {
 	if so.Spec.MaxReplicaCount != nil {
 		return *so.Spec.MaxReplicaCount
 	}
-	return defaultHPAMaxReplicas
+	return defaultMaxReplicas
 }
 
 // checkReplicaCountBoundsAreValid checks that Idle/Min/Max ReplicaCount defined in ScaledObject are correctly specified
 // i.e. that Min is not greater than Max or Idle greater or equal to Min
 func CheckReplicaCountBoundsAreValid(scaledObject *ScaledObject) error {
-	min := int32(0)
-	if scaledObject.Spec.MinReplicaCount != nil {
-		min = *scaledObject.GetHPAMinReplicas()
-	}
-	max := scaledObject.GetHPAMaxReplicas()
+	min := scaledObject.MinReplicaCount()
+	max := scaledObject.MaxReplicaCount()
 
 	if min > max {
 		return fmt.Errorf("MinReplicaCount=%d must be less than MaxReplicaCount=%d", min, max)
diff --git a/controllers/keda/hpa.go b/controllers/keda/hpa.go
index 5fb2c835eb7..cad0752790d 100644
--- a/controllers/keda/hpa.go
+++ b/controllers/keda/hpa.go
@@ -99,25 +99,24 @@ func (r *ScaledObjectReconciler) newHPAForScaledObject(ctx context.Context, logg
 		labels[key] = value
 	}
 
-	minReplicas := scaledObject.GetHPAMinReplicas()
-	maxReplicas := scaledObject.GetHPAMaxReplicas()
+	// minReplicas on HPA must be > 0 without HPAScaleToZero feature.
+	minReplicas := max(scaledObject.MinReplicaCount(), 1)
+	maxReplicas := scaledObject.MaxReplicaCount()
 
 	pausedCount, err := executor.GetPausedReplicaCount(scaledObject)
 	if err != nil {
 		return nil, err
 	}
 	if pausedCount != nil {
-		// MinReplicas on HPA can't be 0
-		if *pausedCount == 0 {
-			*pausedCount = 1
-		}
-		minReplicas = pausedCount
-		maxReplicas = *pausedCount
+		// minReplicas on HPA must be > 0 without HPAScaleToZero feature.
+		replicas := max(*pausedCount, 1)
+		minReplicas = replicas
+		maxReplicas = replicas
 	}
 
 	hpa := &autoscalingv2.HorizontalPodAutoscaler{
 		Spec: autoscalingv2.HorizontalPodAutoscalerSpec{
-			MinReplicas: minReplicas,
+			MinReplicas: &minReplicas,
 			MaxReplicas: maxReplicas,
 			Metrics:     scaledObjectMetricSpecs,
 			Behavior:    behavior,