Skip to content

fix(node): handle properly volume resize when asking for more than the available space #656

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

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

plaffitt
Copy link

Pull Request template

Why is this PR required? What issue does it fix?: closes #649

What this PR does?: It reverts (ref)quota to the previous value of (ref)reservation (the value that we read from zfs before setting those 2 properties together).

Does this PR require any upgrade changes?: Not sure about what this question mean, but I don't think so.

If the changes in this PR are manually verified, list down the scenarios covered::

  • Expand a PVC to an acceptable size and check that it worked as before
  • Expand a PVC to an unreasonable size and check that it reverts to the original size while returning a grpc ResourceExhausted code

Any additional information for your reviewer? :
N/A

Checklist:

Copy link
Member

@Abhinandan-Purkait Abhinandan-Purkait left a comment

Choose a reason for hiding this comment

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

Thanks! Changes look reasonable to me. Can you please add a CI test for the same if possible?

Does this PR require any upgrade changes?:
Not sure about what this question mean, but I don't think so.

It basically means do we need to handle anything for upgrading from an older version to this.
Anyway the changes don't seem to directly impact upgrades but I will double check the scenarios you have mentioned, on a volume created on a previous version and the the resize being performed after upgrade.

@Abhinandan-Purkait
Copy link
Member

@plaffitt I see you have created a new issue on the same, and your suggestion on adding a check on the ControllerExpand. That sounds fair to me, can you add the same in the PR. I tried the same and you can refer the code from below if it looks good to you. Thanks.

https://github.com/openebs/zfs-localpv/compare/add_check_controller?expand=1 cc @tiagolobocastro

@codecov-commenter
Copy link

codecov-commenter commented May 18, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.94%. Comparing base (8c402d3) to head (d18b5cd).
Report is 61 commits behind head on develop.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #656      +/-   ##
===========================================
- Coverage    95.99%   95.94%   -0.06%     
===========================================
  Files            1        1              
  Lines          574      690     +116     
===========================================
+ Hits           551      662     +111     
- Misses          19       23       +4     
- Partials         4        5       +1     
Flag Coverage Δ
bddtests 95.94% <ø> (∅)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@plaffitt plaffitt force-pushed the fix/volume-expansion branch from 9c4ca15 to d9788b4 Compare May 19, 2025 13:24
@plaffitt plaffitt force-pushed the fix/volume-expansion branch from d9788b4 to d18b5cd Compare May 22, 2025 09:19
@plaffitt
Copy link
Author

Hello, is there anything missing to review/merge this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

REFQUOTA is updated even when volume expansion fails due to insufficient space
4 participants