[cinder-csi-plugin] csi-cinder storage capacity #2597

sergelogvinov · 2024-05-16T05:23:22Z

Available capacity of disk storage

What this PR does / why we need it:

To ensure Kubernetes scheduler selects the optimal zone/region based on available disk capacity.
Also to have statistics/alerts in cluster for the specific storageClass

Which issue this PR fixes(if applicable):
for #2035, #2551

Special notes for reviewers:

kubectl get csistoragecapacities -ocustom-columns=CLASS:.storageClassName,AVAIL:.capacity,ZONE:.nodeTopology.matchLabels -A
CLASS                    AVAIL     ZONE
csi-cinder-classic-xfs   19990Gi   map[topology.cinder.csi.openstack.org/zone:nova]

Release note:

required documentation update/helm-chart/migration process

NONE

k8s-ci-robot · 2024-05-16T05:23:31Z

Hi @sergelogvinov. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-ci-robot · 2024-05-16T05:23:35Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign jichenjc for approval. For more information see the Kubernetes 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

jichenjc · 2024-05-16T06:20:54Z

/ok-to-test

sergelogvinov · 2024-05-16T10:32:29Z

/retest

dulek · 2024-05-16T14:08:03Z

pkg/csi/cinder/openstack/openstack_volumes.go

@@ -396,6 +397,16 @@ func (os *OpenStack) GetMaxVolLimit() int64 {
 	return defaultMaxVolAttachLimit
 }

+// GetFreeCapacity returns free capacity of the block storage


Would be nice to say that result is expected in bytes. Which is also a bit weird to me.

fixed, changed to Gb

dulek · 2024-05-16T14:12:43Z

pkg/csi/cinder/openstack/openstack_volumes.go

@@ -396,6 +397,16 @@ func (os *OpenStack) GetMaxVolLimit() int64 {
 	return defaultMaxVolAttachLimit
 }

+// GetFreeCapacity returns free capacity of the block storage
+func (os *OpenStack) GetFreeCapacity() (int64, error) {
+	res, err := volumelimit.Get(os.blockstorage).Extract()


I'm not 100% sure, but this sounds like an admin API. We can't make admin calls from CSIs, all CPO is supposed to work normally on a regular tenant.

https://docs.openstack.org/api-ref/block-storage/v3/#limits-limits i did not find the role/permission list for csi-cinder.
I think this feature should be optional.

Can you confirm it with DevStack and demo tenant?

OVH Cloud, openstack account has computeOperator, volumeOperator permission, here doc: https://help.ovhcloud.com/csm/en-public-cloud-authenticate-api-openstack-service-account?id=kb_article_view&sysparm_article=KB0059364

but i do not know/can get the real openstack permission of it.
If you have a permission list of DevStack - i'd like to check...

I've checked that the API is available. What puzzles me now is - how often is GetCapacity() called? Would calling this API be a significant blow on the Cinder API in the cloud?

/ok-to-test

it calls 1/min (capacity-poll-interval flag of csi-provisioner https://github.com/kubernetes-csi/external-provisioner/blob/c7f94435bd29e49edf1af0eda9c4ee2907c4d160/cmd/csi-provisioner/csi-provisioner.go#L111) for each storageClass and accessibleTopology (av zone).

flag enable-capacity by default is off, so csi-provisioner does not collect this metrics by default.

sergelogvinov · 2024-05-25T18:44:15Z

/retest-required

sergelogvinov · 2024-05-25T19:31:54Z

/retest-required

sergelogvinov · 2024-05-27T10:05:07Z

/retest-required

jichenjc · 2024-05-28T06:48:41Z

pkg/csi/cinder/openstack/openstack_volumes.go

@@ -396,6 +397,21 @@ func (os *OpenStack) GetMaxVolLimit() int64 {
 	return defaultMaxVolAttachLimit
 }

+// GetFreeCapacity returns free capacity of the block storage, in GB


at least this is not my understanding
the free storage usually comes from backend and I don't know whether every storage provider has this

and the API seems from here https://github.com/openstack/cinder/blob/master/cinder/api/v3/limits.py#L25 ?
if so ,it's actually the quota, not the real storage ? e.g you can have 100G max NFS pool
but you can set quota to 1T ?

Yep, this is a quota.
Based on this quota, I cannot allocate more disk space as I am not the owner of the OpenStack cluster.

These metrics provide the scheduler with additional information about which availability zone the pod can be scheduled in.

um... at least the CSI GetFreeSpace is expecting to give real free space?
this might confuse a little bit

and if the quota is bigger than real storage, this is not accurate info?

Oh, now I see that you mean. thanks.
I've tried to get the real size of the pool, but it required rule:admin_api role for the api method scheduler_extension:scheduler_stats:get_pools

So, It is better to rename the function to GetFreeQuotaStorageSpace since we cannot get the real size of the pool.

Available capacity of disk storage Signed-off-by: Serge Logvinov <serge.logvinov@sinextra.dev>

sergelogvinov · 2024-05-29T08:10:40Z

/retest-required

k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 16, 2024

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label May 16, 2024

k8s-ci-robot requested review from kayrus and zetaab May 16, 2024 05:23

k8s-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 16, 2024

sergelogvinov mentioned this pull request May 16, 2024

[cinder-csi-plugin] Multi region/clouds support for controllerServer #2551

Open

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 16, 2024

sergelogvinov force-pushed the csi-cinder-capacity branch from b564f33 to fbe585f Compare May 16, 2024 07:33

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 16, 2024

dulek reviewed May 16, 2024

View reviewed changes

sergelogvinov force-pushed the csi-cinder-capacity branch from fbe585f to f15d7e0 Compare May 16, 2024 15:44

sergelogvinov force-pushed the csi-cinder-capacity branch from f15d7e0 to 8c95830 Compare May 25, 2024 19:17

jichenjc requested changes May 28, 2024

View reviewed changes

sergelogvinov force-pushed the csi-cinder-capacity branch 2 times, most recently from 71ee34a to a5a1e4d Compare May 28, 2024 16:16

feat: csi-cinder storage capacity

57f388b

Available capacity of disk storage Signed-off-by: Serge Logvinov <serge.logvinov@sinextra.dev>

sergelogvinov force-pushed the csi-cinder-capacity branch from a5a1e4d to 57f388b Compare May 29, 2024 08:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[cinder-csi-plugin] csi-cinder storage capacity #2597

[cinder-csi-plugin] csi-cinder storage capacity #2597

sergelogvinov commented May 16, 2024 •

edited

k8s-ci-robot commented May 16, 2024

k8s-ci-robot commented May 16, 2024

jichenjc commented May 16, 2024

sergelogvinov commented May 16, 2024

dulek May 16, 2024

sergelogvinov May 16, 2024

dulek May 16, 2024

sergelogvinov May 16, 2024

dulek May 16, 2024

sergelogvinov May 17, 2024

dulek May 23, 2024

sergelogvinov May 25, 2024

sergelogvinov commented May 25, 2024

sergelogvinov commented May 25, 2024

sergelogvinov commented May 27, 2024

jichenjc May 28, 2024

sergelogvinov May 28, 2024

jichenjc May 28, 2024

sergelogvinov May 29, 2024 •

edited

sergelogvinov commented May 29, 2024

[cinder-csi-plugin] csi-cinder storage capacity #2597

Are you sure you want to change the base?

[cinder-csi-plugin] csi-cinder storage capacity #2597

Conversation

sergelogvinov commented May 16, 2024 • edited

k8s-ci-robot commented May 16, 2024

k8s-ci-robot commented May 16, 2024

jichenjc commented May 16, 2024

sergelogvinov commented May 16, 2024

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

Choose a reason for hiding this comment

sergelogvinov commented May 25, 2024

sergelogvinov commented May 25, 2024

sergelogvinov commented May 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sergelogvinov May 29, 2024 • edited

Choose a reason for hiding this comment

sergelogvinov commented May 29, 2024

sergelogvinov commented May 16, 2024 •

edited

sergelogvinov May 29, 2024 •

edited