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

modify pvc-autoresizer #1120

Merged
merged 1 commit into from
Sep 8, 2023
Merged

Conversation

buffalo1024
Copy link
Contributor

What this PR does / why we need it:

Special notes for your reviewer:

Does this PR introduce a user-facing change?


Signed-off-by: buffalo1024 <yikaichen@yikaichendeMacBook-Pro.local>
@codecov-commenter
Copy link

Codecov Report

Merging #1120 (7500d56) into main (cc4a002) will decrease coverage by 0.58%.
Report is 1 commits behind head on main.
The diff coverage is 6.06%.

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

@@            Coverage Diff             @@
##             main    #1120      +/-   ##
==========================================
- Coverage   39.96%   39.39%   -0.58%     
==========================================
  Files          25       25              
  Lines        1909     1942      +33     
==========================================
+ Hits          763      765       +2     
- Misses       1045     1075      +30     
- Partials      101      102       +1     
Files Changed Coverage Δ
pkg/local-disk-manager/filter/filter_disk.go 50.00% <0.00%> (-19.63%) ⬇️
...k-manager/handler/localdiskclaim/localdiskclaim.go 69.23% <ø> (ø)
.../local-disk-manager/handler/localdisk/localdisk.go 80.48% <100.00%> (+0.32%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

log.Errorf("match pvc err: %v", err)
return false, err
}
if !matched {
Copy link
Member

@sun7927 sun7927 Sep 7, 2023

Choose a reason for hiding this comment

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

why return false? should it continue to be checked more?
should it return true if matched?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's && relation between selectors of a resizepolicy, so if one selector doesn't match, the whole resizepolicy doesn't match.

@sun7927
Copy link
Member

sun7927 commented Sep 7, 2023

In this solution, once a PVC gets updated, it will be checked against the policy and associated with a policy if matched. In case, user wants to disable auto resize for a PVC, how to handle it?

@buffalo1024
Copy link
Contributor Author

In this solution, once a PVC gets updated, it will be checked against the policy and associated with a policy if matched. In case, user wants to disable auto resize for a PVC, how to handle it?

to disable autoresizing for a pvc, keep that no resizepolicy select the pvc.

@sun7927 sun7927 merged commit 39ce353 into hwameistor:main Sep 8, 2023
3 checks passed
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.

None yet

3 participants