server: skip HA restart for VMs in Error state - #13811
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4.20 #13811 +/- ##
=========================================
Coverage 16.26% 16.27%
- Complexity 13434 13437 +3
=========================================
Files 5667 5667
Lines 500731 500737 +6
Branches 60803 60805 +2
=========================================
+ Hits 81455 81471 +16
+ Misses 410172 410159 -13
- Partials 9104 9107 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This pull request prevents CloudStack’s HA subsystem from attempting to restart VMs that are already in VirtualMachine.State.Error, aligning HA behavior with the VM lifecycle (no normal start transition from Error) and addressing issue #13785.
Changes:
- Add an early return in
scheduleRestart(...)to avoid creating HA work and triggering side effects forError-state VMs. - Add an early return in
restart(...)so already-queued HA work forError-state VMs completes without proceeding into investigation/fencing/restart logic. - Add focused regression tests in
HighAvailabilityManagerImplTestcovering both scheduling-time and execution-time guards.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| server/src/main/java/com/cloud/ha/HighAvailabilityManagerImpl.java | Adds Error-state guards in HA scheduling and worker execution paths to skip HA restart processing. |
| server/src/test/java/com/cloud/ha/HighAvailabilityManagerImplTest.java | Adds regression tests validating no HA work/side effects are triggered for Error-state VMs and normal processing continues otherwise. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if (VirtualMachine.State.Error.equals(vm.getState())) { | ||
| logger.info("Skipping HA restart for VM {} because it is in Error state", vm); | ||
| return null; | ||
| } |
Description
Fixes #13785.
Problem
When deployment of an HA-enabled VM fails, the VM can be left in
Errorstate. CloudStack could subsequently pass that VM toHighAvailabilityManagerImpl.scheduleRestart(...)and create HA work for it. If the work item was created while the VM was already inError, the worker's existing state and update checks still matched and HA processing could continue into host investigation, fencing, forced-stop, storage and restart handling.This is inconsistent with the VM lifecycle:
Errorrepresents a failed or inconsistent VM state and there is no normal start transition fromError.Root cause
The shared HA restart entry point did not reject VMs in
Errorstate, and the HA worker did not independently reject an already-persisted work item when the current VM state wasError.Change
This PR enforces the lifecycle invariant at both boundaries:
scheduleRestart(...)returns before any HA work, forced stop, orchestration or alert side effect is attempted for anError-state VM.restart(...)treats an already-queued HA restart for anError-state VM as complete before host investigation, fencing, storage checks or VM start handling.The execution-time check also covers work persisted before an upgrade and the race where a VM enters
Errorafter scheduling but before the HA worker processes it.The change deliberately does not make
Errorstartable, rewrite the VM state, suppress an exception after the operation has begun, or alter HA behaviour for valid VM states. There are no API, database, configuration or UI changes.The branch is based directly on the current
4.20head so that the fix can be merged forward into later release branches.Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate)
Not applicable; this is management-server HA behaviour with no UI change.
How Has This Been Tested?
Focused regression tests were added to
HighAvailabilityManagerImplTest:scheduleRestartVMInErrorStateverifies that anError-state VM cannot create HA work or invoke orchestration or alert side effects.restartVMInErrorStateverifies that already-queued work returns without host lookup, alerting, user-VM start handling, volume restart checks or direct work-step mutation.restartVMNotInErrorStateContinuesProcessingverifies that the new worker guard does not stop normal processing for a valid non-ErrorVM state.The final branch diff was audited against the current Apache
4.20head. It contains one commit and changes only the HA implementation and its regression test class: two files, 58 additions and no deletions.The upstream checks must rerun after the rebase, so no new passing CI result is claimed here yet.
How did you try to break this feature and the system with this change?
ErrorVM.ErrorVM.scheduleRestart(...)call paths and retained all valid-state HA behaviour.ForceHA, valid-state host recovery, migration timeout recovery, host maintenance/degraded handling and out-of-band stop recovery unchanged.