diff --git a/artifacts/flagger/crd.yaml b/artifacts/flagger/crd.yaml index dc203c51..74841cf7 100644 --- a/artifacts/flagger/crd.yaml +++ b/artifacts/flagger/crd.yaml @@ -117,8 +117,10 @@ spec: - ScaledObject name: type: string - primaryScalerQuery: - type: string + primaryScalerQueries: + type: object + additionalProperties: + type: string ingressRef: description: Ingress selector type: object diff --git a/charts/flagger/crds/crd.yaml b/charts/flagger/crds/crd.yaml index 19b24319..578867df 100644 --- a/charts/flagger/crds/crd.yaml +++ b/charts/flagger/crds/crd.yaml @@ -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 diff --git a/kustomize/base/flagger/crd.yaml b/kustomize/base/flagger/crd.yaml index dceb77b8..40d47912 100644 --- a/kustomize/base/flagger/crd.yaml +++ b/kustomize/base/flagger/crd.yaml @@ -117,8 +117,10 @@ spec: - ScaledObject name: type: string - primaryScalerQuery: - type: string + primaryScalerQueries: + type: object + additionalProperties: + type: string ingressRef: description: Ingress selector type: object diff --git a/pkg/apis/flagger/v1beta1/canary.go b/pkg/apis/flagger/v1beta1/canary.go index 1ac07001..693beb74 100644 --- a/pkg/apis/flagger/v1beta1/canary.go +++ b/pkg/apis/flagger/v1beta1/canary.go @@ -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. diff --git a/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go b/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go index 0a5f67e4..cd960dfe 100644 --- a/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go @@ -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 diff --git a/pkg/canary/scaled_object_reconciler.go b/pkg/canary/scaled_object_reconciler.go index c1ac5dfb..cb42ba9a 100644 --- a/pkg/canary/scaled_object_reconciler.go +++ b/pkg/canary/scaled_object_reconciler.go @@ -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 + } } } } diff --git a/pkg/canary/scaled_object_reconciler_test.go b/pkg/canary/scaled_object_reconciler_test.go index ab1a7cf7..de71190c 100644 --- a/pkg/canary/scaled_object_reconciler_test.go +++ b/pkg/canary/scaled_object_reconciler_test.go @@ -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"]) + } + } }