fix: collect dump metrics services across namespaces (#5188)

* fix: collect dump metrics services across namespaces

Signed-off-by: immanuwell <pchpr.00@list.ru>

* Fix expectation on metrics file name

Dumping metrics is now done over multiple namespaces, meaning that
target files names now include the namespace.

---------

Signed-off-by: immanuwell <pchpr.00@list.ru>
Co-authored-by: Corentin Néau <tan.neau@suse.com>

Author: immanuwell

e2e/single-cluster/cli_dump_test.go

@@ -87,7 +87,7 @@ var _ = Describe("Fleet dump", Label("sharding"), func() {
 					continue
 				}
 
-				Expect(fileName).To(HaveLen(2))
+				Expect(fileName).To(HaveLen(3), "metrics file name should have format metrics_<namespace>_<servicename>")
 				Expect(content).ToNot(BeEmpty())
 
 				// Run a few basic checks on expected strings, checking full contents would be cumbersome
@@ -97,11 +97,12 @@ var _ = Describe("Fleet dump", Label("sharding"), func() {
 				Expect(c).To(ContainSubstring("controller_runtime_reconcile_total"))
 
 				exampleMonitoredRsc := "bundle"
-				if strings.Contains(fileName[1], "gitjob") {
+				if strings.Contains(fileName[2], "gitjob") {
 					exampleMonitoredRsc = "gitrepo"
-				} else if !strings.Contains(fileName[1], "shard") {
+				} else if !strings.Contains(fileName[2], "shard") {
 					// Check for fleet_*_desired_ready metrics on non-sharded services
-					Expect(c).To(ContainSubstring(fmt.Sprintf("fleet_%s_desired_ready", exampleMonitoredRsc)))
+					expectedStr := fmt.Sprintf("fleet_%s_desired_ready", exampleMonitoredRsc)
+					Expect(c).To(ContainSubstring(expectedStr), fmt.Sprintf("expected file %q to contain %q", header.Name, expectedStr))
 				}
 
 				Expect(c).To(ContainSubstring(fmt.Sprintf(`workqueue_work_duration_seconds_bucket{controller="%s",name="%s",`, exampleMonitoredRsc, exampleMonitoredRsc)))
@@ -110,14 +111,14 @@ var _ = Describe("Fleet dump", Label("sharding"), func() {
 			}
 
 			Expect(foundFiles).To(HaveLen(8))
-			Expect(foundFiles).To(ContainElement("metrics_monitoring-gitjob"))
-			Expect(foundFiles).To(ContainElement("metrics_monitoring-gitjob-shard-shard0"))
-			Expect(foundFiles).To(ContainElement("metrics_monitoring-gitjob-shard-shard1"))
-			Expect(foundFiles).To(ContainElement("metrics_monitoring-gitjob-shard-shard2"))
-			Expect(foundFiles).To(ContainElement("metrics_monitoring-fleet-controller"))
-			Expect(foundFiles).To(ContainElement("metrics_monitoring-fleet-controller-shard-shard0"))
-			Expect(foundFiles).To(ContainElement("metrics_monitoring-fleet-controller-shard-shard1"))
-			Expect(foundFiles).To(ContainElement("metrics_monitoring-fleet-controller-shard-shard2"))
+			Expect(foundFiles).To(ContainElement("metrics_cattle-fleet-system_monitoring-gitjob"))
+			Expect(foundFiles).To(ContainElement("metrics_cattle-fleet-system_monitoring-gitjob-shard-shard0"))
+			Expect(foundFiles).To(ContainElement("metrics_cattle-fleet-system_monitoring-gitjob-shard-shard1"))
+			Expect(foundFiles).To(ContainElement("metrics_cattle-fleet-system_monitoring-gitjob-shard-shard2"))
+			Expect(foundFiles).To(ContainElement("metrics_cattle-fleet-system_monitoring-fleet-controller"))
+			Expect(foundFiles).To(ContainElement("metrics_cattle-fleet-system_monitoring-fleet-controller-shard-shard0"))
+			Expect(foundFiles).To(ContainElement("metrics_cattle-fleet-system_monitoring-fleet-controller-shard-shard1"))
+			Expect(foundFiles).To(ContainElement("metrics_cattle-fleet-system_monitoring-fleet-controller-shard-shard2"))
 		})
 	})
 

internal/cmd/cli/dump/dump.go

@@ -483,12 +483,10 @@ func addEventsToArchive(
 }
 
 func addMetricsToArchive(ctx context.Context, c client.Client, logger logr.Logger, cfg *rest.Config, w *tar.Writer, opt Options) error {
-	ns := config.DefaultNamespace // XXX: support installation in non-default namespace, and check for services across all namespaces, by label?
-
 	var monitoringSvcs []corev1.Service
 	var svcs corev1.ServiceList
 	for {
-		opts := []client.ListOption{client.InNamespace(ns), client.Limit(opt.FetchLimit), client.Continue(svcs.Continue)}
+		opts := []client.ListOption{client.Limit(opt.FetchLimit), client.Continue(svcs.Continue)}
 
 		if err := c.List(ctx, &svcs, opts...); err != nil {
 			return fmt.Errorf("failed to list services for extracting metrics: %w", err)
@@ -508,7 +506,7 @@ func addMetricsToArchive(ctx context.Context, c client.Client, logger logr.Logge
 	}
 
 	if len(monitoringSvcs) == 0 {
-		logger.Info("No monitoring services found; Fleet has probably been installed with metrics disabled.", "namespace", ns)
+		logger.Info("No monitoring services found; Fleet has probably been installed with metrics disabled.")
 
 		return nil
 	}
@@ -553,7 +551,8 @@ func addMetricsToArchive(ctx context.Context, c client.Client, logger logr.Logge
 
 		logger.Info("Extracted metrics", "service", svc.Name)
 
-		if err := addFileToArchive(body, "metrics_"+svc.Name, w); err != nil {
+		fileName := fmt.Sprintf("metrics_%s_%s", svc.Namespace, svc.Name)
+		if err := addFileToArchive(body, fileName, w); err != nil {
 			return fmt.Errorf("failed to write metrics to archive from service %s: %w", svc.Name, err)
 		}
 	}

internal/cmd/cli/dump/dump_test.go

@@ -388,6 +388,18 @@ func Test_addMetrics(t *testing.T) {
 			},
 			expErrStr: "service cattle-fleet-system/monitoring-prefixed does not have any exposed ports",
 		},
+		{
+			name: "monitoring service in non-default namespace is still discovered",
+			svcs: []corev1.Service{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "monitoring-prefixed",
+						Namespace: "custom-fleet-system",
+					},
+				},
+			},
+			expErrStr: "service custom-fleet-system/monitoring-prefixed does not have any exposed ports",
+		},
 		{
 			name: "monitoring service with exposed ports but no labels",
 			svcs: []corev1.Service{
@@ -501,7 +513,6 @@ func Test_addMetrics(t *testing.T) {
 			mockClient.EXPECT().List(
 				ctx,
 				gomock.AssignableToTypeOf(&corev1.ServiceList{}),
-				client.InNamespace("cattle-fleet-system"),
 				gomock.Any(), // client.Limit(...)
 				gomock.Any(), // client.Continue(...)
 			).