Fixed issue where query with no metric template returned an error

Signed-off-by: LiZhenCheng9527 <[email protected]>
fluxcd · Mar 12, 2024 · 00af576 · 00af576
1 parent f445594
commit 00af576
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 3 deletions.
diff --git 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
@@ -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))
+	})
 }