Skip to content

server: skip HA restart for VMs in Error state - #13811

Open
Dogface2k wants to merge 1 commit into
apache:4.20from
Dogface2k:fix-13785-vm-ha-error-state
Open

server: skip HA restart for VMs in Error state#13811
Dogface2k wants to merge 1 commit into
apache:4.20from
Dogface2k:fix-13785-vm-ha-error-state

Conversation

@Dogface2k

@Dogface2k Dogface2k commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Description

Fixes #13785.

Problem

When deployment of an HA-enabled VM fails, the VM can be left in Error state. CloudStack could subsequently pass that VM to HighAvailabilityManagerImpl.scheduleRestart(...) and create HA work for it. If the work item was created while the VM was already in Error, 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: Error represents a failed or inconsistent VM state and there is no normal start transition from Error.

Root cause

The shared HA restart entry point did not reject VMs in Error state, and the HA worker did not independently reject an already-persisted work item when the current VM state was Error.

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 an Error-state VM.
  • restart(...) treats an already-queued HA restart for an Error-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 Error after scheduling but before the HA worker processes it.

The change deliberately does not make Error startable, 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.20 head so that the fix can be merged forward into later release branches.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • Build/CI
  • Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

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:

  • scheduleRestartVMInErrorState verifies that an Error-state VM cannot create HA work or invoke orchestration or alert side effects.
  • restartVMInErrorState verifies that already-queued work returns without host lookup, alerting, user-VM start handling, volume restart checks or direct work-step mutation.
  • restartVMNotInErrorStateContinuesProcessing verifies that the new worker guard does not stop normal processing for a valid non-Error VM state.

The final branch diff was audited against the current Apache 4.20 head. 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?

  • Covered the scheduling path with no host setup, ensuring the old null-host forced-stop path is not reached for an Error VM.
  • Covered an HA work item that already exists in the database, rather than relying only on prevention at scheduling time.
  • Covered the false side of the worker state guard to prove normal HA processing still continues for a non-Error VM.
  • Reviewed the production scheduleRestart(...) call paths and retained all valid-state HA behaviour.
  • Kept ForceHA, valid-state host recovery, migration timeout recovery, host maintenance/degraded handling and out-of-band stop recovery unchanged.
  • Added or modified no workflow files.

@Dogface2k
Dogface2k marked this pull request as draft August 6, 2026 10:05
@Dogface2k
Dogface2k marked this pull request as ready for review August 6, 2026 10:13

@DaanHoogland DaanHoogland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clgtm

@codecov

codecov Bot commented Aug 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 16.27%. Comparing base (549daae) to head (03006c2).

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     
Flag Coverage Δ
uitests 4.14% <ø> (ø)
unittests 17.12% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 for Error-state VMs.
  • Add an early return in restart(...) so already-queued HA work for Error-state VMs completes without proceeding into investigation/fencing/restart logic.
  • Add focused regression tests in HighAvailabilityManagerImplTest covering 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.

Comment on lines +608 to +611
if (VirtualMachine.State.Error.equals(vm.getState())) {
logger.info("Skipping HA restart for VM {} because it is in Error state", vm);
return null;
}
@DaanHoogland DaanHoogland added this to the 4.20.4 milestone Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants