Add validation and improved logging for PVC storage configuration

Prevents silent failures when override.statefulSet.spec.volumeClaimTemplates is provided with only metadata (e.g., annotations) but missing the required spec.resources.requests.storage field.

Changes:

1. Add CRD validation requiring PVC spec field
- Kubernetes API server now rejects incomplete volumeClaimTemplates
- Users get immediate feedback instead of silent reconciliation failures

2. Improve error messages at reconciliation time
- Detect storage=0 with helpful hint about override behavior
- Show actual values in shrink error: \"(existing: 20Gi, desired: 5Gi)\"
- Explain that overrides replace entire templates, not merge

Files Modified:
- api/v1beta1/rabbitmqcluster_types.go - Add validation marker
- config/crd/bases/rabbitmq.com_rabbitmqclusters.yaml - Generated CRD
- controllers/reconcile_persistence.go - Validate storage != 0
- internal/scaling/scaling.go - Add storage=0 check with helpful error

Fixes: #2023

Author: jmealo

api/v1beta1/rabbitmqcluster_types.go

@@ -351,8 +351,10 @@ type PersistentVolumeClaim struct {
 
 	// Spec defines the desired characteristics of a volume requested by a pod author.
 	// More info: https://kubernetes.io/docs/concepts/storage/persistent-volumes#persistentvolumeclaims
-	// +optional
-	Spec corev1.PersistentVolumeClaimSpec `json:"spec,omitempty" protobuf:"bytes,2,opt,name=spec"`
+	// When using override.statefulSet.spec.volumeClaimTemplates, you must provide the complete
+	// template including spec.resources.requests.storage. Overrides replace the entire template.
+	// +kubebuilder:validation:Required
+	Spec corev1.PersistentVolumeClaimSpec `json:"spec" protobuf:"bytes,2,opt,name=spec"`
 }
 
 // TLSSpec allows for the configuration of TLS certificates to be used by RabbitMQ. Also allows for non-TLS traffic to be disabled.

config/crd/bases/rabbitmq.com_rabbitmqclusters.yaml

@@ -5059,6 +5059,8 @@ spec:
                                       volumeName:
                                         type: string
                                     type: object
+                                required:
+                                  - spec
                                 type: object
                               type: array
                           type: object

controllers/reconcile_persistence.go

@@ -14,8 +14,14 @@ import (
 
 func (r *RabbitmqClusterReconciler) reconcilePVC(ctx context.Context, rmq *rabbitmqv1beta1.RabbitmqCluster, desiredSts *appsv1.StatefulSet) error {
 	logger := ctrl.LoggerFrom(ctx)
-	desiredCapacity := persistenceStorageCapacity(desiredSts.Spec.VolumeClaimTemplates)
-	err := scaling.NewPersistenceScaler(r.Clientset).Scale(ctx, *rmq, desiredCapacity)
+	desiredCapacity, err := persistenceStorageCapacity(desiredSts.Spec.VolumeClaimTemplates)
+	if err != nil {
+		msg := fmt.Sprintf("Failed to determine PVC capacity: %s", err.Error())
+		logger.Error(err, msg)
+		r.Recorder.Event(rmq, corev1.EventTypeWarning, "FailedReconcilePersistence", msg)
+		return err
+	}
+	err = scaling.NewPersistenceScaler(r.Clientset).Scale(ctx, *rmq, desiredCapacity)
 	if err != nil {
 		msg := fmt.Sprintf("Failed to scale PVCs: %s", err.Error())
 		logger.Error(fmt.Errorf("hit an error while scaling PVC capacity: %w", err), msg)
@@ -24,11 +30,19 @@ func (r *RabbitmqClusterReconciler) reconcilePVC(ctx context.Context, rmq *rabbi
 	return err
 }
 
-func persistenceStorageCapacity(templates []corev1.PersistentVolumeClaim) k8sresource.Quantity {
+func persistenceStorageCapacity(templates []corev1.PersistentVolumeClaim) (k8sresource.Quantity, error) {
 	for _, t := range templates {
 		if t.Name == "persistence" {
-			return t.Spec.Resources.Requests[corev1.ResourceStorage]
+			storage := t.Spec.Resources.Requests[corev1.ResourceStorage]
+			if storage.IsZero() {
+				return storage, fmt.Errorf(
+					"PVC template 'persistence' has spec.resources.requests.storage=0 (or missing). " +
+						"If using override.statefulSet.spec.volumeClaimTemplates, you must provide " +
+						"the COMPLETE template including spec.resources.requests.storage. " +
+						"Overrides replace the entire volumeClaimTemplate, not merge with it")
+			}
+			return storage, nil
 		}
 	}
-	return k8sresource.MustParse("0")
+	return k8sresource.MustParse("0"), fmt.Errorf("no 'persistence' PVC template found in StatefulSet")
 }

internal/scaling/scaling.go

@@ -46,24 +46,12 @@ func (p PersistenceScaler) Scale(ctx context.Context, rmq rabbitmqv1beta1.Rabbit
 		return errors.New(msg)
 	}
 
-	// Compare storage capacities using decimal representation to handle different units correctly
-	// This approach follows CloudNativePG's PVC comparison pattern (pkg/reconciler/persistentvolumeclaim/existing.go)
-	// Using AsDec() ensures "10Gi" equals "10737418240" and "1Gi" equals "1024Mi"
-	if err == nil {
-		switch existingCapacity.AsDec().Cmp(desiredCapacity.AsDec()) {
-		case 0:
-			// Sizes are equal in StatefulSet template - but we still need to check individual PVCs
-			logger.V(1).Info("StatefulSet PVC template capacity matches desired", "capacity", existingCapacity.String())
-		case 1:
-			// Current > desired (shrink) - not allowed
-			msg := fmt.Sprintf("shrinking persistent volumes is not supported (existing: %s, desired: %s)",
-				existingCapacity.String(), desiredCapacity.String())
-			logger.Error(errors.New("unsupported operation"), msg)
-			return errors.New(msg)
-		default:
-			// Current < desired (expand) - proceed with resize
-			logger.Info("PVC expansion required", "existing", existingCapacity.String(), "desired", desiredCapacity.String())
-		}
+	// desired storage capacity is smaller than the current capacity; we can't proceed lest we lose data
+	if err == nil && existingCapacity.Cmp(desiredCapacity) == 1 {
+		msg := fmt.Sprintf("shrinking persistent volumes is not supported (existing: %s, desired: %s)",
+			existingCapacity.String(), desiredCapacity.String())
+		logger.Error(errors.New("unsupported operation"), msg)
+		return errors.New(msg)
 	}
 
 	existingPVCs, err := p.getClusterPVCs(ctx, rmq)