Add E2E test for fleet_bundle_state

charts/fleet/templates/deployment.yaml

@@ -78,11 +78,10 @@ spec:
         - --shard-id
         - {{ quote . }}
         {{- end }}
-        {{- if $.Values.debug }}
-        {{- if not .Values.metrics.enabled }}
+        {{- if not $.Values.metrics.enabled }}
         - --disable-metrics
         {{- end }}
-        {{- if .Values.debug }}
+        {{- if $.Values.debug }}
         - --debug
         - --debug-level
         - {{ quote $.Values.debugLevel }}

e2e/metrics/bundle_test.go

@@ -2,6 +2,7 @@ package metrics_test
 
 import (
 	"fmt"
+	"maps"
 	"math/rand"
 	"time"
 
@@ -11,12 +12,13 @@ import (
 	"github.com/rancher/fleet/e2e/metrics"
 	"github.com/rancher/fleet/e2e/testenv"
 	"github.com/rancher/fleet/e2e/testenv/kubectl"
+	fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
 )
 
 var _ = Describe("Bundle Metrics", Label("bundle"), func() {
 	const (
-		objName = "metrics"
-		branch  = "master"
+		gitRepoName = "metrics"
+		branch      = "master"
 	)
 
 	var (
@@ -28,7 +30,7 @@ var _ = Describe("Bundle Metrics", Label("bundle"), func() {
 	BeforeEach(func() {
 		k = env.Kubectl.Namespace(env.Namespace)
 		namespace = testenv.NewNamespaceName(
-			objName,
+			gitRepoName,
 			rand.New(rand.NewSource(time.Now().UnixNano())),
 		)
 		kw = k.Namespace(namespace)
@@ -39,7 +41,7 @@ var _ = Describe("Bundle Metrics", Label("bundle"), func() {
 		err = testenv.CreateGitRepo(
 			kw,
 			namespace,
-			objName,
+			gitRepoName,
 			branch,
 			"simple-manifest",
 		)
@@ -51,105 +53,128 @@ var _ = Describe("Bundle Metrics", Label("bundle"), func() {
 		})
 	})
 
-	When("testing Bundle metrics", func() {
-		bundleMetricNames := []string{
-			"fleet_bundle_desired_ready",
-			"fleet_bundle_err_applied",
-			"fleet_bundle_modified",
-			"fleet_bundle_not_ready",
-			"fleet_bundle_out_of_sync",
-			"fleet_bundle_pending",
-			"fleet_bundle_ready",
-			"fleet_bundle_wait_applied",
+	When("collecting Bundle metrics", func() {
+		bundleMetricNames := map[string]map[string][]string{
+			"fleet_bundle_desired_ready": {},
+			"fleet_bundle_err_applied":   {},
+			"fleet_bundle_modified":      {},
+			"fleet_bundle_not_ready":     {},
+			"fleet_bundle_out_of_sync":   {},
+			"fleet_bundle_pending":       {},
+			"fleet_bundle_ready":         {},
+			"fleet_bundle_wait_applied":  {},
+			"fleet_bundle_state": {
+				"state": []string{
+					string(fleet.Ready),
+					string(fleet.NotReady),
+					string(fleet.WaitApplied),
+					string(fleet.ErrApplied),
+					string(fleet.OutOfSync),
+					string(fleet.Pending),
+					string(fleet.Modified),
+				},
+			},
 		}
 
-		It("should have exactly one metric for the bundle", func() {
-			et := metrics.NewExporterTest(metricsURL)
-			Eventually(func() error {
-				for _, metricName := range bundleMetricNames {
-					metric, err := et.FindOneMetric(
-						metricName,
-						map[string]string{
-							"name":      objName + "-simple-manifest",
-							"namespace": namespace,
-						},
-					)
-					if err != nil {
+		// checkMetrics checks that the metrics exist or not exist. Custom
+		// checks can be added by passing a function to check. This can be used
+		// to check for the value of the metrics.
+		checkMetrics := func(
+			gitRepoName string,
+			expectExists bool,
+			check func(metric *metrics.Metric) error,
+		) func() error {
+			return func() error {
+				et := metrics.NewExporterTest(metricsURL)
+				expectOne := func(metricName string, labels map[string]string) error {
+					metric, err := et.FindOneMetric(metricName, labels)
+					if expectExists && err != nil {
 						return err
+					} else if !expectExists && err == nil {
+						return fmt.Errorf("metric %s found but not expected", metricName)
+					}
+
+					if check != nil {
+						err = check(metric)
+						if err != nil {
+							return err
+						}
+					}
+					return nil
+				}
+
+				for metricName, matchLabels := range bundleMetricNames {
+					identityLabels := map[string]string{
+						"name":      gitRepoName,
+						"namespace": namespace,
+					}
+					labels := map[string]string{}
+					maps.Copy(labels, identityLabels)
+
+					if len(matchLabels) > 0 {
+						for labelName, labelValues := range matchLabels {
+							for _, labelValue := range labelValues {
+								labels[labelName] = labelValue
+								err := expectOne(metricName, labels)
+								if err != nil {
+									return err
+								}
+							}
+						}
+					} else {
+						err := expectOne(metricName, labels)
+						if err != nil {
+							return err
+						}
 					}
-					Expect(metric.Gauge.GetValue()).To(Equal(float64(0)))
 				}
 				return nil
-			}).ShouldNot(HaveOccurred())
+			}
+		}
+
+		It("should have one metric for each specified metric and label value", func() {
+			Eventually(checkMetrics(gitRepoName+"-simple-manifest", true, func(metric *metrics.Metric) error {
+				// No cluster exists in the namespace where our GitRepo has been deployed, hence
+				// we expect the values of the metrics to be 0.
+				if value := metric.Gauge.GetValue(); value != float64(0) {
+					return fmt.Errorf("unexpected metric value: expected 0, found %f", value)
+				}
+				return nil
+			})).ShouldNot(HaveOccurred())
 		})
 
-		Context("when the GitRepo (and therefore Bundle) is changed", Label("bundle-altered"), func() {
-			It("it should not duplicate metrics if Bundle is updated", Label("bundle-update"), func() {
-				et := metrics.NewExporterTest(metricsURL)
+		When("the GitRepo (and therefore Bundle) is changed", Label("bundle-modified"), func() {
+			It("should not duplicate metrics if Bundle is updated", Label("bundle-update"), func() {
+				// et := metrics.NewExporterTest(metricsURL)
 				out, err := kw.Patch(
-					"gitrepo", objName,
+					"gitrepo", gitRepoName,
 					"--type=json",
 					"-p", `[{"op": "replace", "path": "/spec/paths", "value": ["simple-chart"]}]`,
 				)
 				Expect(err).ToNot(HaveOccurred(), out)
 				Expect(out).To(ContainSubstring("gitrepo.fleet.cattle.io/metrics patched"))
 
-				// Wait for it to be changed and fetched.
-				Eventually(func() (string, error) {
-					return kw.Get("gitrepo", objName, "-o", "jsonpath={.status.commit}")
-				}).ShouldNot(BeEmpty())
-
-				var metric *metrics.Metric
-				// Expect still no metrics to be duplicated.
-				Eventually(func() error {
-					for _, metricName := range bundleMetricNames {
-						metric, err = et.FindOneMetric(
-							metricName,
-							map[string]string{
-								"name":      objName + "-simple-chart",
-								"namespace": namespace,
-							},
-						)
-						if err != nil {
-							return err
-						}
-						if metric.LabelValue("paths") == "simple-manifest" {
-							return fmt.Errorf("path for metric %s unchanged", metricName)
-						}
+				Eventually(checkMetrics(gitRepoName+"-simple-chart", true, func(metric *metrics.Metric) error {
+					if metric.LabelValue("paths") == "simple-manifest" {
+						return fmt.Errorf("path for metric %s unchanged", metric.Metric.String())
 					}
 					return nil
-				}).ShouldNot(HaveOccurred())
+				})).ShouldNot(HaveOccurred())
 			})
 
 			It("should not keep metrics if Bundle is deleted", Label("bundle-delete"), func() {
-				et := metrics.NewExporterTest(metricsURL)
-
-				objName := objName + "-simple-manifest"
+				gitRepoName := gitRepoName + "-simple-manifest"
 
 				var (
 					out string
 					err error
 				)
 				Eventually(func() error {
-					out, err = kw.Delete("bundle", objName)
+					out, err = kw.Delete("bundle", gitRepoName)
 					return err
 				}).ShouldNot(HaveOccurred(), out)
 
-				Eventually(func() error {
-					for _, metricName := range bundleMetricNames {
-						_, err := et.FindOneMetric(
-							metricName,
-							map[string]string{
-								"name":      objName,
-								"namespace": namespace,
-							},
-						)
-						if err == nil {
-							return fmt.Errorf("metric %s found but not expected", metricName)
-						}
-					}
-					return nil
-				}).ShouldNot(HaveOccurred())
+				Eventually(checkMetrics(gitRepoName, false, nil)).ShouldNot(HaveOccurred())
 			})
 		})
 	})

e2e/metrics/cluster_test.go

@@ -62,51 +62,47 @@ var _ = Describe("Cluster Metrics", Label("cluster"), func() {
 		"fleet_cluster_state": false,
 	}
 
-	It(
-		"should have as many clusters in metrics as there are objects in the cluster",
-		func() {
-			Eventually(func() error {
-				var (
-					clustersOut string
-					err         error
-				)
-				clustersOut, err = env.Kubectl.Get(
-					"-A", "clusters.fleet.cattle.io",
-					"-o", "json",
-				)
-				Expect(err).ToNot(HaveOccurred())
-
-				var existingClusters clusters
-				err = json.Unmarshal([]byte(clustersOut), &existingClusters)
-				Expect(err).ToNot(HaveOccurred())
-
-				et := metrics.NewExporterTest(metricsURL)
-
-				for _, cluster := range existingClusters {
-					for metricName, expectedExist := range expectedMetricsExist {
-						_, err := et.FindOneMetric(
+	It("should have metrics for all existing cluster resources", func() {
+		Eventually(func() error {
+			var (
+				clustersOut string
+				err         error
+			)
+			clustersOut, err = env.Kubectl.Get(
+				"-A", "clusters.fleet.cattle.io",
+				"-o", "json",
+			)
+			Expect(err).ToNot(HaveOccurred())
+
+			var existingClusters clusters
+			err = json.Unmarshal([]byte(clustersOut), &existingClusters)
+			Expect(err).ToNot(HaveOccurred())
+
+			et := metrics.NewExporterTest(metricsURL)
+
+			Expect(len(existingClusters)).ToNot(BeZero())
+
+			for _, cluster := range existingClusters {
+				for metricName, expectedExist := range expectedMetricsExist {
+					_, err := et.FindOneMetric(
+						metricName,
+						map[string]string{
+							"name":      cluster.Name,
+							"namespace": cluster.Namespace,
+						},
+					)
+					if expectedExist && err != nil {
+						return err
+					} else if !expectedExist && err == nil {
+						return fmt.Errorf(
+							"expected metric %s not to exist, but it exists",
 							metricName,
-							map[string]string{
-								"name":      cluster.Name,
-								"namespace": cluster.Namespace,
-							},
 						)
-						if expectedExist {
-							if err != nil {
-								return err
-							}
-						} else {
-							if err == nil {
-								return fmt.Errorf(
-									"expected metric %s not to exist, but it exists",
-									metricName,
-								)
-							}
-						}
 					}
 				}
-				return nil
-			}).ShouldNot(HaveOccurred())
-		},
+			}
+			return nil
+		}).ShouldNot(HaveOccurred())
+	},
 	)
 })