diff --git a/api/src/main/java/com/cloud/resource/ResourceService.java b/api/src/main/java/com/cloud/resource/ResourceService.java index 3cdf8fc64e99..202d7fe63c49 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 getSupportedHypervisorTypes(long zoneId, boolean forVirtualRouter, Long podId); + List 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 15c45226e00a..36b060fd6150 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 @@ private UserVm deploySharedFSVM(Long zoneId, Account owner, List networkId ServiceOffering serviceOffering = serviceOfferingDao.findById(serviceOfferingId); DataCenter zone = dataCenterDao.findById(zoneId); - List hypervisors = resourceMgr.getSupportedHypervisorTypes(zoneId, false, null); + List hypervisors = resourceMgr.getSupportedHypervisorTypes(zoneId, true, null); if (hypervisors.size() > 0) { Collections.shuffle(hypervisors); } else { @@ -183,8 +183,9 @@ private UserVm deploySharedFSVM(Long zoneId, Account owner, List networkId 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 dd7f873998ea..31d109e7d66d 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.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 @@ private SharedFS prepareDeploySharedFS() throws ResourceUnavailableException, In 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 void testDeploySharedFS() throws ResourceUnavailableException, Insufficie 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 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 void testDeploySharedFSTemplateNotFound() throws ResourceUnavailableExcep 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 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 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 265323547204..275658a27e95 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 boolean configure(final String name, final Map params) th } @Override - public List getSupportedHypervisorTypes(final long zoneId, final boolean forVirtualRouter, final Long podId) { + public List getSupportedHypervisorTypes(final long zoneId, final boolean forSystemVm, final Long podId) { final List hypervisorTypes = new ArrayList<>(); List clustersForZone; @@ -2974,7 +2974,7 @@ public List getSupportedHypervisorTypes(final long zoneId, final 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 8b62861165f9..01be6db73d88 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 DataCenter getZone(Long zoneId) { * @see com.cloud.resource.ResourceService#getSupportedHypervisorTypes(long, boolean, java.lang.Long) */ @Override - public List getSupportedHypervisorTypes(final long zoneId, final boolean forVirtualRouter, final Long podId) { + public List getSupportedHypervisorTypes(final long zoneId, final boolean forSystemVm, final Long podId) { // TODO Auto-generated method stub return null; }