cast to float internally, more SM tests

Signed-off-by: gauron99 <fridrich.david19@gmail.com>

apis/keda/v1alpha1/scaledobject_webhook.go

@@ -22,6 +22,7 @@ import (
  "errors"
  "fmt"
  "strconv"
+ "strings"
 
  "github.com/antonmedv/expr"
  "github.com/antonmedv/expr/vm"
@@ -324,6 +325,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 {
@@ -399,3 +404,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 + ")"
+}

apis/keda/v1alpha1/scaledobject_webhook_test.go

@@ -422,6 +422,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)
@@ -575,7 +578,36 @@ var _ = It("should validate the so creation with ScalingModifiers when formula t
  }).ShouldNot(HaveOccurred())
 })
 
-var _ = It("should validate the so creation with ScalingModifiers.Formula - casting float in formula", func() {
+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)
@@ -614,6 +646,165 @@ var _ = It("should validate the so creation with ScalingModifiers.Formula - cast
  }).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(workload_trig < 5 ? cron_trig + workload_trig : 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")

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) {