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
<[email protected]>
AuthorDate: Fri Aug 20 14:54:14 2021 -0300
Remove storage scope validation on KVM live migration (#5321)
Co-authored-by: GutoVeronezi <[email protected]>
---
.../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
