update primary scaler query handling to consider mutliple triggers

Signed-off-by: Sanskar Jaiswal <sanskar.jaiswal@weave.works>
This commit is contained in:
Sanskar Jaiswal
2022-06-30 17:06:45 +05:30
parent b01e4cf9ec
commit a098d04d64
7 changed files with 94 additions and 42 deletions
+4 -2
View File
@@ -117,8 +117,10 @@ spec:
- ScaledObject
name:
type: string
primaryScalerQuery:
type: string
primaryScalerQueries:
type: object
additionalProperties:
type: string
ingressRef:
description: Ingress selector
type: object
+6 -1
View File
@@ -104,7 +104,7 @@ spec:
name:
type: string
autoscalerRef:
description: HPA selector
description: Scaler selector
type: object
required: ["apiVersion", "kind", "name"]
properties:
@@ -114,8 +114,13 @@ spec:
type: string
enum:
- HorizontalPodAutoscaler
- ScaledObject
name:
type: string
primaryScalerQueries:
type: object
additionalProperties:
type: string
ingressRef:
description: Ingress selector
type: object
+4 -2
View File
@@ -117,8 +117,10 @@ spec:
- ScaledObject
name:
type: string
primaryScalerQuery:
type: string
primaryScalerQueries:
type: object
additionalProperties:
type: string
ingressRef:
description: Ingress selector
type: object
+3 -3
View File
@@ -425,10 +425,10 @@ type AutoscalerRefernce struct {
// Name of the scaler
Name string `json:"name"`
// PrimaryScalerQuery is the query to use for the primary scaler, if
// Flagger generates one.
// PrimaryScalerQueries maps a unique id to a query for the primary
// scaler, if a scaler supports scaling using queries.
// +optional
PrimaryScalerQuery string `json:"primaryScalerQuery"`
PrimaryScalerQueries map[string]string `json:"primaryScalerQueries"`
}
// CustomMetadata holds labels and annotations to set on generated objects.
@@ -154,6 +154,13 @@ func (in *AlertProviderStatus) DeepCopy() *AlertProviderStatus {
// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
func (in *AutoscalerRefernce) DeepCopyInto(out *AutoscalerRefernce) {
*out = *in
if in.PrimaryScalerQueries != nil {
in, out := &in.PrimaryScalerQueries, &out.PrimaryScalerQueries
*out = make(map[string]string, len(*in))
for key, val := range *in {
(*out)[key] = val
}
}
return
}
@@ -439,7 +446,7 @@ func (in *CanarySpec) DeepCopyInto(out *CanarySpec) {
if in.AutoscalerRef != nil {
in, out := &in.AutoscalerRef, &out.AutoscalerRef
*out = new(AutoscalerRefernce)
**out = **in
(*in).DeepCopyInto(*out)
}
if in.IngressRef != nil {
in, out := &in.IngressRef, &out.IngressRef
+30 -22
View File
@@ -37,7 +37,7 @@ func (sor *ScaledObjectReconciler) ReconcilePrimaryScaler(cd *flaggerv1.Canary,
}
func (sor *ScaledObjectReconciler) reconcilePrimaryScaler(cd *flaggerv1.Canary, init bool) error {
primaryName := fmt.Sprintf("%s-primary", cd.Spec.AutoscalerRef.Name)
primaryName := fmt.Sprintf("%s-primary", cd.Spec.TargetRef.Name)
targetSo, err := sor.flaggerClient.KedaV1alpha1().ScaledObjects(cd.Namespace).Get(context.TODO(), cd.Spec.AutoscalerRef.Name, metav1.GetOptions{})
if err != nil {
return fmt.Errorf("Keda ScaledObject %s.%s get query error: %w",
@@ -45,9 +45,7 @@ func (sor *ScaledObjectReconciler) reconcilePrimaryScaler(cd *flaggerv1.Canary,
}
targetSoClone := targetSo.DeepCopy()
for _, trigger := range targetSoClone.Spec.Triggers {
setPrimaryScaledObjectQuery(cd, trigger.Metadata)
}
setPrimaryScaledObjectQueries(cd, targetSoClone.Spec.Triggers)
soSpec := keda.ScaledObjectSpec{
ScaleTargetRef: &keda.ScaleTarget{
@@ -166,24 +164,34 @@ func randSeq() string {
return string(b)
}
func setPrimaryScaledObjectQuery(cd *flaggerv1.Canary, metadata map[string]string) {
for key, val := range metadata {
if key == "query" {
if cd.Spec.AutoscalerRef.PrimaryScalerQuery != "" {
metadata[key] = cd.Spec.AutoscalerRef.PrimaryScalerQuery
} else {
// We could've used regex with negative look-arounds to avoid using a placeholder, but Go does
// not support them. We need them because, we need to replace both "podinfo" and "podinfo-canary"
// (assuming "podinfo" to be the targetRef name), with "podinfo-primary". This placeholder makes
// sure that we don't end up with a query which contains terms like "podinfo-primary-canary" or
// "podinfo-primary-primary". This is a best effort approach, and users should be encouraged to
// check the generated query and opt for using `autoscalerRef.primaryScalerQuery` if the former
// doesn't look correct.
placeholder := randSeq()
replaced := strings.ReplaceAll(val, fmt.Sprintf("%s-canary", cd.Spec.TargetRef.Name), placeholder)
replaced = strings.ReplaceAll(replaced, cd.Spec.TargetRef.Name, fmt.Sprintf("%s-primary", cd.Spec.TargetRef.Name))
replaced = strings.ReplaceAll(replaced, placeholder, fmt.Sprintf("%s-primary", cd.Spec.TargetRef.Name))
metadata[key] = replaced
// setPrimaryScaledObjectQueries accepts a list of ScaleTriggers and modifies the query
// for each of them.
func setPrimaryScaledObjectQueries(cd *flaggerv1.Canary, triggers []keda.ScaleTriggers) {
for _, trigger := range triggers {
if cd.Spec.AutoscalerRef.PrimaryScalerQueries != nil {
// If .spec.autoscalerRef.primaryScalerQueries is specified, the triggers must be named,
// otherwise it might lead to unexpected behaviour.
for name, query := range cd.Spec.AutoscalerRef.PrimaryScalerQueries {
if trigger.Name == name {
trigger.Metadata["query"] = query
}
}
} else {
for key, val := range trigger.Metadata {
if key == "query" {
// We could've used regex with negative look-arounds to avoid using a placeholder, but Go does
// not support them. We need them because, we need to replace both "podinfo" and "podinfo-canary"
// (assuming "podinfo" to be the targetRef name), with "podinfo-primary". This placeholder makes
// sure that we don't end up with a query which contains terms like "podinfo-primary-canary" or
// "podinfo-primary-primary". This is a best effort approach, and users should be encouraged to
// check the generated query and opt for using `autoscalerRef.primaryScalerQuery` if the former
// doesn't look correct.
placeholder := randSeq()
replaced := strings.ReplaceAll(val, fmt.Sprintf("%s-canary", cd.Spec.TargetRef.Name), placeholder)
replaced = strings.ReplaceAll(replaced, cd.Spec.TargetRef.Name, fmt.Sprintf("%s-primary", cd.Spec.TargetRef.Name))
replaced = strings.ReplaceAll(replaced, placeholder, fmt.Sprintf("%s-primary", cd.Spec.TargetRef.Name))
trigger.Metadata[key] = replaced
}
}
}
}
+39 -11
View File
@@ -2,6 +2,7 @@ package canary
import (
"context"
"fmt"
"testing"
"github.com/stretchr/testify/assert"
@@ -28,6 +29,7 @@ func Test_reconcilePrimaryScaledObject(t *testing.T) {
primarySO, err := mocks.flaggerClient.KedaV1alpha1().ScaledObjects("default").Get(context.TODO(), "podinfo-primary", metav1.GetOptions{})
require.NoError(t, err)
assert.Equal(t, primarySO.Spec.ScaleTargetRef.Name, fmt.Sprintf("%s-primary", mocks.canary.Spec.TargetRef.Name))
assert.Equal(t, int(*primarySO.Spec.PollingInterval), 10)
assert.Equal(t, int(*primarySO.Spec.MinReplicaCount), 1)
assert.Equal(t, primarySO.Spec.Triggers[0].Metadata["query"], `sum(rate(http_requests_total{deployment="podinfo-primary"}[2m]))`)
@@ -77,7 +79,7 @@ func Test_resumeScaledObject(t *testing.T) {
assert.False(t, exists)
}
func Test_setPrimaryScaledObjectQuery(t *testing.T) {
func Test_setPrimaryScaledObjectQueries(t *testing.T) {
cd := &flaggerv1.Canary{
Spec: flaggerv1.CanarySpec{
TargetRef: flaggerv1.LocalObjectReference{
@@ -111,18 +113,44 @@ func Test_setPrimaryScaledObjectQuery(t *testing.T) {
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
metadata := make(map[string]string)
metadata["query"] = test.query
setPrimaryScaledObjectQuery(cd, metadata)
assert.Equal(t, metadata["query"], test.wantQuery)
triggers := make([]keda.ScaleTriggers, 0)
triggers = append(triggers, keda.ScaleTriggers{
Metadata: map[string]string{
"query": test.query,
},
})
setPrimaryScaledObjectQueries(cd, triggers)
assert.Equal(t, triggers[0].Metadata["query"], test.wantQuery)
})
}
primaryQuery := `sum(rate(envoy_cluster_upstream_rq{ envoy_cluster_name="test_podinfo-primary_80" }[30s]))`
cd.Spec.AutoscalerRef.PrimaryScalerQuery = primaryQuery
metadata := make(map[string]string)
metadata["query"] = ""
pq1 := `sum(rate(envoy_cluster_upstream_rq{ envoy_cluster_name="test_podinfo-primary_80" }[30s]))`
pq2 := `sum(rate(envoy_cluster_upstream_rq{ envoy_cluster_name="test_podinfo" }[30s]))`
triggers := make([]keda.ScaleTriggers, 0)
triggers = append(triggers, keda.ScaleTriggers{
Name: "trigger1",
Metadata: map[string]string{
"query": pq1,
},
})
triggers = append(triggers, keda.ScaleTriggers{
Name: "trigger2",
Metadata: map[string]string{
"query": pq2,
},
})
cd.Spec.AutoscalerRef.PrimaryScalerQueries = map[string]string{
"trigger1": pq1,
"trigger2": pq2,
}
setPrimaryScaledObjectQuery(cd, metadata)
assert.Equal(t, primaryQuery, metadata["query"])
setPrimaryScaledObjectQueries(cd, triggers)
for _, trigger := range triggers {
if trigger.Name == "trigger1" {
assert.Equal(t, pq1, trigger.Metadata["query"])
}
if trigger.Name == "trigger2" {
assert.Equal(t, pq2, trigger.Metadata["query"])
}
}
}