Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make CephFS plugin stateless reusing RADOS based journal scheme #390

Merged
merged 1 commit into from
Jun 7, 2019

Conversation

ShyamsundarR
Copy link
Contributor

This is a part of the stateless set of commits for CephCSI.

This commit removes the dependency on config maps to store cephFS provisioned
volumes, and instead relies on RADOS based objects and keys, and required
CSI VolumeID encoding to detect the provisioned volumes.

Changes

  • Provide backward compatibility to provisioned volumes by older plugin versions (1.0.0 or older)
  • Remove Create/Delete support for statically provisioned volumes (fixes Use static PV based provisioning for pre-provisioned CephFS volume support #382)
  • Added namespace support to RADOS OMaps and used the same to store RADOS CSI objects and keys in the CephFS metadata pool
  • Added support to mention fsname for CephFS provisioning (fixes Provide ability to select CephFS fsname to provision from in the StorageClass #359)
  • Changed field name in CSI Identifier to 'location', to denote a pool or fscid
  • Updated mounter cache to use new scheme
  • Required Helm manifests are updated
  • Required documentation and other manifests are updated
  • Made driver option 'metadatastorage' as optional, as fresh installs do not need to specify the same

Testing done

  • Create/Mount/Delete PVC
  • Create/Delete 5 PVCs
  • Mount version 1.0.0 PVC
  • Delete version 1.0.0 PV
  • Mount Statically defined PV/PVC/Pod
  • Mount Statically defined version 1.0.0 PV/PVC/Pod
  • Delete Statically defined version 1.0.0 PV/PVC/Pod
  • Node restart when mounted to test mountcache
  • Use InstanceID other than 'default'
  • RBD basic round of tests, as namespace is added to OMaps
  • csitest against ceph-fs plugin
    • NOTE: CephFS plugin still does not detect and address already created
      volumes but of a different size
  • Test not providing any value to the metadata storage parameter

Related issues

Fixes: #382, #359
Implements proposal as stated in #224

Future concerns

  • volumeIDMutex lock can be deleted as it is not providing any further value for retaining idempotent nature of requests, both in RBD and CephFS plugin implementations. Some more study is required before taking this action though.
  • Like missing subvolumes are treated as a success during delete, missing per subvolume credentials should also be treated as a success during deletes and further deletion steps should be followed
  • CephFS plugin should detect subvolume feature compatibility when there are request name conflicts and respond with a success on matches or an error AlreadyExists on mismatches.
  • Need to take a hard re-look at the mount cache, as it is storing secrets in config maps and that can be a violation of trust and have security implications.

Signed-off-by: ShyamsundarR srangana@redhat.com

This is a part of the stateless set of commits for CephCSI.

This commit removes the dependency on config maps to store cephFS provisioned
volumes, and instead relies on RADOS based objects and keys, and required
CSI VolumeID encoding to detect the provisioned volumes.

Changes:
- Provide backward compatibility to provisioned volumes by older plugin versions (1.0.0 or older)
- Remove Create/Delete support for statically provisioned volumes (fixes ceph#382)
- Added namespace support to RADOS OMaps and used the same to store RADOS CSI objects and keys in the CephFS metadata pool
- Added support to mention fsname for CephFS provisioning (fixes ceph#359)
- Changed field name in CSI Identifier to 'location', to denote a pool or fscid
- Updated mounter cache to use new scheme
- Required Helm manifests are updated
- Required documentation and other manifests are updated
- Made driver option 'metadatastorage' as optional, as fresh installs do not need to specify the same

Testing done:
- Create/Mount/Delete PVC
- Create/Delete 5 PVCs
- Mount version 1.0.0 PVC
- Delete version 1.0.0 PV
- Mount Statically defined PV/PVC/Pod
- Mount Statically defined version 1.0.0 PV/PVC/Pod
- Delete Statically defined version 1.0.0 PV/PVC/Pod
- Node restart when mounted to test mountcache
- Use InstanceID other than 'default'
- RBD basic round of tests, as namespace is added to OMaps
- csitest against ceph-fs plugin
  - NOTE: CephFS plugin still does not detect and address already created
  volumes but of a different size
- Test not providing any value to the metadata storage parameter

Signed-off-by: ShyamsundarR <srangana@redhat.com>
@ShyamsundarR ShyamsundarR requested review from Madhu-1 and humblec May 30, 2019 10:32
@ShyamsundarR
Copy link
Contributor Author

Attention: @ajarr @poornimag

For moving forward with #328 and any reviews that can be provided, thanks.

@gman0
Copy link
Contributor

gman0 commented May 30, 2019

Please keep in mind that having the ability to mount pre-provisioned volumes with having access only to that particular volume is absolutely crucial for things like shares managed by OpenStack Manila to work. There are cases where the operators may have limited direct access to the ceph cluster.

NodeStageVolume should be able to get by with {mons, path to the directory} in VolumeContext and ceph user secret in Secret. Could be done as a fallback option perhaps

@ShyamsundarR
Copy link
Contributor Author

Please keep in mind that having the ability to mount pre-provisioned volumes with having access only to that particular volume is absolutely crucial for things like shares managed by OpenStack Manila to work. There are cases where the operators may have limited direct access to the ceph cluster.

NodeStageVolume should be able to get by with {mons, path to the directory} in VolumeContext and ceph user secret in Secret. Could be done as a fallback option perhaps

I have already provided the option for static PVs in the NodeStageVolume code with this PR and this can be traced from here.

I still need to add documentation and possibly example yaml for the same, to help with detailing how this would look.

For my testing I used this manifest and should serve as an example to understand if this satisfies your request. For example it avoids monitors in the VolumeContext and uses the ceph-csi-config config map, and should help with changing MONs more easily even for static PVs. @gman0 Thoughts?

@humblec humblec merged commit 95252dd into ceph:master Jun 7, 2019
@ShyamsundarR ShyamsundarR deleted the stateless-cephfs branch June 12, 2019 19:04
wilmardo pushed a commit to wilmardo/ceph-csi that referenced this pull request Jul 29, 2019
Make CephFS plugin stateless reusing RADOS based journal scheme
yati1998 pushed a commit to yati1998/ceph-csi that referenced this pull request Nov 26, 2024
Syncing latest changes from devel for ceph-csi
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants