WIP: Set object lock for volumes for cephfs encryption
Describe what this PR does
Provide some context for the reviewer
Is there anything that requires special attention
Do you have any questions?
Is the change backward compatible?
Are there concerns around backward compatibility?
Provide any external context for the change, if any.
For example:
- Kubernetes links that explain why the change is required
- CSI spec related changes/catch-up that necessitates this patch
- golang related practices that necessitates this change
Related issues
Mention any github issues relevant to this PR. Adding below line will help to auto close the issue once the PR is merged.
Fixes: #issue_number
Future concerns
List items that are not part of the PR and do not impact it's functionality, but are work items that can be taken up subsequently.
Checklist:
- [ ] Commit Message Formatting: Commit titles and messages follow guidelines in the developer guide.
- [ ] Reviewed the developer guide on Submitting a Pull Request
- [ ] Pending release notes updated with breaking and/or notable changes for the next major release.
- [ ] Documentation has been updated, if necessary.
- [ ] Unit tests have been added, if necessary.
- [ ] Integration tests have been added, if necessary.
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 unrelated failure (please report the failure too!)
@Sunnatillo can you please check the CI failures?
@mergifyio rebase
rebase
❌ Pull request can't be updated with latest base branch changes
Mergify needs the author permission to update the base branch of the pull request. @Nordix needs to authorize modification on its head branch.
Thank you for reviews @nixpanic, @Madhu-1.
I rebased to latest devel branch.
/test ci/centos/k8s-e2e-external-storage/1.30
/test ci/centos/k8s-e2e-external-storage/1.29
/test ci/centos/k8s-e2e-external-storage/1.27
/test ci/centos/mini-e2e-helm/k8s-1.30
/test ci/centos/mini-e2e-helm/k8s-1.29
/test ci/centos/mini-e2e-helm/k8s-1.27
/test ci/centos/mini-e2e/k8s-1.30
/test ci/centos/mini-e2e/k8s-1.29
/test ci/centos/mini-e2e/k8s-1.27
/test ci/centos/k8s-e2e-external-storage/1.28
/test ci/centos/mini-e2e-helm/k8s-1.28
/test ci/centos/mini-e2e/k8s-1.28
/test ci/centos/upgrade-tests-cephfs
/test ci/centos/upgrade-tests-rbd
@nixpanic, I see fscrypt.Unlock call for RBD fscrypt. Do we need to do the same here as well?
do we need to
https://github.com/ceph/ceph-csi/blob/e71a95fece24de8de8f1de51201a7314af4f3480/internal/rbd/nodeserver.go#L467-L474
@nixpanic, I see
fscrypt.Unlockcall for RBD fscrypt. Do we need to do the same here as well?
No, for RBD that is not needed. RBD with a filesystem is never attached to more than one workernode at the same time, so initialization/unlocking of the filesystem will always be on a single node. It is different with ReadWriteMany RBD volumes in block-mode with encryption. The LUKS calls would benefit from a similar locking.
Is there a config to use the lock? We built an image from devel branch (https://github.com/ceph/ceph-csi/tree/devel) and tried it out. However pods failed to come up and we got the following error:
Warning FailedMount 15s (x6 over 31s) kubelet MountVolume.MountDevice failed for volume "pvc-4642810a-7f00-4ae0-8583-ddbcee6ed314" : rpc error: code = Internal desc = Failed to lock volume ID 0001-0009-rook-ceph-00000000000000ad-d71d2949-db34-434e-8008-5497dad5c70b: rados: ret=-1, Operation not permitted
I have tested this and encountered the same error as @z2000l. @Sunnatillo, could you please document any additional settings required or anything we might be overlooking?