Update monitoring reconciliation logic, tests

Author: DmytroPI-dev

api/v4/postgrescluster_types.go

@@ -109,23 +109,23 @@ type PostgresClusterSpec struct {
 	// +optional
 	ClusterDeletionPolicy *string `json:"clusterDeletionPolicy,omitempty"`
 
-	// Observability contains configuration for metrics exposure features.
+	// Monitoring contains configuration for metrics exposure features.
 	// +optional
-	Observability *PostgresObservabilityOverride `json:"observability,omitempty"`
+	Monitoring *PostgresClusterMonitoring `json:"monitoring,omitempty"`
 }
 
-// PostgresObservabilityOverride overrides observability configuration options for PostgresClusterClass.
-type PostgresObservabilityOverride struct {
+// PostgresClusterMonitoring overrides monitoring configuration options for PostgresClusterClass.
+type PostgresClusterMonitoring struct {
 
 	// +optional
-	PostgreSQL *FeatureDisableOverride `json:"postgresql,omitempty"`
+	PostgreSQLMetrics *FeatureDisableOverride `json:"postgresqlMetrics,omitempty"`
 
 	// +optional
-	PgBouncer *FeatureDisableOverride `json:"pgbouncer,omitempty"`
+	ConnectionPoolerMetrics *FeatureDisableOverride `json:"connectionPoolerMetrics,omitempty"`
 }
 
 type FeatureDisableOverride struct {
-	// Disable set to true will disable the feature even if it's enabled in the class.
+	// Disabled set to true will disable the feature even if it's enabled in the class.
 	// +kubebuilder:default=false
 	// +optional
 	Disabled *bool `json:"disabled,omitempty"`

api/v4/postgresclusterclass_types.go

@@ -49,6 +49,7 @@ type PostgresClusterClassSpec struct {
 	CNPG *CNPGConfig `json:"cnpg,omitempty"`
 }
 
+// +kubebuilder:validation:XValidation:rule="!has(self.monitoring) || !has(self.monitoring.connectionPoolerMetrics) || !has(self.monitoring.connectionPoolerMetrics.enabled) || !self.monitoring.connectionPoolerMetrics.enabled || (has(self.connectionPoolerEnabled) && self.connectionPoolerEnabled)",message="connectionPoolerEnabled must be true when monitoring.connectionPoolerMetrics.enabled is true"
 // PostgresClusterClassConfig contains provider-agnostic cluster configuration.
 // These fields define PostgresCluster infrastructure and can be overridden in PostgresCluster CR.
 type PostgresClusterClassConfig struct {
@@ -100,12 +101,12 @@ type PostgresClusterClassConfig struct {
 	// +optional
 	ConnectionPoolerEnabled *bool `json:"connectionPoolerEnabled,omitempty"`
 
-	// Observability contains configuration for metrics exposure.
+	// Monitoring contains configuration for metrics exposure.
 	// When enabled, creates metrics resources for clusters using this class.
 	// Can be overridden in PostgresCluster CR.
 	// +kubebuilder:default={}
 	// +optional
-	Observability *PostgresObservabilityClassConfig `json:"observability,omitempty"`
+	Monitoring *PostgresMonitoringClassConfig `json:"monitoring,omitempty"`
 }
 
 // ConnectionPoolerMode defines the PgBouncer connection pooling strategy.
@@ -179,11 +180,11 @@ type PostgresClusterClassStatus struct {
 	Phase *string `json:"phase,omitempty"`
 }
 
-type PostgresObservabilityClassConfig struct {
+type PostgresMonitoringClassConfig struct {
 	// +optional
-	PostgreSQL *MetricsClassConfig `json:"postgresql,omitempty"`
+	PostgreSQLMetrics *MetricsClassConfig `json:"postgresqlMetrics,omitempty"`
 	// +optional
-	PgBouncer *MetricsClassConfig `json:"pgbouncer,omitempty"`
+	ConnectionPoolerMetrics *MetricsClassConfig `json:"connectionPoolerMetrics,omitempty"`
 }
 
 type MetricsClassConfig struct {

internal/controller/postgrescluster_controller_test.go

@@ -28,14 +28,17 @@ import (
 	cnpgv1 "github.com/cloudnative-pg/cloudnative-pg/api/v1"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/ptr"
 
 	enterprisev4 "github.com/splunk/splunk-operator/api/v4"
 	"github.com/splunk/splunk-operator/pkg/postgresql/cluster/core"
+	corev1 "k8s.io/api/core/v1"
 )
 
 /*
@@ -85,6 +88,28 @@ var _ = Describe("PostgresCluster Controller", func() {
 		}
 	}
 
+	recreateClassWithMonitoring := func(postgresMetricsEnabled bool) {
+		Expect(k8sClient.Delete(ctx, pgClusterClass)).To(Succeed())
+
+		pgClusterClass = &enterprisev4.PostgresClusterClass{
+			ObjectMeta: metav1.ObjectMeta{Name: className},
+			Spec: enterprisev4.PostgresClusterClassSpec{
+				Provisioner: provisioner,
+				Config: &enterprisev4.PostgresClusterClassConfig{
+					Instances:               &[]int32{clusterMemberCount}[0],
+					Storage:                 &[]resource.Quantity{resource.MustParse(storageAmount)}[0],
+					PostgresVersion:         &[]string{postgresVersion}[0],
+					ConnectionPoolerEnabled: &[]bool{poolerEnabled}[0],
+					Monitoring: &enterprisev4.PostgresMonitoringClassConfig{
+						PostgreSQLMetrics: &enterprisev4.MetricsClassConfig{Enabled: ptr.To(postgresMetricsEnabled)},
+					},
+				},
+			},
+		}
+
+		Expect(k8sClient.Create(ctx, pgClusterClass)).To(Succeed())
+	}
+
 	BeforeEach(func() {
 		nameSuffix := fmt.Sprintf("%d-%d-%d",
 			GinkgoParallelProcess(),
@@ -238,6 +263,65 @@ var _ = Describe("PostgresCluster Controller", func() {
 				Expect(cond).NotTo(BeNil())
 				Expect(cond.ObservedGeneration).To(Equal(pc.Generation))
 			})
+
+			It("creates monitoring resources and sets MonitoringReady when monitoring is enabled", func() {
+				recreateClassWithMonitoring(true)
+
+				Expect(k8sClient.Create(ctx, pgCluster)).To(Succeed())
+				reconcileNTimes(3)
+
+				pc := &enterprisev4.PostgresCluster{}
+				Expect(k8sClient.Get(ctx, pgClusterKey, pc)).To(Succeed())
+				cond := meta.FindStatusCondition(pc.Status.Conditions, "MonitoringReady")
+				Expect(cond).NotTo(BeNil())
+				Expect(cond.Status).To(Equal(metav1.ConditionTrue))
+				Expect(cond.Reason).To(Equal("ObservabilityResourcesReady"))
+
+				metricsService := &corev1.Service{}
+				Expect(k8sClient.Get(ctx, types.NamespacedName{
+					Name:      clusterName + "-postgres-metrics",
+					Namespace: namespace,
+				}, metricsService)).To(Succeed())
+
+				serviceMonitor := &monitoringv1.ServiceMonitor{}
+				Expect(k8sClient.Get(ctx, types.NamespacedName{
+					Name:      clusterName + "-postgres-metrics-monitor",
+					Namespace: namespace,
+				}, serviceMonitor)).To(Succeed())
+			})
+
+			It("removes monitoring resources and MonitoringReady when monitoring is disabled by cluster override", func() {
+				recreateClassWithMonitoring(true)
+
+				Expect(k8sClient.Create(ctx, pgCluster)).To(Succeed())
+				reconcileNTimes(3)
+
+				current := &enterprisev4.PostgresCluster{}
+				Expect(k8sClient.Get(ctx, pgClusterKey, current)).To(Succeed())
+				current.Spec.Monitoring = &enterprisev4.PostgresClusterMonitoring{
+					PostgreSQLMetrics: &enterprisev4.FeatureDisableOverride{Disabled: ptr.To(true)},
+				}
+				Expect(k8sClient.Update(ctx, current)).To(Succeed())
+
+				reconcileNTimes(1)
+
+				Expect(k8sClient.Get(ctx, pgClusterKey, current)).To(Succeed())
+				Expect(meta.FindStatusCondition(current.Status.Conditions, "MonitoringReady")).To(BeNil())
+
+				metricsService := &corev1.Service{}
+				err := k8sClient.Get(ctx, types.NamespacedName{
+					Name:      clusterName + "-postgres-metrics",
+					Namespace: namespace,
+				}, metricsService)
+				Expect(apierrors.IsNotFound(err)).To(BeTrue())
+
+				serviceMonitor := &monitoringv1.ServiceMonitor{}
+				err = k8sClient.Get(ctx, types.NamespacedName{
+					Name:      clusterName + "-postgres-metrics-monitor",
+					Namespace: namespace,
+				}, serviceMonitor)
+				Expect(apierrors.IsNotFound(err)).To(BeTrue())
+			})
 		})
 	})
 

internal/controller/suite_test.go

@@ -78,6 +78,7 @@ var _ = BeforeSuite(func(ctx context.Context) {
 		CRDDirectoryPaths: []string{
 			filepath.Join("..", "..", "config", "crd", "bases"),
 			filepath.Join(cnpgModuleDir, "config", "crd", "bases"),
+			filepath.Join("testdata", "crds"),
 		},
 		ErrorIfCRDPathMissing: true,
 	}

internal/controller/testdata/crds/monitoring.coreos.com_servicemonitors.yaml

@@ -0,0 +1,32 @@
+apiVersion: apiextensions.k8s.io/v1
+kind: CustomResourceDefinition
+metadata:
+  name: servicemonitors.monitoring.coreos.com
+spec:
+  group: monitoring.coreos.com
+  scope: Namespaced
+  names:
+    plural: servicemonitors
+    singular: servicemonitor
+    kind: ServiceMonitor
+    listKind: ServiceMonitorList
+  versions:
+    - name: v1
+      served: true
+      storage: true
+      schema:
+        openAPIV3Schema:
+          type: object
+          properties:
+            apiVersion:
+              type: string
+            kind:
+              type: string
+            metadata:
+              type: object
+            spec:
+              type: object
+              x-kubernetes-preserve-unknown-fields: true
+            status:
+              type: object
+              x-kubernetes-preserve-unknown-fields: true

pkg/postgresql/cluster/core/cluster.go

@@ -396,35 +396,54 @@ func PostgresClusterService(ctx context.Context, rc *ReconcileContext, req ctrl.
 		rc.emitPoolerReadyTransition(postgresCluster, oldConditions)
 	}
 
-	if err := reconcilePostgreSQLMetricsService(ctx, c, rc.Scheme, postgresCluster, isPostgreSQLMetricsEnabled(postgresCluster, clusterClass)); err != nil {
-		return ctrl.Result{}, err
+	postgresMetricsEnabled := isPostgreSQLMetricsEnabled(postgresCluster, clusterClass)
+	poolerMetricsEnabled := isConnectionPoolerMetricsEnabled(postgresCluster, clusterClass)
+	rwPoolerMetricsEnabled := poolerMetricsEnabled && poolerEnabled && rwPoolerExists
+	roPoolerMetricsEnabled := poolerMetricsEnabled && poolerEnabled && roPoolerExists
+	monitoringEnabled := postgresMetricsEnabled || (poolerMetricsEnabled && poolerEnabled)
+
+	monitoringFailure := func(reason conditionReasons, eventReason, message string, err error) (ctrl.Result, error) {
+		return ctrl.Result{}, handleMonitoringFailure(ctx, c, rc, postgresCluster, reason, eventReason, message, err)
 	}
 
-	poolerMetricsEnabled := isConnectionPoolerMetricsEnabled(postgresCluster, clusterClass)
-	rwPoolerMetricsEnabled := poolerMetricsEnabled && rwPoolerExists
-	roPoolerMetricsEnabled := poolerMetricsEnabled && roPoolerExists
+	oldConditions := make([]metav1.Condition, len(postgresCluster.Status.Conditions))
+	copy(oldConditions, postgresCluster.Status.Conditions)
+
+	if err := reconcilePostgreSQLMetricsService(ctx, c, rc.Scheme, postgresCluster, postgresMetricsEnabled); err != nil {
+		return monitoringFailure(reasonPostgresMetricsServiceFailed, EventMetricsServiceReconcileFailed, fmt.Sprintf("Failed to reconcile PostgreSQL metrics Service: %v", err), err)
+	}
 	if err := reconcileConnectionPoolerMetricsService(ctx, c, rc.Scheme, postgresCluster, readWriteEndpoint, rwPoolerMetricsEnabled); err != nil {
-		return ctrl.Result{}, err
+		return monitoringFailure(reasonPoolerMetricsServiceFailed, EventMetricsServiceReconcileFailed, fmt.Sprintf("Failed to reconcile RW pooler metrics Service: %v", err), err)
 	}
 	if err := reconcileConnectionPoolerMetricsService(ctx, c, rc.Scheme, postgresCluster, readOnlyEndpoint, roPoolerMetricsEnabled); err != nil {
-		return ctrl.Result{}, err
+		return monitoringFailure(reasonPoolerMetricsServiceFailed, EventMetricsServiceReconcileFailed, fmt.Sprintf("Failed to reconcile RO pooler metrics Service: %v", err), err)
 	}
-
-	if err := reconcilePostgreSQLMetricsServiceMonitor(
-		ctx, c, rc.Scheme, postgresCluster, isPostgreSQLMetricsEnabled(postgresCluster, clusterClass),
-	); err != nil {
-		return ctrl.Result{}, err
+	if err := reconcilePostgreSQLMetricsServiceMonitor(ctx, c, rc.Scheme, postgresCluster, postgresMetricsEnabled); err != nil {
+		return monitoringFailure(reasonPostgresMetricsMonitorFailed, EventServiceMonitorReconcileFailed, fmt.Sprintf("Failed to reconcile PostgreSQL metrics ServiceMonitor: %v", err), err)
 	}
-
-	if err := reconcileConnectionPoolerMetricsServiceMonitor(
-		ctx, c, rc.Scheme, postgresCluster, readWriteEndpoint, rwPoolerMetricsEnabled,
-	); err != nil {
-		return ctrl.Result{}, err
+	if err := reconcileConnectionPoolerMetricsServiceMonitor(ctx, c, rc.Scheme, postgresCluster, readWriteEndpoint, rwPoolerMetricsEnabled); err != nil {
+		return monitoringFailure(reasonPoolerMetricsMonitorFailed, EventServiceMonitorReconcileFailed, fmt.Sprintf("Failed to reconcile RW pooler metrics ServiceMonitor: %v", err), err)
 	}
-	if err := reconcileConnectionPoolerMetricsServiceMonitor(
-		ctx, c, rc.Scheme, postgresCluster, readOnlyEndpoint, roPoolerMetricsEnabled,
-	); err != nil {
-		return ctrl.Result{}, err
+	if err := reconcileConnectionPoolerMetricsServiceMonitor(ctx, c, rc.Scheme, postgresCluster, readOnlyEndpoint, roPoolerMetricsEnabled); err != nil {
+		return monitoringFailure(reasonPoolerMetricsMonitorFailed, EventServiceMonitorReconcileFailed, fmt.Sprintf("Failed to reconcile RO pooler metrics ServiceMonitor: %v", err), err)
+	}
+
+	if !monitoringEnabled {
+		if err := removeCondition(ctx, c, postgresCluster, monitoringReady); err != nil {
+			if apierrors.IsConflict(err) {
+				return ctrl.Result{Requeue: true}, nil
+			}
+			return ctrl.Result{}, err
+		}
+	} else {
+		if err := setCondition(ctx, c, postgresCluster, monitoringReady, metav1.ConditionTrue, reasonObservabilityResourcesReady, "Monitoring resources are ready"); err != nil {
+			if apierrors.IsConflict(err) {
+				return ctrl.Result{Requeue: true}, nil
+			}
+			return ctrl.Result{}, err
+		}
+
+		rc.emitMonitoringReadyTransition(postgresCluster, oldConditions)
 	}
 
 	// Reconcile ConfigMap when CNPG cluster is healthy.
@@ -896,6 +915,50 @@ func setStatus(ctx context.Context, c client.Client, cluster *enterprisev4.Postg
 	return nil
 }
 
+// setCondition updates a specific condition on the PostgresCluster status.
+func setCondition(ctx context.Context, c client.Client, cluster *enterprisev4.PostgresCluster, condType conditionTypes, status metav1.ConditionStatus, reason conditionReasons, message string) error {
+	base := cluster.Status.DeepCopy()
+
+	meta.SetStatusCondition(&cluster.Status.Conditions, metav1.Condition{
+		Type:               string(condType),
+		Status:             status,
+		Reason:             string(reason),
+		Message:            message,
+		ObservedGeneration: cluster.Generation,
+	})
+
+	if equality.Semantic.DeepEqual(*base, cluster.Status) {
+		return nil
+	}
+	if err := c.Status().Update(ctx, cluster); err != nil {
+		return fmt.Errorf("failed to update PostgresCluster condition: %w", err)
+	}
+	return nil
+}
+
+// removeCondition removes a specific condition from the PostgresCluster status.
+func removeCondition(ctx context.Context, c client.Client, cluster *enterprisev4.PostgresCluster, condType conditionTypes) error {
+	base := cluster.Status.DeepCopy()
+
+	meta.RemoveStatusCondition(&cluster.Status.Conditions, string(condType))
+
+	if equality.Semantic.DeepEqual(*base, cluster.Status) {
+		return nil
+	}
+	if err := c.Status().Update(ctx, cluster); err != nil {
+		return fmt.Errorf("failed to remove PostgresCluster condition: %w", err)
+	}
+	return nil
+}
+
+func handleMonitoringFailure(ctx context.Context, c client.Client, rc *ReconcileContext, cluster *enterprisev4.PostgresCluster, reason conditionReasons, eventReason string, message string, err error) error {
+	rc.emitWarning(cluster, eventReason, message)
+	if statusErr := setCondition(ctx, c, cluster, monitoringReady, metav1.ConditionFalse, reason, message); statusErr != nil {
+		return errors.Join(err, fmt.Errorf("failed to update MonitoringReady condition: %w", statusErr))
+	}
+	return err
+}
+
 // generateConfigMap builds a ConfigMap with connection details for the PostgresCluster.
 func generateConfigMap(ctx context.Context, c client.Client, scheme *runtime.Scheme, cluster *enterprisev4.PostgresCluster, cnpgCluster *cnpgv1.Cluster, secretName string) (*corev1.ConfigMap, error) {
 	cmName := fmt.Sprintf("%s%s", cluster.Name, defaultConfigMapSuffix)