api: add radosNamespace field to CephFsConfigSpec #165
Conversation
iPraveenParihar
force-pushed
the
api/cephfs-radosnamespace
branch
from
3826f7a to
0cc6218
Compare
iPraveenParihar
force-pushed
the
api/cephfs-radosnamespace
branch
from
0cc6218 to
4933281
Compare
Ceph-CSI now supports storing CephFS OMAP data in a specified namespace defined in CephFS.radosNamespace field in ceph-csi-config ConfigMap. ref: ceph/ceph-csi#4661. This commit introduces the same by adding the radosNamespace field to the ClientProfile.ClientProfileSpec.CephFsConfigSpec API. Signed-off-by: Praveen M <[email protected]>
Signed-off-by: Praveen M <[email protected]>
Signed-off-by: Praveen M <[email protected]>
Signed-off-by: Praveen M <[email protected]>
iPraveenParihar
force-pushed
the
api/cephfs-radosnamespace
branch
from
4933281 to
0154232
Compare
|
|
||
| //+kubebuilder:validation:XValidation:rule="self == oldSelf",message="radosNamespace is immutable" | ||
| //+kubebuilder:validation:Optional | ||
| RadosNamespace string `json:"radosNamespace,omitempty"` |
There was a problem hiding this comment.
The usage of a Rados namespace for CephFs is sepcificly for OMAP data. The current proposed name for this field does not reflects that. I would suggest the following instead:
| RadosNamespace string `json:"radosNamespace,omitempty"` | |
| OmapNamespace string `json:"OmapNamespace,omitempty"` |
There was a problem hiding this comment.
OMAP are RADOS objects so the term radosNamespace would be more appropriate?
There was a problem hiding this comment.
@iPraveenParihar
I am not sure I agree with the lack of distinction. You want the user to specify a rados namespace for the location of the omap data for the cephfs usage. Not just a general rados namespace for general use.
What I am saying is that the name should capture that
Maybe we could agree on OmapRadosNamespace as an alternative?
There was a problem hiding this comment.
It's radosNamespace in cephcsi, let's keep it the same.
Currently it may well be only used for omap usage but it might change in the future.
There was a problem hiding this comment.
User-facing API design is essential for me, there is already past acknowledgment that the Ceph CSI project did not select good names for some of its front-facing configurations. Bringing them as an example does not make a good case.
Regarding the other part of the comment:
- we are talking about CephFS, not RBD. RDS is the internal mechanism we use to isolate the omap data. It does not capture the value/intent of using the feature/nob
- Using the same field name as we have inside the RBD config section is misleading because in each section the nob had completely different semantics and effects.
- Using RadosNamesapce as the nob name will collide with any future nobs that we might need to allow configuration of additional RDS for other metadata the cephcsi or chepfs will allow us to use. We want future proofing against such a case.
Given the above 3 points, we should go with something like OmapRadosNamaspece which captures both the intent of the nob and the entity type this nob is referring to
There was a problem hiding this comment.
User-facing API design is essential for me, there is already past acknowledgment that the Ceph CSI project did not select good names for some of its front-facing configurations. Bringing them as an example does not make a good case.
I completely disagree with the statement Ceph CSI project did not select good names.
You can always open issues at cephcsi to suggest renaming them.
Changing such conventions in above layers only causes confusion.
Regarding the other part of the comment:
- we are talking about CephFS, not RBD. RDS is the internal mechanism we use to isolate the omap data. It does not capture the value/intent of using the feature/nob
- Using the same field name as we have inside the RBD config section is misleading because in each section the nob had completely different semantics and effects.
- Using RadosNamesapce as the nob name will collide with any future nobs that we might need to allow configuration of additional RDS for other metadata the cephcsi or chepfs will allow us to use. We want future proofing against such a case.
Given the above 3 points, we should go with something like OmapRadosNamaspece which captures both the intent of the nob and the entity type this nob is referring to
RadosNamespace is the key we are using for cephfs section in cephcsi, we should keep it the same in cephcsi Operator. It implies cephcsi will use this radosNS for operations, right now it maybe limited to just omap but might change in the future.
If you would like more specific naming, please open issue in cephcsi, we'll change it there, then in Operator. I would prefer this approach instead of introducing new names abruptly in above layers.
Future additional cases can use another specific key.
There was a problem hiding this comment.
@Madhu-1, can you please provide your inputs here?
| @@ -31,10 +31,15 @@ type CephFsConfigSpec struct { | |||
|
|
|||
| //+kubebuilder:validation:Optional | |||
| FuseMountOptions map[string]string `json:"fuseMountOptions,omitempty"` | |||
|
|
|||
| //+kubebuilder:validation:XValidation:rule="self == oldSelf",message="radosNamespace is immutable" | |||
There was a problem hiding this comment.
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in two weeks if no further activity occurs. Thank you for your contributions. |
Describe what this PR does
Ceph-CSI now supports storing CephFS OMAP data in a
specified namespace defined in CephFS.radosNamespace
field in ceph-csi-config ConfigMap.
ref: ceph/ceph-csi#4661.
This commit introduces the same by adding the radosNamespace field
to the ClientProfile.ClientProfileSpec.CephFsConfigSpec API.
Checklist: