This is an automated email from the ASF dual-hosted git repository. Pearl1594 pushed a commit to branch fix-stg-access-group-validation in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 96704454102f9c0bf8e54f38421bbcba5a54733d Author: Sachin R <[email protected]> AuthorDate: Tue Nov 11 21:04:06 2025 -0800 VM Migration: Storage access groups validation issue fix * VM Migration: Storage access groups validation issue fix * Refactor list empty checks --------- Co-authored-by: Sachin R Doddaguni <[email protected]> --- .../main/java/com/cloud/vm/UserVmManagerImpl.java | 50 +++-- .../java/com/cloud/vm/UserVmManagerImplTest.java | 234 +++++++++++++++------ 2 files changed, 212 insertions(+), 72 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 2e30b4ecbd8..da2689ef1ff 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -410,6 +410,7 @@ import com.cloud.vm.snapshot.dao.VMSnapshotDao; import com.google.gson.Gson; import com.google.gson.reflect.TypeToken; +import com.cloud.storage.dao.StoragePoolAndAccessGroupMapDao; public class UserVmManagerImpl extends ManagerBase implements UserVmManager, VirtualMachineGuru, Configurable { @@ -589,6 +590,8 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir @Inject private StorageManager storageManager; @Inject + private StoragePoolAndAccessGroupMapDao _storagePoolAndAccessGroupMapDao; + @Inject private ServiceOfferingJoinDao serviceOfferingJoinDao; @Inject private BackupDao backupDao; @@ -7224,23 +7227,44 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir return host.checkHostServiceOfferingAndTemplateTags(serviceOffering, template, strictHostTags); } - protected void validateStorageAccessGroupsOnHosts(Host srcHost, Host destinationHost) { - String[] storageAccessGroupsOnSrcHost = storageManager.getStorageAccessGroups(null, null, null, srcHost.getId()); - String[] storageAccessGroupsOnDestHost = storageManager.getStorageAccessGroups(null, null, null, destinationHost.getId()); - - List<String> srcHostStorageAccessGroupsList = storageAccessGroupsOnSrcHost != null ? Arrays.asList(storageAccessGroupsOnSrcHost) : Collections.emptyList(); - List<String> destHostStorageAccessGroupsList = storageAccessGroupsOnDestHost != null ? Arrays.asList(storageAccessGroupsOnDestHost) : Collections.emptyList(); - - if (CollectionUtils.isEmpty(srcHostStorageAccessGroupsList)) { + protected void validateStorageAccessGroupsOnHosts(VMInstanceVO vm, Host destinationHost) { + List<VolumeVO> vmVolumes = _volsDao.findByInstance(vm.getId()); + if (CollectionUtils.isEmpty(vmVolumes)) { + s_logger.debug(String.format("No volumes found for VM %s, skipping storage access group validation", vm)); return; } + String[] storageAccessGroupsOnDestHost = storageManager.getStorageAccessGroups(null, null, null, destinationHost.getId()); + List<String> destHostStorageAccessGroupsList = Arrays.asList(storageAccessGroupsOnDestHost); if (CollectionUtils.isEmpty(destHostStorageAccessGroupsList)) { - throw new CloudRuntimeException("Source host has storage access groups, but destination host has none."); + s_logger.debug(String.format("Destination host %s has no storage access groups", destinationHost.getName())); } - if (!destHostStorageAccessGroupsList.containsAll(srcHostStorageAccessGroupsList)) { - throw new CloudRuntimeException("Storage access groups on the source and destination hosts did not match."); + for (VolumeVO volume : vmVolumes) { + if (volume.getPoolId() == null) { + s_logger.debug(String.format("Volume %s has no storage pool assigned, skipping storage access group validation", volume.getName())); + continue; + } + List<String> poolStorageAccessGroupsList = _storagePoolAndAccessGroupMapDao.getStorageAccessGroups(volume.getPoolId()); + if (CollectionUtils.isEmpty(poolStorageAccessGroupsList)) { + s_logger.debug(String.format("Storage pool %d has no storage access groups, skipping validation for volume %s", + volume.getPoolId(), volume.getName())); + continue; + } + if (Collections.disjoint(poolStorageAccessGroupsList, destHostStorageAccessGroupsList)) { + throw new CloudRuntimeException(String.format( + "Destination host %s does not have any storage access groups that match the requirements for volume %s (Resource Class: %s) on storage pool %d. " + + "Pool requires access groups: %s, but destination host only has: %s", + destinationHost.getName(), + volume.getName(), + volume.getVolumeType(), volume.getPoolId(), + String.join(", ", poolStorageAccessGroupsList), + String.join(", ", destHostStorageAccessGroupsList) + )); + } + s_logger.debug(String.format("Storage access group validation passed for volume %s (Resource Class: %s) on storage pool %d. " + + "Destination host %s has matching access groups.", + volume.getName(), volume.getVolumeType(), volume.getPoolId(), destinationHost.getName())); } } @@ -7286,7 +7310,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir HostVO destinationHostVO = _hostDao.findById(destinationHost.getId()); _hostDao.loadHostTags(destinationHostVO); validateStrictHostTagCheck(vm, destinationHostVO); - validateStorageAccessGroupsOnHosts(srcHost, destinationHost); + validateStorageAccessGroupsOnHosts(vm, destinationHost); checkHostsDedication(vm, srcHost.getId(), destinationHost.getId()); @@ -7647,7 +7671,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir destinationHost.getName(), destinationHost.getUuid())); } - validateStorageAccessGroupsOnHosts(srcHost, destinationHost); + validateStorageAccessGroupsOnHosts(vm, destinationHost); return new Pair<>(srcHost, destinationHost); } diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index fe4ea0838f1..7b9e2222df7 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -171,6 +171,7 @@ import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.GuestOSDao; import com.cloud.storage.dao.SnapshotDao; +import com.cloud.storage.dao.StoragePoolAndAccessGroupMapDao; import com.cloud.storage.dao.VMTemplateDao; import com.cloud.storage.dao.VolumeDao; import com.cloud.template.VirtualMachineTemplate; @@ -415,6 +416,8 @@ public class UserVmManagerImplTest { @Mock StorageManager storageManager; + @Mock + private StoragePoolAndAccessGroupMapDao storagePoolAndAccessGroupMapDao; @Mock private VolumeDataFactory volumeDataFactory; @@ -3206,95 +3209,208 @@ public class UserVmManagerImplTest { } @Test - public void validateStorageAccessGroupsOnHostsMatchingSAGsNoException() { - Host srcHost = Mockito.mock(Host.class); - Host destHost = Mockito.mock(Host.class); + public void testValidateStorageAccessGroupsOnHostsVolumeBasedSuccessWithMatchingGroups() { + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Host destinationHost = Mockito.mock(Host.class); - Mockito.when(srcHost.getId()).thenReturn(1L); - Mockito.when(destHost.getId()).thenReturn(2L); - when(storageManager.getStorageAccessGroups(null, null, null, srcHost.getId())).thenReturn(new String[]{"sag1", "sag2"}); - when(storageManager.getStorageAccessGroups(null, null, null, destHost.getId())).thenReturn(new String[]{"sag1", "sag2", "sag3"}); + List<VolumeVO> vmVolumes = new ArrayList<>(); + VolumeVO volume1 = Mockito.mock(VolumeVO.class); + VolumeVO volume2 = Mockito.mock(VolumeVO.class); + vmVolumes.add(volume1); + vmVolumes.add(volume2); - userVmManagerImpl.validateStorageAccessGroupsOnHosts(srcHost, destHost); + StoragePoolVO storagePool1 = Mockito.mock(StoragePoolVO.class); + StoragePoolVO storagePool2 = Mockito.mock(StoragePoolVO.class); - Mockito.verify(storageManager, times(1)).getStorageAccessGroups(null, null, null, srcHost.getId()); - Mockito.verify(storageManager, times(1)).getStorageAccessGroups(null, null, null, destHost.getId()); - } + Mockito.when(vm.getId()).thenReturn(1L); + Mockito.when(destinationHost.getId()).thenReturn(2L); + Mockito.when(destinationHost.getName()).thenReturn("destHost"); - @Test(expected = CloudRuntimeException.class) - public void validateSAGsOnHostsNonMatchingSAGsThrowsException() { - Host srcHost = Mockito.mock(Host.class); - Host destHost = Mockito.mock(Host.class); + Mockito.when(volume1.getPoolId()).thenReturn(100L); + Mockito.when(volume1.getName()).thenReturn("volume1"); + Mockito.when(volume1.getVolumeType()).thenReturn(Volume.Type.ROOT); + Mockito.when(storagePool1.getName()).thenReturn("pool1"); + + Mockito.when(volume2.getPoolId()).thenReturn(200L); + Mockito.when(volume2.getName()).thenReturn("volume2"); + Mockito.when(volume2.getVolumeType()).thenReturn(Volume.Type.DATADISK); + Mockito.when(storagePool2.getName()).thenReturn("pool2"); - Mockito.when(srcHost.getId()).thenReturn(1L); - Mockito.when(destHost.getId()).thenReturn(2L); - when(storageManager.getStorageAccessGroups(null, null, null, srcHost.getId())).thenReturn(new String[]{"sag1", "sag2"}); - when(storageManager.getStorageAccessGroups(null, null, null, destHost.getId())).thenReturn(new String[]{"sag1", "sag3"}); + Mockito.when(volumeDaoMock.findByInstance(1L)).thenReturn(vmVolumes); - userVmManagerImpl.validateStorageAccessGroupsOnHosts(srcHost, destHost); + Mockito.when(storageManager.getStorageAccessGroups(null, null, null, 2L)) + .thenReturn(new String[]{"group1", "group2", "group3"}); + + Mockito.when(storagePoolAndAccessGroupMapDao.getStorageAccessGroups(100L)) + .thenReturn(Arrays.asList("group1", "group2")); + Mockito.when(storagePoolAndAccessGroupMapDao.getStorageAccessGroups(200L)) + .thenReturn(Arrays.asList("group2", "group3")); + userVmManagerImpl.validateStorageAccessGroupsOnHosts(vm, destinationHost); + + Mockito.verify(volumeDaoMock).findByInstance(1L); + Mockito.verify(storageManager).getStorageAccessGroups(null, null, null, 2L); + Mockito.verify(storagePoolAndAccessGroupMapDao).getStorageAccessGroups(100L); + Mockito.verify(storagePoolAndAccessGroupMapDao).getStorageAccessGroups(200L); } @Test - public void validateEmptyStorageAccessGroupOnHosts() { - Host srcHost = Mockito.mock(Host.class); - Host destHost = Mockito.mock(Host.class); + public void testValidateStorageAccessGroupsOnHostsVolumeBasedSuccessWithIntersection() { + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Host destinationHost = Mockito.mock(Host.class); + + List<VolumeVO> vmVolumes = new ArrayList<>(); + VolumeVO volume1 = Mockito.mock(VolumeVO.class); + vmVolumes.add(volume1); + + StoragePoolVO storagePool1 = Mockito.mock(StoragePoolVO.class); - Mockito.when(srcHost.getId()).thenReturn(1L); - Mockito.when(destHost.getId()).thenReturn(2L); - when(storageManager.getStorageAccessGroups(null, null, null, srcHost.getId())).thenReturn(new String[]{}); - when(storageManager.getStorageAccessGroups(null, null, null, destHost.getId())).thenReturn(new String[]{}); + Mockito.when(vm.getId()).thenReturn(1L); + Mockito.when(destinationHost.getId()).thenReturn(2L); + Mockito.when(destinationHost.getName()).thenReturn("destHost"); + + Mockito.when(volume1.getPoolId()).thenReturn(100L); + Mockito.when(volume1.getName()).thenReturn("volume1"); + Mockito.when(volume1.getVolumeType()).thenReturn(Volume.Type.ROOT); + Mockito.when(storagePool1.getName()).thenReturn("pool1"); + + Mockito.when(volumeDaoMock.findByInstance(1L)).thenReturn(vmVolumes); + + Mockito.when(storageManager.getStorageAccessGroups(null, null, null, 2L)) + .thenReturn(new String[]{"group2", "group4", "group5"}); - userVmManagerImpl.validateStorageAccessGroupsOnHosts(srcHost, destHost); + Mockito.when(storagePoolAndAccessGroupMapDao.getStorageAccessGroups(100L)) + .thenReturn(Arrays.asList("group1", "group2", "group3")); - Mockito.verify(storageManager, times(1)).getStorageAccessGroups(null, null, null, srcHost.getId()); - Mockito.verify(storageManager, times(1)).getStorageAccessGroups(null, null, null, destHost.getId()); + userVmManagerImpl.validateStorageAccessGroupsOnHosts(vm, destinationHost); } @Test - public void validateSAGsOnHostsNullStorageAccessGroups() { - Host srcHost = Mockito.mock(Host.class); - Host destHost = Mockito.mock(Host.class); + public void testValidateStorageAccessGroupsOnHostsSuccessWithEmptyStorageGroups() { + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Host destinationHost = Mockito.mock(Host.class); + + List<VolumeVO> vmVolumes = new ArrayList<>(); + VolumeVO volume1 = Mockito.mock(VolumeVO.class); + vmVolumes.add(volume1); + + StoragePoolVO storagePool1 = Mockito.mock(StoragePoolVO.class); + + Mockito.when(vm.getId()).thenReturn(1L); + Mockito.when(destinationHost.getId()).thenReturn(2L); + Mockito.when(destinationHost.getName()).thenReturn("destHost"); + + Mockito.when(volume1.getPoolId()).thenReturn(100L); + Mockito.when(volume1.getName()).thenReturn("volume1"); + Mockito.when(volume1.getVolumeType()).thenReturn(Volume.Type.ROOT); + Mockito.when(storagePool1.getName()).thenReturn("pool1"); + + Mockito.when(volumeDaoMock.findByInstance(1L)).thenReturn(vmVolumes); + + Mockito.when(storageManager.getStorageAccessGroups(null, null, null, 2L)) + .thenReturn(new String[]{"group1", "group2"}); + Mockito.when(storagePoolAndAccessGroupMapDao.getStorageAccessGroups(100L)) + .thenReturn(List.of()); + userVmManagerImpl.validateStorageAccessGroupsOnHosts(vm, destinationHost); + } - Mockito.when(srcHost.getId()).thenReturn(1L); - Mockito.when(destHost.getId()).thenReturn(2L); - when(storageManager.getStorageAccessGroups(null, null, null, srcHost.getId())).thenReturn(null); - when(storageManager.getStorageAccessGroups(null, null, null, destHost.getId())).thenReturn(null); + @Test(expected = CloudRuntimeException.class) + public void testValidateStorageAccessGroupsOnHostsDestinationHostEmptyGroups() { + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Host destinationHost = Mockito.mock(Host.class); - userVmManagerImpl.validateStorageAccessGroupsOnHosts(srcHost, destHost); + List<VolumeVO> vmVolumes = new ArrayList<>(); + VolumeVO volume1 = Mockito.mock(VolumeVO.class); + vmVolumes.add(volume1); + + StoragePoolVO storagePool1 = Mockito.mock(StoragePoolVO.class); + + Mockito.when(vm.getId()).thenReturn(1L); + Mockito.when(destinationHost.getId()).thenReturn(2L); + Mockito.when(destinationHost.getName()).thenReturn("destHost"); - Mockito.verify(storageManager, times(1)).getStorageAccessGroups(null, null, null, srcHost.getId()); - Mockito.verify(storageManager, times(1)).getStorageAccessGroups(null, null, null, destHost.getId()); + Mockito.when(volume1.getPoolId()).thenReturn(100L); + Mockito.when(volume1.getName()).thenReturn("volume1"); + Mockito.when(volume1.getVolumeType()).thenReturn(Volume.Type.ROOT); + Mockito.when(storagePool1.getName()).thenReturn("pool1"); + + Mockito.when(volumeDaoMock.findByInstance(1L)).thenReturn(vmVolumes); + Mockito.when(storageManager.getStorageAccessGroups(null, null, null, 2L)) + .thenReturn(new String[]{}); + + Mockito.when(storagePoolAndAccessGroupMapDao.getStorageAccessGroups(100L)) + .thenReturn(List.of("group1")); + userVmManagerImpl.validateStorageAccessGroupsOnHosts(vm, destinationHost); } @Test(expected = CloudRuntimeException.class) - public void validateSAGsOnDestHostNullStorageAccessGroups() { - Host srcHost = Mockito.mock(Host.class); - Host destHost = Mockito.mock(Host.class); + public void testValidateStorageAccessGroupsOnHostsFailureNoMatch() { + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Host destinationHost = Mockito.mock(Host.class); + + List<VolumeVO> vmVolumes = new ArrayList<>(); + VolumeVO volume1 = Mockito.mock(VolumeVO.class); + vmVolumes.add(volume1); + + StoragePoolVO storagePool1 = Mockito.mock(StoragePoolVO.class); - Mockito.when(srcHost.getId()).thenReturn(1L); - Mockito.when(destHost.getId()).thenReturn(2L); - when(storageManager.getStorageAccessGroups(null, null, null, srcHost.getId())).thenReturn(new String[]{"sag1", "sag2"}); - when(storageManager.getStorageAccessGroups(null, null, null, destHost.getId())).thenReturn(null); + Mockito.when(vm.getId()).thenReturn(1L); + Mockito.when(destinationHost.getId()).thenReturn(2L); + Mockito.when(destinationHost.getName()).thenReturn("destHost"); + + Mockito.when(volume1.getPoolId()).thenReturn(100L); + Mockito.when(volume1.getName()).thenReturn("volume1"); + Mockito.when(volume1.getVolumeType()).thenReturn(Volume.Type.ROOT); + Mockito.when(storagePool1.getName()).thenReturn("pool1"); - userVmManagerImpl.validateStorageAccessGroupsOnHosts(srcHost, destHost); + Mockito.when(volumeDaoMock.findByInstance(1L)).thenReturn(vmVolumes); + + Mockito.when(storageManager.getStorageAccessGroups(null, null, null, 2L)) + .thenReturn(new String[]{"group4", "group5", "group6"}); + Mockito.when(storagePoolAndAccessGroupMapDao.getStorageAccessGroups(100L)) + .thenReturn(Arrays.asList("group1", "group2", "group3")); + userVmManagerImpl.validateStorageAccessGroupsOnHosts(vm, destinationHost); } - @Test - public void validateNullStorageAccessGroupsOnSrcHost() { + @Test(expected = CloudRuntimeException.class) + public void testValidateStorageAccessGroupsOnHostsMultipleVolumesFailure() { + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Host destinationHost = Mockito.mock(Host.class); - Host srcHost = Mockito.mock(Host.class); - Host destHost = Mockito.mock(Host.class); + List<VolumeVO> vmVolumes = new ArrayList<>(); + VolumeVO volume1 = Mockito.mock(VolumeVO.class); + VolumeVO volume2 = Mockito.mock(VolumeVO.class); + vmVolumes.add(volume1); + vmVolumes.add(volume2); - Mockito.when(srcHost.getId()).thenReturn(1L); - Mockito.when(destHost.getId()).thenReturn(2L); - when(storageManager.getStorageAccessGroups(null, null, null, srcHost.getId())).thenReturn(null); - when(storageManager.getStorageAccessGroups(null, null, null, destHost.getId())).thenReturn(new String[]{"sag1", "sag2"}); + StoragePoolVO storagePool1 = Mockito.mock(StoragePoolVO.class); + StoragePoolVO storagePool2 = Mockito.mock(StoragePoolVO.class); - userVmManagerImpl.validateStorageAccessGroupsOnHosts(srcHost, destHost); + Mockito.when(vm.getId()).thenReturn(1L); + Mockito.when(destinationHost.getId()).thenReturn(2L); + Mockito.when(destinationHost.getName()).thenReturn("destHost"); + + Mockito.when(volume1.getPoolId()).thenReturn(100L); + Mockito.when(volume1.getName()).thenReturn("volume1"); + Mockito.when(volume1.getVolumeType()).thenReturn(Volume.Type.ROOT); + Mockito.when(storagePool1.getName()).thenReturn("pool1"); + + Mockito.when(volume2.getPoolId()).thenReturn(200L); + Mockito.when(volume2.getName()).thenReturn("volume2"); + Mockito.when(volume2.getVolumeType()).thenReturn(Volume.Type.DATADISK); + Mockito.when(storagePool2.getName()).thenReturn("pool2"); - Mockito.verify(storageManager, times(1)).getStorageAccessGroups(null, null, null, srcHost.getId()); - Mockito.verify(storageManager, times(1)).getStorageAccessGroups(null, null, null, destHost.getId()); + Mockito.when(volumeDaoMock.findByInstance(1L)).thenReturn(vmVolumes); + + Mockito.when(storageManager.getStorageAccessGroups(null, null, null, 2L)) + .thenReturn(new String[]{"group1", "group2"}); + + Mockito.when(storagePoolAndAccessGroupMapDao.getStorageAccessGroups(100L)) + .thenReturn(List.of("group1")); + Mockito.when(storagePoolAndAccessGroupMapDao.getStorageAccessGroups(200L)) + .thenReturn(Arrays.asList("group3", "group4")); + userVmManagerImpl.validateStorageAccessGroupsOnHosts(vm, destinationHost); } + @Test public void testAllocateVMFromBackupUsingCmdValues() throws InsufficientCapacityException, ResourceAllocationException, ResourceUnavailableException { Long backupId = 4L;
