Skip to content

fix(linstor): pre-flight check destination is a LINSTOR satellite before live migration - #13077

Merged
DaanHoogland merged 1 commit into
apache:mainfrom
jmsperu:feature/linstor-migration-preflight
May 19, 2026
Merged

fix(linstor): pre-flight check destination is a LINSTOR satellite before live migration#13077
DaanHoogland merged 1 commit into
apache:mainfrom
jmsperu:feature/linstor-migration-preflight

Conversation

@jmsperu

Copy link
Copy Markdown
Collaborator

Summary

LinstorDataMotionStrategy.copyAsync (added in #12830 / Jan 29 2026) calls LinstorUtil.createResource on the destination pool's controller without first verifying the destination KVM host is registered as a LINSTOR satellite there. Two failure modes follow:

  1. Wrong-node placement. The resource group's auto-placement filter picks a different registered satellite that is NOT the migration destination. The resource silently lands on the wrong node. The subsequent migrate then fails because the destination KVM host has no DRBD device for the resource.
  2. Opaque auto-place failure. The auto-placement filter has no candidates and the LINSTOR API returns a generic error. Operator has to correlate the migration failure with an unrelated controller log entry to understand what went wrong.

We've hit (1) on host-5 → host-6 migrations where host-6 was not registered as a LINSTOR satellite. The migration logs were unhelpful.

Change

Adds verifyDestinationIsLinstorSatellite() called at the top of copyAsync. For each LINSTOR-typed pool in volumeDataStoreMap:

  • Fetches the controller's node list via LinstorUtil.getLinstorNodeNames
  • If destHost.getName() is missing from the list, throws CloudRuntimeException with a clear actionable message that lists the known satellites and tells the operator to either linstor node create or pick a different destination
  • Silently skips on transient controller errors (so a controller hiccup doesn't block an otherwise valid migration — the existing flow surfaces the real failure downstream)

Non-LINSTOR destination pools are skipped (mixed-storage migrations are unaffected).

Test plan

  • CI build + unit tests
  • Manual: migrate a VM to a host that IS a LINSTOR satellite — works as before, no behaviour change
  • Manual: migrate a VM to a host that is NOT a LINSTOR satellite — fails fast with the new error message naming the host and listing known satellites

…ore live migration
LinstorDataMotionStrategy.copyAsync would call createResource on the
destination pool's controller without first verifying that the
destination KVM host is registered as a LINSTOR satellite there. Two
failure modes:
1. The resource group's auto-placement filter happens to match a
different node (a registered satellite that is NOT the migration
destination), and the resource is silently created on the wrong
node. The subsequent migrate then fails because the destination
KVM host has no DRBD device for the resource.
2. The auto-placement filter has no candidates and the LINSTOR API
returns an opaque error. The operator has to correlate the
migration failure with an unrelated controller log entry to
understand what happened.
This change adds verifyDestinationIsLinstorSatellite() called at the
top of copyAsync. For each LINSTOR-typed destination pool it:
- fetches the controller's node list via LinstorUtil.getLinstorNodeNames
- throws CloudRuntimeException with a clear actionable message
(lists known satellites) if destHost.getName() is missing from
that list
- silently skips on transient controller errors so a network blip
against the controller doesn't block an otherwise valid migration
Non-LINSTOR destination pools in the volumeDataStoreMap are skipped
(mixed-storage migrations are unaffected).

CopilotAI 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

Adds a pre-flight validation to LINSTOR live migration to ensure the destination KVM host is registered as a LINSTOR satellite on the relevant controller(s), so migrations fail fast with an actionable error instead of failing later via LINSTOR auto-placement.

Changes:

  • Introduces verifyDestinationIsLinstorSatellite(...) to query LINSTOR node lists per destination pool/controller.
  • Calls the pre-flight check at the start of copyAsync(...) to abort early with a clear CloudRuntimeException when the destination host is not a satellite.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov

codecovBot commented Apr 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 18.02%. Comparing base (6f4445c) to head (1a1f8ff).
⚠️ Report is 4 commits behind head on main.

Files with missing linesPatch %Lines
...tack/storage/motion/LinstorDataMotionStrategy.java0.00%29 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #13077 +/- ##
============================================
- Coverage 18.02% 18.02% -0.01% + Complexity 16621 16620 -1 
============================================
Files 6029 6029 Lines 542184 542210 +26 Branches 66451 66455 +4 ============================================
- Hits 97740 97737 -3 - Misses 433428 433457 +29 
Partials 11016 11016 
FlagCoverage Δ
uitests3.52% <ø> (ø)
unittests19.18% <0.00%> (-0.01%)⬇️

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

☔ View full report in Codecov by Sentry.
📢 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.

rp-
rp- approved these changes May 5, 2026
@rp-

rp- commented May 5, 2026

Copy link
Copy Markdown
Contributor

LGTM

@sureshanaparti

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17716

@jmsperu

Copy link
Copy Markdown
CollaboratorAuthor

Thanks @rp- and @sureshanaparti — should be ready to merge whenever convenient.

@sureshanaparti

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

@sureshanaparti a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

@blueorangutan

Copy link
Copy Markdown

[SF] Trillian test result (tid-16060)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 48504 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr13077-t16060-kvm-ol8.zip
Smoke tests completed. 151 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

TestResultTime (s)Test File

@DaanHoogland
DaanHoogland merged commit 68221cf into apache:mainMay 19, 2026
30 of 32 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@jmsperu@rp-@sureshanaparti@blueorangutan@DaanHoogland@winterhazel