Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: validate fallback configuration #5629

Merged
merged 8 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ Here is an overview of all new **experimental** features:
- **General**: Add GRPC Client and Server metrics ([#5502](https://github.com/kedacore/keda/issues/5502))
- **General**: Add OPENTELEMETRY flag in e2e test YAML ([#5375](https://github.com/kedacore/keda/issues/5375))
- **General**: Add support for cross tenant/cloud authentication when using Azure Workload Identity for TriggerAuthentication ([#5441](https://github.com/kedacore/keda/issues/5441))
- **General**: Validate fallback configuration when creating ScaledObjects ([#5515](https://github.com/kedacore/keda/issues/5515))
- **General**: Add `validations.keda.sh/hpa-ownership` annotation to HPA to disable ownership validation ([#5516](https://github.com/kedacore/keda/issues/5516))
- **General**: Support csv-format for WATCH_NAMESPACE env var ([#5670](https://github.com/kedacore/keda/issues/5670))
tomkerkhove marked this conversation as resolved.
Show resolved Hide resolved
- **Azure Event Hub Scaler**: Remove usage of checkpoint offsets to account for SDK checkpointing implementation changes ([#5574](https://github.com/kedacore/keda/issues/5574))
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