Add HPE Storage Pool Allocator implementation & maxfree volume allocation algorithm #11919
Conversation
|
@blueorangutan package |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new HPE Storage Pool Allocator with enhanced storage pool selection capabilities and implements a new "maxfree" volume allocation algorithm that prioritizes storage pools based on available free space.
- Adds HPEStoragePoolAllocator for HPE Primera storage systems with Primera provider name filtering
- Implements maxfree algorithm in AbstractStoragePoolAllocator that reorders pools by free space (descending)
- Registers HPE allocator in Spring configuration with priority placement before existing allocators
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| HPEStoragePoolAllocator.java | New allocator filtering pools by "Primera" storage provider name |
| AbstractStoragePoolAllocator.java | Added reorderPoolsByFreeSpace() method and maxfree algorithm case |
| spring-engine-storage-storage-allocator-context.xml | Registered HPEStoragePoolAllocator bean |
| spring-core-registry-core-context.xml | Added HPEStorage to allocator order configuration |
| VolumeOrchestrationService.java | Updated configuration documentation to include maxfree option |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * HPE Storage Pool Allocator | ||
| * | ||
| * This allocator is specifically designed for HPE storage systems and filters | ||
| * storage pools based on the storage provider name being "Primera". It extends the |
There was a problem hiding this comment.
Documentation mentions 'Primera' but the implementation filters by exact match. Consider documenting that this allocator is hardcoded to 'Primera' provider name only, and clarify whether this is intended to be HPE-specific or Primera-specific.
| } | ||
|
|
||
| // Filter by Primera storage provider name (exact case match) | ||
| if ("Primera".equals(pool.getStorageProviderName())) { |
There was a problem hiding this comment.
The storage provider name 'Primera' is hardcoded as a magic string. Consider extracting this to a private static final constant (e.g., private static final String HPE_STORAGE_PROVIDER_NAME = \"Primera\";) to improve maintainability and make the provider name configurable in the future.
|
|
||
| /** | ||
| * Reorders storage pools based on the configured volume allocation algorithm. | ||
| * Different algorithms provide different strategies for pool selection and ordering. |
There was a problem hiding this comment.
The added method-level documentation doesn't describe the method's parameters, return value, or the specific algorithms supported. Consider expanding this to match the detail level of other method documentation in this file, including @param, @return tags, and a list of supported algorithms.
| * Different algorithms provide different strategies for pool selection and ordering. | |
| * <p> | |
| * Supported algorithms: | |
| * <ul> | |
| * <li><b>random</b>: Randomly shuffles the list of pools.</li> | |
| * <li><b>userconcentratedpod_random</b>: Randomly shuffles the list of pools, concentrating on user pods.</li> | |
| * <li><b>userdispersing</b>: Orders pools to disperse user volumes across different pools, reducing concentration.</li> | |
| * <li><b>firstfitleastconsumed</b>: Orders pools by least consumed capacity, prioritizing those with more available space.</li> | |
| * <li><b>maxfree</b>: Orders pools by maximum available free space (descending).</li> | |
| * </ul> | |
| * If the account is null, or the algorithm is "random" or "userconcentratedpod_random", pools are shuffled randomly. | |
| * | |
| * @param pools The list of storage pools to reorder. | |
| * @param plan The deployment plan, used for context such as data center or pod. | |
| * @param account The account for which the pools are being allocated; may affect algorithm selection. | |
| * @return The reordered list of storage pools according to the selected algorithm. |
| <property name="orderConfigKey" value="storage.pool.allocators.order" /> | ||
| <property name="orderConfigDefault" | ||
| value="LocalStorage,ClusterScopeStoragePoolAllocator,ZoneWideStoragePoolAllocator" /> | ||
| value="HPEStorage,LocalStorage,ClusterScopeStoragePoolAllocator,ZoneWideStoragePoolAllocator" /> |
There was a problem hiding this comment.
Placing HPEStorage first in the allocator order means it will be attempted before LocalStorage for all allocation requests. This could impact existing deployments when they upgrade, as HPE allocator will now be evaluated first even for non-HPE storage. Consider documenting this ordering decision or making it configurable per deployment to avoid unexpected behavior changes.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11919 +/- ##
============================================
- Coverage 17.56% 17.56% -0.01%
+ Complexity 15542 15541 -1
============================================
Files 5909 5910 +1
Lines 529058 529130 +72
Branches 64617 64629 +12
============================================
Hits 92922 92922
- Misses 425683 425755 +72
Partials 10453 10453
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:
|
2 participants
Description
This PR introduces a new HPE Storage Pool Allocator implementation with an enhanced maxfree volume allocation algorithm for improved storage resource management in Apache CloudStack.
Key Features
Algorithm Behavior
Files Modified
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
How Has This Been Tested?
Unit Testing
Integration Testing
System Testing
Breaking Testing Attempts