search

ceph / ceph-csi

CSI driver for Ceph
Apache License 2.0
1.19k stars 528 forks source link

Support storing cephfs omap in a different namespace in rados under same filesystem metadata pool #4599

Open Madhu-1 opened 1 month ago

Madhu-1 commented 1 month ago

Describe the feature you'd like to have

Support storing cephfs omap in a different namespace in rados under the same filesystem metadata pool

What is the value to the end user? (why is it a priority?)

With this feature, even the metadata of the pvc/subvolumes will be stored under a different namespace, where security/isolation is the priority the user/customers can configure csi to use a different namespace.

How will we know we have a good solution? (acceptance criteria)

We can have a new field in the configmap where we have an entry for the subvolumegroup name.

zerotens commented 1 month ago

Hello together, funny story. I wrote today a PR, but i didn't see the issue and that @iPraveenParihar was already working on it. https://github.com/ceph/ceph-csi/compare/devel...zerotens:cephfs-namespace?expand=1

Is this reasonable to you or do you really want to make it configurable via configmap which would need many function calls as the constant is used in many places.

iPraveenParihar commented 3 weeks ago

@zerotens, Thanks for the PR 😃 I had already started working on this issue and was testing the same, and have opened a PR now #4661

Is this reasonable to you or do you really want to make it configurable via configmap which would need many function calls as the constant is used in many places.

This issue actually needs radosNamespace option in the csi ConfigMap as this will allow users to use different radosNamespace for different clusterID, similar to what we have in RBD config section.

However, your PR allow users to overwrite the default radosNamespace which at the point is not the requirement I guess. @Rakshith-R, do you think overwriting the default radosNamespace via flag will be useful?

Rakshith-R commented 3 weeks ago

@zerotens, Thanks for the PR 😃 I had already started working on this issue and was testing the same, and have opened a PR now #4661

Is this reasonable to you or do you really want to make it configurable via configmap which would need many function calls as the constant is used in many places.

This issue actually needs radosNamespace option in the csi ConfigMap as this will allow users to use different radosNamespace for different clusterID, similar to what we have in RBD config section.

However, your PR allow users to overwrite the default radosNamespace which at the point is not the requirement I guess. @Rakshith-R, do you think overwriting the default radosNamespace via flag will be useful?

Making it configurable via configmap allows us to implement multi tenancy with the same ceph filesystem so it is definitely required as praveen mentioned.

But if you would like to have an option to pass it through cli arguement and change the default, that would be welcome as well.

zerotens commented 3 weeks ago

@zerotens, Thanks for the PR 😃 I had already started working on this issue and was testing the same, and have opened a PR now #4661

Is this reasonable to you or do you really want to make it configurable via configmap which would need many function calls as the constant is used in many places.

This issue actually needs radosNamespace option in the csi ConfigMap as this will allow users to use different radosNamespace for different clusterID, similar to what we have in RBD config section. However, your PR allow users to overwrite the default radosNamespace which at the point is not the requirement I guess. @Rakshith-R, do you think overwriting the default radosNamespace via flag will be useful?

Making it configurable via configmap allows us to implement multi tenancy with the same ceph filesystem so it is definitely required as praveen mentioned.

Okay, i agree if the user needs to access multiple ceph clusters from 1 kubernetes cluster.

But if you would like to have an option to pass it through cli arguement and change the default, that would be welcome as well.

Well depending on the usecase i would say it would be enough to just configure it at the configmap level.

I have seen the draft PR of @iPraveenParihar. There are still many fsutil.RadosNamespace constant access calls in the PR https://github.com/ceph/ceph-csi/pull/4661 For example: https://github.com/iPraveenParihar/ceph-csi/blob/fd4e89ed004357c97e7c528af72c6f3e7010c713/internal/cephfs/driver.go#L125 https://github.com/iPraveenParihar/ceph-csi/blob/fd4e89ed004357c97e7c528af72c6f3e7010c713/internal/cephfs/driver.go#L131

I would feel much safer if the constant would be configurable. Any future code change which could rely on this constant could break the support of storing csi metadata in a different namespace. If everything get's refactored to calling the new function GetCephFSRadosNamespace instead of relying on the constant to retrieve the namespace, my PR would be obsolete.

Otherwise i would still like to follow up on the constant to cli configurable global variable.