update storageconsumer api for centralising distribution and making it user facing #3062

leelavg · 2025-02-26T07:10:34Z

new fields with *Classes are intended to be filled by storage admin for distributing to connected client cluster that the storageconsumer represents. Only name is specified and later will be expanded if more config is required.

clientProfiles represents all the aliases that provider needs to send to client for a clientProfile which has RBD & SVG details that the client has access to.

nb-ohad · 2025-02-27T11:42:52Z

api/v1alpha1/storageconsumer_types.go

-	StorageQuotaInGiB int `json:"storageQuotaInGiB,omitempty"`
+	StorageQuotaInGiB          int         `json:"storageQuotaInGiB,omitempty"`
+	RadosNamespace             string      `json:"radosNamespace,omitempty"`
+	SubVolumeGroup             string      `json:"subVolumeGroup,omitempty"`


Should we call this SubVolumeGroup or CephFsSubVolumeGroup?

I picked these names to be consistent with clientprofile fields https://github.com/ceph/ceph-csi-operator/blob/main/api/v1alpha1/clientprofile_types.go

nb-ohad · 2025-02-27T11:43:42Z

api/v1alpha1/storageconsumer_types.go

+	StorageClasses             []ClassSpec `json:"storageClasses,omitempty"`
+	VolumeSnapshotClasses      []ClassSpec `json:"volumeSnapshotClasses,omitempty"`
+	VolumeGroupSnapshotClasses []ClassSpec `json:"volumeGroupSnapshotClasses,omitempty"`
+	ClientProfiles             []string    `json:"clientProfiles,omitempty"`


@leelavg I believe we agreed that ClientProfiles are not admin configurable. Why do we have this as part of the consumer spec?

I'm not sure, we decided to remove the secret (provisioner, node) aliases from spec but not clientProfiles.

Nevertheless, if we already know the storageconsumer upgrade mode we can figure these out, will update.

In one of the meetings, we were talking about having the clientProfile name something similar to storageID (ceph_FSID,radosnamespace) and (ceph_FSID,subvolumegroupName). If we are going with that shouldn't we have two different clientProfiles? one for rbd and another for cephfs?

This was in context when @Madhu-1 raised a point wrt MDR but it was decided that StorageConsumerUID will always be unique (for all intents & purposes) and could be used as clusterID and subsequently as clientProfile, isn't it?

This is all depends on the clientProfile naming scheme. Do we have a PR in place where we are calculating the clientProfile names?

no, didn't we settle w/ storageconsumer uid as they are unique? I don't remember choosing to calculating anything.

@leelavg @Madhu-1 @rewantsoni
Going forward we have a single scheme as discussed and agreed and captured on the weekly call meeting notes.
We will support the older schemes based on the indicator that marks an updated internal or updated provider.
In all cases, this is not an admin nob

api/v1alpha1/storageconsumer_types.go

openshift-ci · 2025-02-28T06:55:41Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: leelavg
Once this PR has been reviewed and has the lgtm label, please ask for approval from nb-ohad. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Madhu-1 · 2025-02-28T12:45:19Z

api/v1alpha1/storageconsumer_types.go

+	RadosNamespace             string                         `json:"radosNamespace,omitempty"`
+	SubVolumeGroup             string                         `json:"subVolumeGroup,omitempty"`


@nb-ohad should we also have this as object struct with name, so can we extend it later based on the requirements? thoughts?

I don't think so. The idea is to point particular consumer to a specific entity by identifying it.
Are you predicting that we need more then a name to uniquely identify these Ceph entities?

api/v1alpha1/storageconsumer_types.go

leelavg · 2025-03-05T12:12:18Z

/hold

Will squash after final review

nb-ohad · 2025-03-06T11:14:43Z

api/v1alpha1/storageconsumer_types.go

+	// +kubebuilder:validation:Optional
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
+	// +kubebuilder:validation:MaxLength=512
+	RadosNamespace string `json:"radosNamespace"`
+	// +kubebuilder:validation:Optional
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
+	// +kubebuilder:validation:MaxLength=512
+	SubVolumeGroup             string                         `json:"subVolumeGroup"`


The validation here does not make sense. This need to be configurable all the way until the Consumer is enabled. At that point we should freeze it.

But I question the need of adding these fields as part of this PR. Why do we need to couple these changes with the rest of the changes in this PR?

nb-ohad · 2025-03-06T11:15:52Z

api/v1alpha1/storageconsumer_types.go

+type RadosNamespaceSpec struct {
+	Name string `json:"name"`
+}
+
+type SubVolumeGroupSpec struct {
+	Name string `json:"name"`
+}
+
+type ClientProfileSpec struct {
+	Name string `json:"name"`
+}
+


I think you meant to have these with a Status safix

umangachapagain · 2025-03-07T05:57:43Z

api/v1alpha1/storageconsumer_types.go

+	// +kubebuilder:validation:Optional
+	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
+	// +kubebuilder:validation:MaxLength=512
+	SubVolumeGroup             string                         `json:"subVolumeGroup"`


Suggested change

SubVolumeGroup string `json:"subVolumeGroup"`

SubVolumeGroup string `json:"subVolumeGroup,omitempty"`

since it is Optional.

omitempty == // +kubebuilder:validation:Optional

Signed-off-by: Leela Venkaiah G <[email protected]>

leelavg force-pushed the consumer-api branch from 0ad3ebd to de21a76 Compare February 26, 2025 10:17

leelavg mentioned this pull request Feb 27, 2025

annotate storageconsumer with deployed mode and local to cluster #3068

Open

nb-ohad requested changes Feb 27, 2025

View reviewed changes

openshift-ci bot assigned nb-ohad Feb 27, 2025

leelavg force-pushed the consumer-api branch from de21a76 to d95bfc1 Compare February 28, 2025 06:55

rewantsoni mentioned this pull request Feb 28, 2025

storageconsumers: create the rns and subvolumegroup #3069

Open

Madhu-1 reviewed Feb 28, 2025

View reviewed changes

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 5, 2025

nb-ohad requested changes Mar 6, 2025

View reviewed changes

umangachapagain requested changes Mar 7, 2025

View reviewed changes

openshift-ci bot assigned umangachapagain Mar 7, 2025

leelavg force-pushed the consumer-api branch from 166a268 to 2542bdc Compare March 7, 2025 07:34

leelavg added 2 commits March 7, 2025 07:34

update storageconsumer api

2542bdc

Signed-off-by: Leela Venkaiah G <[email protected]>

update vendor changes

b6b77d2

Signed-off-by: Leela Venkaiah G <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update storageconsumer api for centralising distribution and making it user facing #3062

update storageconsumer api for centralising distribution and making it user facing #3062

leelavg commented Feb 26, 2025 •

edited

Loading

nb-ohad Feb 27, 2025

leelavg Feb 27, 2025

nb-ohad Feb 27, 2025

leelavg Feb 27, 2025

rewantsoni Feb 27, 2025

leelavg Feb 27, 2025

Madhu-1 Feb 28, 2025

leelavg Feb 28, 2025 •

edited

Loading

nb-ohad Mar 6, 2025

openshift-ci bot commented Feb 28, 2025

Madhu-1 Feb 28, 2025

nb-ohad Mar 6, 2025

leelavg commented Mar 5, 2025

nb-ohad Mar 6, 2025

leelavg Mar 7, 2025

nb-ohad Mar 6, 2025

leelavg Mar 7, 2025

umangachapagain Mar 7, 2025

leelavg Mar 7, 2025

		RadosNamespace string `json:"radosNamespace,omitempty"`
		SubVolumeGroup string `json:"subVolumeGroup,omitempty"`

update storageconsumer api for centralising distribution and making it user facing #3062

Are you sure you want to change the base?

update storageconsumer api for centralising distribution and making it user facing #3062

Conversation

leelavg commented Feb 26, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leelavg Feb 28, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci bot commented Feb 28, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leelavg commented Mar 5, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leelavg commented Feb 26, 2025 •

edited

Loading

leelavg Feb 28, 2025 •

edited

Loading