Skip to content

Commit

Permalink
Scaling Modifiers: cast to float internally and add tests (#5079)
Browse files Browse the repository at this point in the history 
Signed-off-by: gauron99 <fridrich.david19@gmail.com>
  • Loading branch information
@gauron99
gauron99 authored Dec 15, 2023
1 parent 99a1fe5 commit aa5cab1
Show file tree
Hide file tree
Showing 5 changed files with 312 additions and 1 deletion.
1 change: 1 addition & 0 deletions apis/eventing/v1alpha1/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 15 additions & 0 deletions apis/keda/v1alpha1/scaledobject_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"errors"
"fmt"
"strconv"
"strings"

"github.com/antonmedv/expr"
"github.com/antonmedv/expr/vm"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 + ")"
}
234 changes: 234 additions & 0 deletions apis/keda/v1alpha1/scaledobject_webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -590,13 +593,244 @@ 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")
err := testEnv.Stop()
Expect(err).NotTo(HaveOccurred())
})

// -------------------------------------------------------------------------- //
// ----------------------------- HELP FUNCTIONS ----------------------------- //
// -------------------------------------------------------------------------- //

func createNamespace(name string) *v1.Namespace {
return &v1.Namespace{
ObjectMeta: metav1.ObjectMeta{Name: name},
Expand Down
1 change: 1 addition & 0 deletions apis/keda/v1alpha1/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

62 changes: 61 additions & 1 deletion tests/internals/scaling_modifiers/scaling_modifiers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit aa5cab1

Please sign in to comment.