Support storing cephfs omap in a different namespace in rados under same filesystem metadata pool #4652
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zerotens
force-pushed
the
cephfs-namespace
branch
from
de2a831
to
23fe029
Compare
zerotens
changed the title
nixpanic
added
component/cephfs
Add cli argument --radosnamespacecephfs to override radosNamespace "csi" used to store CSI specific objects and keys. Signed-off-by: Andreas <zerotens@users.noreply.github.com>
Add --instanceid / --radosnamespacecephfs for nfs component as it is also using these two variables from cephfs code. Signed-off-by: Andreas <zerotens@users.noreply.github.com>
Add --radosnamespacecephfs cli argument to docs Signed-off-by: Andreas <zerotens@users.noreply.github.com>
Add --radosnamespacecephfs cli argument to helm chart cephfs Add --instance cli argument to helm chart cephfs and rbd Signed-off-by: Andreas <zerotens@users.noreply.github.com>
zerotens
force-pushed
the
cephfs-namespace
branch
from
23fe029
to
ebaa395
Compare
|
@nixpanic Linting was hopefully fixed |
Rakshith-R
reviewed
Jun 5, 2024
Comment on lines
+61
to
+63
| {{- if .Values.instanceID }} | ||
| - "--instanceid={{ .Values.instanceID }}" | ||
| {{- end }} |
There was a problem hiding this comment.
@zerotens
Can you please open another for instanceID change ?
There was a problem hiding this comment.
@Rakshith-R
I made an extra PR for this.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe what this PR does
Fixes #4599
This PR adds a new cli argument to let users override the current hardcoded radosNamespace "csi" used in CephFS.
ceph-csi/internal/cephfs/util/util.go
Line 24 in ce3ec6a
Needed is this for multitenant environments where one ceph cluster with one cephfs filesystem is used by multiple kubernetes clusters.
This allows storing the state of every cephfs instance / kubernetes cluster in their own radosNamespaces and allows finer controlled access for cephfs.
This PR also makes those two needed parameters --instanceid (rbd & cephfs Helm Chart) and --radosnamespacecephfs (cephfs Helm Chart) configurable via helm chart.
Is there anything that requires special attention
Is the change backward compatible?
Yes
Are there concerns around backward compatibility?
No, the default values in code have not been changed but are now overrideable via helm and cli arguments.
Future concerns
Depending on future usage of the NFS Controller, currently the NFS controller is using the CephFS variables cephfs.CSIInstanceID and fsutil.RadosNamespace which is now overrideable with the cli argument
--radosnamespacecephfsand--instanceid.ceph-csi/internal/nfs/controller/controllerserver.go
Line 48 in ce3ec6a
ceph-csi/internal/nfs/controller/controllerserver.go
Line 49 in ce3ec6a
ceph-csi/internal/nfs/controller/volume.go
Line 291 in ce3ec6a
ceph-csi/internal/nfs/controller/volume.go
Line 329 in ce3ec6a
Testing
As i don't have a nice way of testing it and this is my first PR and go code for some time i would appreciate an detailed review.