Skip to content

domain dedication #13654

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

Open
DaanHoogland wants to merge 9 commits into
base: main
Choose a base branch
from
+363 −1
Open

domain dedication #13654

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b26ad39
Initial plan
Copilot Jul 2, 2026
b43afc0
Fix domain-dedicated resource eligibility in ImplicitDedicationPlanne…
Copilot Jul 2, 2026
630c861
Refactor findAvoidSetForNonExplicitUserVM: remove pagination limit an…
Copilot Jul 2, 2026
b6eda5f
Address code review: document filter purpose and clarify test mock in…
Copilot Jul 2, 2026
1d81b04
Apply suggestions from code review
DaanHoogland Jul 6, 2026
52ee410
Apply suggestions from code review
DaanHoogland Jul 6, 2026
70a8698
Fix NPE in ImplicitDedicationPlanner unit tests (issue #5803)
Jul 20, 2026
64c3351
Apply PR #13529 review hardening: type-specific check + early return
Jul 20, 2026
3281a55
minor improvements
Jul 30, 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: 49 additions & 1 deletion server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -963,8 +963,56 @@ private void findAvoiSetForRouterVM(ExcludeList avoids, VirtualMachine vm, List<
}

private void findAvoidSetForNonExplicitUserVM(ExcludeList avoids, VirtualMachine vm, List<Long> allPodsInDc, List<Long> allClustersInDc, List<Long> allHostsInDc) {
if (allPodsInDc.isEmpty() && allClustersInDc.isEmpty() && allHostsInDc.isEmpty()) {
// Nothing in this DC is dedicated to anyone, so there is nothing to avoid or to check dedication for.
return;
}

long vmAccountId = vm.getAccountId();
long vmDomainId = vm.getDomainId();

List<Long> allPodsFromDedicatedID = new ArrayList<>();
List<Long> allClustersFromDedicatedID = new ArrayList<>();
List<Long> allHostsFromDedicatedID = new ArrayList<>();

// If the VM owner's domain has an ExplicitDedication domain-level affinity group,
// resources dedicated to that domain are accessible to the VM owner (fixes issue #5803).
boolean hasDomainExplicitDedicationGroup =
_affinityGroupDao.findDomainLevelGroupByType(vmDomainId, "ExplicitDedication") != null;

Comment thread
DaanHoogland marked this conversation as resolved.
// Sort by id ascending, no pagination — retrieve all dedicated resources for this domain/account
Filter filter = new Filter(DedicatedResourceVO.class, "id", true);

// Always allow resources dedicated to the VM owner's account.
for (DedicatedResourceVO vo : _dedicatedDao.searchDedicatedPods(null, vmDomainId, vmAccountId, null, filter).first()) {
allPodsFromDedicatedID.add(vo.getPodId());
}
for (DedicatedResourceVO vo : _dedicatedDao.searchDedicatedClusters(null, vmDomainId, vmAccountId, null, filter).first()) {
allClustersFromDedicatedID.add(vo.getClusterId());
}
for (DedicatedResourceVO vo : _dedicatedDao.searchDedicatedHosts(null, vmDomainId, vmAccountId, null, filter).first()) {
allHostsFromDedicatedID.add(vo.getHostId());
}

if (hasDomainExplicitDedicationGroup) {
// Domain has explicit dedication affinity groups: also allow resources dedicated to this domain
for (DedicatedResourceVO vo : _dedicatedDao.searchDedicatedPods(null, vmDomainId, null, null, filter).first()) {
allPodsFromDedicatedID.add(vo.getPodId());
}
for (DedicatedResourceVO vo : _dedicatedDao.searchDedicatedClusters(null, vmDomainId, null, null, filter).first()) {
allClustersFromDedicatedID.add(vo.getClusterId());
}
for (DedicatedResourceVO vo : _dedicatedDao.searchDedicatedHosts(null, vmDomainId, null, null, filter).first()) {
allHostsFromDedicatedID.add(vo.getHostId());
}
}

allPodsInDc.removeAll(allPodsFromDedicatedID);
allClustersInDc.removeAll(allClustersFromDedicatedID);
allHostsInDc.removeAll(allHostsFromDedicatedID);

logger.debug(() -> LogUtils.logGsonWithoutException("Adding pods [%s], clusters [%s] and hosts [%s] to the avoid list in the deploy process of user VM [%s], "
+ "because this VM is not explicitly dedicated to these components.", allPodsInDc, allClustersInDc, allHostsInDc, vm));
+ "because this VM is not dedicated to these components.", allPodsInDc, allClustersInDc, allHostsInDc, vm));
avoids.addPodList(allPodsInDc);
avoids.addClusterList(allClustersInDc);
avoids.addHostList(allHostsInDc);
Expand Down
210 changes: 210 additions & 0 deletions server/src/test/java/com/cloud/deploy/DeploymentPlanningManagerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.cloud.dc.ClusterVO;
import com.cloud.dc.DataCenter;
import com.cloud.dc.DataCenterVO;
import com.cloud.dc.DedicatedResourceVO;
import com.cloud.dc.HostPodVO;
import com.cloud.dc.dao.ClusterDao;
import com.cloud.dc.dao.DataCenterDao;
Expand Down Expand Up @@ -84,6 +85,7 @@
import com.cloud.vm.dao.VMInstanceDao;
import org.apache.cloudstack.affinity.AffinityGroupProcessor;
import org.apache.cloudstack.affinity.AffinityGroupService;
import org.apache.cloudstack.affinity.AffinityGroupVO;
import org.apache.cloudstack.affinity.dao.AffinityGroupDao;
import org.apache.cloudstack.affinity.dao.AffinityGroupDomainMapDao;
import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao;
Expand Down Expand Up @@ -227,6 +229,12 @@ public class DeploymentPlanningManagerImplTest {
@Inject
HostPodDao _podDao;

@Inject
HostDao _hostDao;

@Mock
AffinityGroupDao _affinityGroupDao;
Comment thread
DaanHoogland marked this conversation as resolved.
Comment thread
DaanHoogland marked this conversation as resolved.

private static final long dataCenterId = 1L;
private static final long instanceId = 123L;
private static final long hostId = 0L;
Expand Down Expand Up @@ -937,6 +945,208 @@ private void assertAvoidIsEmpty(ExcludeList avoids, boolean isDcEmpty, boolean i
Assert.assertEquals(isHostsEmpty, CollectionUtils.isEmpty(avoids.getHostsToAvoid()));
}

/**
* Helper: set up mocks for checkForNonDedicatedResources tests.
* Sets up a non-root, non-explicit user VM with the given domain and account.
*/
private VMInstanceVO setupNonExplicitUserVmMocks(long vmDomainId, long vmAccountId, DataCenter mockDc) {
// Zone is not dedicated
Mockito.when(_dedicatedDao.findByZoneId(Mockito.anyLong())).thenReturn(null);
// Not root admin
Mockito.when(_accountMgr.isRootAdmin(Mockito.anyLong())).thenReturn(false);
// No explicit dedication affinity group for the VM
Mockito.when(_affinityGroupVMMapDao.findByVmIdType(Mockito.anyLong(), Mockito.eq("ExplicitDedication")))
.thenReturn(new ArrayList<>());

VMInstanceVO mockVm = Mockito.mock(VMInstanceVO.class);
Mockito.when(mockVm.getType()).thenReturn(VirtualMachine.Type.User);
Mockito.when(mockVm.getDomainId()).thenReturn(vmDomainId);
Mockito.when(mockVm.getAccountId()).thenReturn(vmAccountId);
Mockito.when(mockVm.getId()).thenReturn(instanceId);

// No dedicated clusters or hosts in DC
Mockito.when(_clusterDao.listAllClusterIds(Mockito.anyLong())).thenReturn(new ArrayList<>());
Mockito.when(_dedicatedDao.listAllClusters()).thenReturn(new ArrayList<>());
Mockito.when(_hostDao.listAllHosts(Mockito.anyLong())).thenReturn(new ArrayList<>());
Mockito.when(_dedicatedDao.listAllHosts()).thenReturn(new ArrayList<>());
Mockito.when(_dedicatedDao.searchDedicatedPods(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()))
.thenReturn(new Pair<>(new ArrayList<>(), 0));
Mockito.when(_dedicatedDao.searchDedicatedClusters(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()))
.thenReturn(new Pair<>(new ArrayList<>(), 0));
Mockito.when(_dedicatedDao.searchDedicatedHosts(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()))
.thenReturn(new Pair<>(new ArrayList<>(), 0));

return mockVm;
}

/**
* Issue #5803: A regular user in the same domain as a domain-dedicated pod
* must be allowed to deploy on that pod (i.e., the pod must NOT appear in the avoid list).
*/
@Test
public void checkForNonDedicatedResources_domainDedicatedPod_sameDomainUser_podNotInAvoidList() {
long vmDomainId = 10L;
long vmAccountId = 200L;
long dedicatedPodId = 42L;

DataCenter mockDc = Mockito.mock(DataCenter.class);
Mockito.when(mockDc.getId()).thenReturn(dataCenterId);

VMInstanceVO mockVm = setupNonExplicitUserVmMocks(vmDomainId, vmAccountId, mockDc);

// Pod in DC and it is dedicated
List<Long> podsInDc = new ArrayList<>(Arrays.asList(dedicatedPodId));
Mockito.when(_podDao.listAllPods(dataCenterId)).thenReturn(podsInDc);
Mockito.when(_dedicatedDao.listAllPods()).thenReturn(new ArrayList<>(Arrays.asList(dedicatedPodId)));

// Domain has an ExplicitDedication domain-level affinity group (pod dedicated to this domain).
Mockito.when(_affinityGroupDao.findDomainLevelGroupByType(vmDomainId, "ExplicitDedication"))
.thenReturn(Mockito.mock(AffinityGroupVO.class));

// The pod is found when searching by domain (accountId = null)
DedicatedResourceVO dedicatedPod = Mockito.mock(DedicatedResourceVO.class);
Mockito.when(dedicatedPod.getPodId()).thenReturn(dedicatedPodId);
Mockito.when(_dedicatedDao.searchDedicatedPods(Mockito.isNull(), Mockito.eq(vmDomainId),
Mockito.isNull(), Mockito.isNull(), Mockito.any(com.cloud.utils.db.Filter.class)))
.thenReturn(new Pair<>(new ArrayList<>(Arrays.asList(dedicatedPod)), 1));

VirtualMachineProfileImpl mockProfile = Mockito.mock(VirtualMachineProfileImpl.class);
Mockito.when(mockProfile.getVirtualMachine()).thenReturn(mockVm);
Mockito.when(mockProfile.getOwner()).thenReturn(Mockito.mock(com.cloud.user.Account.class));

ExcludeList avoids = new ExcludeList();
_dpm.checkForNonDedicatedResources(mockProfile, mockDc, avoids);

// The dedicated pod belongs to the same domain as the VM owner: must NOT be in avoid list
assertTrue("Domain-dedicated pod should not be in avoid list for same-domain user",
CollectionUtils.isEmpty(avoids.getPodsToAvoid()) || !avoids.getPodsToAvoid().contains(dedicatedPodId));
}

/**
* A user from a different domain must NOT be allowed to deploy on a pod dedicated to another domain.
*/
@Test
public void checkForNonDedicatedResources_domainDedicatedPod_differentDomainUser_podInAvoidList() {
long podDomainId = 10L; // domain the pod is dedicated to
long vmDomainId = 20L; // VM owner belongs to a DIFFERENT domain
long vmAccountId = 300L;
long dedicatedPodId = 43L;

DataCenter mockDc = Mockito.mock(DataCenter.class);
Mockito.when(mockDc.getId()).thenReturn(dataCenterId);

VMInstanceVO mockVm = setupNonExplicitUserVmMocks(vmDomainId, vmAccountId, mockDc);

// Pod in DC and it is dedicated (to podDomainId, NOT to vmDomainId)
List<Long> podsInDc = new ArrayList<>(Arrays.asList(dedicatedPodId));
Mockito.when(_podDao.listAllPods(dataCenterId)).thenReturn(podsInDc);
Mockito.when(_dedicatedDao.listAllPods()).thenReturn(new ArrayList<>(Arrays.asList(dedicatedPodId)));

// VM owner's domain has no ExplicitDedication domain-level affinity group
Mockito.when(_affinityGroupDao.findDomainLevelGroupByType(vmDomainId, "ExplicitDedication"))
.thenReturn(null);

// No pods dedicated to the VM owner's account
Mockito.when(_dedicatedDao.searchDedicatedPods(Mockito.isNull(), Mockito.eq(vmDomainId),
Mockito.eq(vmAccountId), Mockito.isNull(), Mockito.any(com.cloud.utils.db.Filter.class)))
.thenReturn(new Pair<>(new ArrayList<>(), 0));

VirtualMachineProfileImpl mockProfile = Mockito.mock(VirtualMachineProfileImpl.class);
Mockito.when(mockProfile.getVirtualMachine()).thenReturn(mockVm);
Mockito.when(mockProfile.getOwner()).thenReturn(Mockito.mock(com.cloud.user.Account.class));

ExcludeList avoids = new ExcludeList();
_dpm.checkForNonDedicatedResources(mockProfile, mockDc, avoids);

// The pod is dedicated to a different domain: must be in avoid list
assertTrue("Domain-dedicated pod should be in avoid list for different-domain user",
avoids.getPodsToAvoid() != null && avoids.getPodsToAvoid().contains(dedicatedPodId));
}

/**
* An account-dedicated pod should be accessible to the owning account
* (no domain-level affinity groups, but account-level dedicated resource exists).
*/
@Test
public void checkForNonDedicatedResources_accountDedicatedPod_sameAccount_podNotInAvoidList() {
long vmDomainId = 10L;
long vmAccountId = 200L;
long dedicatedPodId = 44L;

DataCenter mockDc = Mockito.mock(DataCenter.class);
Mockito.when(mockDc.getId()).thenReturn(dataCenterId);

VMInstanceVO mockVm = setupNonExplicitUserVmMocks(vmDomainId, vmAccountId, mockDc);

// Pod in DC and it is dedicated
List<Long> podsInDc = new ArrayList<>(Arrays.asList(dedicatedPodId));
Mockito.when(_podDao.listAllPods(dataCenterId)).thenReturn(podsInDc);
Mockito.when(_dedicatedDao.listAllPods()).thenReturn(new ArrayList<>(Arrays.asList(dedicatedPodId)));

// Domain has no ExplicitDedication domain-level affinity group (account-level dedication, not domain-level)
Mockito.when(_affinityGroupDao.findDomainLevelGroupByType(vmDomainId, "ExplicitDedication"))
.thenReturn(null);

// The pod IS found when searching by account
DedicatedResourceVO dedicatedPod = Mockito.mock(DedicatedResourceVO.class);
Mockito.when(dedicatedPod.getPodId()).thenReturn(dedicatedPodId);
Mockito.when(_dedicatedDao.searchDedicatedPods(Mockito.isNull(), Mockito.eq(vmDomainId),
Mockito.eq(vmAccountId), Mockito.isNull(), Mockito.any(com.cloud.utils.db.Filter.class)))
.thenReturn(new Pair<>(new ArrayList<>(Arrays.asList(dedicatedPod)), 1));

VirtualMachineProfileImpl mockProfile = Mockito.mock(VirtualMachineProfileImpl.class);
Mockito.when(mockProfile.getVirtualMachine()).thenReturn(mockVm);
Mockito.when(mockProfile.getOwner()).thenReturn(Mockito.mock(com.cloud.user.Account.class));

ExcludeList avoids = new ExcludeList();
_dpm.checkForNonDedicatedResources(mockProfile, mockDc, avoids);

// The pod is dedicated to this account: must NOT be in avoid list
assertTrue("Account-dedicated pod should not be in avoid list for same-account user",
CollectionUtils.isEmpty(avoids.getPodsToAvoid()) || !avoids.getPodsToAvoid().contains(dedicatedPodId));
}

/**
* A user whose account is different from the account a pod is dedicated to
* must NOT be able to deploy on that pod.
*/
@Test
public void checkForNonDedicatedResources_accountDedicatedPod_differentAccount_podInAvoidList() {
long vmDomainId = 10L;
long vmAccountId = 300L; // different account
long dedicatedPodId = 45L;

DataCenter mockDc = Mockito.mock(DataCenter.class);
Mockito.when(mockDc.getId()).thenReturn(dataCenterId);

VMInstanceVO mockVm = setupNonExplicitUserVmMocks(vmDomainId, vmAccountId, mockDc);

// Pod in DC and it is dedicated (to a different account)
List<Long> podsInDc = new ArrayList<>(Arrays.asList(dedicatedPodId));
Mockito.when(_podDao.listAllPods(dataCenterId)).thenReturn(podsInDc);
Mockito.when(_dedicatedDao.listAllPods()).thenReturn(new ArrayList<>(Arrays.asList(dedicatedPodId)));

// Domain has no ExplicitDedication domain-level affinity group
Mockito.when(_affinityGroupDao.findDomainLevelGroupByType(vmDomainId, "ExplicitDedication"))
.thenReturn(null);

// No pods dedicated to this account
Mockito.when(_dedicatedDao.searchDedicatedPods(Mockito.isNull(), Mockito.eq(vmDomainId),
Mockito.eq(vmAccountId), Mockito.isNull(), Mockito.any(com.cloud.utils.db.Filter.class)))
.thenReturn(new Pair<>(new ArrayList<>(), 0));

VirtualMachineProfileImpl mockProfile = Mockito.mock(VirtualMachineProfileImpl.class);
Mockito.when(mockProfile.getVirtualMachine()).thenReturn(mockVm);
Mockito.when(mockProfile.getOwner()).thenReturn(Mockito.mock(com.cloud.user.Account.class));

ExcludeList avoids = new ExcludeList();
_dpm.checkForNonDedicatedResources(mockProfile, mockDc, avoids);

// The pod is dedicated to a different account: must be in avoid list
assertTrue("Account-dedicated pod should be in avoid list for different-account user",
avoids.getPodsToAvoid() != null && avoids.getPodsToAvoid().contains(dedicatedPodId));
}

@Configuration
@ComponentScan(basePackageClasses = {DeploymentPlanningManagerImpl.class},
includeFilters = {@Filter(value = TestConfiguration.Library.class,
Expand Down
Loading