You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by nv...@apache.org on 2021/08/20 17:54:37 UTC
[cloudstack] branch main updated: Remove storage scope validation
on KVM live migration (#5321)
This is an automated email from the ASF dual-hosted git repository.
nvazquez pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new 9c51009 Remove storage scope validation on KVM live migration (#5321)
9c51009 is described below
commit 9c510091344e7407d0edc3e12ada9e0b04ec8de8
Author: Daniel Augusto Veronezi Salvador <38...@users.noreply.github.com>
AuthorDate: Fri Aug 20 14:54:14 2021 -0300
Remove storage scope validation on KVM live migration (#5321)
Co-authored-by: GutoVeronezi <da...@scclouds.com.br>
---
.../motion/StorageSystemDataMotionStrategy.java | 26 ++++---
.../StorageSystemDataMotionStrategyTest.java | 82 ++++++++++++++++++++++
2 files changed, 98 insertions(+), 10 deletions(-)
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 3c68793..1ee3d66 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,20 +2258,27 @@ 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<String, Storage.StoragePoolType> 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
* @throws CloudRuntimeException if any source storage pool is not mounted on the destination host
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 3793a79..88b7bf5 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<String, Storage.StoragePoolType> mapStringStoragePoolTypeMock;
+
+ List<ScopeType> 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<StoragePoolType> 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<StoragePoolType> 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