feat: validate fallback configuration (kedacore#5629)

Signed-off-by: Guo Peng <[email protected]>
Signed-off-by: guopeng <[email protected]>
Signed-off-by: Tom Kerkhove <[email protected]>
Co-authored-by: Tom Kerkhove <[email protected]>

CHANGELOG.md

@@ -75,6 +75,7 @@ Here is an overview of all new **experimental** features:
 - **General**: Add `validations.keda.sh/hpa-ownership` annotation to HPA to disable ownership validation ([#5516](https://github.com/kedacore/keda/issues/5516))
 - **General**: Improve Prometheus metrics to align with best practices ([#4854](https://github.com/kedacore/keda/issues/4854))
 - **General**: Support csv-format for WATCH_NAMESPACE env var ([#5670](https://github.com/kedacore/keda/issues/5670))
+- **General**: Validate fallback configuration when creating ScaledObjects ([#5515](https://github.com/kedacore/keda/issues/5515))
 - **Azure Event Hub Scaler**: Remove usage of checkpoint offsets to account for SDK checkpointing implementation changes ([#5574](https://github.com/kedacore/keda/issues/5574))
 - **GCP Pub/Sub Scaler**: Add support for resolving resource names from the scale target's environment ([#5693](https://github.com/kedacore/keda/issues/5693))
 - **GCP Stackdriver Scaler**: Add missing parameters 'rate' and 'count' for GCP Stackdriver Scaler alignment ([#5633](https://github.com/kedacore/keda/issues/5633))

apis/keda/v1alpha1/scaledobject_types.go

@@ -269,3 +269,26 @@ func CheckReplicaCountBoundsAreValid(scaledObject *ScaledObject) error {
 
 	return nil
 }
+
+// CheckFallbackValid checks that the fallback supports scalers with an AverageValue metric target.
+// Consequently, it does not support CPU & memory scalers, or scalers targeting a Value metric type.
+func CheckFallbackValid(scaledObject *ScaledObject) error {
+	if scaledObject.Spec.Fallback == nil {
+		return nil
+	}
+
+	if scaledObject.Spec.Fallback.FailureThreshold < 0 || scaledObject.Spec.Fallback.Replicas < 0 {
+		return fmt.Errorf("FailureThreshold=%d & Replicas=%d must both be greater than or equal to 0",
+			scaledObject.Spec.Fallback.FailureThreshold, scaledObject.Spec.Fallback.Replicas)
+	}
+
+	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)
+		}
+		if trigger.MetricType != autoscalingv2.AverageValueMetricType {
+			return fmt.Errorf("MetricType=%s, but Fallback can only be enabled for triggers with metric of type AverageValue", trigger.MetricType)
+		}
+	}
+	return nil
+}

apis/keda/v1alpha1/scaledobject_webhook.go

@@ -135,6 +135,7 @@ func validateWorkload(so *ScaledObject, action string, dryRun bool) (admission.W
 		verifyScaledObjects,
 		verifyHpas,
 		verifyReplicaCount,
+		verifyFallback,
 	}
 
 	for i := range verifyFunctions {
@@ -168,6 +169,15 @@ func verifyReplicaCount(incomingSo *ScaledObject, action string, _ bool) error {
 	return nil
 }
 
+func verifyFallback(incomingSo *ScaledObject, action string, _ bool) error {
+	err := CheckFallbackValid(incomingSo)
+	if err != nil {
+		scaledobjectlog.WithValues("name", incomingSo.Name).Error(err, "validation error")
+		metricscollector.RecordScaledObjectValidatingErrors(incomingSo.Namespace, action, "incorrect-fallback")
+	}
+	return nil
+}
+
 func verifyTriggers(incomingObject interface{}, action string, _ bool) error {
 	var triggers []ScaleTriggers
 	var name string

apis/keda/v1alpha1/scaledobject_webhook_test.go

@@ -158,6 +158,44 @@ var _ = It("shouldn't validate the so creation when the replica counts are wrong
 	}).Should(HaveOccurred())
 })
 
+var _ = It("shouldn't validate the so creation when the fallback is wrong", func() {
+	namespaceName := "wrong-fallback"
+	namespace := createNamespace(namespaceName)
+
+	so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", false, map[string]string{}, "")
+	so.Spec.Fallback = &Fallback{
+		FailureThreshold: -1,
+		Replicas:         -3,
+	}
+
+	err := k8sClient.Create(context.Background(), namespace)
+	Expect(err).ToNot(HaveOccurred())
+
+	Eventually(func() error {
+		return k8sClient.Create(context.Background(), so)
+	}).Should(HaveOccurred())
+})
+
+var _ = It("shouldn't validate the so creation When the fallback are configured and the scaler is either CPU or memory.", func() {
+	namespaceName := "wrong-fallback-cpu-memory"
+	namespace := createNamespace(namespaceName)
+	workload := createDeployment(namespaceName, true, true)
+	so := createScaledObject(soName, namespaceName, workloadName, "apps/v1", "Deployment", true, map[string]string{}, "")
+	so.Spec.Fallback = &Fallback{
+		FailureThreshold: 3,
+		Replicas:         6,
+	}
+	err := k8sClient.Create(context.Background(), namespace)
+	Expect(err).ToNot(HaveOccurred())
+
+	err = k8sClient.Create(context.Background(), workload)
+	Expect(err).ToNot(HaveOccurred())
+
+	Eventually(func() error {
+		return k8sClient.Create(context.Background(), so)
+	}).Should(HaveOccurred())
+})
+
 var _ = It("shouldn't validate the so creation when there is another unmanaged hpa and so has transfer-hpa-ownership activated", func() {
 
 	hpaName := "test-unmanaged-hpa-ownership"