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 3c68793e348..1ee3d662693 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 @@ -102,7 +102,6 @@ import com.cloud.resource.ResourceState; import com.cloud.storage.DataStoreRole; import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.MigrationOptions; -import com.cloud.storage.ScopeType; import com.cloud.storage.Snapshot; import com.cloud.storage.SnapshotVO; import com.cloud.storage.Storage; @@ -2259,19 +2258,26 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { throw new CloudRuntimeException("Destination storage pools must be either all managed or all not managed"); } - if (!destStoragePoolVO.isManaged() && destStoragePoolVO.getPoolType() == StoragePoolType.NetworkFilesystem) { - if (destStoragePoolVO.getScope() != ScopeType.CLUSTER) { - throw new CloudRuntimeException("KVM live storage migrations currently support cluster-wide " + - "not managed NFS destination storage"); - } - if (!sourcePools.containsKey(srcStoragePoolVO.getUuid())) { - sourcePools.put(srcStoragePoolVO.getUuid(), srcStoragePoolVO.getPoolType()); - } - } + addSourcePoolToPoolsMap(sourcePools, srcStoragePoolVO, destStoragePoolVO); } verifyDestinationStorage(sourcePools, destHost); } + /** + * Adds source storage pool to the migration map if the destination pool is not managed and it is NFS. + */ + protected void addSourcePoolToPoolsMap(Map sourcePools, StoragePoolVO srcStoragePoolVO, StoragePoolVO destStoragePoolVO) { + if (destStoragePoolVO.isManaged() || !StoragePoolType.NetworkFilesystem.equals(destStoragePoolVO.getPoolType())) { + LOGGER.trace(String.format("Skipping adding source pool [%s] to map due to destination pool [%s] is managed or not NFS.", srcStoragePoolVO, destStoragePoolVO)); + return; + } + + String sourceStoragePoolUuid = srcStoragePoolVO.getUuid(); + if (!sourcePools.containsKey(sourceStoragePoolUuid)) { + sourcePools.put(sourceStoragePoolUuid, srcStoragePoolVO.getPoolType()); + } + } + /** * Perform storage validation on destination host for KVM live storage migrations. * Validate that volume source storage pools are mounted on the destination host prior the migration 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 3793a795077..88b7bf5fc62 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 @@ -23,6 +23,7 @@ import static org.junit.Assert.assertFalse; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.MockitoAnnotations.initMocks; import java.util.HashMap; @@ -47,17 +48,22 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.Spy; import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.verification.VerificationMode; import com.cloud.agent.api.MigrateCommand; import com.cloud.host.HostVO; import com.cloud.storage.DataStoreRole; import com.cloud.storage.ImageStore; +import com.cloud.storage.ScopeType; import com.cloud.storage.Storage; import com.cloud.storage.Storage.StoragePoolType; 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.LinkedList; +import java.util.List; import java.util.Set; @RunWith(MockitoJUnitRunner.class) @@ -78,6 +84,14 @@ public class StorageSystemDataMotionStrategyTest { @Mock private PrimaryDataStoreDao primaryDataStoreDao; + @Mock + StoragePoolVO sourceStoragePoolVoMock, destinationStoragePoolVoMock; + + @Mock + Map mapStringStoragePoolTypeMock; + + List scopeTypes = Arrays.asList(ScopeType.CLUSTER, ScopeType.ZONE); + @Before public void setUp() throws Exception { sourceStore = mock(PrimaryDataStoreImpl.class); @@ -345,4 +359,72 @@ public class StorageSystemDataMotionStrategyTest { assertFalse(strategy.isStoragePoolTypeInList(StoragePoolType.SharedMountPoint, listTypes)); } + + @Test + public void validateAddSourcePoolToPoolsMapDestinationPoolIsManaged() { + Mockito.doReturn(true).when(destinationStoragePoolVoMock).isManaged(); + strategy.addSourcePoolToPoolsMap(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); + + Mockito.verify(destinationStoragePoolVoMock).isManaged(); + Mockito.verifyNoMoreInteractions(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); + } + + @Test + public void validateAddSourcePoolToPoolsMapDestinationPoolIsNotNFS() { + List storagePoolTypes = new LinkedList<>(Arrays.asList(StoragePoolType.values())); + storagePoolTypes.remove(StoragePoolType.NetworkFilesystem); + + Mockito.doReturn(false).when(destinationStoragePoolVoMock).isManaged(); + storagePoolTypes.forEach(poolType -> { + Mockito.doReturn(poolType).when(destinationStoragePoolVoMock).getPoolType(); + strategy.addSourcePoolToPoolsMap(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); + }); + + VerificationMode times = Mockito.times(storagePoolTypes.size()); + Mockito.verify(destinationStoragePoolVoMock, times).isManaged(); + Mockito.verify(destinationStoragePoolVoMock, times).getPoolType(); + Mockito.verifyNoMoreInteractions(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); + } + + @Test + public void validateAddSourcePoolToPoolsMapMapContainsKey() { + Mockito.doReturn(false).when(destinationStoragePoolVoMock).isManaged(); + Mockito.doReturn(StoragePoolType.NetworkFilesystem).when(destinationStoragePoolVoMock).getPoolType(); + Mockito.doReturn("").when(sourceStoragePoolVoMock).getUuid(); + Mockito.doReturn(true).when(mapStringStoragePoolTypeMock).containsKey(Mockito.anyString()); + strategy.addSourcePoolToPoolsMap(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); + + Mockito.verify(destinationStoragePoolVoMock, never()).getScope(); + Mockito.verify(destinationStoragePoolVoMock).isManaged(); + Mockito.verify(destinationStoragePoolVoMock).getPoolType(); + Mockito.verify(sourceStoragePoolVoMock).getUuid(); + Mockito.verify(mapStringStoragePoolTypeMock).containsKey(Mockito.anyString()); + Mockito.verifyNoMoreInteractions(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); + } + + @Test + public void validateAddSourcePoolToPoolsMapMapDoesNotContainsKey() { + List storagePoolTypes = new LinkedList<>(Arrays.asList(StoragePoolType.values())); + + Mockito.doReturn(false).when(destinationStoragePoolVoMock).isManaged(); + Mockito.doReturn(StoragePoolType.NetworkFilesystem).when(destinationStoragePoolVoMock).getPoolType(); + Mockito.doReturn("").when(sourceStoragePoolVoMock).getUuid(); + Mockito.doReturn(false).when(mapStringStoragePoolTypeMock).containsKey(Mockito.anyString()); + Mockito.doReturn(null).when(mapStringStoragePoolTypeMock).put(Mockito.anyString(), Mockito.any()); + + storagePoolTypes.forEach(poolType -> { + Mockito.doReturn(poolType).when(sourceStoragePoolVoMock).getPoolType(); + strategy.addSourcePoolToPoolsMap(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); + }); + + VerificationMode times = Mockito.times(storagePoolTypes.size()); + Mockito.verify(destinationStoragePoolVoMock, never()).getScope(); + Mockito.verify(destinationStoragePoolVoMock, times).isManaged(); + Mockito.verify(destinationStoragePoolVoMock, times).getPoolType(); + Mockito.verify(sourceStoragePoolVoMock, times).getUuid(); + Mockito.verify(mapStringStoragePoolTypeMock, times).containsKey(Mockito.anyString()); + Mockito.verify(sourceStoragePoolVoMock, times).getPoolType(); + Mockito.verify(mapStringStoragePoolTypeMock, times).put(Mockito.anyString(), Mockito.any()); + Mockito.verifyNoMoreInteractions(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); + } } \ No newline at end of file