Skip to content

Upstream IBM VPC File storage provider#4138

Open
GeraFD14 wants to merge 6 commits into
kubevirt:mainfrom
GeraFD14:main
Open

Upstream IBM VPC File storage provider#4138
GeraFD14 wants to merge 6 commits into
kubevirt:mainfrom
GeraFD14:main

Conversation

@GeraFD14

@GeraFD14 GeraFD14 commented May 15, 2026

Copy link
Copy Markdown

Replaces #4123

What this PR does / why we need it: Need to upstream IBM VPC File Storage Provider into CDI

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 https://redhat.atlassian.net/browse/CNV-84991

Special notes for your reviewer: Adding vpc.file.csi.ibm.io provider including the dynamic SIZE/IOPS required relation based on https://cloud.ibm.com/docs/vpc?topic=vpc-file-storage-profiles&interface=ui#dp2-profile

Release note:

Add support for IBM VPC File Storage driver (vpc.file.csi.ibm.io)

@kubevirt-bot kubevirt-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels May 15, 2026
@kubevirt-bot

Copy link
Copy Markdown
Contributor

Hi @GeraFD14. 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.

Details

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

[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 mhenriks for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

@GeraFD14

GeraFD14 commented May 15, 2026

Copy link
Copy Markdown
Author

@arnongilboa @awels
thank you for your comments, unfortunately I made a mistake an the original PR was closed, will continue in this one.


@arnongilboa from your previous comments:

  1. Please update about the rounding. Not sure we can approve such vendor-specific code.

Lets continue this discussion on slack.

  1. What is the vTPM SC provisioner key? shouldn't it default to RWO (without RWX)?

I've added the new provisioner keys for vTPM SC, please help validating last commit and let me know if I need to change anything else

  1. You also need to align the PR to the repo standards:
    • Add a description to the commits
    • Sign the commits
    • Add a release note to the PR

I'll continue with this and will correct the PR ASAP

gera-fuentes and others added 3 commits May 15, 2026 12:40
Initial commit to upstream required configuration for IVM VPC File storage driver.
Added the storage capability: vpc.file.csi.ibm.io.
Added dynamic minimum disk (PVC) calculation for dp2 IBM profiles

Signed-off-by: GeraFD14 <gera.fd14@outlook.com>
Made corrections from Arnon comments, deleted unused code and create separated capabilities for dp2 and rfs profiles.

Signed-off-by: GeraFD14 <gera.fd14@outlook.com>
Added capabilities keys for vTPM StorageClasses (for dp2 and rfs), this storage class requires RWO.

Signed-off-by: GeraFD14 <gera.fd14@outlook.com>
@kubevirt-bot kubevirt-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels May 15, 2026
@kubevirt-bot

Copy link
Copy Markdown
Contributor

There has been no activity on this PR for 45 days.
To protect limited CI resources, it has been automatically labelled 'stale'.
This PR will automatically rot after an additional 14 days of inactivity, and will be closed shortly after that.

What you can do:

  • If the PR is waiting on you to respond to a question or feedback and/or update the PR, please do so.
  • You can mark the PR as fresh and remove the label with the following command: /remove-lifecycle stale
  • If this PR is safe to close now, please help the project by closing it with: /close
  • If you need attention on this PR from a reviewer, you can raise it on the agenda of the relevant SIG meeting or KubeVirt Community meeting, or ping the kubevirt-dev slack channel.

/lifecycle stale

@kubevirt-prow kubevirt-prow Bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 29, 2026
@GeraFD14

Copy link
Copy Markdown
Author

/remove-lifecycle stale

@kubevirt-prow kubevirt-prow Bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 29, 2026
@kubevirt-prow

kubevirt-prow Bot commented Jul 2, 2026

Copy link
Copy Markdown

[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 mhenriks for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

Comment on lines +332 to +335
// getIBMVPCFileDP2MinimumSize calculates the minimum PVC size for IBM VPC File Storage DP2 profile
// based on the IOPS parameter using a table lookup
// Reference: https://cloud.ibm.com/docs/vpc?topic=vpc-file-storage-profiles&interface=ui#dp2-profile
func getIBMVPCFileDP2MinimumSize(sc *storagev1.StorageClass) (string, bool) {

@arnongilboa arnongilboa Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

As mentioned before, not sure we want this very vendor-specific logic in CDI. It's also quite impossible to maintain over time. @awels @akalenyu?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@gera-fuentes does requesting a pvc size below the minimum result in an error or an auto roundup (to min size)?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@noamasu requesting a pvc size below the minimum results in an error right now

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why don't we have some operator reconcile the storage class with a parameter/annotation that advertises the min size

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@akalenyu they can add an optional "roundup" param in the storage class, so if you set "roundup: true", it will do the roundup logic in the CSI driver. wdyt?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@arnongilboa @noamasu @akalenyu I have removed the IBM's specific logic for the minimum size annotation; we are going to update the provider to do the size round up instead.

@kubevirt-prow kubevirt-prow Bot added dco-signoff: no Indicates the PR's author has not DCO signed all their commits. and removed dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Jul 9, 2026
@GeraFD14 GeraFD14 marked this pull request as ready for review July 9, 2026 19:09
@kubevirt-prow kubevirt-prow Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 9, 2026
@kubevirt-prow kubevirt-prow Bot requested review from alromeros and arnongilboa July 9, 2026 19:09
GeraFD14 added 2 commits July 10, 2026 11:07
Signed-off-by: GeraFD14 <gera.fd14@outlook.com>
Signed-off-by: GeraFD14 <gera.fd14@outlook.com>
@kubevirt-prow kubevirt-prow Bot removed the dco-signoff: no Indicates the PR's author has not DCO signed all their commits. label Jul 10, 2026
@kubevirt-prow kubevirt-prow Bot added the dco-signoff: yes Indicates the PR's author has DCO signed all their commits. label Jul 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dco-signoff: yes Indicates the PR's author has DCO signed all their commits. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants