Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Huawei storagecapabilities #3253

Merged
merged 1 commit into from
Jul 2, 2024

Conversation

ido106
Copy link

@ido106 ido106 commented May 12, 2024

Signed-off-by: Ido Aharon iaharon@redhat.com

What this PR does / why we need it:
Adding storagecapabilities to the Huawei provisioner.
According to eSDK Huawei Storage Kubernetes CSI Plugins V4.3.0 User Guide 02.pdf, the provisioner string is csi.huawei.com and the storage profile is { {rwx, file}, {rwo, file}, {rox, file} } if sc.Parameters["protocol"] == "nfs" and { {rwx, block}, {rwo, block}, {rwo, file}, {rox, block}, {rox, file} } otherwise:

RWO/ROX/RWOP: supported by all types of volumes. RWOP is supported only by kubernetes 1.22 and later versions.
RWX: supported only by Raw Block volumes and NFS volumes

Parameter: parameters.protocol
Description: Storage protocol. The value is a character string. (iscsi, fc, roce, fc-nvme, nfs, dpc, scsi)

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #CNV-41636

Special notes for your reviewer:

Release note:

Add storagecapabilities to csi.huawei.com provisioner

@kubevirt-bot kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels May 12, 2024
@kubevirt-bot
Copy link
Contributor

Hi @ido106. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

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/test-infra repository.

@ido106 ido106 changed the title Add storage capabilities for huawei provisioner Add storage capabilities to huawei provisioner May 12, 2024
@@ -105,6 +105,8 @@ var CapabilitiesByProvisionerKey = map[string][]StorageCapabilities{
"manila.csi.openstack.org": {{rwx, file}},
// ovirt csi
"csi.ovirt.org": createRWOBlockAndFilesystemCapabilities(),
// huawei
"csi.huawei.com": {{rwx, block}},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it only support rwx, block or does it also support rwo, block and rwo, file? If so can you add it like this?

"csi.huawei.com": {{rwx, block}, {rwo, block}, {rwo, file}},

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand from CNV-41531 comments, they only have storage profile configured for iscsi storage class rwx, block

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I am not seeing it, but I don't see any specifics on what that storage in the JIRA card. But I just checked the user guide for the csi driver and it says this

RWO/ROX/RWOP: supported by all types of volumes. RWOP is supported only by kubernetes 1.22 and later versions.
RWX: supported only by Raw Block volumes and NFS volumes

So it looks like it should be what I suggested.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I added {rwx, block}, {rwo, block}, {rwo, file}, {rox, block}, {rox, file} as it says

@awels
Copy link
Member

awels commented May 15, 2024

@ido106 ido106 force-pushed the huawei_storagecapabilities branch from 5fd8aa6 to ab3d1d4 Compare May 19, 2024 09:56
@kubevirt-bot kubevirt-bot added size/S needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed size/XS labels May 19, 2024
@ido106
Copy link
Author

ido106 commented May 21, 2024

The provisioner supports NFS and ISCSI. For NFS it is RWX/FS and for ISCSI it is RWX/Block.
I am following CNV-41531 for samples storage classes that use NFS and ISCSI.

@ido106
Copy link
Author

ido106 commented May 28, 2024

TBH I don't understand why we have to wait for examples. The documentation says:

RWO/ROX/RWOP: supported by all types of volumes. RWOP is supported only by kubernetes 1.22 and later versions.
RWX: supported only by Raw Block volumes and NFS volumes

So apparently we only need to differentiate between NFS and the others. ISCSI doesn't get additional capabilities over the others.
I think the right way to check it is with parameters.protocol as the documentation mentions:

Parameter: parameters.protocol
Description: Storage protocol. The value is a character string. (iscsi, fc, roce, fc-nvme, nfs, dpc, scsi)

So I think it should be { {rwx, file}, {rwo, file}, {rox, file} } if sc.Parameters["protocol"] == "nfs" and { {rwx, block}, {rwo, block}, {rwo, file}, {rox, block}, {rox, file} } otherwise.

@ido106 ido106 changed the title Add storage capabilities to huawei provisioner Add Huawei storagecapabilities May 28, 2024
@ido106 ido106 force-pushed the huawei_storagecapabilities branch from ab3d1d4 to d29813e Compare June 2, 2024 13:13
@kubevirt-bot kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 2, 2024
@awels
Copy link
Member

awels commented Jun 3, 2024

Yes, that sounds good, look at storageClassToProvisionerKeyMapper on how to map certain parameters to certain types of provisioners. We do something similar for other provisioners already.

@ido106 ido106 force-pushed the huawei_storagecapabilities branch from d29813e to d116027 Compare June 10, 2024 11:03
@arnongilboa
Copy link
Collaborator

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 10, 2024
@coveralls
Copy link

Coverage Status

coverage: 59.073% (+0.05%) from 59.023%
when pulling d116027 on ido106:huawei_storagecapabilities
into f3d0060 on kubevirt:main.

@kubevirt-bot kubevirt-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 18, 2024
@ido106 ido106 force-pushed the huawei_storagecapabilities branch from d116027 to ca2f384 Compare June 20, 2024 13:15
@kubevirt-bot kubevirt-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 20, 2024
Signed-off-by: Ido Aharon <iaharon@redhat.com>
@ido106 ido106 force-pushed the huawei_storagecapabilities branch from ca2f384 to 4981b51 Compare June 27, 2024 09:59
@kubevirt-bot kubevirt-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 27, 2024
@ido106
Copy link
Author

ido106 commented Jun 27, 2024

It was a messy rebase with DELL, please double check that everything is o.k

@arnongilboa
Copy link
Collaborator

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 27, 2024
@coveralls
Copy link

Coverage Status

coverage: 59.079% (+0.02%) from 59.055%
when pulling 4981b51 on ido106:huawei_storagecapabilities
into 64d0d16 on kubevirt:main.

@awels
Copy link
Member

awels commented Jul 1, 2024

/approve

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: awels

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

The pull request process is described here

Needs approval from an approver in each of these files:

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

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 1, 2024
@awels
Copy link
Member

awels commented Jul 2, 2024

/override pull-containerized-data-importer-e2e-ceph-wffc

@kubevirt-bot
Copy link
Contributor

@awels: Overrode contexts on behalf of awels: pull-containerized-data-importer-e2e-ceph-wffc

In response to this:

/override pull-containerized-data-importer-e2e-ceph-wffc

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.

@kubevirt-bot kubevirt-bot merged commit 1e7f9db into kubevirt:main Jul 2, 2024
18 checks passed
akalenyu pushed a commit to akalenyu/containerized-data-importer that referenced this pull request Jul 11, 2024
Signed-off-by: Ido Aharon <iaharon@redhat.com>
kubevirt-bot pushed a commit that referenced this pull request Jul 12, 2024
* Add storagecapabilities to vSphere provisioner (#3284)

Signed-off-by: Ido Aharon <iaharon@redhat.com>

* Fix Dell storagecapabilities (#3249)

According to Dell doc:
https://dell.github.io/csm-docs/docs/csidriver/

Signed-off-by: Ido Aharon <iaharon@redhat.com>

* Add storagecapabilities to csi.huawei.com provisioner (#3253)

Signed-off-by: Ido Aharon <iaharon@redhat.com>

---------

Signed-off-by: Ido Aharon <iaharon@redhat.com>
Co-authored-by: Ido Aharon <iaharon@redhat.com>
arnongilboa added a commit to arnongilboa/containerized-data-importer that referenced this pull request Oct 31, 2024
* Add storagecapabilities to vSphere provisioner (kubevirt#3284)

Signed-off-by: Ido Aharon <iaharon@redhat.com>

* Fix Dell storagecapabilities (kubevirt#3249)

Signed-off-by: Ido Aharon <iaharon@redhat.com>

* Add storagecapabilities to csi.huawei.com provisioner (kubevirt#3253)

Signed-off-by: Ido Aharon <iaharon@redhat.com>

* Add Longhorn storagecapabilities (kubevirt#3290)

Signed-off-by: Ido Aharon <iaharon@redhat.com>

---------

Signed-off-by: Ido Aharon <iaharon@redhat.com>
Co-authored-by: Ido Aharon <iaharon@redhat.com>
arnongilboa added a commit to arnongilboa/containerized-data-importer that referenced this pull request Oct 31, 2024
* Add storagecapabilities to vSphere provisioner (kubevirt#3284)

Signed-off-by: Ido Aharon <iaharon@redhat.com>

* Fix Dell storagecapabilities (kubevirt#3249)

Signed-off-by: Ido Aharon <iaharon@redhat.com>

* Add storagecapabilities to csi.huawei.com provisioner (kubevirt#3253)

Signed-off-by: Ido Aharon <iaharon@redhat.com>

* Add Longhorn storagecapabilities (kubevirt#3290)

Signed-off-by: Ido Aharon <iaharon@redhat.com>

---------

Signed-off-by: Ido Aharon <iaharon@redhat.com>
Co-authored-by: Ido Aharon <iaharon@redhat.com>
arnongilboa added a commit to arnongilboa/containerized-data-importer that referenced this pull request Oct 31, 2024
* Add storagecapabilities to vSphere provisioner (kubevirt#3284)

Signed-off-by: Ido Aharon <iaharon@redhat.com>

* Fix Dell storagecapabilities (kubevirt#3249)

Signed-off-by: Ido Aharon <iaharon@redhat.com>

* Add storagecapabilities to csi.huawei.com provisioner (kubevirt#3253)

Signed-off-by: Ido Aharon <iaharon@redhat.com>

* Add Longhorn storagecapabilities (kubevirt#3290)

Signed-off-by: Ido Aharon <iaharon@redhat.com>

---------

Signed-off-by: Ido Aharon <iaharon@redhat.com>
Co-authored-by: Ido Aharon <iaharon@redhat.com>
arnongilboa added a commit to arnongilboa/containerized-data-importer that referenced this pull request Oct 31, 2024
* Add storagecapabilities to vSphere provisioner (kubevirt#3284)

Signed-off-by: Ido Aharon <iaharon@redhat.com>

* Fix Dell storagecapabilities (kubevirt#3249)

Signed-off-by: Ido Aharon <iaharon@redhat.com>

* Add storagecapabilities to csi.huawei.com provisioner (kubevirt#3253)

Signed-off-by: Ido Aharon <iaharon@redhat.com>

* Add Longhorn storagecapabilities (kubevirt#3290)

Signed-off-by: Ido Aharon <iaharon@redhat.com>

---------

Signed-off-by: Ido Aharon <iaharon@redhat.com>
Co-authored-by: Ido Aharon <iaharon@redhat.com>
kubevirt-bot pushed a commit that referenced this pull request Oct 31, 2024
* Add storagecapabilities to vSphere provisioner (#3284)



* Fix Dell storagecapabilities (#3249)



* Add storagecapabilities to csi.huawei.com provisioner (#3253)



* Add Longhorn storagecapabilities (#3290)



---------

Signed-off-by: Ido Aharon <iaharon@redhat.com>
Co-authored-by: Ido Aharon <iaharon@redhat.com>
kubevirt-bot pushed a commit that referenced this pull request Oct 31, 2024
* Add storagecapabilities to vSphere provisioner (#3284)



* Fix Dell storagecapabilities (#3249)



* Add storagecapabilities to csi.huawei.com provisioner (#3253)



* Add Longhorn storagecapabilities (#3290)



---------

Signed-off-by: Ido Aharon <iaharon@redhat.com>
Co-authored-by: Ido Aharon <iaharon@redhat.com>
kubevirt-bot pushed a commit that referenced this pull request Oct 31, 2024
* Add storagecapabilities to vSphere provisioner (#3284)



* Fix Dell storagecapabilities (#3249)



* Add storagecapabilities to csi.huawei.com provisioner (#3253)



* Add Longhorn storagecapabilities (#3290)



---------

Signed-off-by: Ido Aharon <iaharon@redhat.com>
Co-authored-by: Ido Aharon <iaharon@redhat.com>
kubevirt-bot pushed a commit that referenced this pull request Oct 31, 2024
* Add storagecapabilities to vSphere provisioner (#3284)



* Fix Dell storagecapabilities (#3249)



* Add storagecapabilities to csi.huawei.com provisioner (#3253)



* Add Longhorn storagecapabilities (#3290)



---------

Signed-off-by: Ido Aharon <iaharon@redhat.com>
Co-authored-by: Ido Aharon <iaharon@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants