Conversation
server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java
Outdated
Show resolved
Hide resolved
…erviceImpl.java Co-authored-by: Suresh Kumar Anaparti <sureshkumar.anaparti@gmail.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes DRS skip behavior when the SKIP_DRS VM detail is set but not present in the in-memory VM details map by centralizing skip logic and consulting the DB-backed VM details DAO.
Changes:
- Added DB-backed lookup of
VmDetailConstants.SKIP_DRSviaUserVmDetailsDao. - Centralized DRS skip logic into
shouldSkipVMForDRSand reused it in both the main loop andskipDrs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java
Show resolved
Hide resolved
server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 4.20 #12994 +/- ##
============================================
- Coverage 16.26% 16.26% -0.01%
+ Complexity 13434 13430 -4
============================================
Files 5665 5665
Lines 500530 500537 +7
Branches 60787 60780 -7
============================================
+ Hits 81411 81412 +1
- Misses 410028 410034 +6
Partials 9091 9091
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:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
server/src/test/java/org/apache/cloudstack/cluster/ClusterDrsServiceImplTest.java
Show resolved
Hide resolved
|
@blueorangutan package |
|
@vishesh92 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 17444 |
|
[SF] Trillian Build Failed (tid-15852) |
Description
When a VM has
skipFromDRSdetail set, it is still not getting skipped. This PR fixes it.Details
This pull request refactors how the system determines whether a VM should be skipped for DRS (Distributed Resource Scheduler) operations. The main improvement is centralizing and simplifying the skip logic, now using the `UserVmDetailsDao` to check VM details in the database rather than relying solely on in-memory details. This ensures more accurate and consistent behavior.
Key changes:
Refactoring skip logic for DRS:
shouldSkipVMForDRSmethod to encapsulate all logic for determining if a VM should be skipped, making the code cleaner and easier to maintain.shouldSkipVMForDRSmethod, ensuring consistent skip checks throughout the codebase.Improved VM detail retrieval:
SKIP_DRSflag usingUserVmDetailsDaofrom the database, instead of only checking the VM's in-memory details map. This makes the skip logic more robust and reliable.Dependency injection:
UserVmDetailsDaoto enable database access for VM details.UserVmDetailVOandUserVmDetailsDaofor use in the new logic.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?