diff --git a/apis/eventing/v1alpha1/zz_generated.deepcopy.go b/apis/eventing/v1alpha1/zz_generated.deepcopy.go index 4a59e8a0aaa..395be2a4a89 100644 --- a/apis/eventing/v1alpha1/zz_generated.deepcopy.go +++ b/apis/eventing/v1alpha1/zz_generated.deepcopy.go @@ -1,4 +1,5 @@ //go:build !ignore_autogenerated +// +build !ignore_autogenerated /* Copyright 2023 The KEDA Authors diff --git a/apis/keda/v1alpha1/scaledobject_webhook.go b/apis/keda/v1alpha1/scaledobject_webhook.go index 812877b92f0..3c24719a5a3 100644 --- a/apis/keda/v1alpha1/scaledobject_webhook.go +++ b/apis/keda/v1alpha1/scaledobject_webhook.go @@ -22,6 +22,7 @@ import ( "errors" "fmt" "strconv" + "strings" "github.com/antonmedv/expr" "github.com/antonmedv/expr/vm" @@ -334,6 +335,10 @@ func ValidateAndCompileScalingModifiers(so *ScaledObject) (*vm.Program, error) { return nil, fmt.Errorf("error ScalingModifiers.Formula is mandatory") } + // cast return value of formula to float if necessary to avoid wrong value return + // type (ternary operator doesnt return float) + so.Spec.Advanced.ScalingModifiers.Formula = castToFloatIfNecessary(sm.Formula) + // validate formula if not empty compiledFormula, err := validateScalingModifiersFormula(so) if err != nil { @@ -409,3 +414,13 @@ func validateScalingModifiersTarget(so *ScaledObject) error { return nil } + +// castToFloatIfNecessary takes input formula and casts its return value to float +// if necessary to avoid wrong return value type like ternary operator has and/or +// to relief user of having to add it to the formula themselves. +func castToFloatIfNecessary(formula string) string { + if strings.HasPrefix(formula, "float(") { + return formula + } + return "float(" + formula + ")" +} diff --git a/apis/keda/v1alpha1/scaledobject_webhook_test.go b/apis/keda/v1alpha1/scaledobject_webhook_test.go index 6ff6f0b513b..37c0c3a46d8 100644 --- a/apis/keda/v1alpha1/scaledobject_webhook_test.go +++ b/apis/keda/v1alpha1/scaledobject_webhook_test.go @@ -437,6 +437,9 @@ var _ = It("shouldn't create so when stabilizationWindowSeconds exceeds 3600", f Should(HaveOccurred()) }) +// ============================ SCALING MODIFIERS ============================ \\ +// =========================================================================== \\ + var _ = It("should validate the so creation with ScalingModifiers.Formula", func() { namespaceName := "scaling-modifiers-formula-good" namespace := createNamespace(namespaceName) @@ -590,6 +593,233 @@ var _ = It("should validate the so creation with ScalingModifiers when formula t }).ShouldNot(HaveOccurred()) }) +var _ = It("should validate the so creation with ScalingModifiers when formula casts to float already", func() { + namespaceName := "scaling-modifiers-cast-to-float-good" + namespace := createNamespace(namespaceName) + workload := createDeployment(namespaceName, true, true) + + sm := ScalingModifiers{Target: "2", Formula: "float(workload_trig + 1)"} + + triggers := []ScaleTriggers{ + { + Type: "kubernetes-workload", + Name: "workload_trig", + Metadata: map[string]string{ + "podSelector": "pod=workload-test", + "value": "1", + }, + }, + } + + so := createScaledObjectScalingModifiers(namespaceName, sm, triggers) + + 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) + }).ShouldNot(HaveOccurred()) +}) + +var _ = It("should validate the so creation with ScalingModifiers.Formula - casting float from ternary operator", func() { + namespaceName := "scaling-modifiers-formula-casting-float-good" + namespace := createNamespace(namespaceName) + workload := createDeployment(namespaceName, false, false) + + sm := ScalingModifiers{Target: "2", Formula: "float(workload_trig < 5 ? cron_trig + workload_trig : 5)"} + + triggers := []ScaleTriggers{ + { + Type: "cron", + Name: "cron_trig", + Metadata: map[string]string{ + "timezone": "UTC", + "start": "0 * * * *", + "end": "1 * * * *", + "desiredReplicas": "1", + }, + }, + { + Type: "kubernetes-workload", + Name: "workload_trig", + Metadata: map[string]string{ + "podSelector": "pod=workload-test", + "value": "1", + }, + }, + } + + so := createScaledObjectScalingModifiers(namespaceName, sm, triggers) + + 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) + }).ShouldNot(HaveOccurred()) +}) + +// this test checks that internally, the casting to float happened successfully +// to override the return value of ternary operator '?' - because it tries to compile +// in webhook validator or during hpa setup and wouldnt compile without float return +// value +var _ = It("should validate the so creation with ScalingModifiers.Formula - ternary operator without casting float", func() { + namespaceName := "scaling-modifiers-formula-ternary-no-casting-float-good" + namespace := createNamespace(namespaceName) + workload := createDeployment(namespaceName, false, false) + + sm := ScalingModifiers{Target: "2", Formula: "workload_trig < 5 ? cron_trig + workload_trig : 5"} + + triggers := []ScaleTriggers{ + { + Type: "cron", + Name: "cron_trig", + Metadata: map[string]string{ + "timezone": "UTC", + "start": "0 * * * *", + "end": "1 * * * *", + "desiredReplicas": "1", + }, + }, + { + Type: "kubernetes-workload", + Name: "workload_trig", + Metadata: map[string]string{ + "podSelector": "pod=workload-test", + "value": "1", + }, + }, + } + + so := createScaledObjectScalingModifiers(namespaceName, sm, triggers) + + 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) + }).ShouldNot(HaveOccurred()) +}) + +var _ = It("should validate the so creation with ScalingModifiers.Formula - count operator", func() { + namespaceName := "scaling-modifiers-formula-count-function-good" + namespace := createNamespace(namespaceName) + workload := createDeployment(namespaceName, false, false) + + sm := ScalingModifiers{Target: "2", Formula: "count([trig_one,trig_two],{#>1}) > 1 ? 5 : 0"} + + triggers := []ScaleTriggers{ + { + Type: "cron", + Name: "trig_one", + Metadata: map[string]string{ + "timezone": "UTC", + "start": "0 * * * *", + "end": "1 * * * *", + "desiredReplicas": "1", + }, + }, + { + Type: "kubernetes-workload", + Name: "trig_two", + Metadata: map[string]string{ + "podSelector": "pod=workload-test", + "value": "1", + }, + }, + } + + so := createScaledObjectScalingModifiers(namespaceName, sm, triggers) + + 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) + }).ShouldNot(HaveOccurred()) +}) + +var _ = It("should validate the so creation with ScalingModifiers.Formula - complex ternary", func() { + namespaceName := "scaling-modifiers-formula-complex-ternary-good" + namespace := createNamespace(namespaceName) + workload := createDeployment(namespaceName, false, false) + + sm := ScalingModifiers{Target: "2", Formula: "float(trig_one < 2 ? trig_one+trig_two >= 2 ? 5 : 10 : 0)"} + + triggers := []ScaleTriggers{ + { + Type: "cron", + Name: "trig_one", + Metadata: map[string]string{ + "timezone": "UTC", + "start": "0 * * * *", + "end": "1 * * * *", + "desiredReplicas": "1", + }, + }, + { + Type: "kubernetes-workload", + Name: "trig_two", + Metadata: map[string]string{ + "podSelector": "pod=workload-test", + "value": "1", + }, + }, + } + + so := createScaledObjectScalingModifiers(namespaceName, sm, triggers) + + 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) + }).ShouldNot(HaveOccurred()) +}) + +var _ = It("should validate the so creation with ScalingModifiers.Formula - double float cast", func() { + namespaceName := "scaling-modifiers-formula-double-float-cast-good" + namespace := createNamespace(namespaceName) + workload := createDeployment(namespaceName, false, false) + + sm := ScalingModifiers{Target: "2", Formula: "float(float(trig_two < 5 ? trig_one + trig_two : 5))"} + triggers := []ScaleTriggers{ + { + Type: "cron", + Name: "trig_one", + Metadata: map[string]string{ + "timezone": "UTC", + "start": "0 * * * *", + "end": "1 * * * *", + "desiredReplicas": "1", + }, + }, + { + Type: "kubernetes-workload", + Name: "trig_two", + Metadata: map[string]string{ + "podSelector": "pod=workload-test", + "value": "1", + }, + }, + } + + so := createScaledObjectScalingModifiers(namespaceName, sm, triggers) + + 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) + }).ShouldNot(HaveOccurred()) +}) + var _ = AfterSuite(func() { cancel() By("tearing down the test environment") @@ -597,6 +827,10 @@ var _ = AfterSuite(func() { Expect(err).NotTo(HaveOccurred()) }) +// -------------------------------------------------------------------------- // +// ----------------------------- HELP FUNCTIONS ----------------------------- // +// -------------------------------------------------------------------------- // + func createNamespace(name string) *v1.Namespace { return &v1.Namespace{ ObjectMeta: metav1.ObjectMeta{Name: name}, diff --git a/apis/keda/v1alpha1/zz_generated.deepcopy.go b/apis/keda/v1alpha1/zz_generated.deepcopy.go index e0fdc9c51ee..9435a9c4829 100755 --- a/apis/keda/v1alpha1/zz_generated.deepcopy.go +++ b/apis/keda/v1alpha1/zz_generated.deepcopy.go @@ -1,4 +1,5 @@ //go:build !ignore_autogenerated +// +build !ignore_autogenerated /* Copyright 2023 The KEDA Authors diff --git a/tests/internals/scaling_modifiers/scaling_modifiers_test.go b/tests/internals/scaling_modifiers/scaling_modifiers_test.go index d42a7b27022..7e9c3817732 100644 --- a/tests/internals/scaling_modifiers/scaling_modifiers_test.go +++ b/tests/internals/scaling_modifiers/scaling_modifiers_test.go @@ -166,6 +166,48 @@ spec: podSelector: pod=workload-test ` + soComplexFormula = ` +apiVersion: keda.sh/v1alpha1 +kind: ScaledObject +metadata: + name: {{.ScaledObject}} + namespace: {{.TestNamespace}} + labels: + app: {{.DeploymentName}} +spec: + scaleTargetRef: + name: {{.DeploymentName}} + advanced: + horizontalPodAutoscalerConfig: + behavior: + scaleDown: + stabilizationWindowSeconds: 5 + scalingModifiers: + formula: "count([kw_trig,metrics_api],{#>1}) > 1 ? 5 : 0" + target: '2' + activationTarget: '2' + pollingInterval: 5 + cooldownPeriod: 5 + minReplicaCount: 0 + maxReplicaCount: 10 + fallback: + replicas: 5 + failureThreshold: 3 + triggers: + - type: metrics-api + name: metrics_api + metadata: + url: "{{.MetricsServerEndpoint}}" + valueLocation: 'value' + method: "query" + authenticationRef: + name: {{.TriggerAuthName}} + - type: kubernetes-workload + name: kw_trig + metadata: + podSelector: pod=workload-test +` + workloadDeploymentTemplate = ` apiVersion: apps/v1 kind: Deployment @@ -231,7 +273,7 @@ func TestScalingModifiers(t *testing.T) { testFormula(t, kc, data) - templates = append(templates, Template{Name: "soFallbackTemplate", Config: soFallbackTemplate}) + templates = append(templates, Template{Name: "soComplexFormula", Config: soComplexFormula}) DeleteKubernetesResources(t, namespace, data, templates) } @@ -272,6 +314,24 @@ func testFormula(t *testing.T, kc *kubernetes.Clientset, data templateData) { // 2+2=4; target = 2 -> 4/2 replicas should be 2 assert.True(t, WaitForDeploymentReplicaReadyCount(t, kc, deploymentName, namespace, 2, 12, 10), "replica count should be %d after 2 minutes", 2) + + data.MetricValue = 0 + KubectlReplaceWithTemplate(t, data, "updateMetricsTemplate", updateMetricsTemplate) + + // apply new SO + KubectlApplyWithTemplate(t, data, "soComplexFormula", soComplexFormula) + + // formula has count() which needs atleast 2 metrics to have value over 1 to scale up + // now should be 0 + assert.True(t, WaitForDeploymentReplicaReadyCount(t, kc, deploymentName, namespace, 0, 12, 10), + "replica count should be %d after 2 minutes", 0) + + data.MetricValue = 2 + KubectlReplaceWithTemplate(t, data, "updateMetricsTemplate", updateMetricsTemplate) + + // 5//2 = 3 + assert.True(t, WaitForDeploymentReplicaReadyCount(t, kc, deploymentName, namespace, 3, 12, 10), + "replica count should be %d after 2 minutes", 3) } func getTemplateData() (templateData, []Template) {