Skip to content

Commit

Permalink
feat: validate fallback configuration (#5629)
Browse files Browse the repository at this point in the history
Signed-off-by: Guo Peng <370090914@qq.com>
Signed-off-by: guopeng <100843245+guopeng0@users.noreply.github.com>
Signed-off-by: Tom Kerkhove <kerkhove.tom@gmail.com>
Co-authored-by: Tom Kerkhove <kerkhove.tom@gmail.com>
  • Loading branch information
guopeng0 and tomkerkhove authored Apr 23, 2024
1 parent 9a00d1e commit b13e2a4
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
23 changes: 23 additions & 0 deletions apis/keda/v1alpha1/scaledobject_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
10 changes: 10 additions & 0 deletions apis/keda/v1alpha1/scaledobject_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ func validateWorkload(so *ScaledObject, action string, dryRun bool) (admission.W
verifyScaledObjects,
verifyHpas,
verifyReplicaCount,
verifyFallback,
}

for i := range verifyFunctions {
Expand Down Expand Up @@ -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
Expand Down
38 changes: 38 additions & 0 deletions apis/keda/v1alpha1/scaledobject_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit b13e2a4

Please sign in to comment.