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.
|
|
@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 18714 |
|
@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-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?