From ddd83cb1ac19709ea8c4d6a6c57d41f32034063c Mon Sep 17 00:00:00 2001 From: Simon Pasquier Date: Mon, 3 Aug 2026 14:57:56 +0200 Subject: [PATCH 1/6] Update documentation for collection profile tests --- .../prometheus/collection_profiles.go | 44 ++++++++++++++++--- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/test/extended/prometheus/collection_profiles.go b/test/extended/prometheus/collection_profiles.go index dbf74e9923a7..2030379fc4e3 100644 --- a/test/extended/prometheus/collection_profiles.go +++ b/test/extended/prometheus/collection_profiles.go @@ -21,14 +21,29 @@ import ( "k8s.io/client-go/kubernetes" ) +// These constants are defined in the Cluster Monitoring Operator and need to +// be kept in sync. const ( - projectName = "monitoring-collection-profiles" - + // collectionProfileFeatureLabel is the Kubernetes label identifying the + // collection profile associated to the monitoring resource (ServiceMonitor + // or PodMonitor) collectionProfileFeatureLabel = "monitoring.openshift.io/collection-profile" - collectionProfileFull = "full" - collectionProfileDefault = collectionProfileFull - collectionProfileMinimal = "minimal" - collectionProfileNone = "" + + // collectionProfileFull is the profile enabling the collection of all metrics. + collectionProfileFull = "full" + + // collectionProfileMinimal is the profile enabling the collection of + // metrics used for Telemetry, alerting and dashboards. + collectionProfileMinimal = "minimal" + + collectionProfileEmpty = "" + + // collectionProfileDefault is the default collection profile (currently: full). + collectionProfileDefault = collectionProfileFull +) + +const ( + projectName = "monitoring-collection-profiles" operatorName = "cluster-monitoring-operator" operatorNamespaceName = "openshift-monitoring" @@ -39,6 +54,10 @@ const ( ) var ( + // collectionProfilesSupportedList is the list of all collection profiles + // supported by the Cluster Monitoring Operator. + // + // TODO(simonpasquier): the tests should auto-discover the supported collection profiles instead of hardcoding them. To discover the supported profiles, the system can list the ServiceMonitor resources in namespace openshift-monitoring namespace matching the "app.kubernetes.io/managed-by=cluster-monitoring-operator" label the enumerate all the values for the "collectionProfileFeatureLabel" label. The resulting list should be at least 2. collectionProfilesSupportedList = []string{ collectionProfileFull, collectionProfileMinimal, @@ -176,6 +195,7 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil }, pollTimeout, pollInterval).Should(o.BeNil()) } }) + g.It("should have at least one implementation for each collection profile", func() { for _, profile := range collectionProfilesSupportedList { err := r.makeCollectionProfileConfigurationFor(tctx, profile) @@ -194,8 +214,9 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil }, pollTimeout, pollInterval).Should(o.BeNil()) } }) + g.It("should revert to default collection profile when an empty collection profile value is specified", func() { - err := r.makeCollectionProfileConfigurationFor(tctx, collectionProfileNone) + err := r.makeCollectionProfileConfigurationFor(tctx, collectionProfileEmpty) o.Expect(err).To(o.BeNil()) o.Eventually(func() error { @@ -312,6 +333,7 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil }) }) +// TODO(simonpasquier): isProfileEnabled should return an error instead of bool + error. It returns no error when the result's length is 1. func (r runner) isProfileEnabled(ctx context.Context, profile string) (bool, error) { vectorExpression := "max(profile:cluster_monitoring_operator_collection_profile:max{profile=\"%s\"}) == 1" queryResponse, err := helper.RunQuery(ctx, r.pclient, fmt.Sprintf(vectorExpression, profile)) @@ -325,6 +347,13 @@ func (r runner) isProfileEnabled(ctx context.Context, profile string) (bool, err return true, nil } +// TODO(simonpasquier): fetchMonitorsFor should use a custom type to represent label key/value instead of a fixed-size array. +// Example: +// +// type label struct { +// key string +// value string +// } func (r runner) fetchMonitorsFor(ctx context.Context, selectors ...[2]string) (*prometheusoperatorv1.ServiceMonitorList, error) { managedMonitorsSelectors := []string{ fmt.Sprintf("%s=%s", "app.kubernetes.io/managed-by", operatorName), @@ -337,6 +366,7 @@ func (r runner) fetchMonitorsFor(ctx context.Context, selectors ...[2]string) (* }) } +// TODO(simonpasquier): makeCollectionProfileConfigurationFor() should read the CMO configuration and update only the collectionProfile field instead of replacing the full content. The targeted update should use k8s.io/apimachinery/pkg/apis/meta/v1/unstructured and unstructured.SetNestedField(). func (r runner) makeCollectionProfileConfigurationFor(ctx context.Context, collectionProfile string) error { dataConfigYAMLPrometheusK8s := fmt.Sprintf("collectionProfile: %s", collectionProfile) dataConfigYAMLPrometheusK8sStructured := map[string]interface{}{ From ff70abc4ad2780bb61a9cc1b951b36b7e7a35de9 Mon Sep 17 00:00:00 2001 From: Simon Pasquier Date: Mon, 3 Aug 2026 15:05:55 +0200 Subject: [PATCH 2/6] Rename isProfileEnabled() to assertCollectionProfileEnabled() --- .../prometheus/collection_profiles.go | 39 ++++--------------- 1 file changed, 7 insertions(+), 32 deletions(-) diff --git a/test/extended/prometheus/collection_profiles.go b/test/extended/prometheus/collection_profiles.go index 2030379fc4e3..e077046d7c80 100644 --- a/test/extended/prometheus/collection_profiles.go +++ b/test/extended/prometheus/collection_profiles.go @@ -146,15 +146,7 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil err := r.makeCollectionProfileConfigurationFor(tctx, profile) o.Expect(err).To(o.BeNil()) o.Eventually(func() error { - enabled, err := r.isProfileEnabled(tctx, profile) - if err != nil { - return err - } - if !enabled { - return fmt.Errorf("collection profile %q is not enabled", profile) - } - - return nil + return r.assertCollectionProfileEnabled(tctx, profile) }, pollTimeout, pollInterval).Should(o.BeNil()) }) @@ -220,15 +212,7 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil o.Expect(err).To(o.BeNil()) o.Eventually(func() error { - enabled, err := r.isProfileEnabled(tctx, collectionProfileFull) - if err != nil { - return err - } - if !enabled { - return fmt.Errorf("collection profile %q is not enabled", collectionProfileFull) - } - - return nil + return r.assertCollectionProfileEnabled(tctx, collectionProfileFull) }, pollTimeout, pollInterval).Should(o.BeNil()) }) }) @@ -240,15 +224,7 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil err := r.makeCollectionProfileConfigurationFor(tctx, profile) o.Expect(err).To(o.BeNil()) o.Eventually(func() error { - enabled, err := r.isProfileEnabled(tctx, profile) - if err != nil { - return err - } - if !enabled { - return fmt.Errorf("collection profile %q is not enabled", profile) - } - - return nil + return r.assertCollectionProfileEnabled(tctx, profile) }, pollTimeout, pollInterval).Should(o.BeNil()) }) @@ -333,18 +309,17 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil }) }) -// TODO(simonpasquier): isProfileEnabled should return an error instead of bool + error. It returns no error when the result's length is 1. -func (r runner) isProfileEnabled(ctx context.Context, profile string) (bool, error) { +func (r runner) assertCollectionProfileEnabled(ctx context.Context, profile string) error { vectorExpression := "max(profile:cluster_monitoring_operator_collection_profile:max{profile=\"%s\"}) == 1" queryResponse, err := helper.RunQuery(ctx, r.pclient, fmt.Sprintf(vectorExpression, profile)) if err != nil { - return false, err + return err } if len(queryResponse.Data.Result) == 0 { - return false, nil + return fmt.Errorf("collection profile %q is not enabled", profile) } - return true, nil + return nil } // TODO(simonpasquier): fetchMonitorsFor should use a custom type to represent label key/value instead of a fixed-size array. From ca3ddf8e3a481653b5ec1f4d1e5a5026eb9cc194 Mon Sep 17 00:00:00 2001 From: Simon Pasquier Date: Mon, 3 Aug 2026 15:26:57 +0200 Subject: [PATCH 3/6] Read collection profiles from CMO resources --- .../prometheus/collection_profiles.go | 41 +++++++++++++++---- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/test/extended/prometheus/collection_profiles.go b/test/extended/prometheus/collection_profiles.go index e077046d7c80..e3b618f3cfa1 100644 --- a/test/extended/prometheus/collection_profiles.go +++ b/test/extended/prometheus/collection_profiles.go @@ -18,6 +18,7 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/kubernetes" ) @@ -55,13 +56,9 @@ const ( var ( // collectionProfilesSupportedList is the list of all collection profiles - // supported by the Cluster Monitoring Operator. - // - // TODO(simonpasquier): the tests should auto-discover the supported collection profiles instead of hardcoding them. To discover the supported profiles, the system can list the ServiceMonitor resources in namespace openshift-monitoring namespace matching the "app.kubernetes.io/managed-by=cluster-monitoring-operator" label the enumerate all the values for the "collectionProfileFeatureLabel" label. The resulting list should be at least 2. - collectionProfilesSupportedList = []string{ - collectionProfileFull, - collectionProfileMinimal, - } + // supported by the Cluster Monitoring Operator. It is populated at runtime + // to account for new profiles being added over time. + collectionProfilesSupportedList []string ) type runner struct { @@ -109,6 +106,12 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil return nil }, pollTimeout, pollInterval).Should(o.BeNil()) r.originalOperatorConfiguration = operatorConfiguration + + o.Eventually(func() error { + var err error + collectionProfilesSupportedList, err = r.getSupportedCollectionProfiles(tctx) + return err + }, pollTimeout, pollInterval).Should(o.BeNil()) }) g.AfterAll(func() { @@ -341,6 +344,30 @@ func (r runner) fetchMonitorsFor(ctx context.Context, selectors ...[2]string) (* }) } +// getSupportedCollectionProfiles returns the list of supported collection +// profiles interpolating from the monitor resources installed by the Cluster +// Monitoring Operator. +func (r runner) getSupportedCollectionProfiles(ctx context.Context) ([]string, error) { + monitors, err := r.fetchMonitorsFor(ctx) + if err != nil { + return nil, err + } + + seen := sets.New[string]() + for _, monitor := range monitors.Items { + if profile, ok := monitor.Labels[collectionProfileFeatureLabel]; ok && profile != collectionProfileEmpty { + seen.Insert(profile) + } + } + + profiles := sets.List(seen) + if len(profiles) < 2 { + return nil, fmt.Errorf("expected at least 2 supported collection profiles, got %d: %v", len(profiles), profiles) + } + + return profiles, nil +} + // TODO(simonpasquier): makeCollectionProfileConfigurationFor() should read the CMO configuration and update only the collectionProfile field instead of replacing the full content. The targeted update should use k8s.io/apimachinery/pkg/apis/meta/v1/unstructured and unstructured.SetNestedField(). func (r runner) makeCollectionProfileConfigurationFor(ctx context.Context, collectionProfile string) error { dataConfigYAMLPrometheusK8s := fmt.Sprintf("collectionProfile: %s", collectionProfile) From 77895e539b7d637c58ce8f45ba7e9ebe0010052c Mon Sep 17 00:00:00 2001 From: Simon Pasquier Date: Mon, 3 Aug 2026 16:22:27 +0200 Subject: [PATCH 4/6] Rewrite fetchMonitorsFor() --- .../prometheus/collection_profiles.go | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/test/extended/prometheus/collection_profiles.go b/test/extended/prometheus/collection_profiles.go index e3b618f3cfa1..16f2fba640c6 100644 --- a/test/extended/prometheus/collection_profiles.go +++ b/test/extended/prometheus/collection_profiles.go @@ -197,7 +197,7 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil o.Expect(err).To(o.BeNil()) o.Eventually(func() error { - monitors, err := r.fetchMonitorsFor(tctx, [2]string{collectionProfileFeatureLabel, profile}) + monitors, err := r.fetchMonitorsFor(tctx, label{key: collectionProfileFeatureLabel, value: profile}) if err != nil { return err } @@ -237,7 +237,7 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil var kubeStateMetricsMonitor *prometheusoperatorv1.ServiceMonitor o.Eventually(func() error { - monitors, err := r.fetchMonitorsFor(tctx, [2]string{collectionProfileFeatureLabel, profile}, [2]string{appNameSelector, appName}) + monitors, err := r.fetchMonitorsFor(tctx, label{key: collectionProfileFeatureLabel, value: profile}, label{key: appNameSelector, value: appName}) if err != nil { return err } @@ -325,19 +325,17 @@ func (r runner) assertCollectionProfileEnabled(ctx context.Context, profile stri return nil } -// TODO(simonpasquier): fetchMonitorsFor should use a custom type to represent label key/value instead of a fixed-size array. -// Example: -// -// type label struct { -// key string -// value string -// } -func (r runner) fetchMonitorsFor(ctx context.Context, selectors ...[2]string) (*prometheusoperatorv1.ServiceMonitorList, error) { +type label struct { + key string + value string +} + +func (r runner) fetchMonitorsFor(ctx context.Context, selectors ...label) (*prometheusoperatorv1.ServiceMonitorList, error) { managedMonitorsSelectors := []string{ fmt.Sprintf("%s=%s", "app.kubernetes.io/managed-by", operatorName), } for _, selector := range selectors { - managedMonitorsSelectors = append(managedMonitorsSelectors, fmt.Sprintf("%s=%s", selector[0], selector[1])) + managedMonitorsSelectors = append(managedMonitorsSelectors, fmt.Sprintf("%s=%s", selector.key, selector.value)) } return r.mclient.ServiceMonitors(operatorNamespaceName).List(ctx, metav1.ListOptions{ LabelSelector: strings.Join(managedMonitorsSelectors, ","), From 38292298549f8a91c15bcb2306b892c4cdf15684 Mon Sep 17 00:00:00 2001 From: Simon Pasquier Date: Mon, 3 Aug 2026 17:11:30 +0200 Subject: [PATCH 5/6] Patch the CMO config instead of updating it --- .../prometheus/collection_profiles.go | 123 +++++++++--------- 1 file changed, 59 insertions(+), 64 deletions(-) diff --git a/test/extended/prometheus/collection_profiles.go b/test/extended/prometheus/collection_profiles.go index 16f2fba640c6..0ac5e00abbb6 100644 --- a/test/extended/prometheus/collection_profiles.go +++ b/test/extended/prometheus/collection_profiles.go @@ -18,6 +18,7 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/kubernetes" ) @@ -90,6 +91,7 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil } r.pclient = oc.NewPrometheusClient(tctx) + // Save the current configuration and enabled the default collection profile. var operatorConfiguration *v1.ConfigMap o.Eventually(func() error { operatorConfiguration, err = r.kclient.CoreV1().ConfigMaps(operatorNamespaceName).Get(tctx, operatorConfigurationName, metav1.GetOptions{}) @@ -97,7 +99,7 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil if errors.IsNotFound(err) { g.By("initially, creating a configuration for the operator as it did not exist") operatorConfiguration = nil - return r.makeCollectionProfileConfigurationFor(tctx, collectionProfileDefault) + return r.configureCollectionProfile(tctx, collectionProfileDefault) } return err @@ -107,6 +109,7 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil }, pollTimeout, pollInterval).Should(o.BeNil()) r.originalOperatorConfiguration = operatorConfiguration + // Discover all supported collection profiles. o.Eventually(func() error { var err error collectionProfilesSupportedList, err = r.getSupportedCollectionProfiles(tctx) @@ -114,31 +117,35 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil }, pollTimeout, pollInterval).Should(o.BeNil()) }) + // Restore the Cluster Monitoring Operator's configuration. g.AfterAll(func() { - shouldDeleteConfiguration := false currentConfiguration, err := r.kclient.CoreV1().ConfigMaps(operatorNamespaceName).Get(tctx, operatorConfigurationName, metav1.GetOptions{}) o.Expect(err).To(o.BeNil()) + if r.originalOperatorConfiguration != nil { currentConfiguration.Data = r.originalOperatorConfiguration.Data g.By("restoring the original configuration for the operator") _, err = r.kclient.CoreV1().ConfigMaps(operatorNamespaceName).Update(tctx, currentConfiguration, metav1.UpdateOptions{}) } else { - shouldDeleteConfiguration = true - g.By("cleaning up the configuration for the operator as it did not exist pre-job") + g.By("deleting the cluster monitoring operator's configuration since it did not exist pre-job") err = r.kclient.CoreV1().ConfigMaps(operatorNamespaceName).Delete(tctx, operatorConfigurationName, metav1.DeleteOptions{}) } o.Expect(err).To(o.BeNil()) o.Eventually(func() error { - if shouldDeleteConfiguration { - _, err := r.kclient.CoreV1().ConfigMaps(operatorNamespaceName).Get(tctx, operatorConfigurationName, metav1.GetOptions{}) + if r.originalOperatorConfiguration != nil { + return nil + } + + _, err := r.kclient.CoreV1().ConfigMaps(operatorNamespaceName).Get(tctx, operatorConfigurationName, metav1.GetOptions{}) + if err != nil { if errors.IsNotFound(err) { return nil } - return fmt.Errorf("ConfigMap %q still exists after deletion attempt", operatorConfigurationName) + return err } - return nil + return fmt.Errorf("ConfigMap %q still exists after deletion attempt", operatorConfigurationName) }, pollTimeout, pollInterval).Should(o.BeNil()) }) @@ -146,7 +153,7 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil profile := collectionProfileDefault g.BeforeAll(func() { - err := r.makeCollectionProfileConfigurationFor(tctx, profile) + err := r.configureCollectionProfile(tctx, profile) o.Expect(err).To(o.BeNil()) o.Eventually(func() error { return r.assertCollectionProfileEnabled(tctx, profile) @@ -161,6 +168,7 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil if err != nil { return err } + if len(queryResponse.Data.Result) == 0 { return fmt.Errorf("expected %q to be present", defaultOnlyMetric) } @@ -173,7 +181,7 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil g.Context("in a heterogeneous environment,", func() { g.It("should expose information about the applied collection profile using meta-metrics", func() { for _, profile := range collectionProfilesSupportedList { - err := r.makeCollectionProfileConfigurationFor(tctx, profile) + err := r.configureCollectionProfile(tctx, profile) o.Expect(err).To(o.BeNil()) o.Eventually(func() error { @@ -193,7 +201,7 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil g.It("should have at least one implementation for each collection profile", func() { for _, profile := range collectionProfilesSupportedList { - err := r.makeCollectionProfileConfigurationFor(tctx, profile) + err := r.configureCollectionProfile(tctx, profile) o.Expect(err).To(o.BeNil()) o.Eventually(func() error { @@ -211,7 +219,7 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil }) g.It("should revert to default collection profile when an empty collection profile value is specified", func() { - err := r.makeCollectionProfileConfigurationFor(tctx, collectionProfileEmpty) + err := r.configureCollectionProfile(tctx, collectionProfileEmpty) o.Expect(err).To(o.BeNil()) o.Eventually(func() error { @@ -224,7 +232,7 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil profile := collectionProfileMinimal g.BeforeAll(func() { - err := r.makeCollectionProfileConfigurationFor(tctx, profile) + err := r.configureCollectionProfile(tctx, profile) o.Expect(err).To(o.BeNil()) o.Eventually(func() error { return r.assertCollectionProfileEnabled(tctx, profile) @@ -366,62 +374,49 @@ func (r runner) getSupportedCollectionProfiles(ctx context.Context) ([]string, e return profiles, nil } -// TODO(simonpasquier): makeCollectionProfileConfigurationFor() should read the CMO configuration and update only the collectionProfile field instead of replacing the full content. The targeted update should use k8s.io/apimachinery/pkg/apis/meta/v1/unstructured and unstructured.SetNestedField(). -func (r runner) makeCollectionProfileConfigurationFor(ctx context.Context, collectionProfile string) error { - dataConfigYAMLPrometheusK8s := fmt.Sprintf("collectionProfile: %s", collectionProfile) - dataConfigYAMLPrometheusK8sStructured := map[string]interface{}{ - "collectionProfile": collectionProfile, - } - dataConfigYAML := fmt.Sprintf("prometheusK8s:\n %s", dataConfigYAMLPrometheusK8s) - configurationEnableCollectionProfiles := &v1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: operatorConfigurationName, - Namespace: operatorNamespaceName, - }, - Data: map[string]string{ - "config.yaml": dataConfigYAML, - }, +// configureCollectionProfile udpates the Cluster Monitoring +// Operator's configuration to enable a given collection profile. +func (r runner) configureCollectionProfile(ctx context.Context, collectionProfile string) error { + configuration, err := r.kclient.CoreV1().ConfigMaps(operatorNamespaceName).Get(ctx, operatorConfigurationName, metav1.GetOptions{}) + create := errors.IsNotFound(err) + if err != nil && !create { + return err } - configuration, err := r.kclient.CoreV1().ConfigMaps(operatorNamespaceName).Get(ctx, operatorConfigurationName, metav1.GetOptions{}) - if err != nil && errors.IsNotFound(err) { - _, err = r.kclient.CoreV1().ConfigMaps(operatorNamespaceName).Create(ctx, configurationEnableCollectionProfiles, metav1.CreateOptions{}) - if err != nil { - return err + if create { + configuration = &v1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: operatorConfigurationName, + Namespace: operatorNamespaceName, + }, + Data: map[string]string{}, } - } else { - gotDataConfigYAML, ok := configuration.Data["config.yaml"] - if !ok { - configuration.Data = make(map[string]string) - configuration.Data["config.yaml"] = dataConfigYAML - } else { - var gotDataConfigYAMLMap map[string]interface{} - err = yaml.Unmarshal([]byte(gotDataConfigYAML), &gotDataConfigYAMLMap) - if err != nil { - return err - } - if _, ok := gotDataConfigYAMLMap["prometheusK8s"]; !ok { - gotDataConfigYAMLMap["prometheusK8s"] = dataConfigYAMLPrometheusK8sStructured - } else { - gotDataConfigYAMLMap["prometheusK8s"].(map[string]interface{})["collectionProfile"] = collectionProfile - } - gotDataConfigYAMLRaw, err := yaml.Marshal(gotDataConfigYAMLMap) - if err != nil { - return err - } - gotDataConfigYAML = string(gotDataConfigYAMLRaw) - configuration.Data["config.yaml"] = gotDataConfigYAML - } - currentConfiguration, err := r.kclient.CoreV1().ConfigMaps(operatorNamespaceName).Get(ctx, operatorConfigurationName, metav1.GetOptions{}) - if err != nil { - return err - } - currentConfiguration.Data = configuration.Data - _, err = r.kclient.CoreV1().ConfigMaps(operatorNamespaceName).Update(ctx, currentConfiguration, metav1.UpdateOptions{}) - if err != nil { + } + + var configMap map[string]interface{} + if raw, ok := configuration.Data["config.yaml"]; ok { + if err := yaml.Unmarshal([]byte(raw), &configMap); err != nil { return err } } + if configMap == nil { + configMap = make(map[string]interface{}) + } - return nil + if err := unstructured.SetNestedField(configMap, collectionProfile, "prometheusK8s", "collectionProfile"); err != nil { + return err + } + + raw, err := yaml.Marshal(configMap) + if err != nil { + return err + } + configuration.Data["config.yaml"] = string(raw) + + if create { + _, err = r.kclient.CoreV1().ConfigMaps(operatorNamespaceName).Create(ctx, configuration, metav1.CreateOptions{}) + } else { + _, err = r.kclient.CoreV1().ConfigMaps(operatorNamespaceName).Update(ctx, configuration, metav1.UpdateOptions{}) + } + return err } From 32cff6db8e81a54fcbe4681abdc3974c14fa33a8 Mon Sep 17 00:00:00 2001 From: Simon Pasquier Date: Tue, 4 Aug 2026 13:45:54 +0200 Subject: [PATCH 6/6] Check ServiceMonitors in all namespaces Signed-off-by: Simon Pasquier --- .../prometheus/collection_profiles.go | 153 +++++++++++------- 1 file changed, 95 insertions(+), 58 deletions(-) diff --git a/test/extended/prometheus/collection_profiles.go b/test/extended/prometheus/collection_profiles.go index 0ac5e00abbb6..7e4c354683cb 100644 --- a/test/extended/prometheus/collection_profiles.go +++ b/test/extended/prometheus/collection_profiles.go @@ -47,26 +47,27 @@ const ( const ( projectName = "monitoring-collection-profiles" - operatorName = "cluster-monitoring-operator" - operatorNamespaceName = "openshift-monitoring" - operatorConfigurationName = "cluster-monitoring-config" + operatorName = "cluster-monitoring-operator" + openshiftMonitoringNamespace = "openshift-monitoring" + clusterMonitoringConfigMap = "cluster-monitoring-config" pollTimeout = 15 * time.Minute pollInterval = 5 * time.Second ) -var ( +type runner struct { + kclient kubernetes.Interface + mclient *prometheusoperatorv1client.MonitoringV1Client + pclient prometheusv1.API + + // originalOperatorConfiguration is the copy of the CMO configuration's + // configmap to be restored when the test suite finishes. + originalOperatorConfiguration *v1.ConfigMap + // collectionProfilesSupportedList is the list of all collection profiles // supported by the Cluster Monitoring Operator. It is populated at runtime // to account for new profiles being added over time. collectionProfilesSupportedList []string -) - -type runner struct { - kclient kubernetes.Interface - mclient *prometheusoperatorv1client.MonitoringV1Client - pclient prometheusv1.API - originalOperatorConfiguration *v1.ConfigMap } // NOTE: The nested `Context` containers inside the following `Describe` container are used to group certain tests based on the environments they demand. @@ -94,7 +95,7 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil // Save the current configuration and enabled the default collection profile. var operatorConfiguration *v1.ConfigMap o.Eventually(func() error { - operatorConfiguration, err = r.kclient.CoreV1().ConfigMaps(operatorNamespaceName).Get(tctx, operatorConfigurationName, metav1.GetOptions{}) + operatorConfiguration, err = r.kclient.CoreV1().ConfigMaps(openshiftMonitoringNamespace).Get(tctx, clusterMonitoringConfigMap, metav1.GetOptions{}) if err != nil { if errors.IsNotFound(err) { g.By("initially, creating a configuration for the operator as it did not exist") @@ -110,25 +111,28 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil r.originalOperatorConfiguration = operatorConfiguration // Discover all supported collection profiles. + var supportedProfiles []string o.Eventually(func() error { var err error - collectionProfilesSupportedList, err = r.getSupportedCollectionProfiles(tctx) + supportedProfiles, err = r.getSupportedCollectionProfiles(tctx) return err }, pollTimeout, pollInterval).Should(o.BeNil()) + g.GinkgoWriter.Printf("supported collection profiles: %v\n", supportedProfiles) + r.collectionProfilesSupportedList = supportedProfiles }) // Restore the Cluster Monitoring Operator's configuration. g.AfterAll(func() { - currentConfiguration, err := r.kclient.CoreV1().ConfigMaps(operatorNamespaceName).Get(tctx, operatorConfigurationName, metav1.GetOptions{}) + currentConfiguration, err := r.kclient.CoreV1().ConfigMaps(openshiftMonitoringNamespace).Get(tctx, clusterMonitoringConfigMap, metav1.GetOptions{}) o.Expect(err).To(o.BeNil()) if r.originalOperatorConfiguration != nil { currentConfiguration.Data = r.originalOperatorConfiguration.Data g.By("restoring the original configuration for the operator") - _, err = r.kclient.CoreV1().ConfigMaps(operatorNamespaceName).Update(tctx, currentConfiguration, metav1.UpdateOptions{}) + _, err = r.kclient.CoreV1().ConfigMaps(openshiftMonitoringNamespace).Update(tctx, currentConfiguration, metav1.UpdateOptions{}) } else { g.By("deleting the cluster monitoring operator's configuration since it did not exist pre-job") - err = r.kclient.CoreV1().ConfigMaps(operatorNamespaceName).Delete(tctx, operatorConfigurationName, metav1.DeleteOptions{}) + err = r.kclient.CoreV1().ConfigMaps(openshiftMonitoringNamespace).Delete(tctx, clusterMonitoringConfigMap, metav1.DeleteOptions{}) } o.Expect(err).To(o.BeNil()) @@ -137,7 +141,7 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil return nil } - _, err := r.kclient.CoreV1().ConfigMaps(operatorNamespaceName).Get(tctx, operatorConfigurationName, metav1.GetOptions{}) + _, err := r.kclient.CoreV1().ConfigMaps(openshiftMonitoringNamespace).Get(tctx, clusterMonitoringConfigMap, metav1.GetOptions{}) if err != nil { if errors.IsNotFound(err) { return nil @@ -145,7 +149,7 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil return err } - return fmt.Errorf("ConfigMap %q still exists after deletion attempt", operatorConfigurationName) + return fmt.Errorf("ConfigMap %q still exists after deletion attempt", clusterMonitoringConfigMap) }, pollTimeout, pollInterval).Should(o.BeNil()) }) @@ -160,17 +164,16 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil }, pollTimeout, pollInterval).Should(o.BeNil()) }) - g.It("should expose default metrics", func() { + g.It("should expose all metrics", func() { o.Eventually(func() error { - defaultOnlyMetric := "prometheus_engine_query_log_enabled" - defaultMetricQuery := fmt.Sprintf("max(%s)", defaultOnlyMetric) - queryResponse, err := helper.RunQuery(tctx, r.pclient, defaultMetricQuery) + const sentinelMetricForDefaultProfile = "prometheus_engine_query_log_enabled" + queryResponse, err := helper.RunQuery(tctx, r.pclient, fmt.Sprintf("max(%s)", sentinelMetricForDefaultProfile)) if err != nil { return err } if len(queryResponse.Data.Result) == 0 { - return fmt.Errorf("expected %q to be present", defaultOnlyMetric) + return fmt.Errorf("expected %q to be present", sentinelMetricForDefaultProfile) } return nil @@ -180,7 +183,8 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil g.Context("in a heterogeneous environment,", func() { g.It("should expose information about the applied collection profile using meta-metrics", func() { - for _, profile := range collectionProfilesSupportedList { + for _, profile := range r.collectionProfilesSupportedList { + g.GinkgoWriter.Printf("enabling collection profile: %s\n", profile) err := r.configureCollectionProfile(tctx, profile) o.Expect(err).To(o.BeNil()) @@ -190,6 +194,7 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil if err != nil { return err } + if len(queryResponse.Data.Result) == 0 { return fmt.Errorf("no result found for profile %q", profile) } @@ -199,22 +204,49 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil } }) - g.It("should have at least one implementation for each collection profile", func() { - for _, profile := range collectionProfilesSupportedList { - err := r.configureCollectionProfile(tctx, profile) - o.Expect(err).To(o.BeNil()) + g.It("should implement all collection profiles or none", func() { + // Retrieve all service monitors implementing the default collection profile. + var monitors []*prometheusoperatorv1.ServiceMonitor + o.Eventually(func() error { + serviceMonitors, err := r.getServiceMonitors(tctx, metav1.NamespaceAll, label{key: collectionProfileFeatureLabel, value: collectionProfileDefault}) + if err != nil { + return err + } + monitors = serviceMonitors.Items + return nil + }, pollTimeout, pollInterval).Should(o.BeNil()) - o.Eventually(func() error { - monitors, err := r.fetchMonitorsFor(tctx, label{key: collectionProfileFeatureLabel, value: profile}) - if err != nil { - return err - } - if len(monitors.Items) == 0 { - return fmt.Errorf("no monitors found with collection profile %q", profile) + // For each service monitor implementing the default collection + // profile, ensure that all other collection profiles are also + // implemented. + for _, monitor := range monitors { + g.GinkgoWriter.Printf("checking ServiceMonitor %s/%s\n", monitor.Namespace, monitor.Name) + for _, profile := range r.collectionProfilesSupportedList { + if profile == collectionProfileDefault { + continue } - return nil - }, pollTimeout, pollInterval).Should(o.BeNil()) + o.Eventually(func() error { + selectors := []label{{key: collectionProfileFeatureLabel, value: profile}} + for k, v := range monitor.Labels { + if k == collectionProfileFeatureLabel { + continue + } + selectors = append(selectors, label{key: k, value: v}) + } + + monitors, err := r.getServiceMonitors(tctx, monitor.Namespace, selectors...) + if err != nil { + return err + } + + if len(monitors.Items) == 0 { + return fmt.Errorf("%s/%s: no ServiceMonitor found for collection profile %q", monitor.Namespace, monitor.Name, profile) + } + + return nil + }, time.Minute, pollInterval).Should(o.BeNil()) + } } }) @@ -229,13 +261,11 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil }) g.Context("in a homogeneous minimal environment,", func() { - profile := collectionProfileMinimal - g.BeforeAll(func() { - err := r.configureCollectionProfile(tctx, profile) + err := r.configureCollectionProfile(tctx, collectionProfileMinimal) o.Expect(err).To(o.BeNil()) o.Eventually(func() error { - return r.assertCollectionProfileEnabled(tctx, profile) + return r.assertCollectionProfileEnabled(tctx, collectionProfileMinimal) }, pollTimeout, pollInterval).Should(o.BeNil()) }) @@ -245,15 +275,17 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil var kubeStateMetricsMonitor *prometheusoperatorv1.ServiceMonitor o.Eventually(func() error { - monitors, err := r.fetchMonitorsFor(tctx, label{key: collectionProfileFeatureLabel, value: profile}, label{key: appNameSelector, value: appName}) + monitors, err := r.getServiceMonitorsForOpenShiftMonitoring(tctx, label{key: collectionProfileFeatureLabel, value: collectionProfileMinimal}, label{key: appNameSelector, value: appName}) if err != nil { return err } + if len(monitors.Items) == 0 { - return fmt.Errorf("no monitors found with collection profile: %q and %#v=%q", profile, appNameSelector, appName) + return fmt.Errorf("no ServiceMonitor found with collection profile: %q and %#v=%q", collectionProfileMinimal, appNameSelector, appName) } + if len(monitors.Items) > 1 { - return fmt.Errorf("more than one monitor found with collection profile: %q and %#v=%q", profile, appNameSelector, appName) + return fmt.Errorf("more than one ServiceMonitor found with collection profile: %q and %#v=%q", collectionProfileMinimal, appNameSelector, appName) } kubeStateMetricsMonitor = monitors.Items[0] @@ -289,7 +321,7 @@ var _ = g.Describe("[sig-instrumentation][OCPFeatureGate:MetricsCollectionProfil o.Eventually(func() error { postRelabelingMetric := "scrape_samples_post_metric_relabeling" - relabelingMetricQuery := fmt.Sprintf("sum(%s{job=\"%s\",endpoint=\"https-main\",namespace=\"%s\"})", postRelabelingMetric, appName, operatorNamespaceName) + relabelingMetricQuery := fmt.Sprintf("sum(%s{job=\"%s\",endpoint=\"https-main\",namespace=\"%s\"})", postRelabelingMetric, appName, openshiftMonitoringNamespace) queryResponse, err := helper.RunQuery(tctx, r.pclient, relabelingMetricQuery) if err != nil { return err @@ -338,15 +370,20 @@ type label struct { value string } -func (r runner) fetchMonitorsFor(ctx context.Context, selectors ...label) (*prometheusoperatorv1.ServiceMonitorList, error) { - managedMonitorsSelectors := []string{ - fmt.Sprintf("%s=%s", "app.kubernetes.io/managed-by", operatorName), - } +// getServiceMonitorsForOpenShiftMonitoring returns all service monitors managed by the Cluster Monitoring Operator. +func (r runner) getServiceMonitorsForOpenShiftMonitoring(ctx context.Context, selectors ...label) (*prometheusoperatorv1.ServiceMonitorList, error) { + return r.getServiceMonitors(ctx, openshiftMonitoringNamespace, append([]label{{key: "app.kubernetes.io/managed-by", value: operatorName}}, selectors...)...) +} + +// getServiceMonitors returns service monitors in the given namespace (or all namespaces if empty) matching the given label selectors. +func (r runner) getServiceMonitors(ctx context.Context, namespace string, selectors ...label) (*prometheusoperatorv1.ServiceMonitorList, error) { + var labelSelectors []string for _, selector := range selectors { - managedMonitorsSelectors = append(managedMonitorsSelectors, fmt.Sprintf("%s=%s", selector.key, selector.value)) + labelSelectors = append(labelSelectors, fmt.Sprintf("%s=%s", selector.key, selector.value)) } - return r.mclient.ServiceMonitors(operatorNamespaceName).List(ctx, metav1.ListOptions{ - LabelSelector: strings.Join(managedMonitorsSelectors, ","), + + return r.mclient.ServiceMonitors(namespace).List(ctx, metav1.ListOptions{ + LabelSelector: strings.Join(labelSelectors, ","), }) } @@ -354,7 +391,7 @@ func (r runner) fetchMonitorsFor(ctx context.Context, selectors ...label) (*prom // profiles interpolating from the monitor resources installed by the Cluster // Monitoring Operator. func (r runner) getSupportedCollectionProfiles(ctx context.Context) ([]string, error) { - monitors, err := r.fetchMonitorsFor(ctx) + monitors, err := r.getServiceMonitorsForOpenShiftMonitoring(ctx) if err != nil { return nil, err } @@ -377,7 +414,7 @@ func (r runner) getSupportedCollectionProfiles(ctx context.Context) ([]string, e // configureCollectionProfile udpates the Cluster Monitoring // Operator's configuration to enable a given collection profile. func (r runner) configureCollectionProfile(ctx context.Context, collectionProfile string) error { - configuration, err := r.kclient.CoreV1().ConfigMaps(operatorNamespaceName).Get(ctx, operatorConfigurationName, metav1.GetOptions{}) + configuration, err := r.kclient.CoreV1().ConfigMaps(openshiftMonitoringNamespace).Get(ctx, clusterMonitoringConfigMap, metav1.GetOptions{}) create := errors.IsNotFound(err) if err != nil && !create { return err @@ -386,8 +423,8 @@ func (r runner) configureCollectionProfile(ctx context.Context, collectionProfil if create { configuration = &v1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ - Name: operatorConfigurationName, - Namespace: operatorNamespaceName, + Name: clusterMonitoringConfigMap, + Namespace: openshiftMonitoringNamespace, }, Data: map[string]string{}, } @@ -414,9 +451,9 @@ func (r runner) configureCollectionProfile(ctx context.Context, collectionProfil configuration.Data["config.yaml"] = string(raw) if create { - _, err = r.kclient.CoreV1().ConfigMaps(operatorNamespaceName).Create(ctx, configuration, metav1.CreateOptions{}) + _, err = r.kclient.CoreV1().ConfigMaps(openshiftMonitoringNamespace).Create(ctx, configuration, metav1.CreateOptions{}) } else { - _, err = r.kclient.CoreV1().ConfigMaps(operatorNamespaceName).Update(ctx, configuration, metav1.UpdateOptions{}) + _, err = r.kclient.CoreV1().ConfigMaps(openshiftMonitoringNamespace).Update(ctx, configuration, metav1.UpdateOptions{}) } return err }