From b778013e0796fd6bbc1ae7626fcf139042234496 Mon Sep 17 00:00:00 2001 From: LiZhenCheng9527 Date: Thu, 7 Mar 2024 16:06:23 +0800 Subject: [PATCH] Fixed issue where query with no metric template returned an error Signed-off-by: LiZhenCheng9527 --- pkg/controller/scheduler_metrics.go | 4 +-- pkg/controller/scheduler_metrics_test.go | 45 +++++++++++++++++++++++- 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/pkg/controller/scheduler_metrics.go b/pkg/controller/scheduler_metrics.go index 6d2d3c825..9bcd66d9e 100644 --- a/pkg/controller/scheduler_metrics.go +++ b/pkg/controller/scheduler_metrics.go @@ -306,8 +306,8 @@ func (c *Controller) runMetricChecks(canary *flaggerv1.Canary) bool { canary.Name, canary.Namespace, metric.Name, val, metric.Threshold) return false } - } else if metric.Name != "request-success-rate" && metric.Name != "request-duration" { - c.recordEventErrorf(canary, "Metric query failed for no usable metrics template were configured") + } else if metric.Name != "request-success-rate" && metric.Name != "request-duration" && metric.Query == "" { + c.recordEventErrorf(canary, "Metric query failed for no usable metrics template and query were configured") return false } } diff --git a/pkg/controller/scheduler_metrics_test.go b/pkg/controller/scheduler_metrics_test.go index 13ebcb596..92519e68c 100644 --- a/pkg/controller/scheduler_metrics_test.go +++ b/pkg/controller/scheduler_metrics_test.go @@ -88,7 +88,8 @@ func TestController_runMetricChecks(t *testing.T) { t.Run("customVariables", func(t *testing.T) { ctrl := newDeploymentFixture(nil).ctrl analysis := &flaggerv1.CanaryAnalysis{Metrics: []flaggerv1.CanaryMetric{{ - Name: "", TemplateVariables: map[string]string{ + Name: "", + TemplateVariables: map[string]string{ "first": "abc", "second": "def", }, @@ -139,4 +140,46 @@ func TestController_runMetricChecks(t *testing.T) { } assert.Equal(t, true, ctrl.runMetricChecks(canary)) }) + + t.Run("no metric Template is defined, but a query is specified", func(t *testing.T) { + ctrl := newDeploymentFixture(nil).ctrl + analysis := &flaggerv1.CanaryAnalysis{Metrics: []flaggerv1.CanaryMetric{{ + Name: "undefined metric", + ThresholdRange: &flaggerv1.CanaryThresholdRange{ + Min: toFloatPtr(0), + Max: toFloatPtr(100), + }, + Query: ">- sum(logback_events_total{level=\"error\", job=\"some-app\"}) <= bool 0", + }}} + canary := &flaggerv1.Canary{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default"}, + Spec: flaggerv1.CanarySpec{Analysis: analysis}, + } + assert.Equal(t, true, ctrl.runMetricChecks(canary)) + }) + + t.Run("both have metric Template and query", func(t *testing.T) { + ctrl := newDeploymentFixture(nil).ctrl + analysis := &flaggerv1.CanaryAnalysis{Metrics: []flaggerv1.CanaryMetric{{ + Name: "", + TemplateVariables: map[string]string{ + "first": "abc", + "second": "def", + }, + TemplateRef: &flaggerv1.CrossNamespaceObjectReference{ + Name: "custom-vars", + Namespace: "default", + }, + ThresholdRange: &flaggerv1.CanaryThresholdRange{ + Min: toFloatPtr(0), + Max: toFloatPtr(100), + }, + Query: ">- sum(logback_events_total{level=\"error\", job=\"some-app\"}) <= bool 0", + }}} + canary := &flaggerv1.Canary{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default"}, + Spec: flaggerv1.CanarySpec{Analysis: analysis}, + } + assert.Equal(t, true, ctrl.runMetricChecks(canary)) + }) }