refactor storepool automation #11789
Open
+227
−290
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@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. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #11789 +/- ##
=========================================
Coverage 16.17% 16.17%
- Complexity 13297 13298 +1
=========================================
Files 5656 5656
Lines 498331 498326 -5
Branches 60476 60463 -13
=========================================
Hits 80591 80591
+ Misses 408767 408763 -4
+ Partials 8973 8972 -1
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:
|
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15281 |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-14519)
|
DaanHoogland
changed the title
|
[SF] Trillian test result (tid-14533)
|
14 tasks
JoaoJandre
approved these changes
Oct 8, 2025
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the StoragePoolAutomationImpl class by breaking down its two large methods (maintain and cancelMaintain) into smaller, more focused helper methods and consolidating duplicated code.
Key changes:
- Extracted multiple helper methods from the large
maintainandcancelMaintainmethods - Consolidated VM handling logic into reusable methods
- Removed unused imports and injected dependencies
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Comment on lines
+315
to
+317
| if (!(storagePool.getParent().equals(pool.getParent()) || !pool.getParent().equals(storagePool.getId())) && | ||
| (storagePool.getStatus() == StoragePoolStatus.PrepareForMaintenance)) { | ||
| throw new CloudRuntimeException(String.format("Only one storage pool in a cluster can be in PrepareForMaintenance mode, %s is already in PrepareForMaintenance mode ", storagePool)); |
There was a problem hiding this comment.
The boolean logic in this condition is complex and hard to understand. Consider breaking it down into named boolean variables or simplifying the logic for better readability.
Suggested change
| if (!(storagePool.getParent().equals(pool.getParent()) || !pool.getParent().equals(storagePool.getId())) && | |
| (storagePool.getStatus() == StoragePoolStatus.PrepareForMaintenance)) { | |
| throw new CloudRuntimeException(String.format("Only one storage pool in a cluster can be in PrepareForMaintenance mode, %s is already in PrepareForMaintenance mode ", storagePool)); | |
| boolean sameParent = storagePool.getParent().equals(pool.getParent()); | |
| boolean poolIsParentOfStoragePool = pool.getParent().equals(storagePool.getId()); | |
| boolean isOtherPoolInMaintenance = storagePool.getStatus() == StoragePoolStatus.PrepareForMaintenance; | |
| // Only one storage pool in a cluster can be in PrepareForMaintenance mode | |
| if (!(sameParent || !poolIsParentOfStoragePool) && isOtherPoolInMaintenance) { | |
| throw new CloudRuntimeException(String.format("Only one storage pool in a cluster can be in PrepareForMaintenance mode, %s is already in PrepareForMaintenance mode ", storagePool)); |
Copilot uses AI. Check for mistakes.
Comment on lines
+323
to
+325
| * // check if the vm has a root volume. If not, remove the item from the queue, the vm should be | ||
| * // started only when it has at least one root volume attached to it | ||
| * // don't allow to start vm that doesn't have a root volume |
There was a problem hiding this comment.
The JavaDoc comment is malformed. It should use proper JavaDoc format instead of starting each line with * //. These appear to be regular comments that should either be removed or converted to proper JavaDoc.
Suggested change
| * // check if the vm has a root volume. If not, remove the item from the queue, the vm should be | |
| * // started only when it has at least one root volume attached to it | |
| * // don't allow to start vm that doesn't have a root volume | |
| * Checks if the VM has a root volume. If not, removes the item from the queue. | |
| * The VM should be started only when it has at least one root volume attached to it. | |
| * Do not allow starting a VM that does not have a root volume. |
Copilot uses AI. Check for mistakes.
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
1 similar comment
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
|
@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 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15367 |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian Build Failed (tid-14618) |
|
@blueorangutan test |
|
@rosi-shapeblue a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-14651)
|
|
WIP 1.Prepare & Cancel Maintenance on Primary StorageAction: Put pool in maintenance → cancel. Expected Result: Pool transitions Up → PrepareForMaintenance → Up cleanly, no errors. Actual Result
Evidence
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR cleans up the StoragePoolAutomationImpl class to reduce its two big methods and reuse some code.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?