diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java index bcade3a371c4..f94f0fd85640 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java @@ -1964,7 +1964,7 @@ protected MigrationOptions createLinkedCloneMigrationOptions(VolumeInfo srcVolum Storage.StoragePoolType srcPoolType = srcPool.getPoolType(); Long srcPoolClusterId = srcPool.getClusterId(); VMTemplateStoragePoolVO ref = templatePoolDao.findByPoolTemplate(destVolumeInfo.getPoolId(), srcVolumeInfo.getTemplateId(), null); - boolean updateBackingFileReference = ref == null; + boolean updateBackingFileReference = ref == null || !StringUtils.equals(ref.getInstallPath(), srcVolumeBackingFile); String backingFile = !updateBackingFileReference ? ref.getInstallPath() : srcVolumeBackingFile; ScopeType scopeType = srcVolumeInfo.getDataStore().getScope().getScopeType(); return new MigrationOptions(srcPoolUuid, srcPoolType, backingFile, updateBackingFileReference, scopeType, srcPoolClusterId); @@ -2009,6 +2009,49 @@ protected void setVolumeMigrationOptions(VolumeInfo srcVolumeInfo, VolumeInfo de destVolumeInfo.setMigrationOptions(migrationOptions); } + /** + * KVM/libvirt selects linked-clone or full-clone storage migration for the whole VM migration request. + * If any disk is backed by a direct-download template, force the request to full clone so libvirt does + * not use incremental shared-backing semantics for a disk whose backing chain is not guaranteed on the destination. + */ + protected boolean shouldForceFullCloneMigration(Map volumeDataStoreMap, Host destHost) { + return shouldForceFullCloneMigration(volumeDataStoreMap, destHost, new HashMap<>()); + } + + protected boolean shouldForceFullCloneMigration(Map volumeDataStoreMap, Host destHost, Map storagePoolsById) { + for (Map.Entry entry : volumeDataStoreMap.entrySet()) { + VolumeInfo srcVolumeInfo = entry.getKey(); + DataStore destDataStore = entry.getValue(); + StoragePoolVO sourceStoragePool = getStoragePool(storagePoolsById, srcVolumeInfo.getPoolId()); + StoragePoolVO destStoragePool = getStoragePool(storagePoolsById, destDataStore.getId()); + + if (shouldSkipVolumeMigration(sourceStoragePool, destHost, destStoragePool)) { + continue; + } + + if (srcVolumeInfo.isDirectDownload()) { + return true; + } + } + return false; + } + + private StoragePoolVO getStoragePool(Map storagePoolsById, long storagePoolId) { + StoragePoolVO storagePool = storagePoolsById.get(storagePoolId); + if (storagePool == null) { + storagePool = _storagePoolDao.findById(storagePoolId); + if (storagePool != null) { + storagePoolsById.put(storagePoolId, storagePool); + } + } + return storagePool; + } + + protected boolean shouldSkipVolumeMigration(StoragePoolVO sourceStoragePool, Host destHost, StoragePoolVO destStoragePool) { + return (sourceStoragePool.getId() == destStoragePool.getId() && sourceStoragePool.getPoolType() == Storage.StoragePoolType.PowerFlex) || + !shouldMigrateVolume(sourceStoragePool, destHost, destStoragePool); + } + /** * For each disk to migrate: *
    @@ -2036,6 +2079,11 @@ public void copyAsync(Map volumeDataStoreMap, VirtualMach List migrateDiskInfoList = new ArrayList(); Map migrateStorage = new HashMap<>(); + Map storagePoolsById = new HashMap<>(); + boolean forceFullCloneMigration = shouldForceFullCloneMigration(volumeDataStoreMap, destHost, storagePoolsById); + if (forceFullCloneMigration) { + logger.info("Using full clone live storage migration for VM [{}] because one or more migrated volumes are backed by direct-download templates.", vmTO); + } boolean managedStorageDestination = false; boolean migrateNonSharedInc = false; @@ -2044,19 +2092,15 @@ public void copyAsync(Map volumeDataStoreMap, VirtualMach DataStore destDataStore = entry.getValue(); VolumeVO srcVolume = _volumeDao.findById(srcVolumeInfo.getId()); - StoragePoolVO destStoragePool = _storagePoolDao.findById(destDataStore.getId()); - StoragePoolVO sourceStoragePool = _storagePoolDao.findById(srcVolumeInfo.getPoolId()); - - // do not initiate migration for the same PowerFlex/ScaleIO pool - if (sourceStoragePool.getId() == destStoragePool.getId() && sourceStoragePool.getPoolType() == Storage.StoragePoolType.PowerFlex) { - continue; - } + StoragePoolVO destStoragePool = getStoragePool(storagePoolsById, destDataStore.getId()); + StoragePoolVO sourceStoragePool = getStoragePool(storagePoolsById, srcVolumeInfo.getPoolId()); - if (!shouldMigrateVolume(sourceStoragePool, destHost, destStoragePool)) { + if (shouldSkipVolumeMigration(sourceStoragePool, destHost, destStoragePool)) { continue; } - MigrationOptions.Type migrationType = decideMigrationTypeAndCopyTemplateIfNeeded(destHost, vmInstance, srcVolumeInfo, sourceStoragePool, destStoragePool, destDataStore); + MigrationOptions.Type migrationType = decideMigrationTypeAndCopyTemplateIfNeeded(destHost, vmInstance, srcVolumeInfo, sourceStoragePool, destStoragePool, + destDataStore, forceFullCloneMigration); migrateNonSharedInc = migrateNonSharedInc || MigrationOptions.Type.LinkedClone.equals(migrationType); VolumeVO destVolume = duplicateVolumeOnAnotherStorage(srcVolume, destStoragePool); @@ -2068,6 +2112,7 @@ public void copyAsync(Map volumeDataStoreMap, VirtualMach destVolumeInfo.processEvent(Event.MigrationCopySucceeded); // move the volume from Ready to Migrating destVolumeInfo.processEvent(Event.MigrationRequested); + srcVolumeInfoToDestVolumeInfo.put(srcVolumeInfo, destVolumeInfo); setVolumeMigrationOptions(srcVolumeInfo, destVolumeInfo, vmTO, srcHost, destStoragePool, migrationType); @@ -2112,8 +2157,6 @@ public void copyAsync(Map volumeDataStoreMap, VirtualMach prepareDiskWithSecretConsumerDetail(vmTO, srcVolumeInfo, destVolumeInfo.getPath()); migrateStorage.put(srcVolumeInfo.getPath(), migrateDiskInfo); - - srcVolumeInfoToDestVolumeInfo.put(srcVolumeInfo, destVolumeInfo); } PrepareForMigrationCommand pfmc = new PrepareForMigrationCommand(vmTO); @@ -2211,7 +2254,13 @@ public void copyAsync(Map volumeDataStoreMap, VirtualMach } } - private MigrationOptions.Type decideMigrationTypeAndCopyTemplateIfNeeded(Host destHost, VMInstanceVO vmInstance, VolumeInfo srcVolumeInfo, StoragePoolVO sourceStoragePool, StoragePoolVO destStoragePool, DataStore destDataStore) { + protected MigrationOptions.Type decideMigrationTypeAndCopyTemplateIfNeeded(Host destHost, VMInstanceVO vmInstance, VolumeInfo srcVolumeInfo, StoragePoolVO sourceStoragePool, + StoragePoolVO destStoragePool, DataStore destDataStore, boolean forceFullCloneMigration) { + if (forceFullCloneMigration) { + logger.debug("Skipping linked clone migration for volume [{}] because the migration request includes a direct-download backed volume.", srcVolumeInfo.getId()); + return MigrationOptions.Type.FullClone; + } + VMTemplateVO vmTemplate = _vmTemplateDao.findById(vmInstance.getTemplateId()); String srcVolumeBackingFile = getVolumeBackingFile(srcVolumeInfo); if (StringUtils.isNotBlank(srcVolumeBackingFile) && supportStoragePoolType(destStoragePool.getPoolType(), StoragePoolType.Filesystem) && @@ -2449,7 +2498,11 @@ private VolumeVO duplicateVolumeOnAnotherStorage(Volume volume, StoragePoolVO st protected String connectHostToVolume(Host host, long storagePoolId, String iqn) { ModifyTargetsCommand modifyTargetsCommand = getModifyTargetsCommand(storagePoolId, iqn, true); - return sendModifyTargetsCommand(modifyTargetsCommand, host.getId()).get(0); + List connectedPaths = sendModifyTargetsCommand(modifyTargetsCommand, host.getId()); + if (CollectionUtils.isEmpty(connectedPaths)) { + throw new CloudRuntimeException(String.format("Unable to modify targets on the following host: %s because no connected path was returned for target [%s].", host.getId(), iqn)); + } + return connectedPaths.get(0); } private void disconnectHostFromVolume(Host host, long storagePoolId, String iqn) { @@ -2484,14 +2537,25 @@ private ModifyTargetsCommand getModifyTargetsCommand(long storagePoolId, String } private List sendModifyTargetsCommand(ModifyTargetsCommand cmd, long hostId) { - ModifyTargetsAnswer modifyTargetsAnswer = (ModifyTargetsAnswer)agentManager.easySend(hostId, cmd); + Answer answer = agentManager.easySend(hostId, cmd); - if (modifyTargetsAnswer == null) { + if (answer == null) { throw new CloudRuntimeException("Unable to get an answer to the modify targets command"); } + if (!(answer instanceof ModifyTargetsAnswer)) { + String details = StringUtils.defaultIfBlank(answer.getDetails(), + String.format("Unexpected answer type returned: %s", answer.getClass().getName())); + throw new CloudRuntimeException(String.format("Unable to modify targets on the following host: %s due to [%s]", hostId, details)); + } + + ModifyTargetsAnswer modifyTargetsAnswer = (ModifyTargetsAnswer)answer; + if (!modifyTargetsAnswer.getResult()) { String msg = "Unable to modify targets on the following host: " + hostId; + if (StringUtils.isNotBlank(modifyTargetsAnswer.getDetails())) { + msg = String.format("%s due to [%s]", msg, modifyTargetsAnswer.getDetails()); + } throw new CloudRuntimeException(msg); } @@ -2504,14 +2568,25 @@ private List sendModifyTargetsCommand(ModifyTargetsCommand cmd, long hos */ protected void updateCopiedTemplateReference(VolumeInfo srcVolumeInfo, VolumeInfo destVolumeInfo) { VMTemplateStoragePoolVO ref = templatePoolDao.findByPoolTemplate(srcVolumeInfo.getPoolId(), srcVolumeInfo.getTemplateId(), null); - VMTemplateStoragePoolVO newRef = new VMTemplateStoragePoolVO(destVolumeInfo.getPoolId(), ref.getTemplateId(), null); - newRef.setDownloadPercent(100); - newRef.setDownloadState(VMTemplateStorageResourceAssoc.Status.DOWNLOADED); - newRef.setState(ObjectInDataStoreStateMachine.State.Ready); - newRef.setTemplateSize(ref.getTemplateSize()); - newRef.setLocalDownloadPath(ref.getLocalDownloadPath()); - newRef.setInstallPath(ref.getInstallPath()); - templatePoolDao.persist(newRef); + if (ref == null) { + throw new CloudRuntimeException(String.format("Unable to update copied template reference because source template reference was not found for pool [%s] and template [%s].", + srcVolumeInfo.getPoolId(), srcVolumeInfo.getTemplateId())); + } + VMTemplateStoragePoolVO destRef = templatePoolDao.findByPoolTemplate(destVolumeInfo.getPoolId(), ref.getTemplateId(), null); + if (destRef == null) { + destRef = new VMTemplateStoragePoolVO(destVolumeInfo.getPoolId(), ref.getTemplateId(), null); + } + destRef.setDownloadPercent(100); + destRef.setDownloadState(VMTemplateStorageResourceAssoc.Status.DOWNLOADED); + destRef.setState(ObjectInDataStoreStateMachine.State.Ready); + destRef.setTemplateSize(ref.getTemplateSize()); + destRef.setLocalDownloadPath(ref.getLocalDownloadPath()); + destRef.setInstallPath(ref.getInstallPath()); + if (destRef.getId() == 0) { + templatePoolDao.persist(destRef); + } else { + templatePoolDao.update(destRef.getId(), destRef); + } } /** diff --git a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java index 808c319b40f2..2fc67b930ed8 100644 --- a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java +++ b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java @@ -337,6 +337,29 @@ public void migrateTemplateToTargetStorageIfNeededTestTemplateNotOnTargetHost() configureAndTestcopyTemplateToTargetStorageIfNeeded(null, StoragePoolType.Filesystem, 1); } + @Test + public void copyTemplateToTargetStorageIfNeededTestDirectDownloadTemplateAlreadyStaged() { + DataStore destDataStore = Mockito.mock(DataStore.class); + Host destHost = Mockito.mock(Host.class); + VolumeInfo srcVolumeInfo = Mockito.mock(VolumeInfo.class); + StoragePool srcStoragePool = Mockito.mock(StoragePool.class); + StoragePool destStoragePool = Mockito.mock(StoragePool.class); + TemplateInfo directDownloadTemplateInfo = Mockito.mock(TemplateInfo.class); + + Mockito.when(destDataStore.getId()).thenReturn(11L); + Mockito.when(destHost.getId()).thenReturn(12L); + Mockito.when(srcVolumeInfo.getTemplateId()).thenReturn(13L); + Mockito.when(srcVolumeInfo.getVolumeType()).thenReturn(Volume.Type.ROOT); + Mockito.when(templateDataFactory.getReadyBypassedTemplateOnPrimaryStore(13L, 11L, 12L)).thenReturn(directDownloadTemplateInfo); + + kvmNonManagedStorageDataMotionStrategy.copyTemplateToTargetFilesystemStorageIfNeeded(srcVolumeInfo, srcStoragePool, destDataStore, destStoragePool, destHost); + + Mockito.verify(templateDataFactory).getReadyBypassedTemplateOnPrimaryStore(13L, 11L, 12L); + Mockito.verify(vmTemplatePoolDao, Mockito.never()).findByPoolTemplate(Mockito.anyLong(), Mockito.anyLong(), nullable(String.class)); + Mockito.verify(dataStoreManagerImpl, Mockito.never()).getRandomImageStore(Mockito.anyLong()); + Mockito.verify(kvmNonManagedStorageDataMotionStrategy, Mockito.never()).sendCopyCommand(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); + } + @Test public void migrateTemplateToTargetStorageIfNeededTestNonDesiredStoragePoolType() throws AgentUnavailableException, OperationTimedoutException { StoragePoolType[] storagePoolTypeArray = StoragePoolType.values(); diff --git a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategyTest.java b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategyTest.java index 45357fa64b2a..7bb5404bcc70 100644 --- a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategyTest.java +++ b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategyTest.java @@ -25,12 +25,21 @@ import static org.mockito.Mockito.mock; import static org.mockito.MockitoAnnotations.initMocks; +import java.util.AbstractMap; +import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; +import java.util.Set; import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.Scope; import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.storage.datastore.PrimaryDataStoreImpl; @@ -42,26 +51,35 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; +import com.cloud.agent.AgentManager; +import com.cloud.agent.api.Answer; import com.cloud.agent.api.MigrateCommand; +import com.cloud.agent.api.ModifyTargetsAnswer; +import com.cloud.agent.api.ModifyTargetsCommand; +import com.cloud.host.Host; import com.cloud.host.HostVO; import com.cloud.storage.DataStoreRole; import com.cloud.storage.ImageStore; +import com.cloud.storage.MigrationOptions; import com.cloud.storage.ScopeType; import com.cloud.storage.Storage; import com.cloud.storage.Storage.StoragePoolType; +import com.cloud.storage.VMTemplateStoragePoolVO; +import com.cloud.storage.VMTemplateStorageResourceAssoc; +import com.cloud.storage.VMTemplateVO; import com.cloud.storage.Volume; import com.cloud.storage.VolumeVO; -import java.util.AbstractMap; -import java.util.Arrays; -import java.util.HashSet; -import java.util.List; -import java.util.Set; +import com.cloud.storage.dao.VMTemplateDao; +import com.cloud.storage.dao.VMTemplatePoolDao; +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.vm.VMInstanceVO; @RunWith(MockitoJUnitRunner.class) public class StorageSystemDataMotionStrategyTest { @@ -80,6 +98,12 @@ public class StorageSystemDataMotionStrategyTest { private ImageStore destinationStore; @Mock private PrimaryDataStoreDao primaryDataStoreDao; + @Mock + private AgentManager agentManager; + @Mock + private VMTemplatePoolDao templatePoolDao; + @Mock + private VMTemplateDao vmTemplateDao; @Mock StoragePoolVO sourceStoragePoolVoMock, destinationStoragePoolVoMock; @@ -185,6 +209,53 @@ public void generateDestPathTest() { Mockito.verify(strategy).connectHostToVolume(destHost, 0l, "iScsiName"); } + @Test + public void connectHostToVolumeSurfacesOriginalAgentErrorWhenAnswerTypeIsGeneric() { + Host host = mock(Host.class); + StoragePoolVO storagePool = mock(StoragePoolVO.class); + Answer answer = new Answer(mock(ModifyTargetsCommand.class), false, "Can't find volume:volume-1"); + + Mockito.when(host.getId()).thenReturn(42L); + Mockito.doReturn(storagePool).when(primaryDataStoreDao).findById(0L); + Mockito.when(storagePool.getPoolType()).thenReturn(StoragePoolType.NetworkFilesystem); + Mockito.when(storagePool.getUuid()).thenReturn("pool-uuid"); + Mockito.when(storagePool.getHostAddress()).thenReturn("10.0.0.10"); + Mockito.when(storagePool.getPort()).thenReturn(2049); + Mockito.doReturn(answer).when(agentManager).easySend(Mockito.eq(42L), Mockito.any(ModifyTargetsCommand.class)); + + try { + strategy.connectHostToVolume(host, 0L, "volume-1"); + Assert.fail("Expected CloudRuntimeException to be thrown"); + } catch (CloudRuntimeException e) { + Assert.assertTrue(e.getMessage().contains("Can't find volume:volume-1")); + Assert.assertFalse(e.getMessage().contains("ClassCastException")); + } + } + + @Test + public void connectHostToVolumeThrowsWhenConnectedPathIsMissing() { + Host host = mock(Host.class); + StoragePoolVO storagePool = mock(StoragePoolVO.class); + ModifyTargetsAnswer answer = new ModifyTargetsAnswer(); + answer.setConnectedPaths(Collections.emptyList()); + + Mockito.when(host.getId()).thenReturn(42L); + Mockito.doReturn(storagePool).when(primaryDataStoreDao).findById(0L); + Mockito.when(storagePool.getPoolType()).thenReturn(StoragePoolType.NetworkFilesystem); + Mockito.when(storagePool.getUuid()).thenReturn("pool-uuid"); + Mockito.when(storagePool.getHostAddress()).thenReturn("10.0.0.10"); + Mockito.when(storagePool.getPort()).thenReturn(2049); + Mockito.doReturn(answer).when(agentManager).easySend(Mockito.eq(42L), Mockito.any(ModifyTargetsCommand.class)); + + try { + strategy.connectHostToVolume(host, 0L, "volume-1"); + Assert.fail("Expected CloudRuntimeException to be thrown"); + } catch (CloudRuntimeException e) { + Assert.assertTrue(e.getMessage().contains("no connected path was returned")); + Assert.assertTrue(e.getMessage().contains("volume-1")); + } + } + @Test public void configureMigrateDiskInfoTest() { VolumeObject srcVolumeInfo = Mockito.spy(new VolumeObject()); @@ -210,6 +281,224 @@ public void configureMigrateDiskInfoWithBackingTest() { Assert.assertEquals("backingPath", migrateDiskInfo.getBackingStoreText()); } + @Test + public void createLinkedCloneMigrationOptionsUsesSourceBackingFileWhenDestinationReferencePathDiffers() { + VolumeInfo srcVolumeInfo = Mockito.mock(VolumeInfo.class); + VolumeInfo destVolumeInfo = Mockito.mock(VolumeInfo.class); + StoragePoolVO srcPool = Mockito.mock(StoragePoolVO.class); + DataStore srcDataStore = Mockito.mock(DataStore.class); + Scope scope = Mockito.mock(Scope.class); + VMTemplateStoragePoolVO ref = Mockito.mock(VMTemplateStoragePoolVO.class); + + Mockito.when(srcPool.getUuid()).thenReturn("src-pool"); + Mockito.when(srcPool.getPoolType()).thenReturn(StoragePoolType.NetworkFilesystem); + Mockito.when(srcPool.getClusterId()).thenReturn(1L); + Mockito.when(srcVolumeInfo.getTemplateId()).thenReturn(13L); + Mockito.when(srcVolumeInfo.getDataStore()).thenReturn(srcDataStore); + Mockito.when(srcDataStore.getScope()).thenReturn(scope); + Mockito.when(scope.getScopeType()).thenReturn(ScopeType.CLUSTER); + Mockito.when(destVolumeInfo.getPoolId()).thenReturn(4L); + Mockito.when(templatePoolDao.findByPoolTemplate(4L, 13L, null)).thenReturn(ref); + Mockito.when(ref.getInstallPath()).thenReturn("target-backing.qcow2"); + + MigrationOptions options = strategy.createLinkedCloneMigrationOptions(srcVolumeInfo, destVolumeInfo, "source-backing.qcow2", srcPool); + + Assert.assertTrue(options.isCopySrcTemplate()); + Assert.assertEquals("source-backing.qcow2", options.getSrcBackingFilePath()); + } + + @Test + public void updateCopiedTemplateReferenceUpdatesExistingDestinationReference() { + VolumeInfo srcVolumeInfo = Mockito.mock(VolumeInfo.class); + VolumeInfo destVolumeInfo = Mockito.mock(VolumeInfo.class); + VMTemplateStoragePoolVO srcRef = Mockito.mock(VMTemplateStoragePoolVO.class); + VMTemplateStoragePoolVO destRef = Mockito.mock(VMTemplateStoragePoolVO.class); + + Mockito.when(srcVolumeInfo.getPoolId()).thenReturn(5L); + Mockito.when(srcVolumeInfo.getTemplateId()).thenReturn(13L); + Mockito.when(destVolumeInfo.getPoolId()).thenReturn(4L); + Mockito.when(srcRef.getTemplateId()).thenReturn(13L); + Mockito.when(srcRef.getTemplateSize()).thenReturn(1851129856L); + Mockito.when(srcRef.getLocalDownloadPath()).thenReturn("d06b4640-d7d3-45d7-92c1-8cd3c8eb1eb7"); + Mockito.when(srcRef.getInstallPath()).thenReturn("d06b4640-d7d3-45d7-92c1-8cd3c8eb1eb7"); + Mockito.when(destRef.getId()).thenReturn(19L); + Mockito.when(templatePoolDao.findByPoolTemplate(5L, 13L, null)).thenReturn(srcRef); + Mockito.when(templatePoolDao.findByPoolTemplate(4L, 13L, null)).thenReturn(destRef); + + strategy.updateCopiedTemplateReference(srcVolumeInfo, destVolumeInfo); + + Mockito.verify(destRef).setDownloadPercent(100); + Mockito.verify(destRef).setDownloadState(VMTemplateStorageResourceAssoc.Status.DOWNLOADED); + Mockito.verify(destRef).setState(ObjectInDataStoreStateMachine.State.Ready); + Mockito.verify(destRef).setTemplateSize(1851129856L); + Mockito.verify(destRef).setLocalDownloadPath("d06b4640-d7d3-45d7-92c1-8cd3c8eb1eb7"); + Mockito.verify(destRef).setInstallPath("d06b4640-d7d3-45d7-92c1-8cd3c8eb1eb7"); + Mockito.verify(templatePoolDao).update(19L, destRef); + Mockito.verify(templatePoolDao, Mockito.never()).persist(Mockito.any(VMTemplateStoragePoolVO.class)); + } + + @Test + public void updateCopiedTemplateReferencePersistsDestinationReferenceWhenMissing() { + VolumeInfo srcVolumeInfo = Mockito.mock(VolumeInfo.class); + VolumeInfo destVolumeInfo = Mockito.mock(VolumeInfo.class); + VMTemplateStoragePoolVO srcRef = Mockito.mock(VMTemplateStoragePoolVO.class); + ArgumentCaptor captor = ArgumentCaptor.forClass(VMTemplateStoragePoolVO.class); + + Mockito.when(srcVolumeInfo.getPoolId()).thenReturn(5L); + Mockito.when(srcVolumeInfo.getTemplateId()).thenReturn(13L); + Mockito.when(destVolumeInfo.getPoolId()).thenReturn(4L); + Mockito.when(srcRef.getTemplateId()).thenReturn(13L); + Mockito.when(srcRef.getTemplateSize()).thenReturn(1851129856L); + Mockito.when(srcRef.getLocalDownloadPath()).thenReturn("d06b4640-d7d3-45d7-92c1-8cd3c8eb1eb7"); + Mockito.when(srcRef.getInstallPath()).thenReturn("d06b4640-d7d3-45d7-92c1-8cd3c8eb1eb7"); + Mockito.when(templatePoolDao.findByPoolTemplate(5L, 13L, null)).thenReturn(srcRef); + Mockito.when(templatePoolDao.findByPoolTemplate(4L, 13L, null)).thenReturn(null); + + strategy.updateCopiedTemplateReference(srcVolumeInfo, destVolumeInfo); + + Mockito.verify(templatePoolDao).persist(captor.capture()); + VMTemplateStoragePoolVO persistedRef = captor.getValue(); + Assert.assertEquals(4L, persistedRef.getPoolId()); + Assert.assertEquals(13L, persistedRef.getTemplateId()); + Assert.assertEquals(100, persistedRef.getDownloadPercent()); + Assert.assertEquals(VMTemplateStorageResourceAssoc.Status.DOWNLOADED, persistedRef.getDownloadState()); + Assert.assertEquals(ObjectInDataStoreStateMachine.State.Ready, persistedRef.getState()); + Assert.assertEquals(1851129856L, persistedRef.getTemplateSize()); + Assert.assertEquals("d06b4640-d7d3-45d7-92c1-8cd3c8eb1eb7", persistedRef.getLocalDownloadPath()); + Assert.assertEquals("d06b4640-d7d3-45d7-92c1-8cd3c8eb1eb7", persistedRef.getInstallPath()); + Mockito.verify(templatePoolDao, Mockito.never()).update(Mockito.anyLong(), Mockito.any(VMTemplateStoragePoolVO.class)); + } + + @Test + public void updateCopiedTemplateReferenceThrowsWhenSourceReferenceMissing() { + VolumeInfo srcVolumeInfo = Mockito.mock(VolumeInfo.class); + VolumeInfo destVolumeInfo = Mockito.mock(VolumeInfo.class); + + Mockito.when(srcVolumeInfo.getPoolId()).thenReturn(5L); + Mockito.when(srcVolumeInfo.getTemplateId()).thenReturn(13L); + Mockito.when(templatePoolDao.findByPoolTemplate(5L, 13L, null)).thenReturn(null); + + try { + strategy.updateCopiedTemplateReference(srcVolumeInfo, destVolumeInfo); + Assert.fail("Expected CloudRuntimeException to be thrown"); + } catch (CloudRuntimeException e) { + Assert.assertTrue(e.getMessage().contains("source template reference was not found")); + Assert.assertTrue(e.getMessage().contains("pool [5]")); + Assert.assertTrue(e.getMessage().contains("template [13]")); + } + } + + @Test + public void shouldForceFullCloneMigrationReturnsTrueForMixedDirectDownloadVolumes() { + VolumeInfo directDownloadVolume = Mockito.mock(VolumeInfo.class); + VolumeInfo regularVolume = Mockito.mock(VolumeInfo.class); + DataStore destPrimary = Mockito.mock(DataStore.class); + DataStore otherDestPrimary = Mockito.mock(DataStore.class); + Host destHost = Mockito.mock(Host.class); + Map volumeMap = new LinkedHashMap<>(); + + configurePoolLookup(directDownloadVolume, destPrimary, 1L, 2L, StoragePoolType.NetworkFilesystem, StoragePoolType.NetworkFilesystem); + configurePoolLookup(regularVolume, otherDestPrimary, 3L, 4L, StoragePoolType.NetworkFilesystem, StoragePoolType.NetworkFilesystem); + Mockito.when(directDownloadVolume.isDirectDownload()).thenReturn(true); + + volumeMap.put(regularVolume, otherDestPrimary); + volumeMap.put(directDownloadVolume, destPrimary); + + Assert.assertTrue(strategy.shouldForceFullCloneMigration(volumeMap, destHost)); + } + + @Test + public void shouldForceFullCloneMigrationIgnoresSkippedDirectDownloadVolumes() { + VolumeInfo skippedDirectDownloadVolume = Mockito.mock(VolumeInfo.class); + VolumeInfo regularVolume = Mockito.mock(VolumeInfo.class); + DataStore samePowerFlexStore = Mockito.mock(DataStore.class); + DataStore destPrimary = Mockito.mock(DataStore.class); + Host destHost = Mockito.mock(Host.class); + Map volumeMap = new LinkedHashMap<>(); + + configurePoolLookup(skippedDirectDownloadVolume, samePowerFlexStore, 1L, 1L, StoragePoolType.PowerFlex, StoragePoolType.PowerFlex); + configurePoolLookup(regularVolume, destPrimary, 3L, 4L, StoragePoolType.NetworkFilesystem, StoragePoolType.NetworkFilesystem); + lenient().when(skippedDirectDownloadVolume.isDirectDownload()).thenReturn(true); + Mockito.when(regularVolume.isDirectDownload()).thenReturn(false); + + volumeMap.put(skippedDirectDownloadVolume, samePowerFlexStore); + volumeMap.put(regularVolume, destPrimary); + + Assert.assertFalse(strategy.shouldForceFullCloneMigration(volumeMap, destHost)); + Mockito.verify(skippedDirectDownloadVolume, Mockito.never()).isDirectDownload(); + } + + @Test + public void shouldForceFullCloneMigrationReturnsFalseWhenNoVolumeIsDirectDownload() { + VolumeInfo regularVolume = Mockito.mock(VolumeInfo.class); + DataStore destPrimary = Mockito.mock(DataStore.class); + Host destHost = Mockito.mock(Host.class); + Map volumeMap = new HashMap<>(); + + configurePoolLookup(regularVolume, destPrimary, 3L, 4L, StoragePoolType.NetworkFilesystem, StoragePoolType.NetworkFilesystem); + Mockito.when(regularVolume.isDirectDownload()).thenReturn(false); + volumeMap.put(regularVolume, destPrimary); + + Assert.assertFalse(strategy.shouldForceFullCloneMigration(volumeMap, destHost)); + } + + private void configurePoolLookup(VolumeInfo volume, DataStore destStore, long sourcePoolId, long destPoolId, StoragePoolType sourcePoolType, StoragePoolType destPoolType) { + StoragePoolVO sourcePool = Mockito.mock(StoragePoolVO.class); + StoragePoolVO destPool = sourcePoolId == destPoolId ? sourcePool : Mockito.mock(StoragePoolVO.class); + + Mockito.when(volume.getPoolId()).thenReturn(sourcePoolId); + Mockito.when(destStore.getId()).thenReturn(destPoolId); + Mockito.when(sourcePool.getId()).thenReturn(sourcePoolId); + Mockito.lenient().when(destPool.getId()).thenReturn(destPoolId); + Mockito.when(sourcePool.getPoolType()).thenReturn(sourcePoolType); + Mockito.lenient().when(destPool.getPoolType()).thenReturn(destPoolType); + Mockito.when(primaryDataStoreDao.findById(sourcePoolId)).thenReturn(sourcePool); + Mockito.when(primaryDataStoreDao.findById(destPoolId)).thenReturn(destPool); + } + + @Test + public void decideMigrationTypeAndCopyTemplateIfNeededUsesFullCloneWhenForced() { + Host destHost = Mockito.mock(Host.class); + VMInstanceVO vmInstance = Mockito.mock(VMInstanceVO.class); + VolumeInfo srcVolumeInfo = Mockito.mock(VolumeInfo.class); + StoragePoolVO sourceStoragePool = Mockito.mock(StoragePoolVO.class); + StoragePoolVO destStoragePool = Mockito.mock(StoragePoolVO.class); + DataStore destDataStore = Mockito.mock(DataStore.class); + + Mockito.when(srcVolumeInfo.getId()).thenReturn(61L); + + MigrationOptions.Type migrationType = strategy.decideMigrationTypeAndCopyTemplateIfNeeded(destHost, vmInstance, srcVolumeInfo, sourceStoragePool, + destStoragePool, destDataStore, true); + + Assert.assertEquals(MigrationOptions.Type.FullClone, migrationType); + Mockito.verify(strategy, Mockito.never()).copyTemplateToTargetFilesystemStorageIfNeeded(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); + } + + @Test + public void decideMigrationTypeAndCopyTemplateIfNeededUsesLinkedCloneWhenForcedFlagIsFalse() { + Host destHost = Mockito.mock(Host.class); + VMInstanceVO vmInstance = Mockito.mock(VMInstanceVO.class); + VolumeInfo srcVolumeInfo = Mockito.mock(VolumeInfo.class); + StoragePoolVO sourceStoragePool = Mockito.mock(StoragePoolVO.class); + StoragePoolVO destStoragePool = Mockito.mock(StoragePoolVO.class); + DataStore destDataStore = Mockito.mock(DataStore.class); + VMTemplateVO vmTemplate = Mockito.mock(VMTemplateVO.class); + + Mockito.when(vmInstance.getTemplateId()).thenReturn(101L); + Mockito.when(vmTemplateDao.findById(101L)).thenReturn(vmTemplate); + Mockito.when(vmTemplate.getName()).thenReturn("rocky-template"); + Mockito.when(srcVolumeInfo.getId()).thenReturn(61L); + Mockito.when(srcVolumeInfo.getTemplateId()).thenReturn(13L); + Mockito.when(destStoragePool.getPoolType()).thenReturn(StoragePoolType.Filesystem); + Mockito.doReturn("source-backing.qcow2").when(strategy).getVolumeBackingFile(srcVolumeInfo); + + MigrationOptions.Type migrationType = strategy.decideMigrationTypeAndCopyTemplateIfNeeded(destHost, vmInstance, srcVolumeInfo, sourceStoragePool, + destStoragePool, destDataStore, false); + + Assert.assertEquals(MigrationOptions.Type.LinkedClone, migrationType); + Mockito.verify(strategy).copyTemplateToTargetFilesystemStorageIfNeeded(srcVolumeInfo, sourceStoragePool, destDataStore, destStoragePool, destHost); + } + @Test public void setVolumePathTest() { VolumeVO volume = new VolumeVO("name", 0l, 0l, 0l, 0l, 0l, "folder", "path", Storage.ProvisioningType.THIN, 0l, Volume.Type.ROOT); diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeDataFactoryImpl.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeDataFactoryImpl.java index 5c2a774f8a2b..becbebbcf73f 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeDataFactoryImpl.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeDataFactoryImpl.java @@ -56,6 +56,7 @@ public VolumeInfo getVolume(long volumeId, DataStore store) { VolumeVO volumeVO = volumeDao.findById(volumeId); VolumeObject vol = VolumeObject.getVolumeObject(store, volumeVO); + setDirectDownloadFlagIfNeeded(vol); return vol; } @@ -77,6 +78,7 @@ public VolumeInfo getVolume(long volumeId, DataStoreRole storeRole) { vol = VolumeObject.getVolumeObject(store, volumeVO); } } + setDirectDownloadFlagIfNeeded(vol); return vol; } @@ -102,12 +104,7 @@ public VolumeInfo getVolume(long volumeId) { } vol = VolumeObject.getVolumeObject(store, volumeVO); } - if (vol.getTemplateId() != null) { - VMTemplateVO template = templateDao.findById(vol.getTemplateId()); - if (template != null) { - vol.setDirectDownload(template.isDirectDownload()); - } - } + setDirectDownloadFlagIfNeeded(vol); return vol; } @@ -118,6 +115,16 @@ public VolumeInfo getVolume(DataObject volume, DataStore store) { return vol; } + private void setDirectDownloadFlagIfNeeded(VolumeObject vol) { + if (vol == null || vol.getTemplateId() == null) { + return; + } + VMTemplateVO template = templateDao.findByIdIncludingRemoved(vol.getTemplateId()); + if (template != null) { + vol.setDirectDownload(template.isDirectDownload()); + } + } + @Override public List listVolumeOnCache(long volumeId) { List cacheVols = new ArrayList(); diff --git a/engine/storage/volume/src/test/java/org/apache/cloudstack/storage/volume/VolumeDataFactoryImplTest.java b/engine/storage/volume/src/test/java/org/apache/cloudstack/storage/volume/VolumeDataFactoryImplTest.java new file mode 100644 index 000000000000..e3d1992d4b22 --- /dev/null +++ b/engine/storage/volume/src/test/java/org/apache/cloudstack/storage/volume/VolumeDataFactoryImplTest.java @@ -0,0 +1,140 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.cloudstack.storage.volume; + +import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; +import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreVO; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.storage.DataStoreRole; +import com.cloud.storage.VMTemplateVO; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.VMTemplateDao; +import com.cloud.storage.dao.VolumeDao; + +@RunWith(MockitoJUnitRunner.class) +public class VolumeDataFactoryImplTest { + + private VolumeDataFactoryImpl volumeDataFactory; + + @Mock + private VolumeDao volumeDao; + @Mock + private VolumeDataStoreDao volumeStoreDao; + @Mock + private DataStoreManager storeMgr; + @Mock + private VMTemplateDao templateDao; + @Mock + private VolumeVO volumeVO; + @Mock + private VolumeObject volumeObject; + @Mock + private VolumeDataStoreVO volumeDataStoreVO; + @Mock + private DataStore dataStore; + @Mock + private VMTemplateVO templateVO; + + @Before + public void setUp() { + volumeDataFactory = new VolumeDataFactoryImpl(); + volumeDataFactory.volumeDao = volumeDao; + volumeDataFactory.volumeStoreDao = volumeStoreDao; + volumeDataFactory.storeMgr = storeMgr; + volumeDataFactory.templateDao = templateDao; + } + + @Test + public void getVolumeByExplicitStoreSetsDirectDownloadFlag() { + Mockito.when(volumeDao.findById(42L)).thenReturn(volumeVO); + Mockito.when(volumeObject.getTemplateId()).thenReturn(9L); + Mockito.when(templateDao.findByIdIncludingRemoved(9L)).thenReturn(templateVO); + Mockito.when(templateVO.isDirectDownload()).thenReturn(true); + + try (MockedStatic mockedStatic = Mockito.mockStatic(VolumeObject.class)) { + mockedStatic.when(() -> VolumeObject.getVolumeObject(dataStore, volumeVO)).thenReturn(volumeObject); + + Assert.assertSame(volumeObject, volumeDataFactory.getVolume(42L, dataStore)); + } + + Mockito.verify(volumeObject).setDirectDownload(true); + } + + @Test + public void getVolumeByPrimaryStoreRoleSetsDirectDownloadFlag() { + Mockito.when(volumeDao.findById(42L)).thenReturn(volumeVO); + Mockito.when(volumeVO.getPoolId()).thenReturn(7L); + Mockito.when(storeMgr.getDataStore(7L, DataStoreRole.Primary)).thenReturn(dataStore); + Mockito.when(volumeObject.getTemplateId()).thenReturn(9L); + Mockito.when(templateDao.findByIdIncludingRemoved(9L)).thenReturn(templateVO); + Mockito.when(templateVO.isDirectDownload()).thenReturn(true); + + try (MockedStatic mockedStatic = Mockito.mockStatic(VolumeObject.class)) { + mockedStatic.when(() -> VolumeObject.getVolumeObject(dataStore, volumeVO)).thenReturn(volumeObject); + + Assert.assertSame(volumeObject, volumeDataFactory.getVolume(42L, DataStoreRole.Primary)); + } + + Mockito.verify(volumeObject).setDirectDownload(true); + } + + @Test + public void getVolumeByImageStoreRoleSetsDirectDownloadFlag() { + Mockito.when(volumeDao.findById(42L)).thenReturn(volumeVO); + Mockito.when(volumeStoreDao.findByVolume(42L)).thenReturn(volumeDataStoreVO); + Mockito.when(volumeDataStoreVO.getDataStoreId()).thenReturn(11L); + Mockito.when(storeMgr.getDataStore(11L, DataStoreRole.Image)).thenReturn(dataStore); + Mockito.when(volumeObject.getTemplateId()).thenReturn(9L); + Mockito.when(templateDao.findByIdIncludingRemoved(9L)).thenReturn(templateVO); + Mockito.when(templateVO.isDirectDownload()).thenReturn(true); + + try (MockedStatic mockedStatic = Mockito.mockStatic(VolumeObject.class)) { + mockedStatic.when(() -> VolumeObject.getVolumeObject(dataStore, volumeVO)).thenReturn(volumeObject); + + Assert.assertSame(volumeObject, volumeDataFactory.getVolume(42L, DataStoreRole.Image)); + } + + Mockito.verify(volumeObject).setDirectDownload(true); + } + + @Test + public void getVolumeByExplicitStoreSkipsTemplateLookupWhenVolumeHasNoTemplate() { + Mockito.when(volumeDao.findById(42L)).thenReturn(volumeVO); + Mockito.when(volumeObject.getTemplateId()).thenReturn(null); + + try (MockedStatic mockedStatic = Mockito.mockStatic(VolumeObject.class)) { + mockedStatic.when(() -> VolumeObject.getVolumeObject(dataStore, volumeVO)).thenReturn(volumeObject); + + Assert.assertSame(volumeObject, volumeDataFactory.getVolume(42L, dataStore)); + } + + Mockito.verifyNoInteractions(templateDao); + Mockito.verify(volumeObject, Mockito.never()).setDirectDownload(Mockito.anyBoolean()); + } +}