From 6786668684e96109cac6cffb5fd8615498cbe2b7 Mon Sep 17 00:00:00 2001 From: Nelson Johnstone Date: Tue, 31 Jan 2023 16:10:00 +1000 Subject: [PATCH] updated canary CRD and query rendering Signed-off-by: Nelson Johnstone <93178586+njohnstone2@users.noreply.github.com> --- artifacts/flagger/crd.yaml | 5 ++ charts/flagger/crds/crd.yaml | 5 ++ kustomize/base/flagger/crd.yaml | 5 ++ pkg/apis/flagger/v1beta1/canary.go | 4 +- .../flagger/v1beta1/zz_generated.deepcopy.go | 14 ++++ pkg/controller/scheduler_metrics.go | 2 + pkg/metrics/observers/render.go | 2 +- pkg/metrics/observers/render_test.go | 82 +++++++++++++++++++ 8 files changed, 116 insertions(+), 3 deletions(-) create mode 100644 pkg/metrics/observers/render_test.go diff --git a/artifacts/flagger/crd.yaml b/artifacts/flagger/crd.yaml index 02481ccb6..bf8519d4f 100644 --- a/artifacts/flagger/crd.yaml +++ b/artifacts/flagger/crd.yaml @@ -970,6 +970,11 @@ spec: namespace: description: Namespace of this metric template type: string + templateVariables: + description: Additional variables to be used in the metrics query (key-value pairs) + type: object + additionalProperties: + type: string alerts: description: Alert list for this canary analysis type: array diff --git a/charts/flagger/crds/crd.yaml b/charts/flagger/crds/crd.yaml index 02481ccb6..bf8519d4f 100644 --- a/charts/flagger/crds/crd.yaml +++ b/charts/flagger/crds/crd.yaml @@ -970,6 +970,11 @@ spec: namespace: description: Namespace of this metric template type: string + templateVariables: + description: Additional variables to be used in the metrics query (key-value pairs) + type: object + additionalProperties: + type: string alerts: description: Alert list for this canary analysis type: array diff --git a/kustomize/base/flagger/crd.yaml b/kustomize/base/flagger/crd.yaml index 02481ccb6..bf8519d4f 100644 --- a/kustomize/base/flagger/crd.yaml +++ b/kustomize/base/flagger/crd.yaml @@ -970,6 +970,11 @@ spec: namespace: description: Namespace of this metric template type: string + templateVariables: + description: Additional variables to be used in the metrics query (key-value pairs) + type: object + additionalProperties: + type: string alerts: description: Alert list for this canary analysis type: array diff --git a/pkg/apis/flagger/v1beta1/canary.go b/pkg/apis/flagger/v1beta1/canary.go index 8ed98b7cf..304284532 100644 --- a/pkg/apis/flagger/v1beta1/canary.go +++ b/pkg/apis/flagger/v1beta1/canary.go @@ -305,9 +305,9 @@ type CanaryMetric struct { // +optional TemplateRef *CrossNamespaceObjectReference `json:"templateRef,omitempty"` - // TemplateVariables provides a map of key/value pairs to be used on a metric template object + // TemplateVariables provides a map of key/value pairs that can be used to inject variables into a metric query. // +optional - TemplateVariables map[string]string `json:"variables,omitempty"` + TemplateVariables map[string]string `json:"templateVariables,omitempty"` } // CanaryThresholdRange defines the range used for metrics validation diff --git a/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go b/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go index 80bdd286c..123da753e 100644 --- a/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/flagger/v1beta1/zz_generated.deepcopy.go @@ -350,6 +350,13 @@ func (in *CanaryMetric) DeepCopyInto(out *CanaryMetric) { *out = new(CrossNamespaceObjectReference) **out = **in } + if in.TemplateVariables != nil { + in, out := &in.TemplateVariables, &out.TemplateVariables + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } return } @@ -757,6 +764,13 @@ func (in *MetricTemplateList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MetricTemplateModel) DeepCopyInto(out *MetricTemplateModel) { *out = *in + if in.Variables != nil { + in, out := &in.Variables, &out.Variables + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } return } diff --git a/pkg/controller/scheduler_metrics.go b/pkg/controller/scheduler_metrics.go index a94bf0154..960db8119 100644 --- a/pkg/controller/scheduler_metrics.go +++ b/pkg/controller/scheduler_metrics.go @@ -268,6 +268,8 @@ func (c *Controller) runMetricChecks(canary *flaggerv1.Canary) bool { } query, err := observers.RenderQuery(template.Spec.Query, toMetricModel(canary, metric.Interval, metric.TemplateVariables)) + c.logger.With("canary", fmt.Sprintf("%s.%s", canary.Name, namespace)). + Debugf("Metric template %s.%s query: %s", metric.TemplateRef.Name, namespace, query) if err != nil { c.recordEventErrorf(canary, "Metric template %s.%s query render error: %v", metric.TemplateRef.Name, namespace, err) diff --git a/pkg/metrics/observers/render.go b/pkg/metrics/observers/render.go index 7a8ff0621..df8d8542d 100644 --- a/pkg/metrics/observers/render.go +++ b/pkg/metrics/observers/render.go @@ -26,7 +26,7 @@ import ( ) func RenderQuery(queryTemplate string, model flaggerv1.MetricTemplateModel) (string, error) { - t, err := template.New("tmpl").Funcs(model.TemplateFunctions()).Parse(queryTemplate) + t, err := template.New("tmpl").Option("missingkey=error").Funcs(model.TemplateFunctions()).Parse(queryTemplate) if err != nil { return "", fmt.Errorf("template parsing failed: %w", err) } diff --git a/pkg/metrics/observers/render_test.go b/pkg/metrics/observers/render_test.go new file mode 100644 index 000000000..ed6ac8559 --- /dev/null +++ b/pkg/metrics/observers/render_test.go @@ -0,0 +1,82 @@ +/* +Copyright 2020 The Flux authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package observers + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + flaggerv1 "github.com/fluxcd/flagger/pkg/apis/flagger/v1beta1" +) + +func Test_RenderQuery(t *testing.T) { + t.Run("ok_without_variables", func(t *testing.T) { + expected := `sum(envoy_cluster_upstream_rq{envoy_cluster_name=~"default_myapp"})` + templateQuery := `sum(envoy_cluster_upstream_rq{envoy_cluster_name=~"{{ namespace }}_{{ target }}"})` + + model := &flaggerv1.MetricTemplateModel{ + Name: "standard", + Namespace: "default", + Target: "myapp", + Interval: "1m", + } + + actual, err := RenderQuery(templateQuery, *model) + require.NoError(t, err) + + assert.Equal(t, expected, actual) + }) + + t.Run("ok_with_variables", func(t *testing.T) { + expected := `delta(max by (consumer_group) (kafka_consumer_current_offset{cluster="dev", consumer_group="my_consumer"}[1m]))` + templateQuery := `delta(max by (consumer_group) (kafka_consumer_current_offset{cluster="{{ variables.cluster }}", consumer_group="{{ variables.consumer_group }}"}[{{ interval }}]))` + + model := &flaggerv1.MetricTemplateModel{ + Name: "kafka_consumer_offset", + Namespace: "default", + Interval: "1m", + Variables: map[string]string{ + "cluster": "dev", + "consumer_group": "my_consumer", + }, + } + + actual, err := RenderQuery(templateQuery, *model) + require.NoError(t, err) + + assert.Equal(t, expected, actual) + }) + + t.Run("missing_variable_key", func(t *testing.T) { + templateQuery := `delta(max by (consumer_group) (kafka_consumer_current_offset{cluster="{{ variables.cluster }}", consumer_group="{{ variables.consumer_group }}"}[{{ interval }}]))` + + model := &flaggerv1.MetricTemplateModel{ + Name: "kafka_consumer_offset", + Namespace: "default", + Interval: "1m", + Variables: map[string]string{ + "invalid": "dev", + "consumer_group": "my_consumer", + }, + } + + _, err := RenderQuery(templateQuery, *model) + require.Error(t, err) + }) +}