From 0f0a1badd95b3f7118cf1380ed2a31b7b502db3a Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 24 Jan 2025 17:50:52 +0530 Subject: [PATCH 1/6] server: fix attach uploaded volume Fixes #10120 When an uploaded volume is attached to a VM for which no existing volume can be found it was resulting in error. For such volumes, server needs to find a suitable pool first and copy them to the pool from secondary store. Signed-off-by: Abhishek Kumar --- .../cloud/storage/VolumeApiServiceImpl.java | 108 ++++++++++++------ 1 file changed, 72 insertions(+), 36 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 7f867eb01a97..26aa3cac85f4 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -133,7 +133,9 @@ import com.cloud.dc.DataCenter; import com.cloud.dc.DataCenterVO; import com.cloud.dc.Pod; +import com.cloud.dc.dao.ClusterDao; import com.cloud.dc.dao.DataCenterDao; +import com.cloud.dc.dao.HostPodDao; import com.cloud.domain.Domain; import com.cloud.domain.dao.DomainDao; import com.cloud.event.ActionEvent; @@ -153,6 +155,7 @@ import com.cloud.hypervisor.HypervisorCapabilitiesVO; import com.cloud.hypervisor.dao.HypervisorCapabilitiesDao; import com.cloud.offering.DiskOffering; +import com.cloud.org.Cluster; import com.cloud.org.Grouping; import com.cloud.projects.Project; import com.cloud.projects.ProjectManager; @@ -323,6 +326,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic @Inject private VmWorkJobDao _workJobDao; @Inject + ClusterDao clusterDao; + @Inject private ClusterDetailsDao _clusterDetailsDao; @Inject private StorageManager storageMgr; @@ -346,6 +351,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic protected ProjectManager projectManager; @Inject protected StoragePoolDetailsDao storagePoolDetailsDao; + @Inject + HostPodDao podDao; protected Gson _gson; @@ -2380,17 +2387,10 @@ public Volume attachVolumeToVM(AttachVolumeCmd command) { return attachVolumeToVM(command.getVirtualMachineId(), command.getId(), command.getDeviceId()); } - private Volume orchestrateAttachVolumeToVM(Long vmId, Long volumeId, Long deviceId) { - VolumeInfo volumeToAttach = volFactory.getVolume(volumeId); - - if (volumeToAttach.isAttachedVM()) { - throw new CloudRuntimeException("This volume is already attached to a VM."); - } - - UserVmVO vm = _userVmDao.findById(vmId); + protected VolumeVO getVmExistingVolumeForVolumeAttach(UserVmVO vm, VolumeInfo volumeToAttach) { VolumeVO existingVolumeOfVm = null; VMTemplateVO template = _templateDao.findById(vm.getTemplateId()); - List rootVolumesOfVm = _volsDao.findByInstanceAndType(vmId, Volume.Type.ROOT); + List rootVolumesOfVm = _volsDao.findByInstanceAndType(vm.getId(), Volume.Type.ROOT); if (rootVolumesOfVm.size() > 1 && template != null && !template.isDeployAsIs()) { throw new CloudRuntimeException("The VM " + vm.getHostName() + " has more than one ROOT volume and is in an invalid state."); } else { @@ -2398,7 +2398,7 @@ private Volume orchestrateAttachVolumeToVM(Long vmId, Long volumeId, Long device existingVolumeOfVm = rootVolumesOfVm.get(0); } else { // locate data volume of the vm - List diskVolumesOfVm = _volsDao.findByInstanceAndType(vmId, Volume.Type.DATADISK); + List diskVolumesOfVm = _volsDao.findByInstanceAndType(vm.getId(), Volume.Type.DATADISK); for (VolumeVO diskVolume : diskVolumesOfVm) { if (diskVolume.getState() != Volume.State.Allocated) { existingVolumeOfVm = diskVolume; @@ -2407,9 +2407,9 @@ private Volume orchestrateAttachVolumeToVM(Long vmId, Long volumeId, Long device } } } - if (s_logger.isTraceEnabled()) { - String msg = "attaching volume %s/%s to a VM (%s/%s) with an existing volume %s/%s on primary storage %s"; - if (existingVolumeOfVm != null) { + if (existingVolumeOfVm != null) { + if (s_logger.isTraceEnabled()) { + String msg = "attaching volume %s/%s to a VM (%s/%s) with an existing volume %s/%s on primary storage %s"; s_logger.trace(String.format(msg, volumeToAttach.getName(), volumeToAttach.getUuid(), vm.getName(), vm.getUuid(), @@ -2417,35 +2417,76 @@ private Volume orchestrateAttachVolumeToVM(Long vmId, Long volumeId, Long device existingVolumeOfVm.getPoolId())); } } + if (s_logger.isTraceEnabled()) { + s_logger.trace(String.format("No existing volume found for VM (%s/%s) to attach volume %s/%s", + vm.getName(), vm.getUuid(), + volumeToAttach.getName(), volumeToAttach.getUuid())); + } + return null; + } - HypervisorType rootDiskHyperType = vm.getHypervisorType(); - HypervisorType volumeToAttachHyperType = _volsDao.getHypervisorType(volumeToAttach.getId()); + protected StoragePool getPoolForAllocatedOrUploadedVolumeForAttach(final VolumeInfo volumeToAttach, final UserVmVO vm) { + DataCenter zone = _dcDao.findById(vm.getDataCenterId()); + Pair clusterHostId = virtualMachineManager.findClusterAndHostIdForVm(vm, false); + long podId = vm.getPodIdToDeployIn(); + if (clusterHostId.first() != null) { + Cluster cluster = clusterDao.findById(clusterHostId.first()); + podId = cluster.getPodId(); + } + Pod pod = podDao.findById(podId); + DiskOfferingVO offering = _diskOfferingDao.findById(volumeToAttach.getDiskOfferingId()); + DiskProfile diskProfile = new DiskProfile(volumeToAttach.getId(), volumeToAttach.getVolumeType(), + volumeToAttach.getName(), volumeToAttach.getId(), volumeToAttach.getSize(), offering.getTagsArray(), + offering.isUseLocalStorage(), offering.isRecreatable(), + volumeToAttach.getTemplateId()); + StoragePool pool = _volumeMgr.findStoragePool(diskProfile, zone, pod, clusterHostId.first(), clusterHostId.second(), vm, null); + if (pool == null) { + throw new CloudRuntimeException(String.format("Failed to find a primary storage for volume in state: %s", volumeToAttach.getState())); + } + return pool; + } + protected VolumeInfo createVolumeOnSecondaryForAttachIfNeeded(final VolumeInfo volumeToAttach, final UserVmVO vm, VolumeVO existingVolumeOfVm) { VolumeInfo newVolumeOnPrimaryStorage = volumeToAttach; - + boolean volumeOnSecondary = volumeToAttach.getState() == Volume.State.Uploaded; + if (!Arrays.asList(Volume.State.Allocated, Volume.State.Uploaded).contains(volumeToAttach.getState())) { + return newVolumeOnPrimaryStorage; + } //don't create volume on primary storage if its being attached to the vm which Root's volume hasn't been created yet - StoragePoolVO destPrimaryStorage = null; + StoragePool destPrimaryStorage = null; if (existingVolumeOfVm != null && !existingVolumeOfVm.getState().equals(Volume.State.Allocated)) { destPrimaryStorage = _storagePoolDao.findById(existingVolumeOfVm.getPoolId()); if (s_logger.isTraceEnabled() && destPrimaryStorage != null) { s_logger.trace(String.format("decided on target storage: %s/%s", destPrimaryStorage.getName(), destPrimaryStorage.getUuid())); } } + if (destPrimaryStorage == null) { + destPrimaryStorage = getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + } + try { + if (volumeOnSecondary && destPrimaryStorage.getPoolType() == Storage.StoragePoolType.PowerFlex) { + throw new InvalidParameterValueException("Cannot attach uploaded volume, this operation is unsupported on storage pool type " + destPrimaryStorage.getPoolType()); + } + newVolumeOnPrimaryStorage = _volumeMgr.createVolumeOnPrimaryStorage(vm, volumeToAttach, + vm.getHypervisorType(), destPrimaryStorage); + } catch (NoTransitionException e) { + s_logger.debug("Failed to create volume on primary storage", e); + throw new CloudRuntimeException("Failed to create volume on primary storage", e); + } + return newVolumeOnPrimaryStorage; + } - boolean volumeOnSecondary = volumeToAttach.getState() == Volume.State.Uploaded; + private Volume orchestrateAttachVolumeToVM(Long vmId, Long volumeId, Long deviceId) { + VolumeInfo volumeToAttach = volFactory.getVolume(volumeId); - if (destPrimaryStorage != null && (volumeToAttach.getState() == Volume.State.Allocated || volumeOnSecondary)) { - try { - if (volumeOnSecondary && destPrimaryStorage.getPoolType() == Storage.StoragePoolType.PowerFlex) { - throw new InvalidParameterValueException("Cannot attach uploaded volume, this operation is unsupported on storage pool type " + destPrimaryStorage.getPoolType()); - } - newVolumeOnPrimaryStorage = _volumeMgr.createVolumeOnPrimaryStorage(vm, volumeToAttach, rootDiskHyperType, destPrimaryStorage); - } catch (NoTransitionException e) { - s_logger.debug("Failed to create volume on primary storage", e); - throw new CloudRuntimeException("Failed to create volume on primary storage", e); - } + if (volumeToAttach.isAttachedVM()) { + throw new CloudRuntimeException("This volume is already attached to a VM."); } + UserVmVO vm = _userVmDao.findById(vmId); + VolumeVO existingVolumeOfVm = getVmExistingVolumeForVolumeAttach(vm, volumeToAttach); + VolumeInfo newVolumeOnPrimaryStorage = createVolumeOnSecondaryForAttachIfNeeded(volumeToAttach, vm, existingVolumeOfVm); + // reload the volume from db newVolumeOnPrimaryStorage = volFactory.getVolume(newVolumeOnPrimaryStorage.getId()); boolean moveVolumeNeeded = needMoveVolume(existingVolumeOfVm, newVolumeOnPrimaryStorage); @@ -2463,19 +2504,17 @@ private Volume orchestrateAttachVolumeToVM(Long vmId, Long volumeId, Long device StoragePoolVO vmRootVolumePool = _storagePoolDao.findById(existingVolumeOfVm.getPoolId()); try { + HypervisorType volumeToAttachHyperType = _volsDao.getHypervisorType(volumeToAttach.getId()); newVolumeOnPrimaryStorage = _volumeMgr.moveVolume(newVolumeOnPrimaryStorage, vmRootVolumePool.getDataCenterId(), vmRootVolumePool.getPodId(), vmRootVolumePool.getClusterId(), volumeToAttachHyperType); - } catch (ConcurrentOperationException e) { - s_logger.debug("move volume failed", e); - throw new CloudRuntimeException("move volume failed", e); - } catch (StorageUnavailableException e) { + } catch (ConcurrentOperationException | StorageUnavailableException e) { s_logger.debug("move volume failed", e); throw new CloudRuntimeException("move volume failed", e); } } VolumeVO newVol = _volsDao.findById(newVolumeOnPrimaryStorage.getId()); // Getting the fresh vm object in case of volume migration to check the current state of VM - if (moveVolumeNeeded || volumeOnSecondary) { + if (moveVolumeNeeded) { vm = _userVmDao.findById(vmId); if (vm == null) { throw new InvalidParameterValueException("VM not found."); @@ -2659,9 +2698,6 @@ private void checkDeviceId(Long deviceId, VolumeInfo volumeToAttach, UserVmVO vm if (!_volsDao.findByInstanceAndDeviceId(vm.getId(), 0).isEmpty()) { throw new InvalidParameterValueException("Vm already has root volume attached to it"); } - if (volumeToAttach.getState() == Volume.State.Uploaded) { - throw new InvalidParameterValueException("No support for Root volume attach in state " + Volume.State.Uploaded); - } } } From af21605d944298e992fb1b80b053482fffb119c2 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 27 Jan 2025 10:01:29 +0530 Subject: [PATCH 2/6] fix Signed-off-by: Abhishek Kumar --- .../src/main/java/com/cloud/storage/VolumeApiServiceImpl.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 26aa3cac85f4..62b2acc0477b 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -2439,7 +2439,8 @@ protected StoragePool getPoolForAllocatedOrUploadedVolumeForAttach(final VolumeI volumeToAttach.getName(), volumeToAttach.getId(), volumeToAttach.getSize(), offering.getTagsArray(), offering.isUseLocalStorage(), offering.isRecreatable(), volumeToAttach.getTemplateId()); - StoragePool pool = _volumeMgr.findStoragePool(diskProfile, zone, pod, clusterHostId.first(), clusterHostId.second(), vm, null); + StoragePool pool = _volumeMgr.findStoragePool(diskProfile, zone, pod, clusterHostId.first(), + clusterHostId.second(), vm, Collections.emptySet()); if (pool == null) { throw new CloudRuntimeException(String.format("Failed to find a primary storage for volume in state: %s", volumeToAttach.getState())); } From da6af2f3cd2389c976417a0802bf6405af9f1522 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 27 Jan 2025 14:06:27 +0530 Subject: [PATCH 3/6] fix Signed-off-by: Abhishek Kumar --- server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 62b2acc0477b..e4a4bb86e633 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -2439,6 +2439,7 @@ protected StoragePool getPoolForAllocatedOrUploadedVolumeForAttach(final VolumeI volumeToAttach.getName(), volumeToAttach.getId(), volumeToAttach.getSize(), offering.getTagsArray(), offering.isUseLocalStorage(), offering.isRecreatable(), volumeToAttach.getTemplateId()); + diskProfile.setHyperType(vm.getHypervisorType()); StoragePool pool = _volumeMgr.findStoragePool(diskProfile, zone, pod, clusterHostId.first(), clusterHostId.second(), vm, Collections.emptySet()); if (pool == null) { From 4c1548aa8163643226f02ae4274cfb378e8e245a Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 27 Jan 2025 15:29:47 +0530 Subject: [PATCH 4/6] add unit tests Signed-off-by: Abhishek Kumar --- .../cloud/storage/VolumeApiServiceImpl.java | 25 +- .../storage/VolumeApiServiceImplTest.java | 249 ++++++++++++++++++ 2 files changed, 262 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index e4a4bb86e633..68546f185b73 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -2407,22 +2407,23 @@ protected VolumeVO getVmExistingVolumeForVolumeAttach(UserVmVO vm, VolumeInfo vo } } } - if (existingVolumeOfVm != null) { + if (existingVolumeOfVm == null) { if (s_logger.isTraceEnabled()) { - String msg = "attaching volume %s/%s to a VM (%s/%s) with an existing volume %s/%s on primary storage %s"; - s_logger.trace(String.format(msg, - volumeToAttach.getName(), volumeToAttach.getUuid(), + s_logger.trace(String.format("No existing volume found for VM (%s/%s) to attach volume %s/%s", vm.getName(), vm.getUuid(), - existingVolumeOfVm.getName(), existingVolumeOfVm.getUuid(), - existingVolumeOfVm.getPoolId())); + volumeToAttach.getName(), volumeToAttach.getUuid())); } + return null; } if (s_logger.isTraceEnabled()) { - s_logger.trace(String.format("No existing volume found for VM (%s/%s) to attach volume %s/%s", + String msg = "attaching volume %s/%s to a VM (%s/%s) with an existing volume %s/%s on primary storage %s"; + s_logger.trace(String.format(msg, + volumeToAttach.getName(), volumeToAttach.getUuid(), vm.getName(), vm.getUuid(), - volumeToAttach.getName(), volumeToAttach.getUuid())); + existingVolumeOfVm.getName(), existingVolumeOfVm.getUuid(), + existingVolumeOfVm.getPoolId())); } - return null; + return existingVolumeOfVm; } protected StoragePool getPoolForAllocatedOrUploadedVolumeForAttach(final VolumeInfo volumeToAttach, final UserVmVO vm) { @@ -2448,7 +2449,7 @@ protected StoragePool getPoolForAllocatedOrUploadedVolumeForAttach(final VolumeI return pool; } - protected VolumeInfo createVolumeOnSecondaryForAttachIfNeeded(final VolumeInfo volumeToAttach, final UserVmVO vm, VolumeVO existingVolumeOfVm) { + protected VolumeInfo createVolumeOnPrimaryForAttachIfNeeded(final VolumeInfo volumeToAttach, final UserVmVO vm, VolumeVO existingVolumeOfVm) { VolumeInfo newVolumeOnPrimaryStorage = volumeToAttach; boolean volumeOnSecondary = volumeToAttach.getState() == Volume.State.Uploaded; if (!Arrays.asList(Volume.State.Allocated, Volume.State.Uploaded).contains(volumeToAttach.getState())) { @@ -2466,7 +2467,7 @@ protected VolumeInfo createVolumeOnSecondaryForAttachIfNeeded(final VolumeInfo v destPrimaryStorage = getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); } try { - if (volumeOnSecondary && destPrimaryStorage.getPoolType() == Storage.StoragePoolType.PowerFlex) { + if (volumeOnSecondary && Storage.StoragePoolType.PowerFlex.equals(destPrimaryStorage.getPoolType())) { throw new InvalidParameterValueException("Cannot attach uploaded volume, this operation is unsupported on storage pool type " + destPrimaryStorage.getPoolType()); } newVolumeOnPrimaryStorage = _volumeMgr.createVolumeOnPrimaryStorage(vm, volumeToAttach, @@ -2487,7 +2488,7 @@ private Volume orchestrateAttachVolumeToVM(Long vmId, Long volumeId, Long device UserVmVO vm = _userVmDao.findById(vmId); VolumeVO existingVolumeOfVm = getVmExistingVolumeForVolumeAttach(vm, volumeToAttach); - VolumeInfo newVolumeOnPrimaryStorage = createVolumeOnSecondaryForAttachIfNeeded(volumeToAttach, vm, existingVolumeOfVm); + VolumeInfo newVolumeOnPrimaryStorage = createVolumeOnPrimaryForAttachIfNeeded(volumeToAttach, vm, existingVolumeOfVm); // reload the volume from db newVolumeOnPrimaryStorage = volFactory.getVolume(newVolumeOnPrimaryStorage.getId()); diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java index a0f89956df55..3e6f9ec63f9d 100644 --- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java @@ -45,6 +45,7 @@ import org.apache.cloudstack.api.command.user.volume.DetachVolumeCmd; import org.apache.cloudstack.api.command.user.volume.MigrateVolumeCmd; import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore; @@ -86,8 +87,12 @@ import com.cloud.api.query.dao.ServiceOfferingJoinDao; import com.cloud.configuration.Resource; import com.cloud.configuration.Resource.ResourceType; +import com.cloud.dc.ClusterVO; import com.cloud.dc.DataCenterVO; +import com.cloud.dc.HostPodVO; +import com.cloud.dc.dao.ClusterDao; import com.cloud.dc.dao.DataCenterDao; +import com.cloud.dc.dao.HostPodDao; import com.cloud.event.EventTypes; import com.cloud.event.UsageEventUtils; import com.cloud.exception.InvalidParameterValueException; @@ -122,10 +127,12 @@ import com.cloud.utils.db.TransactionLegacy; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.fsm.NoTransitionException; +import com.cloud.vm.DiskProfile; import com.cloud.vm.UserVmManager; import com.cloud.vm.UserVmVO; import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachine.State; +import com.cloud.vm.VirtualMachineManager; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.VMInstanceDao; import com.cloud.vm.snapshot.VMSnapshotVO; @@ -199,6 +206,15 @@ public class VolumeApiServiceImplTest { private DataStoreManager dataStoreMgr; @Mock private SnapshotHelper snapshotHelper; + @Mock + VirtualMachineManager virtualMachineManager; + @Mock + HostPodDao podDao; + @Mock + ClusterDao clusterDao; + @Mock + VolumeOrchestrationService volumeOrchestrationService; + private DetachVolumeCmd detachCmd = new DetachVolumeCmd(); private Class _detachCmdClass = detachCmd.getClass(); @@ -1820,4 +1836,237 @@ public void testValidationsForCheckVolumeAPIWithInvalidVolumeFormat() { volumeApiServiceImpl.validationsForCheckVolumeOperation(volume); } + + private UserVmVO getMockedVm() { + UserVmVO vm = Mockito.mock(UserVmVO.class); + Mockito.when(vm.getId()).thenReturn(1L); + Mockito.when(vm.getTemplateId()).thenReturn(10L); + Mockito.when(vm.getHostName()).thenReturn("test-vm"); + return vm; + } + + private VMTemplateVO getMockedTemplate() { + VMTemplateVO template = Mockito.mock(VMTemplateVO.class); + Mockito.when(template.isDeployAsIs()).thenReturn(false); + return template; + } + + @Test(expected = CloudRuntimeException.class) + public void testGetVmExistingVolumeForVolumeAttach_MultipleRootVolumes_ThrowsException() { + UserVmVO vm = getMockedVm(); + VMTemplateVO template = getMockedTemplate(); + when(templateDao.findById(10L)).thenReturn(template); + when(volumeDaoMock.findByInstanceAndType(1L, Volume.Type.ROOT)) + .thenReturn(Arrays.asList(Mockito.mock(VolumeVO.class), Mockito.mock(VolumeVO.class))); + volumeApiServiceImpl.getVmExistingVolumeForVolumeAttach(vm, Mockito.mock(VolumeInfo.class)); + } + + @Test + public void testGetVmExistingVolumeForVolumeAttach_SingleRootVolume() { + UserVmVO vm = getMockedVm(); + VMTemplateVO template = getMockedTemplate(); + VolumeVO rootVolume = Mockito.mock(VolumeVO.class); + Mockito.when(rootVolume.getId()).thenReturn(20L); + Mockito.when(templateDao.findById(10L)).thenReturn(template); + Mockito.when(volumeDaoMock.findByInstanceAndType(1L, Volume.Type.ROOT)) + .thenReturn(Collections.singletonList(rootVolume)); + VolumeVO result = volumeApiServiceImpl.getVmExistingVolumeForVolumeAttach(vm, Mockito.mock(VolumeInfo.class)); + Assert.assertNotNull(result); + Assert.assertEquals(20L, result.getId()); + } + + private VolumeVO getMockedDataVolume() { + VolumeVO volume = Mockito.mock(VolumeVO.class); + Mockito.when(volume.getId()).thenReturn(30L); + Mockito.when(volume.getState()).thenReturn(Volume.State.Ready); + return volume; + } + + @Test + public void testGetVmExistingVolumeForVolumeAttach_NoRootVolume_DataDiskAvailable() { + UserVmVO vm = getMockedVm(); + VMTemplateVO template = getMockedTemplate(); + VolumeVO dataDisk = getMockedDataVolume(); + List rootVolumes = Collections.emptyList(); + List dataVolumes = Collections.singletonList(dataDisk); + Mockito.when(templateDao.findById(10L)).thenReturn(template); + Mockito.when(volumeDaoMock.findByInstanceAndType(1L, Volume.Type.ROOT)).thenReturn(rootVolumes); + Mockito.when(volumeDaoMock.findByInstanceAndType(1L, Volume.Type.DATADISK)).thenReturn(dataVolumes); + VolumeVO result = volumeApiServiceImpl.getVmExistingVolumeForVolumeAttach(vm, Mockito.mock(VolumeInfo.class)); + Assert.assertNotNull(result); + Assert.assertEquals(30L, result.getId()); + } + + @Test + public void testGetVmExistingVolumeForVolumeAttach_NoVolumesAtAll() { + UserVmVO vm = getMockedVm(); + VMTemplateVO template = getMockedTemplate(); + Mockito.when(templateDao.findById(10L)).thenReturn(template); + Mockito.when(volumeDaoMock.findByInstanceAndType(1L, Volume.Type.ROOT)).thenReturn(Collections.emptyList()); + Mockito.when(volumeDaoMock.findByInstanceAndType(1L, Volume.Type.DATADISK)).thenReturn(Collections.emptyList()); + VolumeVO result = volumeApiServiceImpl.getVmExistingVolumeForVolumeAttach(vm, Mockito.mock(VolumeInfo.class)); + Assert.assertNull(result); + } + + private void mockDiskOffering() { + DiskOfferingVO offering = Mockito.mock(DiskOfferingVO.class); + Mockito.when(_diskOfferingDao.findById(1L)).thenReturn(offering); + Mockito.when(offering.isUseLocalStorage()).thenReturn(true); + Mockito.when(offering.isRecreatable()).thenReturn(false); + } + + private DataCenterVO mockZone() { + DataCenterVO zone = Mockito.mock(DataCenterVO.class); + Mockito.when(_dcDao.findById(1L)).thenReturn(zone); + return zone; + } + + @Test + public void testGetPoolForAllocatedOrUploadedVolumeForAttach_Success() { + VolumeInfo volumeToAttach = Mockito.mock(VolumeInfo.class); + UserVmVO vm = Mockito.mock(UserVmVO.class); + ClusterVO cluster = Mockito.mock(ClusterVO.class); + HostPodVO pod = Mockito.mock(HostPodVO.class); + DataCenterVO zone = mockZone(); + mockDiskOffering(); + StoragePool pool = Mockito.mock(StoragePool.class); + when(vm.getDataCenterId()).thenReturn(1L); + when(virtualMachineManager.findClusterAndHostIdForVm(vm, false)).thenReturn(new Pair<>(1L, 2L)); + when(clusterDao.findById(1L)).thenReturn(cluster); + when(cluster.getPodId()).thenReturn(1L); + when(podDao.findById(1L)).thenReturn(pod); + when(volumeToAttach.getDiskOfferingId()).thenReturn(1L); + when(volumeOrchestrationService.findStoragePool(any(DiskProfile.class), eq(zone), eq(pod), eq(1L), eq(2L), eq(vm), eq(Collections.emptySet()))) + .thenReturn(pool); + StoragePool result = volumeApiServiceImpl.getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + Assert.assertNotNull(result); + Assert.assertEquals(pool, result); + } + + @Test(expected = CloudRuntimeException.class) + public void testGetPoolForAllocatedOrUploadedVolumeForAttach_NoPoolFound_ThrowsException() { + VolumeInfo volumeToAttach = Mockito.mock(VolumeInfo.class); + UserVmVO vm = Mockito.mock(UserVmVO.class); + DataCenterVO zone = mockZone(); + Pair clusterHostId = new Pair<>(1L, 2L); + ClusterVO cluster = Mockito.mock(ClusterVO.class); + HostPodVO pod = Mockito.mock(HostPodVO.class); + mockDiskOffering(); + when(vm.getDataCenterId()).thenReturn(1L); + when(clusterDao.findById(1L)).thenReturn(cluster); + when(virtualMachineManager.findClusterAndHostIdForVm(vm, false)).thenReturn(clusterHostId); + when(podDao.findById(anyLong())).thenReturn(pod); + when(volumeToAttach.getDiskOfferingId()).thenReturn(1L); + when(volumeOrchestrationService.findStoragePool(any(DiskProfile.class), eq(zone), eq(pod), eq(1L), eq(2L), eq(vm), eq(Collections.emptySet()))) + .thenReturn(null); + volumeApiServiceImpl.getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + } + + @Test + public void testGetPoolForAllocatedOrUploadedVolumeForAttach_NoCluster() { + VolumeInfo volumeToAttach = Mockito.mock(VolumeInfo.class); + UserVmVO vm = Mockito.mock(UserVmVO.class); + DataCenterVO zone = mockZone(); + HostPodVO pod = Mockito.mock(HostPodVO.class); + mockDiskOffering(); + StoragePool pool = Mockito.mock(StoragePool.class); + when(vm.getDataCenterId()).thenReturn(1L); + when(vm.getPodIdToDeployIn()).thenReturn(2L); + when(virtualMachineManager.findClusterAndHostIdForVm(vm, false)).thenReturn(new Pair<>(null, 2L)); + when(podDao.findById(2L)).thenReturn(pod); + when(volumeToAttach.getDiskOfferingId()).thenReturn(1L); + when(volumeOrchestrationService.findStoragePool(any(DiskProfile.class), eq(zone), eq(pod), eq(null), eq(2L), eq(vm), eq(Collections.emptySet()))) + .thenReturn(pool); + StoragePool result = volumeApiServiceImpl.getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + Assert.assertNotNull(result); + Assert.assertEquals(pool, result); + } + + + @Test + public void testCreateVolumeOnSecondaryForAttachIfNeeded_VolumeNotAllocatedOrUploaded() { + VolumeInfo volumeToAttach = Mockito.mock(VolumeInfo.class); + Mockito.when(volumeToAttach.getState()).thenReturn(Volume.State.Ready); + VolumeInfo result = volumeApiServiceImpl.createVolumeOnPrimaryForAttachIfNeeded( + volumeToAttach, Mockito.mock(UserVmVO.class), null); + Assert.assertSame(volumeToAttach, result); + Mockito.verifyNoInteractions(primaryDataStoreDaoMock, volumeOrchestrationService); + } + + @Test + public void testCreateVolumeOnSecondaryForAttachIfNeeded_ExistingVolumeDeterminesStoragePool() { + VolumeInfo volumeToAttach = Mockito.mock(VolumeInfo.class); + Mockito.when(volumeToAttach.getState()).thenReturn(Volume.State.Uploaded); + UserVmVO vm = Mockito.mock(UserVmVO.class); + VolumeVO existingVolume = Mockito.mock(VolumeVO.class); + Mockito.when(existingVolume.getState()).thenReturn(Volume.State.Ready); + when(existingVolume.getPoolId()).thenReturn(1L); + StoragePoolVO destPrimaryStorage = Mockito.mock(StoragePoolVO.class); + Mockito.when(destPrimaryStorage.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + Mockito.when(primaryDataStoreDaoMock.findById(1L)).thenReturn(destPrimaryStorage); + VolumeInfo newVolumeOnPrimaryStorage = Mockito.mock(VolumeInfo.class); + try { + Mockito.when(volumeOrchestrationService.createVolumeOnPrimaryStorage(vm, volumeToAttach, vm.getHypervisorType(), destPrimaryStorage)) + .thenReturn(newVolumeOnPrimaryStorage); + } catch (NoTransitionException nte) { + Assert.fail(nte.getMessage()); + } + VolumeInfo result = volumeApiServiceImpl.createVolumeOnPrimaryForAttachIfNeeded(volumeToAttach, vm, existingVolume); + Assert.assertSame(newVolumeOnPrimaryStorage, result); + Mockito.verify(primaryDataStoreDaoMock).findById(1L); + } + + @Test + public void testCreateVolumeOnPrimaryForAttachIfNeeded_UsesGetPoolForAttach() { + VolumeInfo volumeToAttach = Mockito.mock(VolumeInfo.class); + Mockito.when(volumeToAttach.getState()).thenReturn(Volume.State.Allocated); + UserVmVO vm = Mockito.mock(UserVmVO.class); + StoragePool destPrimaryStorage = Mockito.mock(StoragePool.class); + Mockito.doReturn(destPrimaryStorage).when(volumeApiServiceImpl) + .getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + VolumeInfo newVolumeOnPrimaryStorage = Mockito.mock(VolumeInfo.class); + try { + Mockito.when(volumeOrchestrationService.createVolumeOnPrimaryStorage( + vm, volumeToAttach, vm.getHypervisorType(), destPrimaryStorage)) + .thenReturn(newVolumeOnPrimaryStorage); + } catch (NoTransitionException nte) { + Assert.fail(nte.getMessage()); + } + VolumeInfo result = volumeApiServiceImpl.createVolumeOnPrimaryForAttachIfNeeded(volumeToAttach, vm, null); + Assert.assertSame(newVolumeOnPrimaryStorage, result); + verify(volumeApiServiceImpl).getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + } + + @Test(expected = InvalidParameterValueException.class) + public void testCreateVolumeOnPrimaryForAttachIfNeeded_UnsupportedPoolType_ThrowsException() { + VolumeInfo volumeToAttach = Mockito.mock(VolumeInfo.class); + when(volumeToAttach.getState()).thenReturn(Volume.State.Uploaded); + UserVmVO vm = Mockito.mock(UserVmVO.class); + StoragePool destPrimaryStorage = Mockito.mock(StoragePool.class); + when(destPrimaryStorage.getPoolType()).thenReturn(Storage.StoragePoolType.PowerFlex); + Mockito.doReturn(destPrimaryStorage).when(volumeApiServiceImpl) + .getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + volumeApiServiceImpl.createVolumeOnPrimaryForAttachIfNeeded(volumeToAttach, vm, null); + } + + @Test + public void testCreateVolumeOnSecondaryForAttachIfNeeded_CreateVolumeFails_ThrowsException() { + VolumeInfo volumeToAttach = Mockito.mock(VolumeInfo.class); + Mockito.when(volumeToAttach.getState()).thenReturn(Volume.State.Uploaded); + UserVmVO vm = Mockito.mock(UserVmVO.class); + StoragePool destPrimaryStorage = Mockito.mock(StoragePool.class); + Mockito.when(destPrimaryStorage.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + Mockito.doReturn(destPrimaryStorage).when(volumeApiServiceImpl) + .getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + try { + Mockito.when(volumeOrchestrationService.createVolumeOnPrimaryStorage(vm, volumeToAttach, vm.getHypervisorType(), destPrimaryStorage)) + .thenThrow(new NoTransitionException("Mocked exception")); + } catch (NoTransitionException nte) { + Assert.fail(nte.getMessage()); + } + CloudRuntimeException exception = Assert.assertThrows(CloudRuntimeException.class, () -> + volumeApiServiceImpl.createVolumeOnPrimaryForAttachIfNeeded(volumeToAttach, vm, null) + ); + Assert.assertTrue(exception.getMessage().contains("Failed to create volume on primary storage")); + } } From 8d95a514a1d321591272f9b587146e8e629e4904 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 4 Feb 2025 16:03:40 +0530 Subject: [PATCH 5/6] server: fix pod retrieval during volume attach Fixes issue highlighted in https://github.com/apache/cloudstack/pull/9315#issuecomment-2633353671 Signed-off-by: Abhishek Kumar --- .../src/main/java/com/cloud/storage/VolumeApiServiceImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 68546f185b73..a5b9d6ddfe86 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -2429,7 +2429,7 @@ protected VolumeVO getVmExistingVolumeForVolumeAttach(UserVmVO vm, VolumeInfo vo protected StoragePool getPoolForAllocatedOrUploadedVolumeForAttach(final VolumeInfo volumeToAttach, final UserVmVO vm) { DataCenter zone = _dcDao.findById(vm.getDataCenterId()); Pair clusterHostId = virtualMachineManager.findClusterAndHostIdForVm(vm, false); - long podId = vm.getPodIdToDeployIn(); + Long podId = vm.getPodIdToDeployIn(); if (clusterHostId.first() != null) { Cluster cluster = clusterDao.findById(clusterHostId.first()); podId = cluster.getPodId(); From 557b3ba13ebb77233f22288642bf558ceb5f7411 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 5 Feb 2025 14:00:09 +0530 Subject: [PATCH 6/6] changes for attaching allocated vol to a stopped vm Signed-off-by: Abhishek Kumar --- .../cloud/storage/VolumeApiServiceImpl.java | 16 +++--- .../storage/VolumeApiServiceImplTest.java | 51 +++++++++++++++---- 2 files changed, 50 insertions(+), 17 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index a5b9d6ddfe86..3ea8116764a0 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -2426,7 +2426,7 @@ protected VolumeVO getVmExistingVolumeForVolumeAttach(UserVmVO vm, VolumeInfo vo return existingVolumeOfVm; } - protected StoragePool getPoolForAllocatedOrUploadedVolumeForAttach(final VolumeInfo volumeToAttach, final UserVmVO vm) { + protected StoragePool getSuitablePoolForAllocatedOrUploadedVolumeForAttach(final VolumeInfo volumeToAttach, final UserVmVO vm) { DataCenter zone = _dcDao.findById(vm.getDataCenterId()); Pair clusterHostId = virtualMachineManager.findClusterAndHostIdForVm(vm, false); Long podId = vm.getPodIdToDeployIn(); @@ -2441,12 +2441,8 @@ protected StoragePool getPoolForAllocatedOrUploadedVolumeForAttach(final VolumeI offering.isUseLocalStorage(), offering.isRecreatable(), volumeToAttach.getTemplateId()); diskProfile.setHyperType(vm.getHypervisorType()); - StoragePool pool = _volumeMgr.findStoragePool(diskProfile, zone, pod, clusterHostId.first(), + return _volumeMgr.findStoragePool(diskProfile, zone, pod, clusterHostId.first(), clusterHostId.second(), vm, Collections.emptySet()); - if (pool == null) { - throw new CloudRuntimeException(String.format("Failed to find a primary storage for volume in state: %s", volumeToAttach.getState())); - } - return pool; } protected VolumeInfo createVolumeOnPrimaryForAttachIfNeeded(final VolumeInfo volumeToAttach, final UserVmVO vm, VolumeVO existingVolumeOfVm) { @@ -2464,7 +2460,13 @@ protected VolumeInfo createVolumeOnPrimaryForAttachIfNeeded(final VolumeInfo vol } } if (destPrimaryStorage == null) { - destPrimaryStorage = getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + destPrimaryStorage = getSuitablePoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + if (destPrimaryStorage == null) { + if (Volume.State.Allocated.equals(volumeToAttach.getState()) && State.Stopped.equals(vm.getState())) { + return newVolumeOnPrimaryStorage; + } + throw new CloudRuntimeException(String.format("Failed to find a primary storage for volume in state: %s", volumeToAttach.getState())); + } } try { if (volumeOnSecondary && Storage.StoragePoolType.PowerFlex.equals(destPrimaryStorage.getPoolType())) { diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java index 3e6f9ec63f9d..9b087bd384bc 100644 --- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java @@ -1938,13 +1938,13 @@ public void testGetPoolForAllocatedOrUploadedVolumeForAttach_Success() { when(volumeToAttach.getDiskOfferingId()).thenReturn(1L); when(volumeOrchestrationService.findStoragePool(any(DiskProfile.class), eq(zone), eq(pod), eq(1L), eq(2L), eq(vm), eq(Collections.emptySet()))) .thenReturn(pool); - StoragePool result = volumeApiServiceImpl.getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + StoragePool result = volumeApiServiceImpl.getSuitablePoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); Assert.assertNotNull(result); Assert.assertEquals(pool, result); } - @Test(expected = CloudRuntimeException.class) - public void testGetPoolForAllocatedOrUploadedVolumeForAttach_NoPoolFound_ThrowsException() { + @Test + public void testGetPoolForAllocatedOrUploadedVolumeForAttach_NoSuitablePoolFound_ReturnsNull() { VolumeInfo volumeToAttach = Mockito.mock(VolumeInfo.class); UserVmVO vm = Mockito.mock(UserVmVO.class); DataCenterVO zone = mockZone(); @@ -1959,11 +1959,11 @@ public void testGetPoolForAllocatedOrUploadedVolumeForAttach_NoPoolFound_ThrowsE when(volumeToAttach.getDiskOfferingId()).thenReturn(1L); when(volumeOrchestrationService.findStoragePool(any(DiskProfile.class), eq(zone), eq(pod), eq(1L), eq(2L), eq(vm), eq(Collections.emptySet()))) .thenReturn(null); - volumeApiServiceImpl.getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + Assert.assertNull(volumeApiServiceImpl.getSuitablePoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm)); } @Test - public void testGetPoolForAllocatedOrUploadedVolumeForAttach_NoCluster() { + public void testGetSuitablePoolForAllocatedOrUploadedVolumeForAttach_NoCluster() { VolumeInfo volumeToAttach = Mockito.mock(VolumeInfo.class); UserVmVO vm = Mockito.mock(UserVmVO.class); DataCenterVO zone = mockZone(); @@ -1977,7 +1977,7 @@ public void testGetPoolForAllocatedOrUploadedVolumeForAttach_NoCluster() { when(volumeToAttach.getDiskOfferingId()).thenReturn(1L); when(volumeOrchestrationService.findStoragePool(any(DiskProfile.class), eq(zone), eq(pod), eq(null), eq(2L), eq(vm), eq(Collections.emptySet()))) .thenReturn(pool); - StoragePool result = volumeApiServiceImpl.getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + StoragePool result = volumeApiServiceImpl.getSuitablePoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); Assert.assertNotNull(result); Assert.assertEquals(pool, result); } @@ -2023,7 +2023,7 @@ public void testCreateVolumeOnPrimaryForAttachIfNeeded_UsesGetPoolForAttach() { UserVmVO vm = Mockito.mock(UserVmVO.class); StoragePool destPrimaryStorage = Mockito.mock(StoragePool.class); Mockito.doReturn(destPrimaryStorage).when(volumeApiServiceImpl) - .getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + .getSuitablePoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); VolumeInfo newVolumeOnPrimaryStorage = Mockito.mock(VolumeInfo.class); try { Mockito.when(volumeOrchestrationService.createVolumeOnPrimaryStorage( @@ -2034,7 +2034,7 @@ public void testCreateVolumeOnPrimaryForAttachIfNeeded_UsesGetPoolForAttach() { } VolumeInfo result = volumeApiServiceImpl.createVolumeOnPrimaryForAttachIfNeeded(volumeToAttach, vm, null); Assert.assertSame(newVolumeOnPrimaryStorage, result); - verify(volumeApiServiceImpl).getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + verify(volumeApiServiceImpl).getSuitablePoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); } @Test(expected = InvalidParameterValueException.class) @@ -2045,7 +2045,7 @@ public void testCreateVolumeOnPrimaryForAttachIfNeeded_UnsupportedPoolType_Throw StoragePool destPrimaryStorage = Mockito.mock(StoragePool.class); when(destPrimaryStorage.getPoolType()).thenReturn(Storage.StoragePoolType.PowerFlex); Mockito.doReturn(destPrimaryStorage).when(volumeApiServiceImpl) - .getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + .getSuitablePoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); volumeApiServiceImpl.createVolumeOnPrimaryForAttachIfNeeded(volumeToAttach, vm, null); } @@ -2057,7 +2057,7 @@ public void testCreateVolumeOnSecondaryForAttachIfNeeded_CreateVolumeFails_Throw StoragePool destPrimaryStorage = Mockito.mock(StoragePool.class); Mockito.when(destPrimaryStorage.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); Mockito.doReturn(destPrimaryStorage).when(volumeApiServiceImpl) - .getPoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + .getSuitablePoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); try { Mockito.when(volumeOrchestrationService.createVolumeOnPrimaryStorage(vm, volumeToAttach, vm.getHypervisorType(), destPrimaryStorage)) .thenThrow(new NoTransitionException("Mocked exception")); @@ -2069,4 +2069,35 @@ public void testCreateVolumeOnSecondaryForAttachIfNeeded_CreateVolumeFails_Throw ); Assert.assertTrue(exception.getMessage().contains("Failed to create volume on primary storage")); } + + @Test + public void testCreateVolumeOnSecondaryForAttachIfNeeded_NoSuitablePool_ThrowsException() { + VolumeInfo volumeToAttach = Mockito.mock(VolumeInfo.class); + Mockito.when(volumeToAttach.getState()).thenReturn(Volume.State.Uploaded); + UserVmVO vm = Mockito.mock(UserVmVO.class); + Mockito.doReturn(null).when(volumeApiServiceImpl) + .getSuitablePoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + CloudRuntimeException exception = Assert.assertThrows(CloudRuntimeException.class, () -> + volumeApiServiceImpl.createVolumeOnPrimaryForAttachIfNeeded(volumeToAttach, vm, null) + ); + Assert.assertTrue(exception.getMessage().startsWith("Failed to find a primary storage for volume")); + } + + @Test + public void testCreateVolumeOnSecondaryForAttachIfNeeded_NoSuitablePool_ReturnSameVolumeInfo() { + VolumeInfo volumeToAttach = Mockito.mock(VolumeInfo.class); + Mockito.when(volumeToAttach.getState()).thenReturn(Volume.State.Allocated); + UserVmVO vm = Mockito.mock(UserVmVO.class); + Mockito.when(vm.getState()).thenReturn(State.Stopped); + Mockito.doReturn(null).when(volumeApiServiceImpl) + .getSuitablePoolForAllocatedOrUploadedVolumeForAttach(volumeToAttach, vm); + VolumeInfo result = volumeApiServiceImpl.createVolumeOnPrimaryForAttachIfNeeded(volumeToAttach, vm, null); + Assert.assertSame(volumeToAttach, result); + try { + Mockito.verify(volumeOrchestrationService, Mockito.never()).createVolumeOnPrimaryStorage(Mockito.any(), + Mockito.any(), Mockito.any(), Mockito.any()); + } catch (NoTransitionException e) { + Assert.fail(); + } + } }