Skip to content

Conversation

@Dsanatar
Copy link
Collaborator

@Dsanatar Dsanatar commented Sep 4, 2025

What this PR does / why we need it:

Portworx enterprise csi driver, and the PX-CSI driver are currently using the same provisioner string key pxd.portworx.com however they have different storage capabilities. Add a check to see if storage class is PX-CSI and assign the associated capabilities

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 #

Special notes for your reviewer:

Release note:

Support for PX-CSI

@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 Sep 4, 2025
@akalenyu
Copy link
Collaborator

akalenyu commented Sep 4, 2025

makes sense, just recall some comments against this for some reason from #3644 (comment)
/cc @arnongilboa

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Sep 4, 2025
@akalenyu
Copy link
Collaborator

akalenyu commented Sep 4, 2025

/test pull-cdi-goveralls

1 similar comment
@awels
Copy link
Member

awels commented Sep 4, 2025

/test pull-cdi-goveralls

@awels
Copy link
Member

awels commented Sep 4, 2025

makes sense, just recall some comments against this for some reason from #3644 (comment) /cc @arnongilboa

I discussed with the portworx folks and this is the recommendation they gave me for when using the px-csi driver.

@coveralls
Copy link

coveralls commented Sep 4, 2025

Coverage Status

coverage: 59.242% (+0.02%) from 59.225%
when pulling c62e4ab on Dsanatar:add-pxcsi-support
into a0c2594 on kubevirt:main.

@akalenyu
Copy link
Collaborator

akalenyu commented Sep 4, 2025

makes sense, just recall some comments against this for some reason from #3644 (comment) /cc @arnongilboa

I discussed with the portworx folks and this is the recommendation they gave me for when using the px-csi driver.

/approve

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akalenyu

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

The pull request process is described here

Details 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 Sep 4, 2025
@awels
Copy link
Member

awels commented Sep 4, 2025

Probably makes sense to backport this to at least 1.61

@Dsanatar
Copy link
Collaborator Author

Dsanatar commented Sep 4, 2025

/cherry-pick release-v1.62 release-v1.61

@kubevirt-bot
Copy link
Contributor

@Dsanatar: once the present PR merges, I will cherry-pick it on top of release-v1.62 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-v1.62 release-v1.61

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.

@awels
Copy link
Member

awels commented Sep 4, 2025

Don't forget 1.63

@Dsanatar
Copy link
Collaborator Author

Dsanatar commented Sep 4, 2025

/cherry-pick release-v1.63

@kubevirt-bot
Copy link
Contributor

@Dsanatar: once the present PR merges, I will cherry-pick it on top of release-v1.63 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-v1.63

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 a89853c into kubevirt:main Sep 4, 2025
21 checks passed
@kubevirt-bot
Copy link
Contributor

@Dsanatar: new pull request created: #3894

Details

In response to this:

/cherry-pick release-v1.62 release-v1.61

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
Copy link
Contributor

@Dsanatar: new pull request created: #3895

Details

In response to this:

/cherry-pick release-v1.63

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.

@awels
Copy link
Member

awels commented Sep 5, 2025

/cherry-pick release-v1.61

@kubevirt-bot
Copy link
Contributor

@awels: new pull request created: #3896

Details

In response to this:

/cherry-pick release-v1.61

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.

} else if val == "pure_fa_file" {
return "pxd.portworx.com/pure_fa_file"
}
} else {
Copy link
Collaborator

@arnongilboa arnongilboa Sep 7, 2025

Choose a reason for hiding this comment

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

@Dsanatar Are you sure about the else here? afaiu sharedv4_mount_options should be checked even if there is a backend parameter (other than pure_block/pure_fa_file).

Copy link
Member

Choose a reason for hiding this comment

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

I talked to the portworx folks and they said the backend parameter is only valid for PX-CSI, not the enterprise version.

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.

6 participants