Uh oh!
There was an error while loading. Please reload this page.
add a maintenance prepare error event - #13669
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds a new CloudStack event type to record failures while preparing a host for maintenance, and extends unit tests to validate event emission and entity mapping.
Changes:
- Introduce
MAINT.PREPARE.ERRORevent type and map it toHostentity. - Emit an error ActionEvent when a host enters
PrepareForMaintenanceWithErrors. - Add/extend unit tests validating the emitted event and entity mapping.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| server/src/main/java/com/cloud/resource/ResourceManagerImpl.java | Emits a new error event when host preparation for maintenance fails due to VM/migration issues. |
| api/src/main/java/com/cloud/event/EventTypes.java | Adds the new maintenance prepare error event constant and maps it to Host. |
| server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java | Verifies the new event is emitted and mapped to the correct entity class. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
DaanHoogland
commented
Jul 30, 2026
@blueorangutan package |
blueorangutan
commented
Jul 30, 2026
@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. |
blueorangutan
commented
Jul 30, 2026
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18714 |
DaanHoogland
commented
Jul 30, 2026
@blueorangutan test |
blueorangutan
commented
Jul 30, 2026
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
blueorangutan
commented
Jul 31, 2026
[SF] Trillian test result (tid-16660)
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (4)
server/src/main/java/com/cloud/resource/ResourceManagerImpl.java:1731
- The event description uses
%swithhost, which relies onHostVO.toString()and can produce unstable/log-noisy output iftoString()changes. Prefer a stable identifier (e.g., host id/uuid/name) in the message so logs and external consumers get consistent descriptions.
ActionEventUtils.onCompletedActionEvent(CallContext.current().getCallingUserId(), CallContext.current().getCallingAccountId(),
EventVO.LEVEL_ERROR, EventTypes.EVENT_MAINTENANCE_PREPARE_ERROR,
String.format("failed to prepare host %s for maintenance due to migration or VM state errors", host), host.getId(), null, 0);
server/src/main/java/com/cloud/resource/ResourceManagerImpl.java:1729
- CallContext is retrieved twice on the same line. Store
CallContext.current()in a local variable and reuse it to avoid duplicate thread-local lookups and make the code easier to read (and safer if the implementation ever changes).
ActionEventUtils.onCompletedActionEvent(CallContext.current().getCallingUserId(), CallContext.current().getCallingAccountId(),
server/src/test/java/com/cloud/resource/ResourceManagerImplTest.java:459
- This assertion hard-codes the full formatted description using the
hostobject'stoString(), which makes the test brittle iftoString()is modified (or if the mock's string representation changes). Consider asserting on stable substrings/fields (e.g., includes host id/uuid) or matching the description with a less fragile matcher aligned with the intended message contract.
String expectedDescription = String.format("failed to prepare host %s for maintenance due to migration or VM state errors", host);
actionEventUtilsMocked.verify(() -> ActionEventUtils.onCompletedActionEvent(
anyLong(), anyLong(), eq(EventVO.LEVEL_ERROR), eq(EventTypes.EVENT_MAINTENANCE_PREPARE_ERROR),
eq(expectedDescription), eq(hostId), isNull(), eq(0L)));
api/src/main/java/com/cloud/event/EventTypes.java:461
- The PR description template is largely unfilled (type of change, severity/scale, and testing notes), but this change introduces a new externally visible event type. Please update the PR description with concrete behavior change details and how it was tested so automation/maintainers can correctly label and document it.
public static final String EVENT_MAINTENANCE_PREPARE_ERROR = "MAINT.PREPARE.ERROR";



Description
This PR...
Fixes: #13324
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?