Changing storageClassName in CR does not change the PVC resource

[hoyhbx]

Describe the bug

I was trying to change the persistence/storageClassName field in my rabbitmq-cluster's CR, but changing the persistence/storageClassName has no effect on the PVC used by the statefulSet.
The persistence/storageClassName field was initially not specified so the operator used the default storage class "standard". Then I created a new storage class following instruction here https://github.com/rabbitmq/cluster-operator/blob/main/docs/examples/production-ready/ssd-gke.yaml, and changed persistence/storageClassName from null to ssd. This change failed silently.

To Reproduce

Steps to reproduce this behavior:

1. Deploy cluster-operator using the command:
   kubectl apply -f https://github.com/rabbitmq/cluster-operator/releases/latest/download/cluster-operator.yml
2. Deploy rabbitmqCluster with the following yaml file:

apiVersion: rabbitmq.com/v1beta1
kind: RabbitmqCluster
metadata:
  name: sample
spec:
  image: "rabbitmq:3.8.21-management"
  persistence:
    storage: 20Gi
  replicas: 2

3. Create new storage class:
   kubectl apply -f https://github.com/rabbitmq/cluster-operator/blob/main/docs/examples/production-ready/ssd-gke.yaml
4. Change the rabbitmqCluster's persistence/storageClassName to ssd and apply

apiVersion: rabbitmq.com/v1beta1
kind: RabbitmqCluster
metadata:
  name: sample
spec:
  image: "rabbitmq:3.8.21-management"
  persistence:
    storage: 20Gi
    storageClassName: ssd
  replicas: 2

5. Check the PVC used by rabbitmq cluster, the storageClassName change is not applied. It's still "standard" instead of "ssd":

"spec": {
    "accessModes": [
        "ReadWriteOnce"
    ],
    "resources": {
        "requests": {
            "storage": "20Gi"
        }
    },
    "storageClassName": "standard",
    "volumeMode": "Filesystem",
    "volumeName": "pvc-43d66309-2090-4de3-bc82-9edcd0a69361"
}

Expected behavior
The PVC with the desired storage class being created to replace the old PVC.

Screenshots

If applicable, add screenshots to help explain your problem.

Version and environment information

• rabbitmq: rabbitmq:3.8.21-management
• rabbitmq cluster operator: rabbitmqoperator/cluster-operator:1.10.0
• Kubernetes: 1.21.1
• Running on Kind cluster

Addition context

This bug is caused because the operator only reconciles the PVC's storage capacity, but does not reconcile the storageClassName here:

cluster-operator/controllers/reconcile_persistence.go

Line 15 in d657ffb

func (r *RabbitmqClusterReconciler) reconcilePVC(ctx context.Context, rmq *rabbitmqv1beta1.RabbitmqCluster, desiredSts *appsv1.StatefulSet) error {

.

Possible fix is to create the desired storage type and migrate the data over. Or report an error message like PVC scaling down here:

cluster-operator/internal/scaling/scaling.go

Line 51 in d657ffb

msg := "shrinking persistent volumes is not supported"

[Zerpet]

The underlying StatefulSet resource does not allow modifications of the PVC template, therefore, a change in RabbitMQ's spec shouldn't be allowed either. Perhaps the easiest solution would be implement a validating webhook.

[hoyhbx]

Thanks for the response! I think using a validating webhook for this field makes sense if you decide not to support this functionality

[mkuratczyk]

I think it'd be an interesting feature at some point but it's not a simple one. It'd require a new statefulset with new PVCs with new PVs and transferring data from old volumes to the new volumes (or perhaps a blue-green migration on the RabbitMQ level). And on top of that - all the possible issues that can occur in the process (failures, out of disk issues, etc). Here's a blog post explaining how to do that for Percona MySQL https://www.percona.com/blog/2021/04/20/change-storage-class-on-kubernetes-on-the-fly/ to give an idea

[ChunyiLyu]

@hoyhbx Just as Michal commented above, allowing updating storage class name is rather complicated and underlaying infrastructure will play a factor . The team has decided not to implement this feature for the time being.
As for a validating webhook, this is an invalid action from the statefulSet controller directly. Reconcile error will show up in operator logs and in the status&events of the RabbitmqCluster object. The operator currently does not have validating webhook yet, and error logs and events are good enough to surface this issue. I will close this issue for now.

[hoyhbx]

Hi all @ChunyiLyu @Zerpet @mkuratczyk , Kubernetes recently is having a new feature support x-kubernetes-validations in the CRD. You can write CEL expression, which supports implementing immutability. This feature has become alpha in 1.23, and on track to become beta in 1.25 which is releasing in few weeks. controller-gen now also supports generating x-kubernetes-validations via marker.

This feature enables us to reject the storageClassName change elegantly without implementing a validating webhook, we can add a marker to the field in types.go:
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="StorageClass is immutable"

We can create a PR fixing this using x-kubernetes-validations if you agree. Thanks