SharedFS: fix NPE when accessing systemvm templates for unsupported hypervisors - #13930
Conversation
… in deploySharedFSVM()
There was a problem hiding this comment.
Pull request overview
This PR addresses an intermittent NullPointerException during SharedFS VM deployment by ensuring unsupported hypervisors (or hypervisors without a downloaded SystemVM template) don’t cause template dereferences, and by tightening the hypervisor list used for SystemVM-based deployments.
Changes:
- Treat the
getSupportedHypervisorTypesboolean flag as “for SystemVM” and use it from the SharedFS deployment path. - Fix SharedFS deployment to continue iterating hypervisors when a SystemVM template is missing, instead of dereferencing a null template.
- Update the SharedFS unit test stubbing and related method signatures to match the new boolean flag name.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java | Updates mock method signature to match the renamed boolean parameter. |
| server/src/main/java/com/cloud/resource/ResourceManagerImpl.java | Renames the boolean parameter and keeps filtering logic for SystemVM-oriented hypervisor selection. |
| plugins/storage/sharedfs/storagevm/src/test/java/org/apache/cloudstack/storage/sharedfs/lifecycle/StorageVmSharedFSLifeCycleTest.java | Updates stubs to call getSupportedHypervisorTypes(..., true, ...) for SharedFS deployment tests. |
| plugins/storage/sharedfs/storagevm/src/main/java/org/apache/cloudstack/storage/sharedfs/lifecycle/StorageVmSharedFSLifeCycle.java | Uses SystemVM-filtered hypervisors and prevents NPE by continuing when templates are missing. |
| api/src/main/java/com/cloud/resource/ResourceService.java | Renames the boolean parameter in the API interface to reflect SystemVM intent. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.22 #13930 +/- ##
============================================
+ Coverage 17.86% 17.92% +0.06%
- Complexity 16037 16113 +76
============================================
Files 5928 5928
Lines 534479 534964 +485
Branches 65410 65463 +53
============================================
+ Hits 95468 95876 +408
- Misses 428173 428180 +7
- Partials 10838 10908 +70
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:
|
|
@blueorangutan package |
|
@abh1sar 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 18928 |
|
@GaOrtiga , did you happen to test? |
|
No @DaanHoogland , only code review. |
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
|
ping @abh1sar |
|
Although the main issue was already fixed by #13830, this PR offers a more complete fix. |
|
@blueorangutan package |
|
@abh1sar 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. |
b343252 to
52210d1
Compare
|
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 19181 |
|
@blueorangutan package |
|
@abh1sar 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 19197 |
There was a problem hiding this comment.
LGTM
Tested on a zone with an External (Proxmox) cluster next to KVM and VMware, which is the setup from #13837. SystemVM templates were present for KVM and VMware only and none for External, so the conditions for the NPE were in place.
Created Shared FileSystems repeatedly, 12 in total, since the original failure was intermittent and depended on where External landed in the shuffled list. No NPE on any of them and nothing in the management server log. Every deploy that had capacity landed on KVM and never on External.
The only failures I saw were InsufficientServerCapacityException, from the VMware hosts in this zone being down and from my own testing filling the environment up. Neither is related to this change.
Great work @abh1sar !
github-project-automation
Bot
moved this from conflict/waiting
to Done
in CloudStack Testing
Description
This PR fixes #13837
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?