Skip to content

Commit 1a2bb15

Browse files
committed
Fix PVC storage validation and error messages
Prevents silent reconciliation failures when override.statefulSet.spec.volumeClaimTemplates is provided with incomplete configuration (e.g., only metadata.annotations without spec.resources.requests.storage). Changes: - Add CRD validation requiring PVC spec field (rejected at admission time) - Detect and error on storage=0 with hints about override behavior - Show actual values in shrink errors: "(existing: 20Gi, desired: 5Gi)" Fixes #2023
1 parent e20d5f4 commit 1a2bb15

File tree

5 files changed

+30
-8
lines changed

5 files changed

+30
-8
lines changed

api/v1beta1/rabbitmqcluster_types.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -351,8 +351,10 @@ type PersistentVolumeClaim struct {
351351

352352
// Spec defines the desired characteristics of a volume requested by a pod author.
353353
// More info: https://kubernetes.io/docs/concepts/storage/persistent-volumes#persistentvolumeclaims
354-
// +optional
355-
Spec corev1.PersistentVolumeClaimSpec `json:"spec,omitempty" protobuf:"bytes,2,opt,name=spec"`
354+
// When using override.statefulSet.spec.volumeClaimTemplates, you must provide the complete
355+
// template including spec.resources.requests.storage. Overrides replace the entire template.
356+
// +kubebuilder:validation:Required
357+
Spec corev1.PersistentVolumeClaimSpec `json:"spec" protobuf:"bytes,2,opt,name=spec"`
356358
}
357359

358360
// 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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5059,6 +5059,8 @@ spec:
50595059
volumeName:
50605060
type: string
50615061
type: object
5062+
required:
5063+
- spec
50625064
type: object
50635065
type: array
50645066
type: object

controllers/reconcile_persistence.go

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,14 @@ import (
1414

1515
func (r *RabbitmqClusterReconciler) reconcilePVC(ctx context.Context, rmq *rabbitmqv1beta1.RabbitmqCluster, desiredSts *appsv1.StatefulSet) error {
1616
logger := ctrl.LoggerFrom(ctx)
17-
desiredCapacity := persistenceStorageCapacity(desiredSts.Spec.VolumeClaimTemplates)
18-
err := scaling.NewPersistenceScaler(r.Clientset).Scale(ctx, *rmq, desiredCapacity)
17+
desiredCapacity, err := persistenceStorageCapacity(desiredSts.Spec.VolumeClaimTemplates)
18+
if err != nil {
19+
msg := fmt.Sprintf("Failed to determine PVC capacity: %s", err.Error())
20+
logger.Error(err, msg)
21+
r.Recorder.Event(rmq, corev1.EventTypeWarning, "FailedReconcilePersistence", msg)
22+
return err
23+
}
24+
err = scaling.NewPersistenceScaler(r.Clientset).Scale(ctx, *rmq, desiredCapacity)
1925
if err != nil {
2026
msg := fmt.Sprintf("Failed to scale PVCs: %s", err.Error())
2127
logger.Error(fmt.Errorf("hit an error while scaling PVC capacity: %w", err), msg)
@@ -24,11 +30,20 @@ func (r *RabbitmqClusterReconciler) reconcilePVC(ctx context.Context, rmq *rabbi
2430
return err
2531
}
2632

27-
func persistenceStorageCapacity(templates []corev1.PersistentVolumeClaim) k8sresource.Quantity {
33+
func persistenceStorageCapacity(templates []corev1.PersistentVolumeClaim) (k8sresource.Quantity, error) {
2834
for _, t := range templates {
2935
if t.Name == "persistence" {
30-
return t.Spec.Resources.Requests[corev1.ResourceStorage]
36+
storage := t.Spec.Resources.Requests[corev1.ResourceStorage]
37+
if storage.IsZero() {
38+
return storage, fmt.Errorf(
39+
"PVC template 'persistence' has spec.resources.requests.storage=0 (or missing). " +
40+
"If using override.statefulSet.spec.volumeClaimTemplates, you must provide " +
41+
"the COMPLETE template including spec.resources.requests.storage. " +
42+
"Overrides replace the entire volumeClaimTemplate, not merge with it")
43+
}
44+
return storage, nil
3145
}
3246
}
33-
return k8sresource.MustParse("0")
47+
// No persistence template found - this is valid for ephemeral storage (storage: "0Gi")
48+
return k8sresource.MustParse("0"), nil
3449
}

docs/api/rabbitmq.com.ref.asciidoc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,8 @@ More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-
114114

115115
| *`spec`* __link:https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.23/#persistentvolumeclaimspec-v1-core[$$PersistentVolumeClaimSpec$$]__ | Spec defines the desired characteristics of a volume requested by a pod author.
116116
More info: https://kubernetes.io/docs/concepts/storage/persistent-volumes#persistentvolumeclaims
117+
When using override.statefulSet.spec.volumeClaimTemplates, you must provide the complete
118+
template including spec.resources.requests.storage. Overrides replace the entire template.
117119
|===
118120

119121

internal/scaling/scaling.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ func (p PersistenceScaler) Scale(ctx context.Context, rmq rabbitmqv1beta1.Rabbit
4848

4949
// desired storage capacity is smaller than the current capacity; we can't proceed lest we lose data
5050
if err == nil && existingCapacity.Cmp(desiredCapacity) == 1 {
51-
msg := "shrinking persistent volumes is not supported"
51+
msg := fmt.Sprintf("shrinking persistent volumes is not supported (existing: %s, desired: %s)",
52+
existingCapacity.String(), desiredCapacity.String())
5253
logger.Error(errors.New("unsupported operation"), msg)
5354
return errors.New(msg)
5455
}

0 commit comments

Comments
 (0)