-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Introduce volume allocation algorithm global configuration #10696
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
base: main
Are you sure you want to change the base?
Conversation
Volume allocation has been using vm.allocation.algorithm config value. This change will allow them to be configured in isolation.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10696 +/- ##
============================================
+ Coverage 16.31% 16.52% +0.20%
- Complexity 13464 13626 +162
============================================
Files 5676 5673 -3
Lines 499412 500101 +689
Branches 60395 60504 +109
============================================
+ Hits 81503 82626 +1123
+ Misses 408829 408263 -566
- Partials 9080 9212 +132
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@blueorangutan package |
@rajujith a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13019 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clgtm
...rage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
Outdated
Show resolved
Hide resolved
...rage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
Outdated
Show resolved
Hide resolved
...rage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
Outdated
Show resolved
Hide resolved
.../src/main/java/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java
Show resolved
Hide resolved
@blueorangutan package |
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13020 |
...rage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
Outdated
Show resolved
Hide resolved
@blueorangutan package |
@sudo87 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13046 |
@blueorangutan test |
@sudo87 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
code lgtm However, I think we need to think of the upgrade |
[SF] Trillian test result (tid-12988)
|
Thank you @weizhouapache, I have handled it in latest commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code lgtm
"random" can be replaced by VolumeAllocationAlgorithm.getDefaultValue() |
@blueorangutan package |
@sudo87 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13089 |
@blueorangutan test |
@sudo87 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code LGTM
[SF] Trillian test result (tid-13028)
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
@blueorangutan package |
@sudo87 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13137 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Tested with multiple shared NFS primary storage. If the logging for the firstfitleastconsumed can print the available capacity as well in the logs it will be useful.
'List of pools in descending order of available capacity [[1, 2]].'
Thanks @rajujith for suggestion! Change is made to enhance log to show available capacity for host/pool. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code LGTM
Description
This PR introduces separate global configuration for Volume allocation algorithm.
Volume allocation has been using vm.allocation.algorithm config value. It can be confusing and limitting user to use same allocation algorithm for both VM and Volume.
This new configuration will allow vm and volume allocators to use different algoritms independently without being tied together.
Doc PR apache/cloudstack-documentation#499
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
Local dev testing is done by deployment of instances against simulator after setting different values for "volume.allocation.algorithm".
How did you try to break this feature and the system with this change?