Fix NPE on external/unmanaged instance import using custom offerings#12884
Fix NPE on external/unmanaged instance import using custom offerings#12884abh1sar merged 4 commits intoapache:4.20from
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #12884 +/- ##
=========================================
Coverage 16.25% 16.26%
- Complexity 13419 13428 +9
=========================================
Files 5664 5664
Lines 500467 500509 +42
Branches 60780 60785 +5
=========================================
+ Hits 81354 81399 +45
Misses 410018 410018
+ Partials 9095 9092 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Fixes a regression where importing unmanaged/external instances with custom (dynamic) compute offerings can hit NPEs when CPU/RAM are not populated on the offering, by deriving CPU/RAM from the hypervisor (unmanaged) or from provided details (external/disk-based imports) and moving resource-limit reservations closer to the specific import paths.
Changes:
- Move VM (cpu/memory/vm count) resource-limit reservations out of top-level import methods and into specific import flows.
- Add pre-checks to determine CPU/RAM for unmanaged-instance imports (hypervisor vs offering) and for external KVM imports (offering vs
details). - Ensure reservations are closed via
ReservationHelper.closeAll(...)around import/conversion flows.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java
Show resolved
Hide resolved
|
@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 17237 |
|
Tested the following test cases with custom and fixed service offerings and verified that the resource counts were being incremented accordingly
|
|
@blueorangutan test |
|
@abh1sar a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian Build Failed (tid-15736) |
|
@blueorangutan test |
|
@abh1sar a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
As @abh1sar already tested most of the affected workflows (#12884 (comment)), I focused on testing the remaining one (import of VMs belonging to remote hosts). My tests consisted of attempting to import an instance from a remote host using both fixed and custom offerings, validating that the process works as intended while ensuring that it is not possible to exceed the configured limits. EvidenceImport using a custom offeringImport using a fixed offeringOnly the unit tests suggested at #12884 (comment) are pending now. |
|
[SF] Trillian test result (tid-15742)
|
|
@blueorangutan package |
|
@winterhazel 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 17257 |
Description
This PR addresses a regression in the import of unmanaged/external VMs using a custom compute offering (reported in https://lists.apache.org/thread/1bvxjc197zhj61mtjxpm3tz1o27znjmv).
As both
serviceOffering.getCpu()andserviceOffering.getRamSize()return null when the offering is custom constrained/unconstrained, we need to check the amount of CPUs and memory returned by the hypervisor in case an unmanaged instance is being imported, or thecpuNumberandmemorydetails in case the instance belongs to a remote host/is being imported from its disk.Types of changes
Feature/Enhancement Scale or Bug Severity
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
The following operations were validated for both fixed and custom offerings (see #12884 (comment) and #12884 (comment)):