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. Signed-off-by: Yury Akudovich <yorik13@gmail.com>
kedacore · Mar 4, 2024 · 7cf4c6c · 7cf4c6c
1 parent 610f812
commit 7cf4c6c
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 25 deletions.
diff --git 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
@@ -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,