This is an automated email from the ASF dual-hosted git repository.
RosiKyu pushed a commit to branch 4.22
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.22 by this push:
new 9030443eac5 SharedFS: fix NPE when accessing systemvm templates for
unsupported hypervisors (#13930)
9030443eac5 is described below
commit 9030443eac555c26b06bbf8e82249f2aac385fcc
Author: Abhisar Sinha <[email protected]>
AuthorDate: Fri Sep 11 20:14:32 2026 +0530
SharedFS: fix NPE when accessing systemvm templates for unsupported
hypervisors (#13930)
* fix NPE when accessing systemvm templates for unsupported hypervisors in
deploySharedFSVM()
* systemvm -> SystemVM
Co-authored-by: Copilot Autofix powered by AI
<[email protected]>
* Add unit test for skip-hypervisor-without-template fix in
deploySharedFSVM()
* sharedfs: drop duplicate skip-hypervisor test after 4.22 merge
---------
Co-authored-by: Copilot Autofix powered by AI
<[email protected]>
---
.../java/com/cloud/resource/ResourceService.java | 2 +-
.../lifecycle/StorageVmSharedFSLifeCycle.java | 5 +-
.../lifecycle/StorageVmSharedFSLifeCycleTest.java | 101 ++++++++++++---------
.../com/cloud/resource/ResourceManagerImpl.java | 4 +-
.../cloud/resource/MockResourceManagerImpl.java | 2 +-
5 files changed, 66 insertions(+), 48 deletions(-)
diff --git a/api/src/main/java/com/cloud/resource/ResourceService.java
b/api/src/main/java/com/cloud/resource/ResourceService.java
index 3cdf8fc64e9..202d7fe63c4 100644
--- a/api/src/main/java/com/cloud/resource/ResourceService.java
+++ b/api/src/main/java/com/cloud/resource/ResourceService.java
@@ -91,7 +91,7 @@ public interface ResourceService {
DataCenter getZone(Long zoneId);
- List<HypervisorType> getSupportedHypervisorTypes(long zoneId, boolean
forVirtualRouter, Long podId);
+ List<HypervisorType> getSupportedHypervisorTypes(long zoneId, boolean
forSystemVm, Long podId);
boolean releaseHostReservation(Long hostId);
diff --git
a/plugins/storage/sharedfs/storagevm/src/main/java/org/apache/cloudstack/storage/sharedfs/lifecycle/StorageVmSharedFSLifeCycle.java
b/plugins/storage/sharedfs/storagevm/src/main/java/org/apache/cloudstack/storage/sharedfs/lifecycle/StorageVmSharedFSLifeCycle.java
index 15c45226e00..36b060fd615 100644
---
a/plugins/storage/sharedfs/storagevm/src/main/java/org/apache/cloudstack/storage/sharedfs/lifecycle/StorageVmSharedFSLifeCycle.java
+++
b/plugins/storage/sharedfs/storagevm/src/main/java/org/apache/cloudstack/storage/sharedfs/lifecycle/StorageVmSharedFSLifeCycle.java
@@ -160,7 +160,7 @@ public class StorageVmSharedFSLifeCycle implements
SharedFSLifeCycle {
ServiceOffering serviceOffering =
serviceOfferingDao.findById(serviceOfferingId);
DataCenter zone = dataCenterDao.findById(zoneId);
- List<Hypervisor.HypervisorType> hypervisors =
resourceMgr.getSupportedHypervisorTypes(zoneId, false, null);
+ List<Hypervisor.HypervisorType> hypervisors =
resourceMgr.getSupportedHypervisorTypes(zoneId, true, null);
if (hypervisors.size() > 0) {
Collections.shuffle(hypervisors);
} else {
@@ -183,8 +183,9 @@ public class StorageVmSharedFSLifeCycle implements
SharedFSLifeCycle {
if (template == null) {
if (iter.hasNext()) {
continue;
+ } else {
+ throw new CloudRuntimeException(String.format("Unable to
find the SystemVM template for any of the available hypervisors in zone %s.",
zone.toString()));
}
- throw new CloudRuntimeException(String.format("Unable to find
the systemvm template for %s or it was not downloaded in %s.",
hypervisor.toString(), zone.toString()));
}
LaunchPermissionVO existingPermission =
launchPermissionDao.findByTemplateAndAccount(template.getId(), owner.getId());
diff --git
a/plugins/storage/sharedfs/storagevm/src/test/java/org/apache/cloudstack/storage/sharedfs/lifecycle/StorageVmSharedFSLifeCycleTest.java
b/plugins/storage/sharedfs/storagevm/src/test/java/org/apache/cloudstack/storage/sharedfs/lifecycle/StorageVmSharedFSLifeCycleTest.java
index dd7f873998e..31d109e7d66 100644
---
a/plugins/storage/sharedfs/storagevm/src/test/java/org/apache/cloudstack/storage/sharedfs/lifecycle/StorageVmSharedFSLifeCycleTest.java
+++
b/plugins/storage/sharedfs/storagevm/src/test/java/org/apache/cloudstack/storage/sharedfs/lifecycle/StorageVmSharedFSLifeCycleTest.java
@@ -81,9 +81,12 @@ import static org.mockito.ArgumentMatchers.anyList;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.anyMap;
import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@RunWith(MockitoJUnitRunner.class)
@@ -230,7 +233,7 @@ public class StorageVmSharedFSLifeCycleTest {
DataCenterVO zone = mock(DataCenterVO.class);
when(dataCenterDao.findById(s_zoneId)).thenReturn(zone);
- when(resourceMgr.getSupportedHypervisorTypes(s_zoneId, false,
null)).thenReturn(List.of(Hypervisor.HypervisorType.KVM));
+ when(resourceMgr.getSupportedHypervisorTypes(s_zoneId, true,
null)).thenReturn(List.of(Hypervisor.HypervisorType.KVM));
ServiceOfferingVO serviceOffering = mock(ServiceOfferingVO.class);
when(serviceOfferingDao.findById(s_serviceOfferingId)).thenReturn(serviceOffering);
@@ -274,46 +277,6 @@ public class StorageVmSharedFSLifeCycleTest {
Assert.assertEquals(Optional.ofNullable(result.second()),
Optional.ofNullable(s_vmId));
}
- @Test
- public void
testDeploySharedFSContinuesWhenTemplateIsMissingForNonLastHypervisor() throws
ResourceUnavailableException, InsufficientCapacityException,
ResourceAllocationException, IOException, OperationTimedoutException {
- SharedFS sharedFS = prepareDeploySharedFS();
- when(resourceMgr.getSupportedHypervisorTypes(s_zoneId, false,
null)).thenReturn(new ArrayList<>(List.of(Hypervisor.HypervisorType.External,
Hypervisor.HypervisorType.KVM)) {
- @Override
- public Hypervisor.HypervisorType set(int index,
Hypervisor.HypervisorType element) {
- // Keep the test order stable while exercising the production
shuffle call.
- return get(index);
- }
- });
- when(templateDao.findSystemVMReadyTemplate(s_zoneId,
Hypervisor.HypervisorType.External,
ResourceManager.SystemVmPreferredArchitecture.defaultValue())).thenReturn(null);
-
- Account owner = mock(Account.class);
- when(owner.getId()).thenReturn(s_ownerId);
- when(accountMgr.getActiveAccountById(s_ownerId)).thenReturn(owner);
-
- UserVm vm = mock(UserVm.class);
- when(vm.getId()).thenReturn(s_vmId);
- when(userVmService.createAdvancedVirtualMachine(
- any(DataCenter.class), any(ServiceOffering.class),
any(VirtualMachineTemplate.class), anyList(), any(Account.class), anyString(),
- anyString(), anyLong(), anyLong(), any(), isNull(),
any(Hypervisor.HypervisorType.class), any(BaseCmd.HTTPMethod.class),
anyString(),
- isNull(), isNull(), anyList(), isNull(),
any(Network.IpAddresses.class), isNull(), isNull(), isNull(),
- anyMap(), isNull(), isNull(), isNull(), isNull(),
- anyBoolean(), anyString(), isNull(), isNull(),
isNull())).thenReturn(vm);
-
- VolumeVO rootVol = mock(VolumeVO.class);
- when(rootVol.getVolumeType()).thenReturn(Volume.Type.ROOT);
- when(rootVol.getName()).thenReturn("ROOT-1");
- VolumeVO dataVol = mock(VolumeVO.class);
- when(dataVol.getId()).thenReturn(s_volumeId);
- when(dataVol.getName()).thenReturn("DATA-1");
- when(dataVol.getVolumeType()).thenReturn(Volume.Type.DATADISK);
- when(volumeDao.findByInstance(s_vmId)).thenReturn(List.of(rootVol,
dataVol));
-
- Pair<Long, Long> result = lifeCycle.deploySharedFS(sharedFS,
s_networkId, s_diskOfferingId, s_size, s_minIops, s_maxIops);
-
- Assert.assertEquals(Optional.ofNullable(result.first()),
Optional.ofNullable(s_volumeId));
- Assert.assertEquals(Optional.ofNullable(result.second()),
Optional.ofNullable(s_vmId));
- }
-
@Test(expected = CloudRuntimeException.class)
public void testDeploySharedFSHypervisorNotFound() throws
ResourceUnavailableException, InsufficientCapacityException,
ResourceAllocationException, IOException, OperationTimedoutException {
SharedFS sharedFS = mock(SharedFS.class);
@@ -341,11 +304,65 @@ public class StorageVmSharedFSLifeCycleTest {
when(accountMgr.getActiveAccountById(s_ownerId)).thenReturn(null);
DataCenterVO zone = mock(DataCenterVO.class);
when(dataCenterDao.findById(s_zoneId)).thenReturn(zone);
- when(resourceMgr.getSupportedHypervisorTypes(s_zoneId, false,
null)).thenReturn(List.of(Hypervisor.HypervisorType.KVM));
+ when(resourceMgr.getSupportedHypervisorTypes(s_zoneId, true,
null)).thenReturn(List.of(Hypervisor.HypervisorType.KVM));
lifeCycle.deploySharedFS(sharedFS, s_networkId, s_diskOfferingId,
s_size, s_minIops, s_maxIops);
}
+ @Test
+ public void testDeploySharedFSSkipsHypervisorWithoutTemplate() throws
ResourceUnavailableException, InsufficientCapacityException,
ResourceAllocationException, IOException, OperationTimedoutException {
+ // Verifies that when the systemvm template is missing for one of the
supported hypervisors,
+ // deploySharedFSVM() skips it and tries the next one instead of
failing with an NPE.
+ SharedFS sharedFS = mock(SharedFS.class);
+ when(sharedFS.getDataCenterId()).thenReturn(s_zoneId);
+ when(sharedFS.getName()).thenReturn(s_name);
+ when(sharedFS.getServiceOfferingId()).thenReturn(s_serviceOfferingId);
+
when(sharedFS.getFsType()).thenReturn(SharedFS.FileSystemType.valueOf(s_fsFormat));
+ when(sharedFS.getAccountId()).thenReturn(s_ownerId);
+
+ Account owner = mock(Account.class);
+ when(owner.getId()).thenReturn(s_ownerId);
+ when(accountMgr.getActiveAccountById(s_ownerId)).thenReturn(owner);
+
+ DataCenterVO zone = mock(DataCenterVO.class);
+ when(dataCenterDao.findById(s_zoneId)).thenReturn(zone);
+
+ List<Hypervisor.HypervisorType> hypervisors = new
ArrayList<>(List.of(Hypervisor.HypervisorType.XenServer,
Hypervisor.HypervisorType.KVM));
+ when(resourceMgr.getSupportedHypervisorTypes(s_zoneId, true,
null)).thenReturn(hypervisors);
+
+ ServiceOfferingVO serviceOffering = mock(ServiceOfferingVO.class);
+
when(serviceOfferingDao.findById(s_serviceOfferingId)).thenReturn(serviceOffering);
+
+ // The hypervisor list is shuffled before iteration, so instead of
pinning which hypervisor is
+ // tried first, the first lookup (whichever hypervisor that is)
returns no template and the
+ // second lookup returns a valid one, exercising the "skip and try the
next hypervisor" fix.
+ VMTemplateVO template = mock(VMTemplateVO.class);
+ when(template.getId()).thenReturn(s_templateId);
+ when(templateDao.findSystemVMReadyTemplate(eq(s_zoneId),
any(Hypervisor.HypervisorType.class),
eq(ResourceManager.SystemVmPreferredArchitecture.defaultValue())))
+ .thenReturn(null, template);
+
+ UserVm vm = mock(UserVm.class);
+ when(vm.getId()).thenReturn(s_vmId);
+ when(userVmService.createAdvancedVirtualMachine(
+ any(DataCenter.class), any(ServiceOffering.class),
any(VirtualMachineTemplate.class), anyList(), any(Account.class), anyString(),
+ anyString(), anyLong(), anyLong(), any(), isNull(),
any(Hypervisor.HypervisorType.class), any(BaseCmd.HTTPMethod.class),
anyString(),
+ isNull(), isNull(), anyList(), isNull(),
any(Network.IpAddresses.class), isNull(), isNull(), isNull(),
+ anyMap(), isNull(), isNull(), isNull(), isNull(),
+ anyBoolean(), anyString(), isNull(), isNull(),
isNull())).thenReturn(vm);
+
+ VolumeVO dataVol = mock(VolumeVO.class);
+ when(dataVol.getId()).thenReturn(s_volumeId);
+ when(dataVol.getName()).thenReturn("DATA-1");
+ when(dataVol.getVolumeType()).thenReturn(Volume.Type.DATADISK);
+ when(volumeDao.findByInstance(s_vmId)).thenReturn(List.of(dataVol));
+
+ Pair<Long, Long> result = lifeCycle.deploySharedFS(sharedFS,
s_networkId, s_diskOfferingId, s_size, s_minIops, s_maxIops);
+ Assert.assertEquals(Optional.of(s_volumeId),
Optional.ofNullable(result.first()));
+ Assert.assertEquals(Optional.of(s_vmId),
Optional.ofNullable(result.second()));
+
+ verify(templateDao, times(2)).findSystemVMReadyTemplate(eq(s_zoneId),
any(Hypervisor.HypervisorType.class),
eq(ResourceManager.SystemVmPreferredArchitecture.defaultValue()));
+ }
+
@Test
public void testDeleteSharedFS() throws ResourceUnavailableException {
SharedFS sharedFS = mock(SharedFS.class);
diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
index 26532354720..275658a27e9 100755
--- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
+++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
@@ -2962,7 +2962,7 @@ public class ResourceManagerImpl extends ManagerBase
implements ResourceManager,
}
@Override
- public List<HypervisorType> getSupportedHypervisorTypes(final long zoneId,
final boolean forVirtualRouter, final Long podId) {
+ public List<HypervisorType> getSupportedHypervisorTypes(final long zoneId,
final boolean forSystemVm, final Long podId) {
final List<HypervisorType> hypervisorTypes = new ArrayList<>();
List<ClusterVO> clustersForZone;
@@ -2974,7 +2974,7 @@ public class ResourceManagerImpl extends ManagerBase
implements ResourceManager,
for (final ClusterVO cluster : clustersForZone) {
final HypervisorType hType = cluster.getHypervisorType();
- if (!forVirtualRouter || (hType != HypervisorType.BareMetal &&
hType != HypervisorType.External && hType != HypervisorType.Ovm)) {
+ if (!forSystemVm || (hType != HypervisorType.BareMetal && hType !=
HypervisorType.External && hType != HypervisorType.Ovm)) {
hypervisorTypes.add(hType);
}
}
diff --git
a/server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java
b/server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java
index 8b62861165f..01be6db73d8 100755
--- a/server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java
+++ b/server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java
@@ -205,7 +205,7 @@ public class MockResourceManagerImpl extends ManagerBase
implements ResourceMana
* @see
com.cloud.resource.ResourceService#getSupportedHypervisorTypes(long, boolean,
java.lang.Long)
*/
@Override
- public List<HypervisorType> getSupportedHypervisorTypes(final long zoneId,
final boolean forVirtualRouter, final Long podId) {
+ public List<HypervisorType> getSupportedHypervisorTypes(final long zoneId,
final boolean forSystemVm, final Long podId) {
// TODO Auto-generated method stub
return null;
}
