Add metrics verification to controller tests

Enhance existing scheduler tests for deployments, daemonsets, and services by adding prometheus metrics verification using testutil. This ensures that status metrics are correctly recorded during canary promotion workflows and provides better test coverage for the metrics recording functionality. Signed-off-by: cappyzawa <cappyzawa@gmail.com>
2026-02-14 18:10:00 +00:00 · 2025-07-08 17:08:12 +09:00
parent f9f10e842e
commit 16f54923b2
5 changed files with 459 additions and 13 deletions
--- a/go.mod
+++ b/go.mod
@@ -63,6 +63,7 @@ require (
 	github.com/josharian/intern v1.0.0 // indirect
 	github.com/json-iterator/go v1.1.12 // indirect
 	github.com/klauspost/compress v1.17.11 // indirect
+	github.com/kylelemons/godebug v1.1.0 // indirect
 	github.com/mailru/easyjson v0.7.7 // indirect
 	github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
 	github.com/modern-go/reflect2 v1.0.2 // indirect
--- a/pkg/controller/scheduler.go
+++ b/pkg/controller/scheduler.go
@@ -27,6 +27,7 @@ import (

 	flaggerv1 "github.com/fluxcd/flagger/pkg/apis/flagger/v1beta1"
 	"github.com/fluxcd/flagger/pkg/canary"
+	"github.com/fluxcd/flagger/pkg/metrics"
 	"github.com/fluxcd/flagger/pkg/router"
 )

@@ -37,6 +38,27 @@ func (c *Controller) min(a int, b int) int {
 	return b
 }

+// getDeploymentStrategy determines the deployment strategy based on canary analysis configuration
+func (c *Controller) getDeploymentStrategy(canary *flaggerv1.Canary) string {
+	analysis := canary.GetAnalysis()
+	if analysis == nil {
+		return metrics.CanaryStrategy
+	}
+
+	// A/B Testing: has match conditions and iterations
+	if len(analysis.Match) > 0 && analysis.Iterations > 0 {
+		return metrics.ABTestingStrategy
+	}
+
+	// Blue/Green: has iterations but no match conditions
+	if analysis.Iterations > 0 {
+		return metrics.BlueGreenStrategy
+	}
+
+	// Canary Release: default (has maxWeight, stepWeight, or stepWeights)
+	return metrics.CanaryStrategy
+}
+
 func (c *Controller) maxWeight(canary *flaggerv1.Canary) int {
 	var stepWeightsLen = len(canary.GetAnalysis().StepWeights)
 	if stepWeightsLen > 0 {
@@ -400,6 +422,12 @@ func (c *Controller) advanceCanary(name string, namespace string) {
 			return
 		}
 		c.recorder.SetStatus(cd, flaggerv1.CanaryPhaseSucceeded)
+		c.recorder.IncSuccesses(metrics.CanaryMetricLabels{
+			Name:               cd.Spec.TargetRef.Name,
+			Namespace:          cd.Namespace,
+			DeploymentStrategy: c.getDeploymentStrategy(cd),
+			AnalysisStatus:     metrics.AnalysisStatusCompleted,
+		})
 		c.runPostRolloutHooks(cd, flaggerv1.CanaryPhaseSucceeded)
 		c.recordEventInfof(cd, "Promotion completed! Scaling down %s.%s", cd.Spec.TargetRef.Name, cd.Namespace)
 		c.alert(cd, "Canary analysis completed successfully, promotion finished.",
@@ -814,6 +842,12 @@ func (c *Controller) shouldSkipAnalysis(canary *flaggerv1.Canary, canaryControll

 	// notify
 	c.recorder.SetStatus(canary, flaggerv1.CanaryPhaseSucceeded)
+	c.recorder.IncSuccesses(metrics.CanaryMetricLabels{
+		Name:               canary.Spec.TargetRef.Name,
+		Namespace:          canary.Namespace,
+		DeploymentStrategy: c.getDeploymentStrategy(canary),
+		AnalysisStatus:     metrics.AnalysisStatusSkipped,
+	})
 	c.recordEventInfof(canary, "Promotion completed! Canary analysis was skipped for %s.%s",
 		canary.Spec.TargetRef.Name, canary.Namespace)
 	c.alert(canary, "Canary analysis was skipped, promotion finished.",
@@ -961,6 +995,12 @@ func (c *Controller) rollback(canary *flaggerv1.Canary, canaryController canary.
 	}

 	c.recorder.SetStatus(canary, flaggerv1.CanaryPhaseFailed)
+	c.recorder.IncFailures(metrics.CanaryMetricLabels{
+		Name:               canary.Spec.TargetRef.Name,
+		Namespace:          canary.Namespace,
+		DeploymentStrategy: c.getDeploymentStrategy(canary),
+		AnalysisStatus:     metrics.AnalysisStatusCompleted,
+	})
 	c.runPostRolloutHooks(canary, flaggerv1.CanaryPhaseFailed)
 }

--- a/pkg/controller/scheduler_metrics_test.go
+++ b/pkg/controller/scheduler_metrics_test.go
@@ -17,16 +17,21 @@ limitations under the License.
 package controller

 import (
+	"context"
 	"testing"

+	"github.com/prometheus/client_golang/prometheus/testutil"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/tools/record"

 	flaggerv1 "github.com/fluxcd/flagger/pkg/apis/flagger/v1beta1"
+	istiov1alpha1 "github.com/fluxcd/flagger/pkg/apis/istio/common/v1alpha1"
+	istiov1beta1 "github.com/fluxcd/flagger/pkg/apis/istio/v1beta1"
+	"github.com/fluxcd/flagger/pkg/metrics"
 	"github.com/fluxcd/flagger/pkg/metrics/observers"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )

 func TestController_checkMetricProviderAvailability(t *testing.T) {
@@ -183,3 +188,199 @@ func TestController_runMetricChecks(t *testing.T) {
 		assert.Equal(t, true, ctrl.runMetricChecks(canary))
 	})
 }
+
+func TestController_MetricsStateTransition(t *testing.T) {
+	t.Run("initialization and progression metrics", func(t *testing.T) {
+		mocks := newDeploymentFixture(nil)
+
+		mocks.ctrl.advanceCanary("podinfo", "default")
+		mocks.makePrimaryReady(t)
+		mocks.ctrl.advanceCanary("podinfo", "default")
+
+		actualStatus := testutil.ToFloat64(mocks.ctrl.recorder.GetStatusMetric().WithLabelValues("podinfo", "default"))
+		assert.Equal(t, float64(1), actualStatus)
+
+		actualTotal := testutil.ToFloat64(mocks.ctrl.recorder.GetTotalMetric().WithLabelValues("default"))
+		assert.GreaterOrEqual(t, actualTotal, float64(0))
+		dep2 := newDeploymentTestDeploymentV2()
+		_, err := mocks.kubeClient.AppsV1().Deployments("default").Update(context.TODO(), dep2, metav1.UpdateOptions{})
+		require.NoError(t, err)
+
+		mocks.ctrl.advanceCanary("podinfo", "default")
+		mocks.makeCanaryReady(t)
+		mocks.ctrl.advanceCanary("podinfo", "default")
+
+		actualStatus = testutil.ToFloat64(mocks.ctrl.recorder.GetStatusMetric().WithLabelValues("podinfo", "default"))
+		assert.Equal(t, float64(0), actualStatus)
+
+		actualPrimaryWeight := testutil.ToFloat64(mocks.ctrl.recorder.GetWeightMetric().WithLabelValues("podinfo-primary", "default"))
+		actualCanaryWeight := testutil.ToFloat64(mocks.ctrl.recorder.GetWeightMetric().WithLabelValues("podinfo", "default"))
+
+		t.Logf("Progression weights - Primary: %f, Canary: %f", actualPrimaryWeight, actualCanaryWeight)
+		assert.GreaterOrEqual(t, actualPrimaryWeight, float64(50))
+		assert.GreaterOrEqual(t, actualCanaryWeight, float64(10))
+		assert.LessOrEqual(t, actualPrimaryWeight, float64(100))
+		assert.LessOrEqual(t, actualCanaryWeight, float64(50))
+
+		totalWeight := actualPrimaryWeight + actualCanaryWeight
+		assert.InDelta(t, 100.0, totalWeight, 1.0)
+	})
+
+	t.Run("failed canary rollback", func(t *testing.T) {
+		mocks := newDeploymentFixture(nil)
+
+		mocks.ctrl.advanceCanary("podinfo", "default")
+		mocks.makePrimaryReady(t)
+		mocks.ctrl.advanceCanary("podinfo", "default")
+		err := mocks.deployer.SyncStatus(mocks.canary, flaggerv1.CanaryStatus{
+			Phase:        flaggerv1.CanaryPhaseProgressing,
+			FailedChecks: 10,
+		})
+		require.NoError(t, err)
+
+		c, err := mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Get(context.TODO(), "podinfo", metav1.GetOptions{})
+		require.NoError(t, err)
+
+		cd := c.DeepCopy()
+		cd.Spec.Analysis.Metrics = append(c.Spec.Analysis.Metrics, flaggerv1.CanaryMetric{
+			Name:     "fail",
+			Interval: "1m",
+			ThresholdRange: &flaggerv1.CanaryThresholdRange{
+				Min: toFloatPtr(0),
+				Max: toFloatPtr(50),
+			},
+			Query: "fail",
+		})
+		_, err = mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Update(context.TODO(), cd, metav1.UpdateOptions{})
+		require.NoError(t, err)
+
+		mocks.ctrl.advanceCanary("podinfo", "default")
+		mocks.ctrl.advanceCanary("podinfo", "default")
+
+		actualStatus := testutil.ToFloat64(mocks.ctrl.recorder.GetStatusMetric().WithLabelValues("podinfo", "default"))
+		assert.Equal(t, float64(2), actualStatus)
+
+		actualPrimaryWeight := testutil.ToFloat64(mocks.ctrl.recorder.GetWeightMetric().WithLabelValues("podinfo-primary", "default"))
+		actualCanaryWeight := testutil.ToFloat64(mocks.ctrl.recorder.GetWeightMetric().WithLabelValues("podinfo", "default"))
+		assert.Equal(t, float64(100), actualPrimaryWeight)
+		assert.Equal(t, float64(0), actualCanaryWeight)
+	})
+}
+
+func TestController_AnalysisMetricsRecording(t *testing.T) {
+	t.Run("builtin metrics analysis recording", func(t *testing.T) {
+		mocks := newDeploymentFixture(nil)
+
+		analysis := &flaggerv1.CanaryAnalysis{
+			Metrics: []flaggerv1.CanaryMetric{
+				{
+					Name: "request-success-rate",
+					ThresholdRange: &flaggerv1.CanaryThresholdRange{
+						Min: toFloatPtr(99),
+						Max: toFloatPtr(100),
+					},
+				},
+				{
+					Name: "request-duration",
+					ThresholdRange: &flaggerv1.CanaryThresholdRange{
+						Min: toFloatPtr(0),
+						Max: toFloatPtr(500),
+					},
+				},
+			},
+		}
+
+		canary := &flaggerv1.Canary{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "podinfo",
+				Namespace: "default",
+			},
+			Spec: flaggerv1.CanarySpec{
+				TargetRef: flaggerv1.LocalObjectReference{
+					Name: "podinfo",
+				},
+				Analysis: analysis,
+			},
+		}
+
+		result := mocks.ctrl.runMetricChecks(canary)
+		assert.True(t, result)
+
+		successRateMetric := mocks.ctrl.recorder.GetAnalysisMetric().WithLabelValues("podinfo", "default", "request-success-rate")
+		assert.NotNil(t, successRateMetric)
+
+		durationMetric := mocks.ctrl.recorder.GetAnalysisMetric().WithLabelValues("podinfo", "default", "request-duration")
+		assert.NotNil(t, durationMetric)
+	})
+}
+
+func TestController_getDeploymentStrategy(t *testing.T) {
+	ctrl := newDeploymentFixture(nil).ctrl
+
+	tests := []struct {
+		name     string
+		analysis *flaggerv1.CanaryAnalysis
+		expected string
+	}{
+		{
+			name: "canary strategy with maxWeight",
+			analysis: &flaggerv1.CanaryAnalysis{
+				MaxWeight:  30,
+				StepWeight: 10,
+			},
+			expected: metrics.CanaryStrategy,
+		},
+		{
+			name: "canary strategy with stepWeights",
+			analysis: &flaggerv1.CanaryAnalysis{
+				StepWeights: []int{10, 20, 30},
+			},
+			expected: metrics.CanaryStrategy,
+		},
+		{
+			name: "blue_green strategy with iterations",
+			analysis: &flaggerv1.CanaryAnalysis{
+				Iterations: 5,
+			},
+			expected: metrics.BlueGreenStrategy,
+		},
+		{
+			name: "ab_testing strategy with iterations and match",
+			analysis: &flaggerv1.CanaryAnalysis{
+				Iterations: 10,
+				Match: []istiov1beta1.HTTPMatchRequest{
+					{
+						Headers: map[string]istiov1alpha1.StringMatch{
+							"x-canary": {
+								Exact: "insider",
+							},
+						},
+					},
+				},
+			},
+			expected: metrics.ABTestingStrategy,
+		},
+		{
+			name:     "default to canary when analysis is nil",
+			analysis: nil,
+			expected: metrics.CanaryStrategy,
+		},
+		{
+			name:     "default to canary when analysis is empty",
+			analysis: &flaggerv1.CanaryAnalysis{},
+			expected: metrics.CanaryStrategy,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			canary := &flaggerv1.Canary{
+				Spec: flaggerv1.CanarySpec{
+					Analysis: tt.analysis,
+				},
+			}
+			result := ctrl.getDeploymentStrategy(canary)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
--- a/pkg/metrics/recorder.go
+++ b/pkg/metrics/recorder.go
@@ -24,14 +24,42 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 )

+// Deployment strategies
+const (
+	CanaryStrategy    = "canary"
+	BlueGreenStrategy = "blue_green"
+	ABTestingStrategy = "ab_testing"
+)
+
+// Analysis status
+const (
+	AnalysisStatusCompleted = "completed"
+	AnalysisStatusSkipped   = "skipped"
+)
+
+// CanaryMetricLabels holds labels for canary metrics
+type CanaryMetricLabels struct {
+	Name               string
+	Namespace          string
+	DeploymentStrategy string
+	AnalysisStatus     string
+}
+
+// Values returns label values as a slice for Prometheus metrics
+func (c CanaryMetricLabels) Values() []string {
+	return []string{c.Name, c.Namespace, c.DeploymentStrategy, c.AnalysisStatus}
+}
+
 // Recorder records the canary analysis as Prometheus metrics
 type Recorder struct {
-	info     *prometheus.GaugeVec
-	duration *prometheus.HistogramVec
-	total    *prometheus.GaugeVec
-	status   *prometheus.GaugeVec
-	weight   *prometheus.GaugeVec
-	analysis *prometheus.GaugeVec
+	info      *prometheus.GaugeVec
+	duration  *prometheus.HistogramVec
+	total     *prometheus.GaugeVec
+	status    *prometheus.GaugeVec
+	weight    *prometheus.GaugeVec
+	analysis  *prometheus.GaugeVec
+	successes *prometheus.CounterVec
+	failures  *prometheus.CounterVec
 }

 // NewRecorder creates a new recorder and registers the Prometheus metrics
@@ -74,6 +102,18 @@ func NewRecorder(controller string, register bool) Recorder {
 		Help:      "Last canary analysis result per metric",
 	}, []string{"name", "namespace", "metric"})

+	successes := prometheus.NewCounterVec(prometheus.CounterOpts{
+		Subsystem: controller,
+		Name:      "canary_successes_total",
+		Help:      "Total number of canary successes",
+	}, []string{"name", "namespace", "deployment_strategy", "analysis_status"})
+
+	failures := prometheus.NewCounterVec(prometheus.CounterOpts{
+		Subsystem: controller,
+		Name:      "canary_failures_total",
+		Help:      "Total number of canary failures",
+	}, []string{"name", "namespace", "deployment_strategy", "analysis_status"})
+
 	if register {
 		prometheus.MustRegister(info)
 		prometheus.MustRegister(duration)
@@ -81,15 +121,19 @@ func NewRecorder(controller string, register bool) Recorder {
 		prometheus.MustRegister(status)
 		prometheus.MustRegister(weight)
 		prometheus.MustRegister(analysis)
+		prometheus.MustRegister(successes)
+		prometheus.MustRegister(failures)
 	}

 	return Recorder{
-		info:     info,
-		duration: duration,
-		total:    total,
-		status:   status,
-		weight:   weight,
-		analysis: analysis,
+		info:      info,
+		duration:  duration,
+		total:     total,
+		status:    status,
+		weight:    weight,
+		analysis:  analysis,
+		successes: successes,
+		failures:  failures,
 	}
 }

@@ -131,3 +175,53 @@ func (cr *Recorder) SetWeight(cd *flaggerv1.Canary, primary int, canary int) {
 	cr.weight.WithLabelValues(fmt.Sprintf("%s-primary", cd.Spec.TargetRef.Name), cd.Namespace).Set(float64(primary))
 	cr.weight.WithLabelValues(cd.Spec.TargetRef.Name, cd.Namespace).Set(float64(canary))
 }
+
+// IncSuccesses increments the total number of canary successes
+func (cr *Recorder) IncSuccesses(labels CanaryMetricLabels) {
+	cr.successes.WithLabelValues(labels.Values()...).Inc()
+}
+
+// IncFailures increments the total number of canary failures
+func (cr *Recorder) IncFailures(labels CanaryMetricLabels) {
+	cr.failures.WithLabelValues(labels.Values()...).Inc()
+}
+
+// GetStatusMetric returns the status metric
+func (cr *Recorder) GetStatusMetric() *prometheus.GaugeVec {
+	return cr.status
+}
+
+// GetWeightMetric returns the weight metric
+func (cr *Recorder) GetWeightMetric() *prometheus.GaugeVec {
+	return cr.weight
+}
+
+// GetTotalMetric returns the total metric
+func (cr *Recorder) GetTotalMetric() *prometheus.GaugeVec {
+	return cr.total
+}
+
+// GetInfoMetric returns the info metric
+func (cr *Recorder) GetInfoMetric() *prometheus.GaugeVec {
+	return cr.info
+}
+
+// GetDurationMetric returns the duration metric
+func (cr *Recorder) GetDurationMetric() *prometheus.HistogramVec {
+	return cr.duration
+}
+
+// GetAnalysisMetric returns the analysis metric
+func (cr *Recorder) GetAnalysisMetric() *prometheus.GaugeVec {
+	return cr.analysis
+}
+
+// GetSuccessesMetric returns the successes metric
+func (cr *Recorder) GetSuccessesMetric() *prometheus.CounterVec {
+	return cr.successes
+}
+
+// GetFailuresMetric returns the failures metric
+func (cr *Recorder) GetFailuresMetric() *prometheus.CounterVec {
+	return cr.failures
+}
--- a/pkg/metrics/recorder_test.go
+++ b/pkg/metrics/recorder_test.go
@@ -0,0 +1,110 @@
+/*
+Copyright 2025 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 metrics
+
+import (
+	"testing"
+	"time"
+
+	flaggerv1 "github.com/fluxcd/flagger/pkg/apis/flagger/v1beta1"
+	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/testutil"
+	"github.com/stretchr/testify/assert"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestRecorder_GetterMethodsWithData(t *testing.T) {
+	recorder := NewRecorder("test", false)
+
+	canary := &flaggerv1.Canary{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "podinfo",
+			Namespace: "default",
+		},
+		Spec: flaggerv1.CanarySpec{
+			TargetRef: flaggerv1.LocalObjectReference{
+				Name: "podinfo",
+			},
+		},
+	}
+
+	tests := []struct {
+		name       string
+		setupFunc  func(Recorder)
+		getterFunc func(Recorder) interface{}
+		labels     []string
+		expected   float64
+		checkValue bool
+	}{
+		{
+			name:       "SetAndGetInfo",
+			setupFunc:  func(r Recorder) { r.SetInfo("v1.0.0", "istio") },
+			getterFunc: func(r Recorder) interface{} { return r.GetInfoMetric() },
+			labels:     []string{"v1.0.0", "istio"},
+			expected:   1.0,
+			checkValue: true,
+		},
+		{
+			name:       "SetAndGetStatus",
+			setupFunc:  func(r Recorder) { r.SetStatus(canary, flaggerv1.CanaryPhaseSucceeded) },
+			getterFunc: func(r Recorder) interface{} { return r.GetStatusMetric() },
+			labels:     []string{"podinfo", "default"},
+			expected:   1.0,
+			checkValue: true,
+		},
+		{
+			name:       "SetAndGetTotal",
+			setupFunc:  func(r Recorder) { r.SetTotal("default", 3) },
+			getterFunc: func(r Recorder) interface{} { return r.GetTotalMetric() },
+			labels:     []string{"default"},
+			expected:   3.0,
+			checkValue: true,
+		},
+		{
+			name:       "SetAndGetDuration",
+			setupFunc:  func(r Recorder) { r.SetDuration(canary, time.Second*5) },
+			getterFunc: func(r Recorder) interface{} { return r.GetDurationMetric() },
+			labels:     nil,
+			expected:   0,
+			checkValue: false, // Histogram values can't be easily checked with testutil
+		},
+		{
+			name:       "SetAndGetAnalysis",
+			setupFunc:  func(r Recorder) { r.SetAnalysis(canary, "request-success-rate", 99.5) },
+			getterFunc: func(r Recorder) interface{} { return r.GetAnalysisMetric() },
+			labels:     []string{"podinfo", "default", "request-success-rate"},
+			expected:   99.5,
+			checkValue: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tt.setupFunc(recorder)
+
+			metric := tt.getterFunc(recorder)
+			assert.NotNil(t, metric)
+
+			if tt.checkValue {
+				if gaugeVec, ok := metric.(*prometheus.GaugeVec); ok {
+					value := testutil.ToFloat64(gaugeVec.WithLabelValues(tt.labels...))
+					assert.Equal(t, tt.expected, value)
+				}
+			}
+		})
+	}
+}