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

Fix uploading for block devices that exceed requested size #3461

Merged
merged 1 commit into from
Oct 16, 2024

Conversation

kvaps
Copy link
Member

@kvaps kvaps commented Oct 14, 2024

What this PR does / why we need it:

Various storage providers may provision block devices larger than the size requested by the user. This is often due to LVM or ZFS rounding up to the nearest extent size. (see LINBIT/linstor-server#421 (comment) and #3159 (comment))

Currently, CDI checks the requested size in the PVC and fails the upload if the original block device is larger than the requested size.

This PR changes logic to take requestImageSize into account only for filesystem volumes

Which issue(s) this PR fixes:

Another try to fix #3159

Special notes for your reviewer:

Release note:

Fix uploading for block devices that exceed requested size

@kubevirt-bot kubevirt-bot added dco-signoff: no Indicates the PR's author has not 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. size/S labels Oct 14, 2024
@kvaps kvaps force-pushed the fix-check-size-for-block-devices branch 2 times, most recently from c90ef04 to 47c361e Compare October 14, 2024 10:41
@kubevirt-bot kubevirt-bot added 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. and removed dco-signoff: no Indicates the PR's author has not 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 Oct 14, 2024
@coveralls
Copy link

coveralls commented Oct 14, 2024

Coverage Status

coverage: 59.228% (+0.007%) from 59.221%
when pulling 38e6d3c on kvaps:fix-check-size-for-block-devices
into 7b330eb on kubevirt:main.

@kvaps kvaps force-pushed the fix-check-size-for-block-devices branch from 47c361e to 9c19512 Compare October 14, 2024 14:20
@kvaps kvaps changed the title Fix: checking available image size for block volumes Fix uploading for block devices that exceed requested size in PVC Oct 14, 2024
@kvaps kvaps force-pushed the fix-check-size-for-block-devices branch from 9c19512 to 15d8a4e Compare October 14, 2024 15:27
@kvaps kvaps changed the title Fix uploading for block devices that exceed requested size in PVC Fix uploading for block devices that exceed requested size Oct 14, 2024
@kvaps
Copy link
Member Author

kvaps commented Oct 14, 2024

/assign @awels

@kvaps kvaps force-pushed the fix-check-size-for-block-devices branch 2 times, most recently from e633d9f to 90fdd7f Compare October 14, 2024 21:42
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
@kvaps kvaps force-pushed the fix-check-size-for-block-devices branch from 90fdd7f to 38e6d3c Compare October 14, 2024 22:21
@awels
Copy link
Member

awels commented Oct 14, 2024

/test pull-cdi-unit-test

kvaps added a commit to aenix-io/cozystack that referenced this pull request Oct 16, 2024
Upstream fix:
kubevirt/containerized-data-importer#3461

Signed-off-by: Andrei Kvapil <kvapss@gmail.com>

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Introduced a new version (`v1beta1`) for the CDI operator alongside
the existing version, enhancing configuration options.
- Expanded `spec` section with detailed descriptions for various
configurations including data volume management and TLS security
profiles.
- Added a new Ingress resource for the `cdi-uploadproxy` service,
improving traffic routing capabilities.
- Introduced new configuration parameters for dynamic upload proxy URL
management.

- **Improvements**
- Updated permissions for the CDI operator to manage additional
resources, improving its data handling capabilities.
- Refined deployment configuration with updated container image
references and environment variables for better operational control.
- Enhanced network policy definitions by adding specific rules for new
services while maintaining existing policies.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
@awels
Copy link
Member

awels commented Oct 16, 2024

/lgtm
/approve

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 16, 2024
@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 Oct 16, 2024
@kubevirt-bot kubevirt-bot merged commit 5f66979 into kubevirt:main Oct 16, 2024
20 checks passed
universal-itengineer pushed a commit to deckhouse/3p-containerized-data-importer that referenced this pull request Oct 29, 2024
universal-itengineer pushed a commit to deckhouse/3p-containerized-data-importer that referenced this pull request Oct 29, 2024
@akalenyu
Copy link
Collaborator

/cherrypick release-v1.60

@kubevirt-bot
Copy link
Contributor

@akalenyu: new pull request created: #3487

In response to this:

/cherrypick release-v1.60

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.

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.

Virtual image size is larger than the reported available storage
5 participants