Move the NFS server from the storageclass to the configmap

[Madhu-1]

Currently NFS SC contains the details of the server https://github.com/ceph/ceph-csi/blob/66b3e23928a7e97890bda1e7f89cca6be4a150d3/examples/nfs/storageclass.yaml#L13. Once the value is set it goes to the PV object and becomes immutable. There can be cases like the NFS server is running on a host network and the NFS is required to move to a different host or different IP, There should be an easy option to update the details and restart the application pods. It will be hard to delete and recreate the PV in certain cases.

cc @nixpanic @Rakshith-R WDYT?

[nixpanic]

This should not be in a configmap, but it should become a mutable parameter to stay aligned with the CSI specification.

https://github.com/container-storage-interface/spec/blob/master/spec.md#controllermodifyvolume

[Madhu-1]

yes sound good to put it in the VolumeAttributeClass, we need to store it in the metadata on the subvolume instead of the volume Context(volumeAttributes in the PV)

[Madhu-1]

https://github.com/ceph/ceph-csi/issues/4662 is added to support the attributeclass

[Rakshith-R]

yes sound good to put it in the VolumeAttributeClass, we need to store it in the metadata on the subvolume instead of the volume Context(volumeAttributes in the PV)

This should not be in a configmap, but it should become a mutable parameter to stay aligned with the CSI specification.

https://github.com/container-storage-interface/spec/blob/master/spec.md#controllermodifyvolume

Parameters inside VAClass are immutable.

The users have to create new volumeAttributeClass and change it for each PVC manually https://kubernetes.io/docs/concepts/storage/volume-attributes-classes/#the-volumeattributesclass-api

I think configmap is a better option.

[Madhu-1]

is VAC feature enabled by default, it is in beta state and disabled by default. its going to GA in 1.34

[nixpanic]

@Rakshith-R

The users have to create new volumeAttributeClass and change it for each PVC manually

Users can not create a VolumeAttributeClass, that can only be done by admins. The same counts for a ConfigMap for Ceph-CSI.

By using a VolumeAttributeClass for the NFS-servername users are still somewhat in control of when to move to an other NFS-server. This would be more beneficial for managing application uptime and maintenance windows. Also, it is thinkable that there are multiple functional NFS-servers in any case, possibly depending on the region, or even related to performance tuning in the NFS-server itself.

For options like this, I think it is more appropriate to introduce ControllerModifyVolume instead of a global Ceph-CSI configuration option.

[Madhu-1]

@nixpanic i think the only downside is that if there a change in the server details, a new VAC need to be created(re). with the VAC is that all the PVC need to be updated to a new VAC, This will become two step process 1. Admin create the VAC and all the application owners need to update PVC for the new VAC (i havent tested it thought). with configmap we have this advantage where the csi configuration lives in one place and its source of truth for this kind of mutable objects.

[nixpanic]

There is always a 2-step process when changing the NFS-server:

1. configure the new server somewhere (configmap or VolumeAttributeClass)
2. apply the configuration to the volumes (requires at least restarting Pods)

Having the admin take care of 1, and the user do 2, is pretty common in migration scenarios. Users usually get a deadline until they can apply the changes (and restart pods) themselves, otherwise the admin applies it.

Changing the NFS-server underneath and causing unexpected interruptions of apps that are managed by users, isn't a very friendly approach. It also doesn't allow for a multi-tier (location or performance wise) configuration of NFS-servers.

[github-actions[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.