Skip to content

Fix stg access group validation #13562

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Pearl1594 wants to merge 5 commits into
base: 4.22
Choose a base branch
from
+202 −72
Draft

Fix stg access group validation #13562

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9670445
VM Migration: Storage access groups validation issue fix
sachindoddaguni Nov 12, 2025
935dcc3
update logger
Jul 7, 2026
f274f33
fix logger
Jul 7, 2026
f14f9dd
Merge branch '4.22' of https://github.com/apache/cloudstack into fix-…
Jul 7, 2026
35f8ee7
remove unnecessary stubbings
Jul 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 37 additions & 13 deletions server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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<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)) {
logger.debug("No volumes found for VM {}, 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.");
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<String> 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()));
}
}

Expand Down Expand Up @@ -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());

Expand Down Expand Up @@ -7735,7 +7759,7 @@ private Pair<Host, Host> getHostsForMigrateVmWithStorage(VMInstanceVO vm, Host d
destinationHost.getName(), destinationHost.getUuid()));
}

validateStorageAccessGroupsOnHosts(srcHost, destinationHost);
validateStorageAccessGroupsOnHosts(vm, destinationHost);

return new Pair<>(srcHost, destinationHost);
}
Expand Down
224 changes: 165 additions & 59 deletions server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -422,6 +423,8 @@ public class UserVmManagerImplTest {

@Mock
StorageManager storageManager;
@Mock
private StoragePoolAndAccessGroupMapDao storagePoolAndAccessGroupMapDao;

@Mock
private VolumeDataFactory volumeDataFactory;
Expand Down Expand Up @@ -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<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(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<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(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(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<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(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);

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(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;
Expand Down
Loading