Checkfallbackvalid (#6407)

Signed-off-by: Shane <imu_xxd@163.com>
Signed-off-by: Zbynek Roubalik <zroubalik@gmail.com>
Co-authored-by: Zbynek Roubalik <zroubalik@gmail.com>

CHANGELOG.md

@@ -76,6 +76,9 @@ Here is an overview of all new **experimental** features:
 
 ### Fixes
 
+- **General**: Centralize and improve automaxprocs configuration with proper structured logging ([#5970](https://github.com/kedacore/keda/issues/5970))
+- **General**: Paused ScaledObject count is reported correctly after operator restart ([#6321](https://github.com/kedacore/keda/issues/6321))
+- **General**: ScaledJobs ready status set to true when recoverred problem ([#6329](https://github.com/kedacore/keda/pull/6329))
 - **General**: Fix event text when deactivation fails ([#6469](https://github.com/kedacore/keda/issues/6469))
 - **AWS Scalers**: Add AWS region to the AWS Config Cache key ([#6128](https://github.com/kedacore/keda/issues/6128))
 - **Redis Streams**: Allow default value of 0 for activationLagCount ([#6478](https://github.com/kedacore/keda/issues/6478))
@@ -98,6 +101,7 @@ New deprecation(s):
 ### Other
 
 - **General**: Add debug logs tracking validation of ScaledObjects on webhook ([#6498](https://github.com/kedacore/keda/pull/6498))
+- **General**: Fix fallback validation check bug ([#6407](https://github.com/kedacore/keda/pull/6407))
 - **General**: New eventreason KEDAScalersInfo to display important information ([#6328](https://github.com/kedacore/keda/issues/6328))
 
 ## v2.16.1

apis/keda/v1alpha1/scaledobject_types.go

@@ -23,8 +23,11 @@ import (
 
 	autoscalingv2 "k8s.io/api/autoscaling/v2"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	logf "sigs.k8s.io/controller-runtime/pkg/log"
 )
 
+var scaledobjecttypeslog = logf.Log.WithName("scaledobject-types")
+
 // +genclient
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
@@ -237,7 +240,7 @@ 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
+// 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
@@ -246,15 +249,15 @@ func (so *ScaledObject) GetHPAMinReplicas() *int32 {
 	return &tmp
 }
 
-// getHPAMaxReplicas returns MaxReplicas based on definition in ScaledObject or default value if not defined
+// GetHPAMaxReplicas returns MaxReplicas based on definition in ScaledObject or default value if not defined
 func (so *ScaledObject) GetHPAMaxReplicas() int32 {
 	if so.Spec.MaxReplicaCount != nil {
 		return *so.Spec.MaxReplicaCount
 	}
 	return defaultHPAMaxReplicas
 }
 
-// checkReplicaCountBoundsAreValid checks that Idle/Min/Max ReplicaCount defined in ScaledObject are correctly specified
+// 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 {
 	minReplicas := int32(0)
@@ -288,10 +291,10 @@ func CheckFallbackValid(scaledObject *ScaledObject) error {
 
 	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)
+			scaledobjecttypeslog.Error(nil, fmt.Sprintf("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 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

@@ -197,7 +197,7 @@ func verifyFallback(incomingSo *ScaledObject, action string, _ bool) error {
 		scaledobjectlog.WithValues("name", incomingSo.Name).Error(err, "validation error")
 		metricscollector.RecordScaledObjectValidatingErrors(incomingSo.Namespace, action, "incorrect-fallback")
 	}
-	return nil
+	return err
 }
 
 func verifyTriggers(incomingObject interface{}, action string, _ bool) error {

apis/keda/v1alpha1/scaledobject_webhook_test.go

@@ -176,15 +176,18 @@ var _ = It("shouldn't validate the so creation when the fallback is wrong", func
 	}).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"
+var _ = It("should validate the so creation When the fallback are configured and the scaler is either CPU or memory.", func() {
+	namespaceName := "right-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,
 	}
+	for index := range so.Spec.Triggers {
+		so.Spec.Triggers[index].MetricType = "AverageValue"
+	}
 	err := k8sClient.Create(context.Background(), namespace)
 	Expect(err).ToNot(HaveOccurred())
 
@@ -193,7 +196,7 @@ var _ = It("shouldn't validate the so creation When the fallback are configured
 
 	Eventually(func() error {
 		return k8sClient.Create(context.Background(), so)
-	}).Should(HaveOccurred())
+	}).ShouldNot(HaveOccurred())
 })
 
 var _ = It("shouldn't validate the so creation when there is another unmanaged hpa and so has transfer-hpa-ownership activated", func() {

tests/internals/fallback/fallback_test.go

@@ -189,6 +189,7 @@ spec:
       url: "{{.MetricsServerEndpoint}}"
       valueLocation: 'value'
       method: "query"
+    metricType: "AverageValue"
     authenticationRef:
       name: {{.TriggerAuthName}}
 `

tests/internals/scaling_modifiers/scaling_modifiers_test.go

@@ -163,12 +163,14 @@ spec:
       url: "{{.MetricsServerEndpoint}}"
       valueLocation: 'value'
       method: "query"
+    metricType: "AverageValue"
     authenticationRef:
       name: {{.TriggerAuthName}}
   - type: kubernetes-workload
     name: kw_trig
     metadata:
       podSelector: pod=workload-test
+    metricType: "AverageValue"
 `
 
 	soComplexFormula = `
@@ -205,12 +207,14 @@ spec:
       url: "{{.MetricsServerEndpoint}}"
       valueLocation: 'value'
       method: "query"
+    metricType: "AverageValue"
     authenticationRef:
       name: {{.TriggerAuthName}}
   - type: kubernetes-workload
     name: kw_trig
     metadata:
       podSelector: pod=workload-test
+    metricType: "AverageValue"
 `
 
 	workloadDeploymentTemplate = `