diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 5c9d54e19374..3db1ce6f3d96 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -413,6 +413,7 @@ 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 { @@ -592,6 +593,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; @@ -7311,23 +7314,44 @@ private boolean checkEnforceStrictHostTagCheck(HostVO host, ServiceOffering serv 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 srcHostStorageAccessGroupsList = storageAccessGroupsOnSrcHost != null ? Arrays.asList(storageAccessGroupsOnSrcHost) : Collections.emptyList(); - List destHostStorageAccessGroupsList = storageAccessGroupsOnDestHost != null ? Arrays.asList(storageAccessGroupsOnDestHost) : Collections.emptyList(); - - if (CollectionUtils.isEmpty(srcHostStorageAccessGroupsList)) { + protected void validateStorageAccessGroupsOnHosts(VMInstanceVO vm, Host destinationHost) { + List vmVolumes = _volsDao.findByInstance(vm.getId()); + if (CollectionUtils.isEmpty(vmVolumes)) { + logger.debug("No volumes found for VM {}, skipping storage access group validation", vm); return; } + String[] storageAccessGroupsOnDestHost = storageManager.getStorageAccessGroups(null, null, null, destinationHost.getId()); + List destHostStorageAccessGroupsList = Arrays.asList(storageAccessGroupsOnDestHost); if (CollectionUtils.isEmpty(destHostStorageAccessGroupsList)) { - throw new CloudRuntimeException("Source host has storage access groups, but destination host has none."); + logger.debug("Destination host {} 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) { + logger.debug("Volume {} has no storage pool assigned, skipping storage access group validation", volume.getName()); + continue; + } + List poolStorageAccessGroupsList = _storagePoolAndAccessGroupMapDao.getStorageAccessGroups(volume.getPoolId()); + if (CollectionUtils.isEmpty(poolStorageAccessGroupsList)) { + logger.debug("Storage pool {} has no storage access groups, skipping validation for volume {}", + 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) + )); + } + 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())); } } @@ -7374,7 +7398,7 @@ private DeployDestination checkVmMigrationDestination(VMInstanceVO vm, Host srcH HostVO destinationHostVO = _hostDao.findById(destinationHost.getId()); _hostDao.loadHostTags(destinationHostVO); validateStrictHostTagCheck(vm, destinationHostVO); - validateStorageAccessGroupsOnHosts(srcHost, destinationHost); + validateStorageAccessGroupsOnHosts(vm, destinationHost); checkHostsDedication(vm, srcHost.getId(), destinationHost.getId()); @@ -7735,7 +7759,7 @@ private Pair getHostsForMigrateVmWithStorage(VMInstanceVO vm, Host d 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 f752dc31f181..8644ebf51cda 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -176,6 +176,7 @@ 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.SnapshotPolicyDao; import com.cloud.storage.dao.VMTemplateDao; import com.cloud.storage.dao.VolumeDao; @@ -422,6 +423,8 @@ public class UserVmManagerImplTest { @Mock StorageManager storageManager; + @Mock + private StoragePoolAndAccessGroupMapDao storagePoolAndAccessGroupMapDao; @Mock private VolumeDataFactory volumeDataFactory; @@ -3196,95 +3199,198 @@ public void executeStepsToChangeOwnershipOfVmTestResourceCountRunningVmsOnlyEnab } @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 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(volume2.getPoolId()).thenReturn(200L); + Mockito.when(volume2.getName()).thenReturn("volume2"); + Mockito.when(volume2.getVolumeType()).thenReturn(Volume.Type.DATADISK); - 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 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(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 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(volume1.getPoolId()).thenReturn(100L); + Mockito.when(volume1.getName()).thenReturn("volume1"); + + 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 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(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 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); - 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 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(volume2.getPoolId()).thenReturn(200L); + Mockito.when(volume2.getName()).thenReturn("volume2"); + Mockito.when(volume2.getVolumeType()).thenReturn(Volume.Type.DATADISK); - 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;