Skip to content

Allow deploy of VRs on dedicated resources #9531

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
GaOrtiga wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Allow deploy of VRs on dedicated resources #9531

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9e19ab3
allow deploy of VRs on dedicated resources
GaOrtiga Aug 6, 2024
42c5625
Move configuration to Planner hierarchy
Feb 5, 2025
e2197f0
allow deploy when resource is dedicated to account
GaOrtiga Apr 20, 2026
8b20abe
fix tests
GaOrtiga May 25, 2026
e35c299
Update engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDao…
GaOrtiga Jun 11, 2026
1bdb771
Update engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDao…
GaOrtiga Jun 11, 2026
65524e3
remove_config
GaOrtiga Jun 22, 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
3 changes: 2 additions & 1 deletion engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDao.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.cloud.utils.db.GenericDao;

public interface CapacityDao extends GenericDao<CapacityVO, Long> {

CapacityVO findByHostIdType(Long hostId, short capacityType);

List<CapacityVO> listByHostIdTypes(Long hostId, List<Short> capacityTypes);
Expand All @@ -40,7 +41,7 @@ public interface CapacityDao extends GenericDao<CapacityVO, Long> {

List<SummedCapacity> findNonSharedStorageForClusterPodZone(Long zoneId, Long podId, Long clusterId);

Pair<List<Long>, Map<Long, Double>> orderClustersByAggregateCapacity(long id, long vmId, short capacityType, boolean isZone);
Pair<List<Long>, Map<Long, Double>> orderClustersByAggregateCapacity(long id, long vmId, Long ownerId, short capacityType, boolean isVr, boolean isZone);

Ternary<Long, Long, Long> findCapacityByZoneAndHostTag(Long zoneId, String hostTag);

Expand Down
17 changes: 14 additions & 3 deletions engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDaoImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,13 @@ public class CapacityDaoImpl extends GenericDaoBase<CapacityVO, Long> implements
"LEFT JOIN dedicated_resources dr_cluster ON dr_cluster.cluster_id IS NOT NULL AND dr_cluster.cluster_id = host.cluster_id " +
"LEFT JOIN dedicated_resources dr_host ON dr_host.host_id IS NOT NULL AND dr_host.host_id = host.id ";

private static final String ORDER_CLUSTERS_BY_AGGREGATE_CAPACITY_INCLUDE_DEDICATED_JOIN_CLAUSE =
"JOIN host ON capacity.host_id = host.id " +
"LEFT JOIN (SELECT affinity_group.id, agvm.instance_id FROM affinity_group_vm_map agvm JOIN affinity_group ON agvm.affinity_group_id = affinity_group.id AND affinity_group.type='ExplicitDedication') AS ag ON ag.instance_id = ? " +
"LEFT JOIN dedicated_resources dr_pod ON dr_pod.pod_id IS NOT NULL AND dr_pod.pod_id = host.pod_id AND dr_pod.account_id IS NOT NULL AND dr_pod.account_id != ownerId " +

@weizhouapache weizhouapache Jun 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about resources dedicated to domain ?

@GaOrtiga GaOrtiga Jun 22, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dedication to domains is already being addressed in this query

@weizhouapache weizhouapache Jun 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh really ?

I couldn't even find a domain or domain_id field in the SQL query...

@GaOrtiga GaOrtiga Jun 22, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll run a test, been a while since I altered this query, so I might be confusing

"LEFT JOIN dedicated_resources dr_cluster ON dr_cluster.cluster_id IS NOT NULL AND dr_cluster.cluster_id = host.cluster_id AND dr_cluster.account_id IS NOT NULL AND dr_cluster.account_id != ownerId " +
"LEFT JOIN dedicated_resources dr_host ON dr_host.host_id IS NOT NULL AND dr_host.host_id = host.id AND dr_host.account_id IS NOT NULL AND dr_host.account_id != ownerId ";

private static final String ORDER_CLUSTERS_BY_AGGREGATE_CAPACITY_JOIN_2 =
" AND ((ag.id IS NULL AND dr_pod.pod_id IS NULL AND dr_cluster.cluster_id IS NULL AND dr_host.host_id IS NULL) OR " +
"(dr_pod.affinity_group_id = ag.id OR dr_cluster.affinity_group_id = ag.id OR dr_host.affinity_group_id = ag.id))";
Expand Down Expand Up @@ -986,7 +993,7 @@ public boolean removeBy(Short capacityType, Long zoneId, Long podId, Long cluste
}

@Override
public Pair<List<Long>, Map<Long, Double>> orderClustersByAggregateCapacity(long id, long vmId, short capacityTypeForOrdering, boolean isZone) {
public Pair<List<Long>, Map<Long, Double>> orderClustersByAggregateCapacity(long id, long vmId, Long ownerId, short capacityTypeForOrdering, boolean isVr, boolean isZone) {
TransactionLegacy txn = TransactionLegacy.currentTxn();
PreparedStatement pstmt = null;
List<Long> result = new ArrayList<Long>();
Expand All @@ -998,7 +1005,12 @@ public Pair<List<Long>, Map<Long, Double>> orderClustersByAggregateCapacity(long
sql.append(ORDER_CLUSTERS_BY_AGGREGATE_OVERCOMMIT_CAPACITY_PART1);
}

sql.append(ORDER_CLUSTERS_BY_AGGREGATE_CAPACITY_JOIN_1);
if (isVr) {
sql.append(ORDER_CLUSTERS_BY_AGGREGATE_CAPACITY_INCLUDE_DEDICATED_JOIN_CLAUSE.replace("ownerId", ownerId.toString()));

@weizhouapache weizhouapache Jun 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not elegant I think

@DaanHoogland DaanHoogland Jun 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is very elegant, like a pig on clogs . What do you suggest, @weizhouapache ?

@weizhouapache weizhouapache Jun 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can refer to

    private static final String ORDER_CLUSTERS_BY_AGGREGATE_CAPACITY_JOIN_1 =
            "JOIN host ON capacity.host_id = host.id " +
                    "LEFT JOIN (SELECT affinity_group.id, agvm.instance_id FROM affinity_group_vm_map agvm JOIN affinity_group ON agvm.affinity_group_id = affinity_group.id AND affinity_group.type='ExplicitDedication') AS ag ON ag.instance_id = ? " +
                    "LEFT JOIN dedicated_resources dr_pod ON dr_pod.pod_id IS NOT NULL AND dr_pod.pod_id = host.pod_id " +
                    "LEFT JOIN dedicated_resources dr_cluster ON dr_cluster.cluster_id IS NOT NULL AND dr_cluster.cluster_id = host.cluster_id " +
                    "LEFT JOIN dedicated_resources dr_host ON dr_host.host_id IS NOT NULL AND dr_host.host_id = host.id ";

@GaOrtiga GaOrtiga Jun 22, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@weizhouapache @DaanHoogland

Using ? was my first option aswell, since it's how it's done elsewhere in the method, however, the values for ? are injected informing the order (how many ?s before it). Since this one is optional, it may or may not change the order of the remaining ? that come after it, thus, I'd have to add at least 3 more ifs to account for this possible change of order, which IMO ends up being even less elegant.

@weizhouapache weizhouapache Jun 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's ok, thanks for explanation.

Maybe we could use a placeholder syntax such as {{ ownerId }} to make it clear that this is a variable that will be replaced at runtime.

@GaOrtiga GaOrtiga Jun 22, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good idea.

} else {
sql.append(ORDER_CLUSTERS_BY_AGGREGATE_CAPACITY_JOIN_1);
}

if (isZone) {
sql.append("WHERE capacity.capacity_state = 'Enabled' AND capacity.data_center_id = ?");
} else {
Expand Down Expand Up @@ -1290,5 +1302,4 @@ public float findClusterConsumption(Long clusterId, short capacityType, long com
}
return 0;
}

}
2 changes: 1 addition & 1 deletion engine/schema/src/test/java/com/cloud/capacity/dao/CapacityDaoImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ public void testOrderClustersByAggregateCapacityEmptyResult() throws Exception {
when(pstmt.executeQuery()).thenReturn(resultSet);
when(resultSet.next()).thenReturn(false);

Pair<List<Long>, Map<Long, Double>> result = capacityDao.orderClustersByAggregateCapacity(1L, 1L, (short) 1, true);
Pair<List<Long>, Map<Long, Double>> result = capacityDao.orderClustersByAggregateCapacity(1L, 1L, 1L, (short) 1, false, true);
assertNotNull(result);
assertTrue(result.first().isEmpty());
assertTrue(result.second().isEmpty());
Expand Down
2 changes: 1 addition & 1 deletion ...t-dedication/src/test/java/org/apache/cloudstack/implicitplanner/ImplicitPlannerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ private void initializeForTest(VirtualMachineProfileImpl vmProfile, DataCenterDe
clusterCapacityMap.put(2L, 2048D);
clusterCapacityMap.put(3L, 2048D);
Pair<List<Long>, Map<Long, Double>> clustersOrderedByCapacity = new Pair<List<Long>, Map<Long, Double>>(clustersWithEnoughCapacity, clusterCapacityMap);
when(capacityDao.orderClustersByAggregateCapacity(dataCenterId, 12L, Capacity.CAPACITY_TYPE_CPU, true)).thenReturn(clustersOrderedByCapacity);
when(capacityDao.orderClustersByAggregateCapacity(dataCenterId, 12L, 0L, Capacity.CAPACITY_TYPE_CPU, false, true)).thenReturn(clustersOrderedByCapacity);

List<Long> disabledClusters = new ArrayList<Long>();
List<Long> clustersWithDisabledPods = new ArrayList<Long>();
Expand Down
12 changes: 7 additions & 5 deletions server/src/main/java/com/cloud/deploy/FirstFitPlanner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,9 +398,11 @@ private List<Long> scanClustersForDestinationInZoneOrPod(long id, boolean isZone
DataCenter dc = dcDao.findById(vm.getDataCenterId());
int requiredCpu = offering.getCpu() * offering.getSpeed();
long requiredRam = offering.getRamSize() * 1024L * 1024L;
boolean isVr = VirtualMachine.Type.DomainRouter.equals(vmProfile.getType());
Long ownerId = vm.getAccountId();

//list clusters under this zone by cpu and ram capacity
Pair<List<Long>, Map<Long, Double>> clusterCapacityInfo = listClustersByCapacity(id, vmProfile.getId(), requiredCpu, requiredRam, avoid, isZone);
Pair<List<Long>, Map<Long, Double>> clusterCapacityInfo = listClustersByCapacity(id, vmProfile.getId(), ownerId, requiredCpu, requiredRam, isVr, isZone);
List<Long> prioritizedClusterIds = clusterCapacityInfo.first();
if (!prioritizedClusterIds.isEmpty()) {
if (avoid.getClustersToAvoid() != null) {
Expand Down Expand Up @@ -458,7 +460,7 @@ protected List<Long> reorderPods(Pair<List<Long>, Map<Long, Double>> podCapacity
return podIdsByCapacity;
}

protected Pair<List<Long>, Map<Long, Double>> listClustersByCapacity(long id, long vmId, int requiredCpu, long requiredRam, ExcludeList avoid, boolean isZone) {
protected Pair<List<Long>, Map<Long, Double>> listClustersByCapacity(long id, long vmId, Long ownerId, int requiredCpu, long requiredRam, boolean isVr, boolean isZone) {
//look at the aggregate available cpu and ram per cluster
//although an aggregate value may be false indicator that a cluster can host a vm, it will at the least eliminate those clusters which definitely cannot

Expand All @@ -474,7 +476,7 @@ protected Pair<List<Long>, Map<Long, Double>> listClustersByCapacity(long id, lo
}


Pair<List<Long>, Map<Long, Double>> result = getOrderedClustersByCapacity(id, vmId, isZone);
Pair<List<Long>, Map<Long, Double>> result = getOrderedClustersByCapacity(id, vmId, ownerId, isVr, isZone);
List<Long> clusterIdsOrderedByAggregateCapacity = result.first();
//only keep the clusters that have enough capacity to host this VM
if (logger.isTraceEnabled()) {
Expand Down Expand Up @@ -554,14 +556,14 @@ public Map<Long, Double> getPodByCombinedCapacities(List<CapacityVO> capacities,
}


private Pair<List<Long>, Map<Long, Double>> getOrderedClustersByCapacity(long id, long vmId, boolean isZone) {
private Pair<List<Long>, Map<Long, Double>> getOrderedClustersByCapacity(long id, long vmId, Long ownerId, boolean isVr, boolean isZone) {
double cpuToMemoryWeight = ConfigurationManager.HostCapacityTypeCpuMemoryWeight.value();
short capacityType = getHostCapacityTypeToOrderCluster(
configDao.getValue(Config.HostCapacityTypeToOrderClusters.key()), cpuToMemoryWeight);

logger.debug("CapacityType: {} is used for Cluster ordering", getCapacityTypeName(capacityType));
if (capacityType >= 0) { // for capacityType other than COMBINED
return capacityDao.orderClustersByAggregateCapacity(id, vmId, capacityType, isZone);
return capacityDao.orderClustersByAggregateCapacity(id, vmId, ownerId, capacityType, isVr, isZone);
}

Long zoneId = isZone ? id : null;
Expand Down
2 changes: 1 addition & 1 deletion server/src/test/java/com/cloud/vm/FirstFitPlannerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ private void initializeForTest(VirtualMachineProfileImpl vmProfile, DataCenterDe
clusterCapacityMap.put(6L, 2048D);

Pair<List<Long>, Map<Long, Double>> clustersOrderedByCapacity = new Pair<List<Long>, Map<Long, Double>>(clustersWithEnoughCapacity, clusterCapacityMap);
when(capacityDao.orderClustersByAggregateCapacity(dataCenterId, 12L, Capacity.CAPACITY_TYPE_CPU, true)).thenReturn(clustersOrderedByCapacity);
when(capacityDao.orderClustersByAggregateCapacity(dataCenterId, 12L, 0L, Capacity.CAPACITY_TYPE_CPU, false, true)).thenReturn(clustersOrderedByCapacity);

List<Long> disabledClusters = new ArrayList<Long>();
List<Long> clustersWithDisabledPods = new ArrayList<Long>();
Expand Down
Loading