nfs: implement ControllerModifyVolume to change NFS-server hostname#5829
Conversation
d006863 to
1a11979
Compare
Still trying to add some e2e tests, the csi-provisioner and csi-resizer sidecars have just gained the ability to pass credentials to the |
1a11979 to
ca55fc1
Compare
Pull request has been modified.
|
/test ci/centos/mini-e2e-helm/k8s-1.34/nfs |
|
/test ci/centos/mini-e2e/k8s-1.34/nfs |
|
/test ci/centos/mini-e2e/k8s-1.34 |
Verified 😜 that it fails with this: |
ca55fc1 to
fe5d071
Compare
|
/test ci/centos/mini-e2e/k8s-1.34 |
2 similar comments
|
/test ci/centos/mini-e2e/k8s-1.34 |
|
/test ci/centos/mini-e2e/k8s-1.34 |
|
/test ci/centos/mini-e2e/k8s-1.34/cephfs |
0215938 to
fc84b94
Compare
|
/test ci/centos/mini-e2e/k8s-1.34/cephfs |
fc84b94 to
df7f5dd
Compare
|
/test ci/centos/mini-e2e/k8s-1.34/cephfs |
df7f5dd to
0f12bea
Compare
|
/test ci/centos/mini-e2e/k8s-1.34/cephfs |
|
Still trying to use the old nfs-server: |
|
It seems I missed handling of |
0f12bea to
dc81558
Compare
|
/test ci/centos/mini-e2e/k8s-1.34/cephfs |
1 similar comment
|
/test ci/centos/mini-e2e/k8s-1.34/cephfs |
dc81558 to
8899dcc
Compare
|
@Madhu-1 , NFS (and CephFS) uses admin credentials everywhere. Just using user credentials for ControllerModifyVolume and NodePublishVolume isn't straight forward. Reverted to admin credentials again, so that things at least work again. |
|
This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏 |
|
@nixpanic lets address the merge conflict and have a tracker to address the admin creds |
Signed-off-by: Niels de Vos <ndevos@ibm.com>
All functions use a fail-early-continue-on-success flow, NodeGetVolumeStats was not following this. Signed-off-by: Niels de Vos <ndevos@ibm.com>
Signed-off-by: Niels de Vos <ndevos@ibm.com>
Signed-off-by: Niels de Vos <ndevos@ibm.com>
The new hostname of the NFS-server is stored in a journal entry for the volume. The new NFS-server will only be used the next time the NFS Volume is mounted on a worker node. It is not possible to update the VolumeContext of a Volume, so the old hostname of the NFS=server will be stored there forever. Closes: ceph#5420 Signed-off-by: Niels de Vos <ndevos@ibm.com>
Signed-off-by: Niels de Vos <ndevos@ibm.com>
|
/test ci/centos/mini-e2e/k8s-1.35 |
Signed-off-by: Niels de Vos <ndevos@ibm.com>
Signed-off-by: Niels de Vos <ndevos@ibm.com>
Only the canary version suppors the VolumeAttributesClass with secrets in the ControllerModifyVolume gRPC. Signed-off-by: Niels de Vos <ndevos@ibm.com>
Signed-off-by: Niels de Vos <ndevos@ibm.com>
The NFS node-plugin does not support staging, it only uses publishing. For VolumeAttributeClass / ControllerModifyVolume the mutable parameters are stored in the Ceph backend. This means that during volume publishing the node-plugin needs to get the updated parameters from the Ceph cluster (hence the publish secret requirement). /tmp/csi has been added to the node-plugin Pod so that the temporary credentials file can be written. Signed-off-by: Niels de Vos <ndevos@ibm.com>
Signed-off-by: Niels de Vos <ndevos@ibm.com>
Only Kubernetes 1.34 and newer support VolumeAttributeClass functionality (GA). Ceph-CSI also depends on certain versions of the Kubernetes CSI external-provisioner and external-resizer. See-also: kubernetes-csi/docs#631 Signed-off-by: Niels de Vos <ndevos@ibm.com>
|
aaah! after renaming the VAC example, there was still a typo somewhere 😢 |
|
/test ci/centos/mini-e2e/k8s-1.35 |
|
@Madhu-1 and @Rakshith-R , this is now really ready for approval. The CI job is still running, but the NFS tests have passed. |
|
@Mergifyio requeue |
✅ This pull request will be embarked automaticallyDetailsThe head sha of this pull request, b4565bd, was never embarked in the merge queue. But don't worry, Mergify will embark it automatically for you. |
✅ The pull request has been merged automaticallyDetailsThe pull request has been merged automatically at 94c5e36 |
Merge Queue Status✅ The pull request has been merged at b4565bd This pull request spent 3 hours 5 minutes 16 seconds in the queue, including 3 hours 4 minutes 56 seconds running CI. Required conditions to merge
|
The new hostname of the NFS-server is stored in a journal entry for the
volume. The new NFS-server will only be used the next time the NFS
Volume is mounted on a worker node.
It is not possible to update the VolumeContext of a Volume, so the old
hostname of the NFS-server will be stored there forever.
Show available bot commands
These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:
/retest ci/centos/<job-name>: retest the<job-name>after unrelatedfailure (please report the failure too!)