Update after feedback

Signed-off-by: rickbrouwer <[email protected]>

apis/keda/v1alpha1/scaledobject_types.go

@@ -56,6 +56,8 @@ const ScaledObjectTransferHpaOwnershipAnnotation = "scaledobject.keda.sh/transfe
 const ValidationsHpaOwnershipAnnotation = "validations.keda.sh/hpa-ownership"
 const PausedReplicasAnnotation = "autoscaling.keda.sh/paused-replicas"
 const PausedAnnotation = "autoscaling.keda.sh/paused"
+const FallbackBehaviorStatic = "static"
+const FallbackBehaviorUseCurrentReplicasAsMin = "useCurrentReplicasAsMinimum"
 
 // HealthStatus is the status for a ScaledObject's health
 type HealthStatus struct {
@@ -110,7 +112,7 @@ type Fallback struct {
 	FailureThreshold int32 `json:"failureThreshold"`
 	Replicas         int32 `json:"replicas"`
 	// +optional
-	UseCurrentReplicasAsMinimum *bool `json:"useCurrentReplicasAsMinimum,omitempty"`
+	Behavior string `json:"behavior,omitempty"`
 }
 
 // AdvancedConfig specifies advance scaling options
@@ -288,6 +290,16 @@ func CheckFallbackValid(scaledObject *ScaledObject) error {
 			scaledObject.Spec.Fallback.FailureThreshold, scaledObject.Spec.Fallback.Replicas)
 	}
 
+	// Check if behavior is valid when specified
+	if scaledObject.Spec.Fallback.Behavior != "" &&
+		scaledObject.Spec.Fallback.Behavior != FallbackBehaviorStatic &&
+		scaledObject.Spec.Fallback.Behavior != FallbackBehaviorUseCurrentReplicasAsMin {
+		return fmt.Errorf("Behaviour=%s must be either '%s' or '%s'",
+			  scaledObject.Spec.Fallback.Behavior,
+			  FallbackBehaviorStatic,
+			  FallbackBehaviorUseCurrentReplicasAsMin)
+	}
+
 	for _, trigger := range scaledObject.Spec.Triggers {
 		if trigger.Type == cpuString || trigger.Type == memoryString {
 			return fmt.Errorf("type is %s , but fallback it is not supported by the CPU & memory scalers", trigger.Type)

config/crd/bases/keda.sh_scaledobjects.yaml

@@ -231,8 +231,8 @@ spec:
                   replicas:
                     format: int32
                     type: integer
-                  useCurrentReplicasAsMinimum:
-                    type: boolean
+                  behavior:
+                    type: string
                 required:
                 - failureThreshold
                 - replicas

pkg/fallback/fallback.go

@@ -111,10 +111,10 @@ func HasValidFallback(scaledObject *kedav1alpha1.ScaledObject) bool {
 
 func doFallback(scaledObject *kedav1alpha1.ScaledObject, metricSpec v2.MetricSpec, metricName string, currentReplicas int32, suppressedError error) []external_metrics.ExternalMetricValue {
 	replicas := int64(scaledObject.Spec.Fallback.Replicas)
+	fallbackBehavior := scaledObject.Spec.Fallback.Behavior
 
-	// Check if we should use current replicas as minimum
-	if scaledObject.Spec.Fallback.UseCurrentReplicasAsMinimum != nil &&
-		*scaledObject.Spec.Fallback.UseCurrentReplicasAsMinimum {
+	// Check behavior 'UseCurrentReplicasAsMin'
+	if fallbackBehavior == kedav1alpha1.FallbackBehaviorUseCurrentReplicasAsMin {
 		currentReplicasCount := int64(currentReplicas)
 		if currentReplicasCount > replicas {
 			replicas = currentReplicasCount
@@ -141,7 +141,9 @@ func doFallback(scaledObject *kedav1alpha1.ScaledObject, metricSpec v2.MetricSpe
 		"scaledObject.Namespace", scaledObject.Namespace,
 		"scaledObject.Name", scaledObject.Name,
 		"suppressedError", suppressedError,
-		"fallback.replicas", replicas)
+		"fallback.behavior", fallbackBehavior,
+		"fallback.replicas", replicas,
+		"workload.currentReplicas", currentReplicas)
 	return fallbackMetrics
 }
 

pkg/fallback/fallback_test.go

@@ -378,13 +378,13 @@ var _ = Describe("fallback", func() {
 	It("should use fallback replicas when current replicas is lower", func() {
 		scaler.EXPECT().GetMetricsAndActivity(gomock.Any(), gomock.Eq(metricName)).Return(nil, false, errors.New("some error"))
 		startingNumberOfFailures := int32(3)
-		useCurrentAsMin := true
+		behavior := "useCurrentReplicasAsMinimum"
 
 		so := buildScaledObject(
 			&kedav1alpha1.Fallback{
-				FailureThreshold:            int32(3),
-				Replicas:                    int32(10),
-				UseCurrentReplicasAsMinimum: &useCurrentAsMin,
+				FailureThreshold: int32(3),
+				Replicas:         int32(10),
+				Behavior:         behavior,
 			},
 			&kedav1alpha1.ScaledObjectStatus{
 				Health: map[string]kedav1alpha1.HealthStatus{
@@ -409,49 +409,16 @@ var _ = Describe("fallback", func() {
 		Expect(value).Should(Equal(expectedValue))
 	})
 
-	It("should ignore current replicas when UseCurrentReplicasAsMinimum is false", func() {
-		scaler.EXPECT().GetMetricsAndActivity(gomock.Any(), gomock.Eq(metricName)).Return(nil, false, errors.New("some error"))
-		startingNumberOfFailures := int32(3)
-		useCurrentAsMin := false
-
-		so := buildScaledObject(
-			&kedav1alpha1.Fallback{
-				FailureThreshold:            int32(3),
-				Replicas:                    int32(10),
-				UseCurrentReplicasAsMinimum: &useCurrentAsMin,
-			},
-			&kedav1alpha1.ScaledObjectStatus{
-				Health: map[string]kedav1alpha1.HealthStatus{
-					metricName: {
-						NumberOfFailures: &startingNumberOfFailures,
-						Status:           kedav1alpha1.HealthStatusHappy,
-					},
-				},
-			},
-		)
-		metricSpec := createMetricSpec(10)
-		expectStatusPatch(ctrl, client)
-
-		mockScaleAndDeployment(ctrl, client, scaleClient, 15)
-
-		metrics, _, err := scaler.GetMetricsAndActivity(context.Background(), metricName)
-		metrics, _, err = GetMetricsWithFallback(context.Background(), client, scaleClient, metrics, err, metricName, so, metricSpec)
-
-		Expect(err).ToNot(HaveOccurred())
-		value := metrics[0].Value.AsApproximateFloat64()
-		expectedValue := float64(100) // 10 replicas * 10 target value, ignoring current 15
-		Expect(value).Should(Equal(expectedValue))
-	})
-
-	It("should handle nil UseCurrentReplicasAsMinimum the same as false", func() {
+	It("should ignore current replicas when behavior is 'static'", func() {
 		scaler.EXPECT().GetMetricsAndActivity(gomock.Any(), gomock.Eq(metricName)).Return(nil, false, errors.New("some error"))
 		startingNumberOfFailures := int32(3)
+		behavior := "static"
 
 		so := buildScaledObject(
 			&kedav1alpha1.Fallback{
 				FailureThreshold: int32(3),
 				Replicas:         int32(10),
-				// UseCurrentReplicasAsMinimum is nil
+				Behavior:         behavior,
 			},
 			&kedav1alpha1.ScaledObjectStatus{
 				Health: map[string]kedav1alpha1.HealthStatus{

pkg/scaling/executor/scale_scaledobjects.go

@@ -207,15 +207,16 @@ func (e *scaleExecutor) RequestScale(ctx context.Context, scaledObject *kedav1al
 
 func (e *scaleExecutor) doFallbackScaling(ctx context.Context, scaledObject *kedav1alpha1.ScaledObject, currentScale *autoscalingv1.Scale, logger logr.Logger, currentReplicas int32) {
 	targetReplicas := scaledObject.Spec.Fallback.Replicas
+	fallbackBehavior := scaledObject.Spec.Fallback.Behavior
 
-	// Check if we should use current replicas as minimum
-	if scaledObject.Spec.Fallback.UseCurrentReplicasAsMinimum != nil && *scaledObject.Spec.Fallback.UseCurrentReplicasAsMinimum && currentReplicas > targetReplicas {
+	// Check behavior 'UseCurrentReplicasAsMin'
+	if fallbackBehavior == kedav1alpha1.FallbackBehaviorUseCurrentReplicasAsMin && currentReplicas > targetReplicas {
 		targetReplicas = currentReplicas
 	}
 
 	_, err := e.updateScaleOnScaleTarget(ctx, scaledObject, currentScale, targetReplicas)
 	if err == nil {
-		logger.Info("Successfully set ScaleTarget replicas count to calculated fallback replicas", "Original Replicas Count", currentReplicas, "New Replicas Count", targetReplicas)
+		logger.Info("Successfully set ScaleTarget replicas count to calculated fallback replicas", "Original Replicas Count", currentReplicas, "New Replicas Count", targetReplicas, "Behavior", fallbackBehavior)
 	}
 	if e := e.setFallbackCondition(ctx, logger, scaledObject, metav1.ConditionTrue, "FallbackExists", "At least one trigger is falling back on this scaled object"); e != nil {
 		logger.Error(e, "Error setting fallback condition")

pkg/scaling/executor/scale_scaledobjects_test.go

@@ -523,7 +523,7 @@ func TestEventWitTriggerInfo(t *testing.T) {
 	assert.Equal(t, "Normal KEDAScaleTargetActivated Scaled  namespace/name from 2 to 5, triggered by testTrigger", eventstring)
 }
 
-// UseCurrentReplicasAsMinimum is true and current replicas is higher than fallback replicas
+// Behavior is UseCurrentReplicasAsMinimum and current replicas is higher than fallback replicas
 func TestScaleToFallbackWithCurrentReplicasAsMinimum(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	client := mock_client.NewMockClient(ctrl)
@@ -534,7 +534,7 @@ func TestScaleToFallbackWithCurrentReplicasAsMinimum(t *testing.T) {
 
 	scaleExecutor := NewScaleExecutor(client, mockScaleClient, nil, recorder)
 
-	useCurrentAsMin := true
+	behavior := "useCurrentReplicasAsMinimum"
 	scaledObject := v1alpha1.ScaledObject{
 		ObjectMeta: v1.ObjectMeta{
 			Name:      "name",
@@ -545,9 +545,9 @@ func TestScaleToFallbackWithCurrentReplicasAsMinimum(t *testing.T) {
 				Name: "name",
 			},
 			Fallback: &v1alpha1.Fallback{
-				FailureThreshold:            3,
-				Replicas:                    5,
-				UseCurrentReplicasAsMinimum: &useCurrentAsMin,
+				FailureThreshold: 3,
+				Replicas:         5,
+				Behavior:         behavior,
 			},
 		},
 		Status: v1alpha1.ScaledObjectStatus{
@@ -590,7 +590,7 @@ func TestScaleToFallbackWithCurrentReplicasAsMinimum(t *testing.T) {
 	assert.Equal(t, true, condition.IsTrue())
 }
 
-// UseCurrentReplicasAsMinimum is true and current replicas is lower than fallback replicas
+// Behavior is UseCurrentReplicasAsMinimum and is true and current replicas is lower than fallback replicas
 func TestScaleToFallbackIgnoringLowerCurrentReplicas(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	client := mock_client.NewMockClient(ctrl)
@@ -601,7 +601,7 @@ func TestScaleToFallbackIgnoringLowerCurrentReplicas(t *testing.T) {
 
 	scaleExecutor := NewScaleExecutor(client, mockScaleClient, nil, recorder)
 
-	useCurrentAsMin := true
+	behavior := "useCurrentReplicasAsMinimum"
 	scaledObject := v1alpha1.ScaledObject{
 		ObjectMeta: v1.ObjectMeta{
 			Name:      "name",
@@ -612,9 +612,9 @@ func TestScaleToFallbackIgnoringLowerCurrentReplicas(t *testing.T) {
 				Name: "name",
 			},
 			Fallback: &v1alpha1.Fallback{
-				FailureThreshold:            3,
-				Replicas:                    5,
-				UseCurrentReplicasAsMinimum: &useCurrentAsMin,
+				FailureThreshold: 3,
+				Replicas:         5,
+				Behavior:         behavior,
 			},
 		},
 		Status: v1alpha1.ScaledObjectStatus{
@@ -657,7 +657,7 @@ func TestScaleToFallbackIgnoringLowerCurrentReplicas(t *testing.T) {
 	assert.Equal(t, true, condition.IsTrue())
 }
 
-// UseCurrentReplicasAsMinimum is false and current replicas is higher than fallback replicas
+// Behavior is Static and current replicas is higher than fallback replicas
 func TestScaleToFallbackWithoutCurrentReplicasAsMinimum(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	client := mock_client.NewMockClient(ctrl)
@@ -668,7 +668,7 @@ func TestScaleToFallbackWithoutCurrentReplicasAsMinimum(t *testing.T) {
 
 	scaleExecutor := NewScaleExecutor(client, mockScaleClient, nil, recorder)
 
-	useCurrentAsMin := false
+	behavior := "static"
 	scaledObject := v1alpha1.ScaledObject{
 		ObjectMeta: v1.ObjectMeta{
 			Name:      "name",
@@ -679,9 +679,9 @@ func TestScaleToFallbackWithoutCurrentReplicasAsMinimum(t *testing.T) {
 				Name: "name",
 			},
 			Fallback: &v1alpha1.Fallback{
-				FailureThreshold:            3,
-				Replicas:                    5,
-				UseCurrentReplicasAsMinimum: &useCurrentAsMin,
+				FailureThreshold: 3,
+				Replicas:         5,
+				Behavior:         behavior,
 			},
 		},
 		Status: v1alpha1.ScaledObjectStatus{