diff --git a/api/src/main/java/com/cloud/server/ManagementService.java b/api/src/main/java/com/cloud/server/ManagementService.java index d6d7f7a59c68..2670b657df13 100644 --- a/api/src/main/java/com/cloud/server/ManagementService.java +++ b/api/src/main/java/com/cloud/server/ManagementService.java @@ -71,6 +71,8 @@ import com.cloud.capacity.Capacity; import com.cloud.dc.Pod; import com.cloud.dc.Vlan; +import com.cloud.deploy.DeploymentPlan; +import com.cloud.deploy.DeploymentPlanner.ExcludeList; import com.cloud.exception.ConcurrentOperationException; import com.cloud.exception.ManagementServerException; import com.cloud.exception.ResourceUnavailableException; @@ -91,6 +93,7 @@ import com.cloud.vm.InstanceGroup; import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachine.Type; +import com.cloud.vm.VirtualMachineProfile; /** * Hopefully this is temporary. @@ -452,6 +455,19 @@ public interface ManagementService { Ternary, Integer>, List, Map> listHostsForMigrationOfVM(VirtualMachine vm, Long startIndex, Long pageSize, String keyword, List vmList); + /** + * Apply affinity group constraints and other exclusion rules for VM migration. + * This is a helper method that can be used independently for per-iteration affinity checks in DRS. + * + * @param vm The virtual machine to migrate + * @param vmProfile The VM profile + * @param plan The deployment plan + * @param vmList List of VMs with current/simulated placements for affinity processing + * @return ExcludeList containing hosts to avoid + */ + ExcludeList applyAffinityConstraints(VirtualMachine vm, VirtualMachineProfile vmProfile, + DeploymentPlan plan, List vmList); + /** * List storage pools for live migrating of a volume. The API returns list of all pools in the cluster to which the * volume can be migrated. Current pool is not included in the list. In case of vSphere datastore cluster storage pools, diff --git a/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java b/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java index 665f95842b0d..368487c2b9b0 100644 --- a/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java +++ b/api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java @@ -22,10 +22,10 @@ import com.cloud.host.Host; import com.cloud.offering.ServiceOffering; import com.cloud.org.Cluster; -import com.cloud.utils.Pair; import com.cloud.utils.Ternary; import com.cloud.utils.component.Adapter; import com.cloud.vm.VirtualMachine; +import org.apache.commons.collections.CollectionUtils; import org.apache.commons.math3.stat.descriptive.moment.Mean; import org.apache.commons.math3.stat.descriptive.moment.StandardDeviation; @@ -40,6 +40,9 @@ public interface ClusterDrsAlgorithm extends Adapter { + Mean MEAN_CALCULATOR = new Mean(); + StandardDeviation STDDEV_CALCULATOR = new StandardDeviation(false); + /** * Determines whether a DRS operation is needed for a given cluster and host-VM * mapping. @@ -59,79 +62,121 @@ public interface ClusterDrsAlgorithm extends Adapter { boolean needsDrs(Cluster cluster, List> cpuList, List> memoryList) throws ConfigurationException; - /** - * Determines the metrics for a given virtual machine and destination host in a DRS cluster. - * - * @param clusterId - * the ID of the cluster to check - * @param vm - * the virtual machine to check - * @param serviceOffering - * the service offering for the virtual machine - * @param destHost - * the destination host for the virtual machine - * @param hostCpuMap - * a map of host IDs to the Ternary of used, reserved and total CPU on each host - * @param hostMemoryMap - * a map of host IDs to the Ternary of used, reserved and total memory on each host - * @param requiresStorageMotion - * whether storage motion is required for the virtual machine + * Calculates the metrics (improvement, cost, benefit) for migrating a VM to a destination host. Improvement is + * calculated based on the change in cluster imbalance before and after the migration. * + * @param cluster the cluster to check + * @param vm the virtual machine to check + * @param serviceOffering the service offering for the virtual machine + * @param destHost the destination host for the virtual machine + * @param hostCpuMap a map of host IDs to the Ternary of used, reserved and total CPU on each host + * @param hostMemoryMap a map of host IDs to the Ternary of used, reserved and total memory on each host + * @param requiresStorageMotion whether storage motion is required for the virtual machine + * @param preImbalance the pre-calculated cluster imbalance before migration (null to calculate it) + * @param baseMetricsArray pre-calculated array of all host metrics before migration + * @param hostIdToIndexMap mapping from host ID to index in the metrics array * @return a ternary containing improvement, cost, benefit */ Ternary getMetrics(Cluster cluster, VirtualMachine vm, ServiceOffering serviceOffering, Host destHost, Map> hostCpuMap, Map> hostMemoryMap, - Boolean requiresStorageMotion) throws ConfigurationException; + Boolean requiresStorageMotion, Double preImbalance, + double[] baseMetricsArray, Map hostIdToIndexMap) throws ConfigurationException; /** - * Calculates the imbalance of the cluster after a virtual machine migration. + * Calculates the cluster imbalance after migrating a VM to a destination host. * - * @param serviceOffering - * the service offering for the virtual machine - * @param vm - * the virtual machine being migrated - * @param destHost - * the destination host for the virtual machine - * @param hostCpuMap - * a map of host IDs to the Ternary of used, reserved and total CPU on each host - * @param hostMemoryMap - * a map of host IDs to the Ternary of used, reserved and total memory on each host + * @param vm the virtual machine being migrated + * @param destHost the destination host for the virtual machine + * @param clusterId the cluster ID + * @param vmMetric the VM's resource consumption metric + * @param baseMetricsArray pre-calculated array of all host metrics before migration + * @param hostIdToIndexMap mapping from host ID to index in the metrics array + * @return the cluster imbalance after migration + */ + default Double getImbalancePostMigration(VirtualMachine vm, + Host destHost, Long clusterId, long vmMetric, double[] baseMetricsArray, + Map hostIdToIndexMap, Map> hostCpuMap, + Map> hostMemoryMap) { + // Create a copy of the base array and adjust only the two affected hosts + double[] adjustedMetrics = new double[baseMetricsArray.length]; + System.arraycopy(baseMetricsArray, 0, adjustedMetrics, 0, baseMetricsArray.length); + + long destHostId = destHost.getId(); + long vmHostId = vm.getHostId(); + + // Adjust source host (remove VM resources) + Integer sourceIndex = hostIdToIndexMap.get(vmHostId); + if (sourceIndex != null && sourceIndex < adjustedMetrics.length) { + Map> sourceMetricsMap = getClusterDrsMetric(clusterId).equals("cpu") ? hostCpuMap : hostMemoryMap; + Ternary sourceMetrics = sourceMetricsMap.get(vmHostId); + if (sourceMetrics != null) { + adjustedMetrics[sourceIndex] = getMetricValuePostMigration(clusterId, sourceMetrics, vmMetric, vmHostId, destHostId, vmHostId); + } + } + + // Adjust destination host (add VM resources) + Integer destIndex = hostIdToIndexMap.get(destHostId); + if (destIndex != null && destIndex < adjustedMetrics.length) { + Map> destMetricsMap = getClusterDrsMetric(clusterId).equals("cpu") ? hostCpuMap : hostMemoryMap; + Ternary destMetrics = destMetricsMap.get(destHostId); + if (destMetrics != null) { + adjustedMetrics[destIndex] = getMetricValuePostMigration(clusterId, destMetrics, vmMetric, destHostId, destHostId, vmHostId); + } + } + + return calculateImbalance(adjustedMetrics); + } + + /** + * Calculate imbalance from an array of metric values. + * Imbalance is defined as standard deviation divided by mean. * - * @return a pair containing the CPU and memory imbalance of the cluster after the migration + * Uses reusable stateless calculator objects to avoid object creation overhead. + * @param values array of metric values + * @return calculated imbalance */ - default Double getImbalancePostMigration(ServiceOffering serviceOffering, VirtualMachine vm, - Host destHost, Map> hostCpuMap, - Map> hostMemoryMap) throws ConfigurationException { - Pair>> pair = getHostMetricsMapAndType(destHost.getClusterId(), serviceOffering, hostCpuMap, hostMemoryMap); - long vmMetric = pair.first(); - Map> hostMetricsMap = pair.second(); + private static double calculateImbalance(double[] values) { + if (values == null || values.length == 0) { + return 0.0; + } - List list = new ArrayList<>(); - for (Long hostId : hostMetricsMap.keySet()) { - list.add(getMetricValuePostMigration(destHost.getClusterId(), hostMetricsMap.get(hostId), vmMetric, hostId, destHost.getId(), vm.getHostId())); + double mean = MEAN_CALCULATOR.evaluate(values); + if (mean == 0.0) { + return 0.0; // Avoid division by zero } - return getImbalance(list); + double stdDev = STDDEV_CALCULATOR.evaluate(values, mean); + return stdDev / mean; } - private Pair>> getHostMetricsMapAndType(Long clusterId, - ServiceOffering serviceOffering, Map> hostCpuMap, - Map> hostMemoryMap) throws ConfigurationException { + /** + * Helper method to get VM metric based on cluster configuration. + */ + static long getVmMetric(ServiceOffering serviceOffering, Long clusterId) throws ConfigurationException { String metric = getClusterDrsMetric(clusterId); - Pair>> pair; switch (metric) { case "cpu": - pair = new Pair<>((long) serviceOffering.getCpu() * serviceOffering.getSpeed(), hostCpuMap); - break; + return (long) serviceOffering.getCpu() * serviceOffering.getSpeed(); case "memory": - pair = new Pair<>(serviceOffering.getRamSize() * 1024L * 1024L, hostMemoryMap); - break; + return serviceOffering.getRamSize() * 1024L * 1024L; default: throw new ConfigurationException( String.format("Invalid metric: %s for cluster: %d", metric, clusterId)); } - return pair; + } + + /** + * Helper method to calculate metrics from pre and post imbalance values. + */ + default Ternary calculateMetricsFromImbalances(Double preImbalance, Double postImbalance) { + // This needs more research to determine the cost and benefit of a migration + // TODO: Cost should be a factor of the VM size and the host capacity + // TODO: Benefit should be a factor of the VM size and the host capacity and the number of VMs on the host + final double improvement = preImbalance - postImbalance; + final double cost = 0.0; + final double benefit = 1.0; + return new Ternary<>(improvement, cost, benefit); } private Double getMetricValuePostMigration(Long clusterId, Ternary metrics, long vmMetric, @@ -151,9 +196,26 @@ private Double getMetricValuePostMigration(Long clusterId, Ternary metricList) { - Double clusterMeanMetric = getClusterMeanMetric(metricList); - Double clusterStandardDeviation = getClusterStandardDeviation(metricList, clusterMeanMetric); - return clusterStandardDeviation / clusterMeanMetric; + if (CollectionUtils.isEmpty(metricList)) { + return 0.0; + } + // Convert List to double[] once, avoiding repeated conversions + double[] values = new double[metricList.size()]; + int index = 0; + for (Double value : metricList) { + if (value != null) { + values[index++] = value; + } + } + + // Trim array if some values were null + if (index < values.length) { + double[] trimmed = new double[index]; + System.arraycopy(values, 0, trimmed, 0, index); + values = trimmed; + } + + return calculateImbalance(values); } static String getClusterDrsMetric(long clusterId) { @@ -181,36 +243,6 @@ static Double getMetricValue(long clusterId, long used, long free, long total, F return null; } - /** - * Mean is the average of a collection or set of metrics. In context of a DRS - * cluster, the cluster metrics defined as the average metrics value for some - * metric (such as CPU, memory etc.) for every resource such as host. - * Cluster Mean Metric, mavg = (∑mi) / N, where mi is a measurable metric for a - * resource ‘i’ in a cluster with total N number of resources. - */ - static Double getClusterMeanMetric(List metricList) { - return new Mean().evaluate(metricList.stream().mapToDouble(i -> i).toArray()); - } - - /** - * Standard deviation is defined as the square root of the absolute squared sum - * of difference of a metric from its mean for every resource divided by the - * total number of resources. In context of the DRS, the cluster standard - * deviation is the standard deviation based on a metric of resources in a - * cluster such as for the allocation or utilisation CPU/memory metric of hosts - * in a cluster. - * Cluster Standard Deviation, σc = sqrt((∑∣mi−mavg∣^2) / N), where mavg is the - * mean metric value and mi is a measurable metric for some resource ‘i’ in the - * cluster with total N number of resources. - */ - static Double getClusterStandardDeviation(List metricList, Double mean) { - if (mean != null) { - return new StandardDeviation(false).evaluate(metricList.stream().mapToDouble(i -> i).toArray(), mean); - } else { - return new StandardDeviation(false).evaluate(metricList.stream().mapToDouble(i -> i).toArray()); - } - } - static boolean getDrsMetricUseRatio(long clusterId) { return ClusterDrsMetricUseRatio.valueIn(clusterId); } diff --git a/plugins/drs/cluster/balanced/src/main/java/org/apache/cloudstack/cluster/Balanced.java b/plugins/drs/cluster/balanced/src/main/java/org/apache/cloudstack/cluster/Balanced.java index b6a5ed1aac1a..902ab0900bd7 100644 --- a/plugins/drs/cluster/balanced/src/main/java/org/apache/cloudstack/cluster/Balanced.java +++ b/plugins/drs/cluster/balanced/src/main/java/org/apache/cloudstack/cluster/Balanced.java @@ -68,23 +68,26 @@ public String getName() { return "balanced"; } + @Override public Ternary getMetrics(Cluster cluster, VirtualMachine vm, ServiceOffering serviceOffering, Host destHost, Map> hostCpuMap, Map> hostMemoryMap, - Boolean requiresStorageMotion) throws ConfigurationException { - Double preImbalance = ClusterDrsAlgorithm.getClusterImbalance(cluster.getId(), new ArrayList<>(hostCpuMap.values()), new ArrayList<>(hostMemoryMap.values()), null); - Double postImbalance = getImbalancePostMigration(serviceOffering, vm, destHost, hostCpuMap, hostMemoryMap); + Boolean requiresStorageMotion, Double preImbalance, + double[] baseMetricsArray, Map hostIdToIndexMap) throws ConfigurationException { + // Use provided pre-imbalance if available, otherwise calculate it + if (preImbalance == null) { + preImbalance = ClusterDrsAlgorithm.getClusterImbalance(cluster.getId(), new ArrayList<>(hostCpuMap.values()), new ArrayList<>(hostMemoryMap.values()), null); + } + + // Use optimized post-imbalance calculation that adjusts only affected hosts + Double postImbalance = getImbalancePostMigration(vm, destHost, + cluster.getId(), ClusterDrsAlgorithm.getVmMetric(serviceOffering, cluster.getId()), + baseMetricsArray, hostIdToIndexMap, hostCpuMap, hostMemoryMap); - logger.debug("Cluster {} pre-imbalance: {} post-imbalance: {} Algorithm: {} VM: {} srcHost: {} destHost: {}", + logger.trace("Cluster {} pre-imbalance: {} post-imbalance: {} Algorithm: {} VM: {} srcHost ID: {} destHost: {}", cluster, preImbalance, postImbalance, getName(), vm, vm.getHostId(), destHost); - // This needs more research to determine the cost and benefit of a migration - // TODO: Cost should be a factor of the VM size and the host capacity - // TODO: Benefit should be a factor of the VM size and the host capacity and the number of VMs on the host - final double improvement = preImbalance - postImbalance; - final double cost = 0.0; - final double benefit = 1.0; - return new Ternary<>(improvement, cost, benefit); + return calculateMetricsFromImbalances(preImbalance, postImbalance); } } diff --git a/plugins/drs/cluster/balanced/src/test/java/org/apache/cloudstack/cluster/BalancedTest.java b/plugins/drs/cluster/balanced/src/test/java/org/apache/cloudstack/cluster/BalancedTest.java index d51606719584..e955a10f2a56 100644 --- a/plugins/drs/cluster/balanced/src/test/java/org/apache/cloudstack/cluster/BalancedTest.java +++ b/plugins/drs/cluster/balanced/src/test/java/org/apache/cloudstack/cluster/BalancedTest.java @@ -43,6 +43,9 @@ import java.util.List; import java.util.Map; +import static org.apache.cloudstack.cluster.ClusterDrsAlgorithm.getClusterDrsMetric; +import static org.apache.cloudstack.cluster.ClusterDrsAlgorithm.getClusterImbalance; +import static org.apache.cloudstack.cluster.ClusterDrsAlgorithm.getMetricValue; import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsImbalanceThreshold; import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsMetric; import static org.junit.Assert.assertEquals; @@ -119,6 +122,48 @@ public void tearDown() throws Exception { closeable.close(); } + /** + * Helper method to prepare metrics data for getMetrics calls with optimized signature. + * Calculates pre-imbalance and builds baseMetricsArray and hostIdToIndexMap. + * + * @return a Ternary containing preImbalance, baseMetricsArray, and hostIdToIndexMap + */ + private Ternary> prepareMetricsData() throws ConfigurationException { + // Calculate pre-imbalance + Double preImbalance = getClusterImbalance(clusterId, new ArrayList<>(hostCpuFreeMap.values()), + new ArrayList<>(hostMemoryFreeMap.values()), null); + + // Build baseMetricsArray and hostIdToIndexMap + String metricType = getClusterDrsMetric(clusterId); + Map> baseMetricsMap = "cpu".equals(metricType) ? hostCpuFreeMap : hostMemoryFreeMap; + double[] baseMetricsArray = new double[baseMetricsMap.size()]; + Map hostIdToIndexMap = new HashMap<>(); + + int index = 0; + for (Map.Entry> entry : baseMetricsMap.entrySet()) { + Long hostId = entry.getKey(); + Ternary metrics = entry.getValue(); + long used = metrics.first(); + long actualTotal = metrics.third() - metrics.second(); + long free = actualTotal - metrics.first(); + Double metricValue = getMetricValue(clusterId, used, free, actualTotal, null); + if (metricValue != null) { + baseMetricsArray[index] = metricValue; + hostIdToIndexMap.put(hostId, index); + index++; + } + } + + // Trim array if some values were null + if (index < baseMetricsArray.length) { + double[] trimmed = new double[index]; + System.arraycopy(baseMetricsArray, 0, trimmed, 0, index); + baseMetricsArray = trimmed; + } + + return new Ternary<>(preImbalance, baseMetricsArray, hostIdToIndexMap); + } + /** * needsDrs tests *

Scenarios to test for needsDrs @@ -183,8 +228,14 @@ public void needsDrsWithUnknown() throws NoSuchFieldException, IllegalAccessExce @Test public void getMetricsWithCpu() throws NoSuchFieldException, IllegalAccessException, ConfigurationException { overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "cpu"); + + Ternary> metricsData = prepareMetricsData(); + Double preImbalance = metricsData.first(); + double[] baseMetricsArray = metricsData.second(); + Map hostIdToIndexMap = metricsData.third(); + Ternary result = balanced.getMetrics(cluster, vm3, serviceOffering, destHost, - hostCpuFreeMap, hostMemoryFreeMap, false); + hostCpuFreeMap, hostMemoryFreeMap, false, preImbalance, baseMetricsArray, hostIdToIndexMap); assertEquals(0.0, result.first(), 0.01); assertEquals(0.0, result.second(), 0.0); assertEquals(1.0, result.third(), 0.0); @@ -197,8 +248,14 @@ public void getMetricsWithCpu() throws NoSuchFieldException, IllegalAccessExcept @Test public void getMetricsWithMemory() throws NoSuchFieldException, IllegalAccessException, ConfigurationException { overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "memory"); + + Ternary> metricsData = prepareMetricsData(); + Double preImbalance = metricsData.first(); + double[] baseMetricsArray = metricsData.second(); + Map hostIdToIndexMap = metricsData.third(); + Ternary result = balanced.getMetrics(cluster, vm3, serviceOffering, destHost, - hostCpuFreeMap, hostMemoryFreeMap, false); + hostCpuFreeMap, hostMemoryFreeMap, false, preImbalance, baseMetricsArray, hostIdToIndexMap); assertEquals(0.4, result.first(), 0.01); assertEquals(0, result.second(), 0.0); assertEquals(1, result.third(), 0.0); diff --git a/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java b/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java index 70c5acd951fe..d672ddfda615 100644 --- a/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java +++ b/plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java @@ -75,20 +75,22 @@ public String getName() { public Ternary getMetrics(Cluster cluster, VirtualMachine vm, ServiceOffering serviceOffering, Host destHost, Map> hostCpuMap, Map> hostMemoryMap, - Boolean requiresStorageMotion) throws ConfigurationException { - Double preImbalance = ClusterDrsAlgorithm.getClusterImbalance(cluster.getId(), new ArrayList<>(hostCpuMap.values()), - new ArrayList<>(hostMemoryMap.values()), null); - Double postImbalance = getImbalancePostMigration(serviceOffering, vm, destHost, hostCpuMap, hostMemoryMap); + Boolean requiresStorageMotion, Double preImbalance, + double[] baseMetricsArray, Map hostIdToIndexMap) throws ConfigurationException { + // Use provided pre-imbalance if available, otherwise calculate it + if (preImbalance == null) { + preImbalance = ClusterDrsAlgorithm.getClusterImbalance(cluster.getId(), new ArrayList<>(hostCpuMap.values()), + new ArrayList<>(hostMemoryMap.values()), null); + } + + // Use optimized post-imbalance calculation that adjusts only affected hosts + Double postImbalance = getImbalancePostMigration(vm, destHost, + cluster.getId(), ClusterDrsAlgorithm.getVmMetric(serviceOffering, cluster.getId()), + baseMetricsArray, hostIdToIndexMap, hostCpuMap, hostMemoryMap); - logger.debug("Cluster {} pre-imbalance: {} post-imbalance: {} Algorithm: {} VM: {} srcHost: {} destHost: {}", + logger.trace("Cluster {} pre-imbalance: {} post-imbalance: {} Algorithm: {} VM: {} srcHost ID: {} destHost: {}", cluster, preImbalance, postImbalance, getName(), vm, vm.getHostId(), destHost); - // This needs more research to determine the cost and benefit of a migration - // TODO: Cost should be a factor of the VM size and the host capacity - // TODO: Benefit should be a factor of the VM size and the host capacity and the number of VMs on the host - final double improvement = postImbalance - preImbalance; - final double cost = 0; - final double benefit = 1; - return new Ternary<>(improvement, cost, benefit); + return calculateMetricsFromImbalances(postImbalance, preImbalance); } } diff --git a/plugins/drs/cluster/condensed/src/test/java/org/apache/cloudstack/cluster/CondensedTest.java b/plugins/drs/cluster/condensed/src/test/java/org/apache/cloudstack/cluster/CondensedTest.java index 3d3896704dab..b2a5e6bf84f1 100644 --- a/plugins/drs/cluster/condensed/src/test/java/org/apache/cloudstack/cluster/CondensedTest.java +++ b/plugins/drs/cluster/condensed/src/test/java/org/apache/cloudstack/cluster/CondensedTest.java @@ -43,6 +43,9 @@ import java.util.List; import java.util.Map; +import static org.apache.cloudstack.cluster.ClusterDrsAlgorithm.getClusterDrsMetric; +import static org.apache.cloudstack.cluster.ClusterDrsAlgorithm.getClusterImbalance; +import static org.apache.cloudstack.cluster.ClusterDrsAlgorithm.getMetricValue; import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsImbalanceThreshold; import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsMetric; import static org.junit.Assert.assertEquals; @@ -121,6 +124,48 @@ public void tearDown() throws Exception { closeable.close(); } + /** + * Helper method to prepare metrics data for getMetrics calls with optimized signature. + * Calculates pre-imbalance and builds baseMetricsArray and hostIdToIndexMap. + * + * @return a Ternary containing preImbalance, baseMetricsArray, and hostIdToIndexMap + */ + private Ternary> prepareMetricsData() throws ConfigurationException { + // Calculate pre-imbalance + Double preImbalance = getClusterImbalance(clusterId, new ArrayList<>(hostCpuFreeMap.values()), + new ArrayList<>(hostMemoryFreeMap.values()), null); + + // Build baseMetricsArray and hostIdToIndexMap + String metricType = getClusterDrsMetric(clusterId); + Map> baseMetricsMap = "cpu".equals(metricType) ? hostCpuFreeMap : hostMemoryFreeMap; + double[] baseMetricsArray = new double[baseMetricsMap.size()]; + Map hostIdToIndexMap = new HashMap<>(); + + int index = 0; + for (Map.Entry> entry : baseMetricsMap.entrySet()) { + Long hostId = entry.getKey(); + Ternary metrics = entry.getValue(); + long used = metrics.first(); + long actualTotal = metrics.third() - metrics.second(); + long free = actualTotal - metrics.first(); + Double metricValue = getMetricValue(clusterId, used, free, actualTotal, null); + if (metricValue != null) { + baseMetricsArray[index] = metricValue; + hostIdToIndexMap.put(hostId, index); + index++; + } + } + + // Trim array if some values were null + if (index < baseMetricsArray.length) { + double[] trimmed = new double[index]; + System.arraycopy(baseMetricsArray, 0, trimmed, 0, index); + baseMetricsArray = trimmed; + } + + return new Ternary<>(preImbalance, baseMetricsArray, hostIdToIndexMap); + } + /** *

needsDrs tests *

Scenarios to test for needsDrs @@ -185,8 +230,14 @@ public void needsDrsWithUnknown() throws NoSuchFieldException, IllegalAccessExce @Test public void getMetricsWithCpu() throws NoSuchFieldException, IllegalAccessException, ConfigurationException { overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "cpu"); + + Ternary> metricsData = prepareMetricsData(); + Double preImbalance = metricsData.first(); + double[] baseMetricsArray = metricsData.second(); + Map hostIdToIndexMap = metricsData.third(); + Ternary result = condensed.getMetrics(cluster, vm3, serviceOffering, destHost, - hostCpuFreeMap, hostMemoryFreeMap, false); + hostCpuFreeMap, hostMemoryFreeMap, false, preImbalance, baseMetricsArray, hostIdToIndexMap); assertEquals(0.0, result.first(), 0.0); assertEquals(0, result.second(), 0.0); assertEquals(1, result.third(), 0.0); @@ -199,8 +250,14 @@ public void getMetricsWithCpu() throws NoSuchFieldException, IllegalAccessExcept @Test public void getMetricsWithMemory() throws NoSuchFieldException, IllegalAccessException, ConfigurationException { overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "memory"); + + Ternary> metricsData = prepareMetricsData(); + Double preImbalance = metricsData.first(); + double[] baseMetricsArray = metricsData.second(); + Map hostIdToIndexMap = metricsData.third(); + Ternary result = condensed.getMetrics(cluster, vm3, serviceOffering, destHost, - hostCpuFreeMap, hostMemoryFreeMap, false); + hostCpuFreeMap, hostMemoryFreeMap, false, preImbalance, baseMetricsArray, hostIdToIndexMap); assertEquals(-0.4, result.first(), 0.01); assertEquals(0, result.second(), 0.0); assertEquals(1, result.third(), 0.0); diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index 58ac6891e5f5..1e744aa62906 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -692,6 +692,7 @@ import com.cloud.dc.dao.PodVlanMapDao; import com.cloud.dc.dao.VlanDao; import com.cloud.deploy.DataCenterDeployment; +import com.cloud.deploy.DeploymentPlan; import com.cloud.deploy.DeploymentPlanner; import com.cloud.deploy.DeploymentPlanner.ExcludeList; import com.cloud.deploy.DeploymentPlanningManager; @@ -1454,17 +1455,27 @@ protected boolean zoneWideVolumeRequiresStorageMotion(PrimaryDataStore volumeDat return false; } - @Override - public Ternary, Integer>, List, Map> listHostsForMigrationOfVM(final VirtualMachine vm, final Long startIndex, final Long pageSize, - final String keyword, List vmList) { - - validateVmForHostMigration(vm); + /** + * Get technically compatible hosts for VM migration (storage, hypervisor, UEFI filtering). + * This determines which hosts are technically capable of hosting the VM based on + * storage requirements, hypervisor capabilities, and UEFI requirements. + * + * @param vm The virtual machine to migrate + * @param startIndex Starting index for pagination + * @param pageSize Page size for pagination + * @param keyword Keyword filter for host search + * @return Ternary containing: (all hosts with count, filtered compatible hosts, storage motion requirements map) + */ + Ternary, Integer>, List, Map> getTechnicallyCompatibleHosts( + final VirtualMachine vm, + final Long startIndex, + final Long pageSize, + final String keyword) { + // GPU check if (_serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString()) != null) { - logger.info(" Live Migration of GPU enabled VM : " + vm.getInstanceName() + " is not supported"); - // Return empty list. - return new Ternary<>(new Pair<>(new ArrayList<>(), 0), - new ArrayList<>(), new HashMap<>()); + logger.info("Live Migration of GPU enabled VM : {} is not supported", vm); + return new Ternary<>(new Pair<>(new ArrayList<>(), 0), new ArrayList<>(), new HashMap<>()); } final long srcHostId = vm.getHostId(); @@ -1478,6 +1489,7 @@ public Ternary, Integer>, List, Map, Integer>, List, Map allHosts = null; List filteredHosts = null; final Map requiresStorageMotion = new HashMap<>(); - DataCenterDeployment plan = null; + if (canMigrateWithStorage) { Long podId = !VirtualMachine.Type.User.equals(vm.getType()) ? srcHost.getPodId() : null; allHostsPair = searchForServers(startIndex, pageSize, null, hostType, null, srcHost.getDataCenterId(), podId, null, null, keyword, @@ -1562,7 +1574,6 @@ public Ternary, Integer>, List, Map(new Pair<>(allHosts, allHostsPair.second()), new ArrayList<>(), new HashMap<>()); } - plan = new DataCenterDeployment(srcHost.getDataCenterId(), podId, null, null, null, null); } else { final Long cluster = srcHost.getClusterId(); if (logger.isDebugEnabled()) { @@ -1571,22 +1582,38 @@ public Ternary, Integer>, List, Map, Integer> otherHosts = new Pair<>(allHosts, allHostsPair.second()); + final Pair, Integer> allHostsPairResult = new Pair<>(allHosts, allHostsPair.second()); Pair> uefiFilteredResult = filterUefiHostsForMigration(allHosts, filteredHosts, vm); if (!uefiFilteredResult.first()) { - return new Ternary<>(otherHosts, new ArrayList<>(), new HashMap<>()); + return new Ternary<>(allHostsPairResult, new ArrayList<>(), new HashMap<>()); } filteredHosts = uefiFilteredResult.second(); - List suitableHosts = new ArrayList<>(); + return new Ternary<>(allHostsPairResult, filteredHosts, requiresStorageMotion); + } + + /** + * Apply affinity group constraints and other exclusion rules for VM migration. + * This builds an ExcludeList based on affinity groups, DPDK requirements, and dedicated resources. + * + * @param vm The virtual machine to migrate + * @param vmProfile The VM profile + * @param plan The deployment plan + * @param vmList List of VMs with current/simulated placements for affinity processing + * @return ExcludeList containing hosts to avoid + */ + @Override + public ExcludeList applyAffinityConstraints(VirtualMachine vm, VirtualMachineProfile vmProfile, DeploymentPlan plan, List vmList) { final ExcludeList excludes = new ExcludeList(); - excludes.addHost(srcHostId); + excludes.addHost(vm.getHostId()); if (dpdkHelper.isVMDpdkEnabled(vm.getId())) { - excludeNonDPDKEnabledHosts(plan, excludes); + if (plan instanceof DataCenterDeployment) { + excludeNonDPDKEnabledHosts((DataCenterDeployment) plan, excludes); + } } // call affinitygroup chain @@ -1599,13 +1626,37 @@ public Ternary, Integer>, List, Map getCapableSuitableHosts( + final VirtualMachine vm, + final VirtualMachineProfile vmProfile, + final DataCenterDeployment plan, + final List compatibleHosts, + final ExcludeList excludes, + final Host srcHost) { + + List suitableHosts = new ArrayList<>(); + for (final HostAllocator allocator : hostAllocators) { - if (CollectionUtils.isNotEmpty(filteredHosts)) { - suitableHosts = allocator.allocateTo(vmProfile, plan, Host.Type.Routing, excludes, filteredHosts, HostAllocator.RETURN_UPTO_ALL, false); + if (CollectionUtils.isNotEmpty(compatibleHosts)) { + suitableHosts = allocator.allocateTo(vmProfile, plan, Host.Type.Routing, excludes, compatibleHosts, HostAllocator.RETURN_UPTO_ALL, false); } else { suitableHosts = allocator.allocateTo(vmProfile, plan, Host.Type.Routing, excludes, HostAllocator.RETURN_UPTO_ALL, false); } @@ -1631,6 +1682,43 @@ public Ternary, Integer>, List, Map, Integer>, List, Map> listHostsForMigrationOfVM(final VirtualMachine vm, final Long startIndex, final Long pageSize, + final String keyword, List vmList) { + + validateVmForHostMigration(vm); + + // Get technically compatible hosts (storage, hypervisor, UEFI) + Ternary, Integer>, List, Map> compatibilityResult = + getTechnicallyCompatibleHosts(vm, startIndex, pageSize, keyword); + + Pair, Integer> allHostsPair = compatibilityResult.first(); + List filteredHosts = compatibilityResult.second(); + Map requiresStorageMotion = compatibilityResult.third(); + + // If no compatible hosts, return early + if (CollectionUtils.isEmpty(filteredHosts)) { + final Pair, Integer> otherHosts = new Pair<>(allHostsPair.first(), allHostsPair.second()); + return new Ternary<>(otherHosts, new ArrayList<>(), requiresStorageMotion); + } + + // Create deployment plan and VM profile + final Host srcHost = _hostDao.findById(vm.getHostId()); + final DataCenterDeployment plan = new DataCenterDeployment( + srcHost.getDataCenterId(), srcHost.getPodId(), srcHost.getClusterId(), null, null, null); + final VirtualMachineProfile vmProfile = new VirtualMachineProfileImpl( + vm, null, _offeringDao.findById(vm.getId(), vm.getServiceOfferingId()), null, null); + + // Apply affinity constraints + final ExcludeList excludes = applyAffinityConstraints(vm, vmProfile, plan, vmList); + + // Get hosts with capacity + List suitableHosts = getCapableSuitableHosts(vm, vmProfile, plan, filteredHosts, excludes, srcHost); + + final Pair, Integer> otherHosts = new Pair<>(allHostsPair.first(), allHostsPair.second()); return new Ternary<>(otherHosts, suitableHosts, requiresStorageMotion); } @@ -1923,7 +2011,7 @@ private List findAllSuitableStoragePoolsForDetachedVolume(Volume vo return suitablePools; } - private Pair, Integer> searchForServers(final Long startIndex, final Long pageSize, final Object name, final Object type, + Pair, Integer> searchForServers(final Long startIndex, final Long pageSize, final Object name, final Object type, final Object state, final Object zone, final Object pod, final Object cluster, final Object id, final Object keyword, final Object resourceState, final Object haHosts, final Object hypervisorType, final Object hypervisorVersion, final Object... excludes) { final Filter searchFilter = new Filter(HostVO.class, "id", Boolean.TRUE, startIndex, pageSize); diff --git a/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java b/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java index a662d47d4541..7e1011ff9314 100644 --- a/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java +++ b/server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java @@ -24,6 +24,8 @@ import com.cloud.api.query.vo.HostJoinVO; import com.cloud.dc.ClusterVO; import com.cloud.dc.dao.ClusterDao; +import com.cloud.deploy.DataCenterDeployment; +import com.cloud.deploy.DeploymentPlanner.ExcludeList; import com.cloud.domain.Domain; import com.cloud.event.ActionEventUtils; import com.cloud.event.EventTypes; @@ -51,6 +53,8 @@ import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.VMInstanceVO; import com.cloud.vm.VirtualMachine; +import com.cloud.vm.VirtualMachineProfile; +import com.cloud.vm.VirtualMachineProfileImpl; import com.cloud.vm.VmDetailConstants; import com.cloud.vm.dao.VMInstanceDao; import org.apache.cloudstack.api.ApiCommandResourceType; @@ -62,6 +66,8 @@ import org.apache.cloudstack.api.response.ClusterDrsPlanMigrationResponse; import org.apache.cloudstack.api.response.ClusterDrsPlanResponse; import org.apache.cloudstack.api.response.ListResponse; +import org.apache.cloudstack.affinity.AffinityGroupVMMapVO; +import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao; import org.apache.cloudstack.cluster.dao.ClusterDrsPlanDao; import org.apache.cloudstack.cluster.dao.ClusterDrsPlanMigrationDao; import org.apache.cloudstack.context.CallContext; @@ -71,6 +77,7 @@ import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO; import org.apache.cloudstack.jobs.JobInfo; import org.apache.cloudstack.managed.context.ManagedContextTimerTask; +import org.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.MapUtils; import org.apache.commons.lang3.time.DateUtils; @@ -81,13 +88,18 @@ import java.util.Collections; import java.util.Date; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.Timer; import java.util.TimerTask; import java.util.stream.Collectors; import static com.cloud.org.Grouping.AllocationState.Disabled; +import static org.apache.cloudstack.cluster.ClusterDrsAlgorithm.getClusterImbalance; +import static org.apache.cloudstack.cluster.ClusterDrsAlgorithm.getClusterDrsMetric; +import static org.apache.cloudstack.cluster.ClusterDrsAlgorithm.getMetricValue; public class ClusterDrsServiceImpl extends ManagerBase implements ClusterDrsService, PluggableService { @@ -125,6 +137,9 @@ public class ClusterDrsServiceImpl extends ManagerBase implements ClusterDrsServ @Inject ManagementServer managementServer; + @Inject + AffinityGroupVMMapDao affinityGroupVMMapDao; + List drsAlgorithms = new ArrayList<>(); Map drsAlgorithmMap = new HashMap<>(); @@ -318,19 +333,14 @@ void generateDrsPlanForAllClusters() { * @throws ConfigurationException * If there is an error in the DRS configuration. */ - List> getDrsPlan(Cluster cluster, - int maxIterations) throws ConfigurationException { - List> migrationPlan = new ArrayList<>(); + List> getDrsPlan(Cluster cluster, int maxIterations) throws ConfigurationException { if (cluster.getAllocationState() == Disabled || maxIterations <= 0) { return Collections.emptyList(); } - ClusterDrsAlgorithm algorithm = getDrsAlgorithm(ClusterDrsAlgorithm.valueIn(cluster.getId())); List hostList = hostDao.findByClusterId(cluster.getId()); List vmList = new ArrayList<>(vmInstanceDao.listByClusterId(cluster.getId())); - int iteration = 0; - Map hostMap = hostList.stream().collect(Collectors.toMap(HostVO::getId, host -> host)); Map> hostVmMap = getHostVmMap(hostList, vmList); @@ -357,10 +367,39 @@ List> getDrsPlan(Cluster cluster, serviceOfferingDao.findByIdIncludingRemoved(vm.getId(), vm.getServiceOfferingId())); } + Pair>, Map>> hostCache = getCompatibleHostAndVmStorageMotionCache(vmList); + Map> vmToCompatibleHostsCache = hostCache.first(); + Map> vmToStorageMotionCache = hostCache.second(); + + Set vmsWithAffinityGroups = getVmsWithAffinityGroups(vmList, vmToCompatibleHostsCache); + + return getMigrationPlans(maxIterations, cluster, hostMap, vmList, vmsWithAffinityGroups, vmToCompatibleHostsCache, + vmToStorageMotionCache, vmIdServiceOfferingMap, originalHostIdVmIdMap, hostVmMap, hostCpuMap, hostMemoryMap); + } + + private List> getMigrationPlans( + long maxIterations, Cluster cluster, Map hostMap, List vmList, + Set vmsWithAffinityGroups, Map> vmToCompatibleHostsCache, + Map> vmToStorageMotionCache, Map vmIdServiceOfferingMap, + Map> originalHostIdVmIdMap, Map> hostVmMap, + Map> hostCpuMap, Map> hostMemoryMap + ) throws ConfigurationException { + ClusterDrsAlgorithm algorithm = getDrsAlgorithm(ClusterDrsAlgorithm.valueIn(cluster.getId())); + int iteration = 0; + List> migrationPlan = new ArrayList<>(); while (iteration < maxIterations && algorithm.needsDrs(cluster, new ArrayList<>(hostCpuMap.values()), new ArrayList<>(hostMemoryMap.values()))) { + + logger.debug("Starting DRS iteration {} for cluster {}", iteration + 1, cluster); + // Re-evaluate affinity constraints with current (simulated) VM placements + Map vmToExcludesMap = getVmToExcludesMap(vmList, hostMap, vmsWithAffinityGroups, + vmToCompatibleHostsCache, vmIdServiceOfferingMap); + + logger.debug("Completed affinity evaluation for DRS iteration {} for cluster {}", iteration + 1, cluster); + Pair bestMigration = getBestMigration(cluster, algorithm, vmList, - vmIdServiceOfferingMap, hostCpuMap, hostMemoryMap); + vmIdServiceOfferingMap, hostCpuMap, hostMemoryMap, + vmToCompatibleHostsCache, vmToStorageMotionCache, vmToExcludesMap); VirtualMachine vm = bestMigration.first(); Host destHost = bestMigration.second(); if (destHost == null || vm == null || originalHostIdVmIdMap.get(destHost.getId()).contains(vm.getId())) { @@ -372,8 +411,6 @@ List> getDrsPlan(Cluster cluster, ServiceOffering serviceOffering = vmIdServiceOfferingMap.get(vm.getId()); migrationPlan.add(new Ternary<>(vm, hostMap.get(vm.getHostId()), hostMap.get(destHost.getId()))); - hostVmMap.get(vm.getHostId()).remove(vm); - hostVmMap.get(destHost.getId()).add(vm); hostVmMap.get(vm.getHostId()).remove(vm); hostVmMap.get(destHost.getId()).add(vm); @@ -391,6 +428,106 @@ List> getDrsPlan(Cluster cluster, return migrationPlan; } + private Map getVmToExcludesMap(List vmList, Map hostMap, + Set vmsWithAffinityGroups, Map> vmToCompatibleHostsCache, + Map vmIdServiceOfferingMap) { + Map vmToExcludesMap = new HashMap<>(); + for (VirtualMachine vm : vmList) { + if (vmToCompatibleHostsCache.containsKey(vm.getId())) { + Host srcHost = hostMap.get(vm.getHostId()); + if (srcHost != null) { + // Only call expensive applyAffinityConstraints for VMs with affinity groups + // For VMs without affinity groups, create minimal ExcludeList (just source host) + ExcludeList excludes; + if (vmsWithAffinityGroups.contains(vm.getId())) { + DataCenterDeployment plan = new DataCenterDeployment( + srcHost.getDataCenterId(), srcHost.getPodId(), srcHost.getClusterId(), + null, null, null); + VirtualMachineProfile vmProfile = new VirtualMachineProfileImpl(vm, null, + vmIdServiceOfferingMap.get(vm.getId()), null, null); + + excludes = managementServer.applyAffinityConstraints( + vm, vmProfile, plan, vmList); + } else { + // VM has no affinity groups - create minimal ExcludeList (just source host) + excludes = new ExcludeList(); + excludes.addHost(vm.getHostId()); + } + vmToExcludesMap.put(vm.getId(), excludes); + } + } + } + return vmToExcludesMap; + } + + + /** + * Pre-compute suitable hosts (once per eligible VM - never changes) + * Use listHostsForMigrationOfVM to get hosts validated by getCapableSuitableHosts + * This ensures DRS uses the same validation as "find host for migration" command + * + * @param vmList List of VMs to pre-compute suitable hosts for + * @return Pair of VM to compatible hosts map and VM to storage motion requirement map + */ + private Pair>, Map>> getCompatibleHostAndVmStorageMotionCache( + List vmList + ) { + Map> vmToCompatibleHostsCache = new HashMap<>(); + Map> vmToStorageMotionCache = new HashMap<>(); + + for (VirtualMachine vm : vmList) { + // Skip ineligible VMs + if (vm.getType().isUsedBySystem() || + vm.getState() != VirtualMachine.State.Running || + (MapUtils.isNotEmpty(vm.getDetails()) && + "true".equalsIgnoreCase(vm.getDetails().get(VmDetailConstants.SKIP_DRS)))) { + continue; + } + + try { + // Use listHostsForMigrationOfVM to get suitable hosts (validated by getCapableSuitableHosts) + // This ensures the same validation as the "find host for migration" command + Ternary, Integer>, List, Map> hostsForMigration = + managementServer.listHostsForMigrationOfVM(vm, 0L, 500L, null, vmList); + + List suitableHosts = hostsForMigration.second(); // Get suitable hosts (validated by HostAllocator) + Map requiresStorageMotion = hostsForMigration.third(); + + if (suitableHosts != null && !suitableHosts.isEmpty()) { + vmToCompatibleHostsCache.put(vm.getId(), suitableHosts); + vmToStorageMotionCache.put(vm.getId(), requiresStorageMotion); + } + } catch (Exception e) { + logger.debug("Could not get suitable hosts for VM {}: {}", vm, e.getMessage()); + } + } + return new Pair<>(vmToCompatibleHostsCache, vmToStorageMotionCache); + } + + /** + * Pre-fetch affinity group mappings for all eligible VMs (once, before iterations) + * This allows us to skip expensive affinity processing for VMs without affinity groups + * + * @param vmList List of VMs to check for affinity groups + * @param vmToCompatibleHostsCache Cached map of VM IDs to their compatible hosts + * @return Set of VM IDs that have affinity groups + */ + private Set getVmsWithAffinityGroups( + List vmList, Map> vmToCompatibleHostsCache + ) { + Set vmsWithAffinityGroups = new HashSet<>(); + for (VirtualMachine vm : vmList) { + if (vmToCompatibleHostsCache.containsKey(vm.getId())) { + // Check if VM has any affinity groups - if list is empty, VM has no affinity groups + List affinityGroupMappings = affinityGroupVMMapDao.listByInstanceId(vm.getId()); + if (CollectionUtils.isNotEmpty(affinityGroupMappings)) { + vmsWithAffinityGroups.add(vm.getId()); + } + } + } + return vmsWithAffinityGroups; + } + private ClusterDrsAlgorithm getDrsAlgorithm(String algoName) { if (drsAlgorithmMap.containsKey(algoName)) { return drsAlgorithmMap.get(algoName); @@ -429,6 +566,12 @@ Map> getHostVmMap(List hostList, List getBestMigration(Cluster cluster, ClusterDrsAlgorithm List vmList, Map vmIdServiceOfferingMap, Map> hostCpuCapacityMap, - Map> hostMemoryCapacityMap) throws ConfigurationException { + Map> hostMemoryCapacityMap, + Map> vmToCompatibleHostsCache, + Map> vmToStorageMotionCache, + Map vmToExcludesMap) throws ConfigurationException { + // Pre-calculate cluster imbalance once per iteration (same for all VM-host combinations) + Double preImbalance = getClusterImbalance(cluster.getId(), + new ArrayList<>(hostCpuCapacityMap.values()), + new ArrayList<>(hostMemoryCapacityMap.values()), + null); + + // Pre-calculate base metrics array once per iteration for optimized imbalance calculation + String metricType = getClusterDrsMetric(cluster.getId()); + Map> baseMetricsMap = "cpu".equals(metricType) ? hostCpuCapacityMap : hostMemoryCapacityMap; + Pair> baseMetricsAndIndexMap = getBaseMetricsArrayAndHostIdIndexMap(cluster, baseMetricsMap); + double[] baseMetricsArray = baseMetricsAndIndexMap.first(); + Map hostIdToIndexMap = baseMetricsAndIndexMap.second(); + double improvement = 0; Pair bestMigration = new Pair<>(null, null); for (VirtualMachine vm : vmList) { - if (vm.getType().isUsedBySystem() || vm.getState() != VirtualMachine.State.Running || - (MapUtils.isNotEmpty(vm.getDetails()) && - vm.getDetails().get(VmDetailConstants.SKIP_DRS).equalsIgnoreCase("true")) - ) { + List compatibleHosts = vmToCompatibleHostsCache.get(vm.getId()); + Map requiresStorageMotion = vmToStorageMotionCache.get(vm.getId()); + ExcludeList excludes = vmToExcludesMap.get(vm.getId()); + + ServiceOffering serviceOffering = vmIdServiceOfferingMap.get(vm.getId()); + if (skipDrs(vm, compatibleHosts, serviceOffering)) { continue; } - Ternary, Integer>, List, Map> hostsForMigrationOfVM = managementServer - .listHostsForMigrationOfVM( - vm, 0L, 500L, null, vmList); - List compatibleDestinationHosts = hostsForMigrationOfVM.first().first(); - List suitableDestinationHosts = hostsForMigrationOfVM.second(); - Map requiresStorageMotion = hostsForMigrationOfVM.third(); + long vmCpu = (long) serviceOffering.getCpu() * serviceOffering.getSpeed(); + long vmMemory = serviceOffering.getRamSize() * 1024L * 1024L; - for (Host destHost : compatibleDestinationHosts) { - if (!suitableDestinationHosts.contains(destHost) || cluster.getId() != destHost.getClusterId()) { + for (Host destHost : compatibleHosts) { + Ternary metrics = getMetricsForMigration(cluster, algorithm, vm, vmCpu, + vmMemory, serviceOffering, destHost, hostCpuCapacityMap, hostMemoryCapacityMap, + requiresStorageMotion, preImbalance, baseMetricsArray, hostIdToIndexMap, excludes); + if (metrics == null) { continue; } - Ternary metrics = algorithm.getMetrics(cluster, vm, - vmIdServiceOfferingMap.get(vm.getId()), destHost, hostCpuCapacityMap, hostMemoryCapacityMap, - requiresStorageMotion.get(destHost)); - Double currentImprovement = metrics.first(); Double cost = metrics.second(); Double benefit = metrics.third(); @@ -477,6 +633,86 @@ Pair getBestMigration(Cluster cluster, ClusterDrsAlgorithm return bestMigration; } + private boolean skipDrs(VirtualMachine vm, List compatibleHosts, ServiceOffering serviceOffering) { + if (vm.getType().isUsedBySystem() || vm.getState() != VirtualMachine.State.Running) { + return true; + } + if (MapUtils.isNotEmpty(vm.getDetails()) && + "true".equalsIgnoreCase(vm.getDetails().get(VmDetailConstants.SKIP_DRS))) { + return true; + } + if (CollectionUtils.isEmpty(compatibleHosts)) { + return true; + } + if (serviceOffering == null) { + return true; + } + return false; + } + + private Pair> getBaseMetricsArrayAndHostIdIndexMap( + Cluster cluster, Map> baseMetricsMap + ) { + double[] baseMetricsArray = new double[baseMetricsMap.size()]; + Map hostIdToIndexMap = new HashMap<>(); + + int index = 0; + for (Map.Entry> entry : baseMetricsMap.entrySet()) { + Long hostId = entry.getKey(); + Ternary metrics = entry.getValue(); + long used = metrics.first(); + long actualTotal = metrics.third() - metrics.second(); + long free = actualTotal - metrics.first(); + Double metricValue = getMetricValue(cluster.getId(), used, free, actualTotal, null); + if (metricValue != null) { + baseMetricsArray[index] = metricValue; + hostIdToIndexMap.put(hostId, index); + index++; + } + } + + // Trim array if some values were null + if (index < baseMetricsArray.length) { + double[] trimmed = new double[index]; + System.arraycopy(baseMetricsArray, 0, trimmed, 0, index); + baseMetricsArray = trimmed; + } + return new Pair<>(baseMetricsArray, hostIdToIndexMap); + } + + private Ternary getMetricsForMigration( + Cluster cluster, ClusterDrsAlgorithm algorithm, VirtualMachine vm, long vmCpu, long vmMemory, + ServiceOffering serviceOffering, Host destHost, Map> hostCpuCapacityMap, + Map> hostMemoryCapacityMap, Map requiresStorageMotion, + Double preImbalance, double[] baseMetricsArray, Map hostIdToIndexMap, ExcludeList excludes + ) throws ConfigurationException { + if (cluster.getId() != destHost.getClusterId()) { + return null; + } + + // Check affinity constraints + if (excludes != null && excludes.shouldAvoid(destHost)) { + return null; + } + + // Quick capacity pre-filter: skip hosts that don't have enough capacity + Ternary destHostCpu = hostCpuCapacityMap.get(destHost.getId()); + Ternary destHostMemory = hostMemoryCapacityMap.get(destHost.getId()); + if (destHostCpu == null || destHostMemory == null) { + return null; + } + + long destHostAvailableCpu = (destHostCpu.third() - destHostCpu.second()) - destHostCpu.first(); + long destHostAvailableMemory = (destHostMemory.third() - destHostMemory.second()) - destHostMemory.first(); + + if (destHostAvailableCpu < vmCpu || destHostAvailableMemory < vmMemory) { + return null; // Skip hosts without sufficient capacity + } + + return algorithm.getMetrics(cluster, vm, serviceOffering, destHost, hostCpuCapacityMap, hostMemoryCapacityMap, + requiresStorageMotion.getOrDefault(destHost, false), preImbalance, baseMetricsArray, hostIdToIndexMap); + } + /** * Saves a DRS plan for a given cluster and returns the saved plan along with the list of migrations to be executed. diff --git a/server/src/test/java/com/cloud/server/ManagementServerImplTest.java b/server/src/test/java/com/cloud/server/ManagementServerImplTest.java index b26cd455cfbb..da2005f61368 100644 --- a/server/src/test/java/com/cloud/server/ManagementServerImplTest.java +++ b/server/src/test/java/com/cloud/server/ManagementServerImplTest.java @@ -16,17 +16,34 @@ // under the License. package com.cloud.server; +import com.cloud.api.ApiDBUtils; +import com.cloud.dc.DataCenterVO; import com.cloud.dc.Vlan.VlanType; +import com.cloud.dc.dao.DataCenterDao; +import com.cloud.deploy.DeploymentPlanningManager; import com.cloud.exception.InvalidParameterValueException; +import com.cloud.exception.PermissionDeniedException; +import com.cloud.gpu.GPU; import com.cloud.host.DetailVO; import com.cloud.host.Host; +import com.cloud.host.Host.Type; import com.cloud.host.HostVO; +import com.cloud.host.dao.HostDao; import com.cloud.host.dao.HostDetailsDao; +import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.network.IpAddress; import com.cloud.network.IpAddressManagerImpl; import com.cloud.network.dao.IPAddressVO; +import com.cloud.service.ServiceOfferingVO; +import com.cloud.service.dao.ServiceOfferingDao; +import com.cloud.service.dao.ServiceOfferingDetailsDao; +import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.VMTemplateVO; +import com.cloud.storage.Volume; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.VMTemplateDao; +import com.cloud.storage.dao.VolumeDao; import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.user.SSHKeyPair; @@ -37,14 +54,25 @@ import com.cloud.user.dao.SSHKeyPairDao; import com.cloud.user.dao.UserDataDao; import com.cloud.utils.Pair; +import com.cloud.utils.Ternary; import com.cloud.utils.db.Filter; import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.UserVmDetailVO; import com.cloud.vm.UserVmVO; +import com.cloud.vm.VMInstanceVO; +import com.cloud.vm.VirtualMachine; +import com.cloud.vm.VirtualMachine.State; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.UserVmDetailsDao; +import com.cloud.vm.dao.VMInstanceDao; +import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import com.cloud.hypervisor.dao.HypervisorCapabilitiesDao; +import com.cloud.hypervisor.kvm.dpdk.DpdkHelper; +import com.cloud.agent.manager.allocator.HostAllocator; import org.apache.cloudstack.annotation.dao.AnnotationDao; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseCmd; @@ -63,6 +91,7 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.MockedStatic; import org.mockito.Mockito; @@ -74,6 +103,7 @@ import java.lang.reflect.Field; import java.util.ArrayList; import java.util.List; +import java.util.Map; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.nullable; @@ -123,15 +153,60 @@ public class ManagementServerImplTest { @Mock UserDataManager userDataManager; - @Spy - ManagementServerImpl spy = new ManagementServerImpl(); - @Mock UserVmDetailsDao userVmDetailsDao; @Mock HostDetailsDao hostDetailsDao; + + @Mock + VMInstanceDao vmInstanceDao; + + @Mock + HostDao hostDao; + + @Mock + ServiceOfferingDetailsDao serviceOfferingDetailsDao; + + @Mock + VolumeDao volumeDao; + + @Mock + ServiceOfferingDao offeringDao; + + @Mock + DiskOfferingDao diskOfferingDao; + + @Mock + HypervisorCapabilitiesDao hypervisorCapabilitiesDao; + + @Mock + DataStoreManager dataStoreManager; + + @Mock + PrimaryDataStoreDao primaryDataStoreDao; + + @Mock + DpdkHelper dpdkHelper; + + @Mock + AffinityGroupVMMapDao affinityGroupVMMapDao; + + @Mock + DeploymentPlanningManager dpMgr; + + @Mock + DataCenterDao dcDao; + + @Mock + HostAllocator hostAllocator; + + @InjectMocks + @Spy + ManagementServerImpl spy; + private AutoCloseable closeable; + private MockedStatic apiDBUtilsMock; @Before public void setup() throws IllegalAccessException, NoSuchFieldException { @@ -145,10 +220,21 @@ public void setup() throws IllegalAccessException, NoSuchFieldException { spy._UserVmDetailsDao = userVmDetailsDao; spy._detailsDao = hostDetailsDao; spy.userDataManager = userDataManager; + + spy.setHostAllocators(List.of(hostAllocator)); + + // Mock ApiDBUtils static method + apiDBUtilsMock = Mockito.mockStatic(ApiDBUtils.class); + // Return empty list to avoid architecture filtering in most tests + apiDBUtilsMock.when(() -> ApiDBUtils.listZoneClustersArchs(Mockito.anyLong())) + .thenReturn(new ArrayList<>()); } @After public void tearDown() throws Exception { + if (apiDBUtilsMock != null) { + apiDBUtilsMock.close(); + } CallContext.unregister(); closeable.close(); } @@ -684,4 +770,1308 @@ public void testZoneWideVolumeRequiresStorageMotionDriverDependent() { Mockito.when(driver.zoneWideVolumesAvailableWithoutClusterMotion()).thenReturn(false); Assert.assertTrue(spy.zoneWideVolumeRequiresStorageMotion(dataStore, host1, host2)); } + + // ============= Tests for listHostsForMigrationOfVM ============= + + @Test(expected = PermissionDeniedException.class) + public void testListHostsForMigrationOfVMNonRootAdmin() { + mockRunningVM(1L, HypervisorType.KVM); + Account caller = Mockito.mock(Account.class); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(_accountMgr.isRootAdmin(caller.getId())).thenReturn(false); + + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + } + + @Test(expected = InvalidParameterValueException.class) + public void testListHostsForMigrationOfVMNullVM() { + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(null); + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + } + + @Test(expected = InvalidParameterValueException.class) + public void testListHostsForMigrationOfVMNotRunning() { + VMInstanceVO vm = mockVM(1L, HypervisorType.KVM, State.Stopped); + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + } + + @Test(expected = InvalidParameterValueException.class) + public void testListHostsForMigrationOfVMUnsupportedHypervisor() { + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.BareMetal); + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + } + + @Test(expected = InvalidParameterValueException.class) + public void testListHostsForMigrationOfVMLxcUserVM() { + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.LXC); + Mockito.when(vm.getType()).thenReturn(VirtualMachine.Type.User); + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + } + + @Test + public void testListHostsForMigrationOfVMGpuEnabled() { + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.KVM); + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + + // Mock GPU detail + Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) + .thenReturn(Mockito.mock(com.cloud.service.ServiceOfferingDetailsVO.class)); + + Ternary, Integer>, List, java.util.Map> result = + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + + Assert.assertNotNull(result); + Assert.assertEquals(0, result.first().first().size()); + Assert.assertEquals(Integer.valueOf(0), result.first().second()); + Assert.assertEquals(0, result.second().size()); + } + + @Test + public void testListHostsForMigrationOfVMWithSystemVM() { + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.VMware); + Mockito.when(vm.getType()).thenReturn(VirtualMachine.Type.ConsoleProxy); + + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) + .thenReturn(null); + + HostVO srcHost = mockHost(100L, 1L, 1L, 1L, HypervisorType.VMware); + Mockito.when(hostDao.findById(vm.getHostId())).thenReturn(srcHost); + + // System VMs can use storage motion + Mockito.when(hypervisorCapabilitiesDao.isStorageMotionSupported(HypervisorType.VMware, null)) + .thenReturn(true); + + ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(offeringDao.findById(vm.getId(), vm.getServiceOfferingId())).thenReturn(offering); + + VolumeVO volume = mockVolume(1L, 1L); + Mockito.when(volumeDao.findCreatedByInstance(vm.getId())).thenReturn(List.of(volume)); + + DiskOfferingVO diskOffering = mockSharedDiskOffering(1L); + Mockito.when(diskOfferingDao.findById(volume.getDiskOfferingId())).thenReturn(diskOffering); + + // Mock searchForServers with zone-wide scope (storage motion enabled) + HostVO host1 = mockHost(101L, 1L, 1L, 1L, HypervisorType.VMware); + HostVO host2 = mockHost(102L, 1L, 1L, 1L, HypervisorType.VMware); + List hosts = List.of(host1, host2); + Pair, Integer> hostsPair = new Pair<>(hosts, 2); + Mockito.doReturn(hostsPair).when(spy).searchForServers( + Mockito.anyLong(), Mockito.anyLong(), Mockito.isNull(), Mockito.any(Type.class), + Mockito.isNull(), Mockito.anyLong(), Mockito.anyLong(), Mockito.isNull(), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), + Mockito.any(HypervisorType.class), Mockito.isNull(), Mockito.anyLong()); + + setupMigrationMocks(vm, srcHost, hosts, volume, true); + + // Verify this doesn't throw exception - system VMs should be migratable + Ternary, Integer>, List, Map> result = + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + + // Verify storage motion capability was checked + Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.VMware, null); + + // Verify result structure and data + Assert.assertNotNull(result); + Assert.assertNotNull("All hosts list should not be null", result.first()); + Assert.assertNotNull("Suitable hosts list should not be null", result.second()); + Assert.assertNotNull("Storage motion map should not be null", result.third()); + + // Verify all hosts returned (from searchForServers) + Assert.assertEquals("Should return 2 total hosts", Integer.valueOf(2), result.first().second()); + Assert.assertEquals("All hosts list should contain 2 hosts", 2, result.first().first().size()); + + // Verify suitable hosts (from host allocator) + Assert.assertEquals("Should return 2 suitable hosts", 2, result.second().size()); + Assert.assertTrue("Suitable hosts should contain host1", + result.second().stream().anyMatch(h -> h.getId() == 101L)); + Assert.assertTrue("Suitable hosts should contain host2", + result.second().stream().anyMatch(h -> h.getId() == 102L)); + } + + @Test + public void testListHostsForMigrationOfVMWithDomainRouter() { + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.KVM); + Mockito.when(vm.getType()).thenReturn(VirtualMachine.Type.DomainRouter); + + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) + .thenReturn(null); + + HostVO srcHost = mockHost(100L, 1L, 1L, 1L, HypervisorType.KVM); + Mockito.when(hostDao.findById(vm.getHostId())).thenReturn(srcHost); + + Mockito.when(hypervisorCapabilitiesDao.isStorageMotionSupported(HypervisorType.KVM, "")) + .thenReturn(false); + + ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(offeringDao.findById(vm.getId(), vm.getServiceOfferingId())).thenReturn(offering); + + VolumeVO volume = mockVolume(1L, 1L); + Mockito.when(volumeDao.findCreatedByInstance(vm.getId())).thenReturn(List.of(volume)); + + DiskOfferingVO diskOffering = mockSharedDiskOffering(1L); + Mockito.when(diskOfferingDao.findById(volume.getDiskOfferingId())).thenReturn(diskOffering); + + // Mock searchForServers for cluster-scoped search + HostVO host1 = mockHost(101L, 1L, 1L, 1L, HypervisorType.KVM); + HostVO host2 = mockHost(102L, 1L, 1L, 1L, HypervisorType.KVM); + List hosts = List.of(host1, host2); + Pair, Integer> hostsPair = new Pair<>(hosts, 2); + Mockito.doReturn(hostsPair).when(spy).searchForServers( + Mockito.anyLong(), Mockito.anyLong(), Mockito.isNull(), Mockito.any(Type.class), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.anyLong(), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), + Mockito.isNull(), Mockito.isNull(), Mockito.anyLong()); + + setupMigrationMocks(vm, srcHost, hosts, volume); + + // Verify domain router can be migrated + Ternary, Integer>, List, Map> result = + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + + // Verify hypervisor capabilities were checked + Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.KVM, ""); + + // Verify result contains expected hosts + Assert.assertNotNull(result); + Assert.assertEquals("Should return 2 total hosts", Integer.valueOf(2), result.first().second()); + Assert.assertEquals("Should return 2 suitable hosts", 2, result.second().size()); + Assert.assertTrue("Result should contain host 101", + result.second().stream().anyMatch(h -> h.getId() == 101L)); + Assert.assertTrue("Result should contain host 102", + result.second().stream().anyMatch(h -> h.getId() == 102L)); + } + + @Test + public void testListHostsForMigrationOfVMWithMultipleVolumes() { + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.KVM); + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) + .thenReturn(null); + + HostVO srcHost = mockHost(100L, 1L, 1L, 1L, HypervisorType.KVM); + Mockito.when(hostDao.findById(vm.getHostId())).thenReturn(srcHost); + + Mockito.when(hypervisorCapabilitiesDao.isStorageMotionSupported(HypervisorType.KVM, "")) + .thenReturn(false); + + ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(offeringDao.findById(vm.getId(), vm.getServiceOfferingId())).thenReturn(offering); + + // Multiple volumes - root and data disk + VolumeVO rootVolume = mockVolume(1L, 1L); + VolumeVO dataVolume = mockVolume(2L, 1L); + Mockito.when(volumeDao.findCreatedByInstance(vm.getId())).thenReturn(List.of(rootVolume, dataVolume)); + + DiskOfferingVO sharedOffering = mockSharedDiskOffering(1L); + Mockito.when(diskOfferingDao.findById(1L)).thenReturn(sharedOffering); + + // Mock searchForServers for cluster-scoped search + HostVO host1 = mockHost(101L, 1L, 1L, 1L, HypervisorType.KVM); + HostVO host2 = mockHost(102L, 1L, 1L, 1L, HypervisorType.KVM); + List hosts = List.of(host1, host2); + Pair, Integer> hostsPair = new Pair<>(hosts, 2); + Mockito.doReturn(hostsPair).when(spy).searchForServers( + Mockito.anyLong(), Mockito.anyLong(), Mockito.isNull(), Mockito.any(Type.class), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.anyLong(), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), + Mockito.isNull(), Mockito.isNull(), Mockito.anyLong()); + + setupMigrationMocks(vm, srcHost, hosts, rootVolume); + + // Verify multiple volumes are handled + Ternary, Integer>, List, Map> result = + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + + // Verify all volumes were checked + Mockito.verify(volumeDao).findCreatedByInstance(vm.getId()); + Mockito.verify(diskOfferingDao, Mockito.times(2)).findById(1L); + + // Verify result + Assert.assertNotNull(result); + Assert.assertEquals("Should return 2 total hosts", Integer.valueOf(2), result.first().second()); + Assert.assertEquals("Should return 2 suitable hosts for migration", 2, result.second().size()); + Assert.assertTrue("Suitable hosts should include host 101", + result.second().stream().anyMatch(h -> h.getId() == 101L)); + Assert.assertTrue("Suitable hosts should include host 102", + result.second().stream().anyMatch(h -> h.getId() == 102L)); + } + + @Test(expected = InvalidParameterValueException.class) + public void testListHostsForMigrationOfVMWithMixedStorage() { + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.XenServer); + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) + .thenReturn(null); + + HostVO srcHost = mockHost(100L, 1L, 1L, 1L, HypervisorType.XenServer); + Mockito.when(hostDao.findById(vm.getHostId())).thenReturn(srcHost); + + // No storage motion support + Mockito.when(hypervisorCapabilitiesDao.isStorageMotionSupported(HypervisorType.XenServer, null)) + .thenReturn(false); + + ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(offeringDao.findById(vm.getId(), vm.getServiceOfferingId())).thenReturn(offering); + + // Mixed storage - one shared, one local + VolumeVO sharedVolume = mockVolume(1L, 1L); + VolumeVO localVolume = mockVolume(2L, 2L); + Mockito.when(volumeDao.findCreatedByInstance(vm.getId())).thenReturn(List.of(sharedVolume, localVolume)); + + DiskOfferingVO sharedOffering = mockSharedDiskOffering(1L); + DiskOfferingVO localOffering = mockLocalDiskOffering(2L); + Mockito.when(diskOfferingDao.findById(sharedVolume.getDiskOfferingId())).thenReturn(sharedOffering); + Mockito.when(diskOfferingDao.findById(localVolume.getDiskOfferingId())).thenReturn(localOffering); + + // Should throw exception because we have local storage without storage motion support + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + } + + @Test + public void testListHostsForMigrationOfVMKVMWithNullHypervisorVersion() { + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.KVM); + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) + .thenReturn(null); + + // KVM host with null hypervisor version + HostVO srcHost = mockHost(100L, 1L, 1L, 1L, HypervisorType.KVM); + Mockito.when(hostDao.findById(vm.getHostId())).thenReturn(srcHost); + + // KVM null version should be treated as empty string + Mockito.when(hypervisorCapabilitiesDao.isStorageMotionSupported(HypervisorType.KVM, "")) + .thenReturn(true); + + ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(offeringDao.findById(vm.getId(), vm.getServiceOfferingId())).thenReturn(offering); + + VolumeVO volume = mockVolume(1L, 1L); + Mockito.when(volumeDao.findCreatedByInstance(vm.getId())).thenReturn(List.of(volume)); + + DiskOfferingVO diskOffering = mockSharedDiskOffering(1L); + Mockito.when(diskOfferingDao.findById(volume.getDiskOfferingId())).thenReturn(diskOffering); + + // Mock searchForServers with zone-wide scope (storage motion enabled) + HostVO host1 = mockHost(101L, 1L, 1L, 1L, HypervisorType.KVM); + HostVO host2 = mockHost(102L, 1L, 1L, 1L, HypervisorType.KVM); + List hosts = List.of(host1, host2); + Pair, Integer> hostsPair = new Pair<>(hosts, 2); + Mockito.doReturn(hostsPair).when(spy).searchForServers( + Mockito.anyLong(), Mockito.anyLong(), Mockito.isNull(), Mockito.any(Type.class), + Mockito.isNull(), Mockito.anyLong(), Mockito.isNull(), Mockito.isNull(), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), + Mockito.any(HypervisorType.class), Mockito.isNull(), Mockito.anyLong()); + + setupMigrationMocks(vm, srcHost, hosts, volume, true); + + Ternary, Integer>, List, Map> result = + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + + // Verify KVM null version was converted to empty string + Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.KVM, ""); + + // Verify result data + Assert.assertNotNull(result); + Assert.assertEquals("Total hosts should be 2", Integer.valueOf(2), result.first().second()); + Assert.assertEquals("Suitable hosts should be 2", 2, result.second().size()); + Assert.assertTrue("Should contain host 101", + result.second().stream().anyMatch(h -> h.getId() == 101L)); + Assert.assertTrue("Should contain host 102", + result.second().stream().anyMatch(h -> h.getId() == 102L)); + } + + @Test + public void testListHostsForMigrationOfVMNonUefiVm() { + // Test VM migration for non-UEFI VM (regular VM migration flow) + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.KVM); + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) + .thenReturn(null); + + HostVO srcHost = mockHost(100L, 1L, 1L, 1L, HypervisorType.KVM); + Mockito.when(hostDao.findById(vm.getHostId())).thenReturn(srcHost); + + Mockito.when(hypervisorCapabilitiesDao.isStorageMotionSupported(HypervisorType.KVM, "")) + .thenReturn(false); + + ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(offeringDao.findById(vm.getId(), vm.getServiceOfferingId())).thenReturn(offering); + + VolumeVO volume = mockVolume(1L, 1L); + Mockito.when(volumeDao.findCreatedByInstance(vm.getId())).thenReturn(List.of(volume)); + + DiskOfferingVO diskOffering = mockSharedDiskOffering(1L); + Mockito.when(diskOfferingDao.findById(volume.getDiskOfferingId())).thenReturn(diskOffering); + + // Mock searchForServers for cluster-scoped search + HostVO host1 = mockHost(101L, 1L, 1L, 1L, HypervisorType.KVM); + HostVO host2 = mockHost(102L, 1L, 1L, 1L, HypervisorType.KVM); + List hosts = List.of(host1, host2); + Pair, Integer> hostsPair = new Pair<>(hosts, 2); + Mockito.doReturn(hostsPair).when(spy).searchForServers( + Mockito.anyLong(), Mockito.anyLong(), Mockito.isNull(), Mockito.any(Type.class), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.anyLong(), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), + Mockito.isNull(), Mockito.isNull(), Mockito.anyLong()); + + setupMigrationMocks(vm, srcHost, hosts, volume); + + Ternary, Integer>, List, Map> result = + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + + // Verify hosts are returned for migration (non-UEFI VMs don't need UEFI-enabled hosts) + Assert.assertNotNull(result); + Assert.assertEquals("Should have 2 total hosts", Integer.valueOf(2), result.first().second()); + Assert.assertEquals("Should have 2 suitable hosts", 2, result.second().size()); + Assert.assertTrue("Should contain host 101", + result.second().stream().anyMatch(h -> h.getId() == 101L)); + Assert.assertTrue("Should contain host 102", + result.second().stream().anyMatch(h -> h.getId() == 102L)); + } + + @Test + public void testListHostsForMigrationOfVMWithUefiVmClusterScope() { + // Test UEFI VM migration with cluster-scoped search (no storage motion) + // This exercises the code path where filteredHosts is NULL and allocateTo without list is called + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.KVM); + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) + .thenReturn(null); + + HostVO srcHost = mockHost(100L, 1L, 1L, 1L, HypervisorType.KVM); + Mockito.when(hostDao.findById(vm.getHostId())).thenReturn(srcHost); + + // No storage motion support - cluster-scoped search + Mockito.when(hypervisorCapabilitiesDao.isStorageMotionSupported(HypervisorType.KVM, "")) + .thenReturn(false); + + ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(offeringDao.findById(vm.getId(), vm.getServiceOfferingId())).thenReturn(offering); + + VolumeVO volume = mockVolume(1L, 1L); + Mockito.when(volumeDao.findCreatedByInstance(vm.getId())).thenReturn(List.of(volume)); + + DiskOfferingVO diskOffering = mockSharedDiskOffering(1L); + Mockito.when(diskOfferingDao.findById(volume.getDiskOfferingId())).thenReturn(diskOffering); + + // Mock searchForServers for cluster-scoped search + HostVO host1 = mockHost(101L, 1L, 1L, 1L, HypervisorType.KVM); + HostVO host2 = mockHost(102L, 1L, 1L, 1L, HypervisorType.KVM); + List hosts = List.of(host1, host2); + Pair, Integer> hostsPair = new Pair<>(hosts, 2); + Mockito.doReturn(hostsPair).when(spy).searchForServers( + Mockito.anyLong(), Mockito.anyLong(), Mockito.isNull(), Mockito.any(Type.class), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.anyLong(), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), + Mockito.isNull(), Mockito.isNull(), Mockito.anyLong()); + + // Mock filterUefiHostsForMigration to return success with filtered hosts (only host1 is UEFI-compatible) + List uefiCompatibleHosts = List.of(host1); + Pair> uefiFilterResult = new Pair<>(true, uefiCompatibleHosts); + Mockito.doReturn(uefiFilterResult).when(spy).filterUefiHostsForMigration( + Mockito.anyList(), Mockito.anyList(), Mockito.any()); + + // Setup other mocks + Mockito.when(dpdkHelper.isVMDpdkEnabled(vm.getId())).thenReturn(false); + Mockito.when(affinityGroupVMMapDao.countAffinityGroupsForVm(vm.getId())).thenReturn(0L); + DataCenterVO dc = Mockito.mock(DataCenterVO.class); + Mockito.when(dcDao.findById(srcHost.getDataCenterId())).thenReturn(dc); + Mockito.doNothing().when(dpMgr).checkForNonDedicatedResources(Mockito.any(), Mockito.any(), Mockito.any()); + Mockito.doNothing().when(dpMgr).reorderHostsByPriority(Mockito.any(), Mockito.anyList()); + + // After UEFI filtering, filteredHosts is set to uefiCompatibleHosts (line 1582) + // So allocateTo WITH list parameter is called (line 1608) even in cluster scope + Mockito.when(hostAllocator.allocateTo(Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.anyList(), Mockito.anyInt(), Mockito.anyBoolean())) + .thenReturn(new ArrayList<>(uefiCompatibleHosts)); + + Ternary, Integer>, List, Map> result = + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + + // Verify result structure + Assert.assertNotNull("Result should not be null", result); + Assert.assertEquals("Should return 2 total hosts", Integer.valueOf(2), result.first().second()); + Assert.assertEquals("All hosts list should contain 2 hosts", 2, result.first().first().size()); + + // Verify only UEFI-compatible hosts are in suitable list + Assert.assertEquals("Should have 1 UEFI-compatible suitable host", 1, result.second().size()); + Assert.assertTrue("Host 101 should be the only UEFI-compatible host", + result.second().stream().anyMatch(h -> h.getId() == 101L)); + Assert.assertFalse("Host 102 should not be in suitable hosts (not UEFI-compatible)", + result.second().stream().anyMatch(h -> h.getId() == 102L)); + } + + @Test + public void testListHostsForMigrationOfVMWithUefiVmZoneWideScope() { + // Test UEFI VM migration with zone-wide search (storage motion enabled) + // This exercises the code path where filteredHosts IS populated and allocateTo WITH list is called + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.VMware); + Mockito.when(vm.getType()).thenReturn(VirtualMachine.Type.User); + + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) + .thenReturn(null); + + HostVO srcHost = mockHost(100L, 1L, 1L, 1L, HypervisorType.VMware); + Mockito.when(hostDao.findById(vm.getHostId())).thenReturn(srcHost); + + // Storage motion supported - zone-wide search with filteredHosts + Mockito.when(hypervisorCapabilitiesDao.isStorageMotionSupported(HypervisorType.VMware, null)) + .thenReturn(true); + + ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(offeringDao.findById(vm.getId(), vm.getServiceOfferingId())).thenReturn(offering); + + VolumeVO volume = mockVolume(1L, 1L); + Mockito.when(volumeDao.findCreatedByInstance(vm.getId())).thenReturn(List.of(volume)); + + DiskOfferingVO diskOffering = mockSharedDiskOffering(1L); + Mockito.when(diskOfferingDao.findById(volume.getDiskOfferingId())).thenReturn(diskOffering); + + // Mock searchForServers for zone-wide search (storage motion enabled) + HostVO host1 = mockHost(101L, 1L, 1L, 1L, HypervisorType.VMware); + HostVO host2 = mockHost(102L, 2L, 1L, 1L, HypervisorType.VMware); // Different cluster + List hosts = List.of(host1, host2); + Pair, Integer> hostsPair = new Pair<>(hosts, 2); + Mockito.doReturn(hostsPair).when(spy).searchForServers( + Mockito.anyLong(), Mockito.anyLong(), Mockito.isNull(), Mockito.any(Type.class), + Mockito.isNull(), Mockito.anyLong(), Mockito.isNull(), Mockito.isNull(), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), + Mockito.any(HypervisorType.class), Mockito.isNull(), Mockito.anyLong()); + + setupMigrationMocks(vm, srcHost, hosts, volume, true); + + // Mock filterUefiHostsForMigration to return success with filtered hosts (only host1 is UEFI-compatible) + List uefiCompatibleHosts = List.of(host1); + Pair> uefiFilterResult = new Pair<>(true, uefiCompatibleHosts); + Mockito.doReturn(uefiFilterResult).when(spy).filterUefiHostsForMigration( + Mockito.anyList(), Mockito.anyList(), Mockito.any()); + + // Override hostAllocator to return only UEFI-compatible hosts + // Uses allocateTo WITH list parameter (filteredHosts is populated in zone-wide search) + Mockito.when(hostAllocator.allocateTo(Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.anyList(), Mockito.anyInt(), Mockito.anyBoolean())) + .thenReturn(new ArrayList<>(uefiCompatibleHosts)); + + Ternary, Integer>, List, Map> result = + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + + // Verify result structure + Assert.assertNotNull("Result should not be null", result); + Assert.assertEquals("Should return 2 total hosts", Integer.valueOf(2), result.first().second()); + Assert.assertEquals("All hosts list should contain 2 hosts", 2, result.first().first().size()); + + // Verify only UEFI-compatible hosts are in suitable list + Assert.assertEquals("Should have 1 UEFI-compatible suitable host", 1, result.second().size()); + Assert.assertTrue("Host 101 should be the only UEFI-compatible host", + result.second().stream().anyMatch(h -> h.getId() == 101L)); + Assert.assertFalse("Host 102 should not be in suitable hosts (not UEFI-compatible)", + result.second().stream().anyMatch(h -> h.getId() == 102L)); + + // Verify storage motion map is populated + Assert.assertNotNull("Storage motion map should not be null", result.third()); + } + + @Test + public void testListHostsForMigrationOfVMUefiFilteringReturnsEmpty() { + // Test case where UEFI filtering results in no suitable hosts + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.KVM); + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) + .thenReturn(null); + + HostVO srcHost = mockHost(100L, 1L, 1L, 1L, HypervisorType.KVM); + Mockito.when(hostDao.findById(vm.getHostId())).thenReturn(srcHost); + + Mockito.when(hypervisorCapabilitiesDao.isStorageMotionSupported(HypervisorType.KVM, "")) + .thenReturn(false); + + ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(offeringDao.findById(vm.getId(), vm.getServiceOfferingId())).thenReturn(offering); + + VolumeVO volume = mockVolume(1L, 1L); + Mockito.when(volumeDao.findCreatedByInstance(vm.getId())).thenReturn(List.of(volume)); + + DiskOfferingVO diskOffering = mockSharedDiskOffering(1L); + Mockito.when(diskOfferingDao.findById(volume.getDiskOfferingId())).thenReturn(diskOffering); + + // Mock searchForServers for cluster-scoped search + HostVO host1 = mockHost(101L, 1L, 1L, 1L, HypervisorType.KVM); + HostVO host2 = mockHost(102L, 1L, 1L, 1L, HypervisorType.KVM); + List hosts = List.of(host1, host2); + Pair, Integer> hostsPair = new Pair<>(hosts, 2); + Mockito.doReturn(hostsPair).when(spy).searchForServers( + Mockito.anyLong(), Mockito.anyLong(), Mockito.isNull(), Mockito.any(Type.class), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.anyLong(), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), + Mockito.isNull(), Mockito.isNull(), Mockito.anyLong()); + + // Mock filterUefiHostsForMigration FIRST to return false (no UEFI-enabled hosts found) + // This simulates the scenario where UEFI VM has no compatible hosts + Pair> uefiFilterResult = new Pair<>(false, null); + Mockito.doReturn(uefiFilterResult).when(spy).filterUefiHostsForMigration( + Mockito.anyList(), Mockito.anyList(), Mockito.any()); + + // Note: No other mocks needed because when filterUefiHostsForMigration returns false, + // the method returns early and doesn't proceed to host allocation or other processing + + Ternary, Integer>, List, Map> result = + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + + // Verify result structure + Assert.assertNotNull("Result should not be null", result); + Assert.assertNotNull("All hosts list should not be null", result.first()); + Assert.assertNotNull("Suitable hosts list should not be null", result.second()); + Assert.assertNotNull("Storage motion map should not be null", result.third()); + + // Verify all hosts are still returned (from searchForServers) + Assert.assertEquals("Should still return 2 total hosts", Integer.valueOf(2), result.first().second()); + Assert.assertEquals("All hosts list should contain 2 hosts", 2, result.first().first().size()); + + // Verify suitable hosts list is empty due to UEFI filtering + Assert.assertEquals("Should have 0 suitable hosts after UEFI filtering", 0, result.second().size()); + + // Verify storage motion map is empty + Assert.assertTrue("Storage motion map should be empty when no suitable hosts", result.third().isEmpty()); + } + + @Test + public void testListHostsForMigrationOfVMStorageMotionCapabilityCheck() { + // Test User VM with VMware - should check storage motion for User VMs + VMInstanceVO userVm = mockRunningVM(1L, HypervisorType.VMware); + Mockito.when(userVm.getType()).thenReturn(VirtualMachine.Type.User); + + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(userVm); + Mockito.when(serviceOfferingDetailsDao.findDetail(userVm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) + .thenReturn(null); + + HostVO srcHost = mockHost(100L, 1L, 1L, 1L, HypervisorType.VMware); + Mockito.when(hostDao.findById(userVm.getHostId())).thenReturn(srcHost); + + Mockito.when(hypervisorCapabilitiesDao.isStorageMotionSupported(HypervisorType.VMware, null)) + .thenReturn(true); + + ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(offeringDao.findById(userVm.getId(), userVm.getServiceOfferingId())).thenReturn(offering); + + VolumeVO volume = mockVolume(1L, 1L); + Mockito.when(volumeDao.findCreatedByInstance(userVm.getId())).thenReturn(List.of(volume)); + + DiskOfferingVO diskOffering = mockSharedDiskOffering(1L); + Mockito.when(diskOfferingDao.findById(volume.getDiskOfferingId())).thenReturn(diskOffering); + + // Mock searchForServers with zone-wide scope (storage motion enabled) + HostVO host1 = mockHost(101L, 1L, 1L, 1L, HypervisorType.VMware); + HostVO host2 = mockHost(102L, 1L, 1L, 1L, HypervisorType.VMware); + List hosts = List.of(host1, host2); + Pair, Integer> hostsPair = new Pair<>(hosts, 2); + Mockito.doReturn(hostsPair).when(spy).searchForServers( + Mockito.anyLong(), Mockito.anyLong(), Mockito.isNull(), Mockito.any(Type.class), + Mockito.isNull(), Mockito.anyLong(), Mockito.isNull(), Mockito.isNull(), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), + Mockito.any(HypervisorType.class), Mockito.isNull(), Mockito.anyLong()); + + setupMigrationMocks(userVm, srcHost, hosts, volume, true); + + Ternary, Integer>, List, Map> result = + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + + // Verify storage motion capability was checked for User VM + Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.VMware, null); + + // Verify response data + Assert.assertNotNull(result); + Assert.assertEquals("Should return 2 total hosts", Integer.valueOf(2), result.first().second()); + Assert.assertEquals("Should have 2 suitable hosts", 2, result.second().size()); + Assert.assertTrue("Host 101 should be in suitable list", + result.second().stream().anyMatch(h -> h.getId() == 101L)); + Assert.assertTrue("Host 102 should be in suitable list", + result.second().stream().anyMatch(h -> h.getId() == 102L)); + } + + @Test + public void testListHostsForMigrationOfVMWithAllSupportedHypervisors() { + // Test each supported hypervisor type + HypervisorType[] supportedTypes = { + HypervisorType.XenServer, + HypervisorType.VMware, + HypervisorType.KVM, + HypervisorType.Ovm, + HypervisorType.Hyperv, + HypervisorType.Ovm3 + }; + + for (HypervisorType hypervisorType : supportedTypes) { + VMInstanceVO vm = mockRunningVM(1L, hypervisorType); + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) + .thenReturn(null); + + HostVO srcHost = mockHost(100L, 1L, 1L, 1L, hypervisorType); + Mockito.when(hostDao.findById(vm.getHostId())).thenReturn(srcHost); + + String version = hypervisorType == HypervisorType.KVM ? "" : null; + Mockito.when(hypervisorCapabilitiesDao.isStorageMotionSupported(hypervisorType, version)) + .thenReturn(false); + + ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(offeringDao.findById(vm.getId(), vm.getServiceOfferingId())).thenReturn(offering); + + VolumeVO volume = mockVolume(1L, 1L); + Mockito.when(volumeDao.findCreatedByInstance(vm.getId())).thenReturn(List.of(volume)); + + DiskOfferingVO diskOffering = mockSharedDiskOffering(1L); + Mockito.when(diskOfferingDao.findById(volume.getDiskOfferingId())).thenReturn(diskOffering); + + // Mock searchForServers for cluster-scoped search + HostVO host1 = mockHost(101L, 1L, 1L, 1L, hypervisorType); + HostVO host2 = mockHost(102L, 1L, 1L, 1L, hypervisorType); + List hosts = List.of(host1, host2); + Pair, Integer> hostsPair = new Pair<>(hosts, 2); + Mockito.doReturn(hostsPair).when(spy).searchForServers( + Mockito.anyLong(), Mockito.anyLong(), Mockito.isNull(), Mockito.any(Type.class), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.anyLong(), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), + Mockito.isNull(), Mockito.isNull(), Mockito.anyLong()); + + setupMigrationMocks(vm, srcHost, hosts, volume); + + Ternary, Integer>, List, Map> result = + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + + // Verify hypervisor is in supported hypervisors list + Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(hypervisorType, version); + + // Verify validation passed for this hypervisor + Assert.assertNotNull("Result should not be null for " + hypervisorType, result); + Assert.assertEquals("Should return 2 total hosts for " + hypervisorType, + Integer.valueOf(2), result.first().second()); + Assert.assertEquals("Should have 2 suitable hosts for " + hypervisorType, + 2, result.second().size()); + Assert.assertTrue("Host 101 should be available for " + hypervisorType, + result.second().stream().anyMatch(h -> h.getId() == 101L)); + Assert.assertTrue("Host 102 should be available for " + hypervisorType, + result.second().stream().anyMatch(h -> h.getId() == 102L)); + + // Reset mocks for next iteration + Mockito.reset(vmInstanceDao, hostDao, serviceOfferingDetailsDao, volumeDao, + diskOfferingDao, hypervisorCapabilitiesDao, offeringDao, dpdkHelper, + affinityGroupVMMapDao, dpMgr, dcDao, hostAllocator, dataStoreManager); + Mockito.reset(spy); + } + } + + @Test(expected = InvalidParameterValueException.class) + public void testListHostsForMigrationOfVMSourceHostNotFound() { + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.KVM); + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) + .thenReturn(null); + Mockito.when(hostDao.findById(vm.getHostId())).thenReturn(null); + + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + } + + @Test(expected = InvalidParameterValueException.class) + public void testListHostsForMigrationOfVMLocalStorageNoStorageMotion() { + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.XenServer); + HostVO srcHost = mockHost(100L, 1L, 1L, 1L, HypervisorType.XenServer); + Account caller = mockRootAdminAccount(); + + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) + .thenReturn(null); + Mockito.when(hostDao.findById(vm.getHostId())).thenReturn(srcHost); + + // Mock storage motion not supported + Mockito.when(hypervisorCapabilitiesDao.isStorageMotionSupported(HypervisorType.XenServer, null)) + .thenReturn(false); + + // Mock local storage usage + ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(offeringDao.findById(vm.getId(), vm.getServiceOfferingId())).thenReturn(offering); + + VolumeVO volume = mockVolume(1L, 1L); + Mockito.when(volumeDao.findCreatedByInstance(vm.getId())).thenReturn(List.of(volume)); + + DiskOfferingVO diskOffering = mockLocalDiskOffering(1L); + Mockito.when(diskOfferingDao.findById(volume.getDiskOfferingId())).thenReturn(diskOffering); + + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + } + + @Test + public void testListHostsForMigrationOfVMStorageMotionCheckForSystemVM() { + // Test that storage motion capability is checked for System VMs + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.VMware); + Mockito.when(vm.getType()).thenReturn(VirtualMachine.Type.ConsoleProxy); + + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) + .thenReturn(null); + + HostVO srcHost = mockHost(100L, 1L, 1L, 1L, HypervisorType.VMware); + Mockito.when(hostDao.findById(vm.getHostId())).thenReturn(srcHost); + + // Storage motion supported for VMware + Mockito.when(hypervisorCapabilitiesDao.isStorageMotionSupported(HypervisorType.VMware, null)) + .thenReturn(true); + + ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(offeringDao.findById(vm.getId(), vm.getServiceOfferingId())).thenReturn(offering); + + VolumeVO volume = mockVolume(1L, 1L); + Mockito.when(volumeDao.findCreatedByInstance(vm.getId())).thenReturn(List.of(volume)); + + DiskOfferingVO diskOffering = mockSharedDiskOffering(1L); + Mockito.when(diskOfferingDao.findById(volume.getDiskOfferingId())).thenReturn(diskOffering); + + // Mock searchForServers with zone-wide scope (storage motion enabled) + HostVO host1 = mockHost(101L, 1L, 1L, 1L, HypervisorType.VMware); + HostVO host2 = mockHost(102L, 1L, 1L, 1L, HypervisorType.VMware); + List hosts = List.of(host1, host2); + Pair, Integer> hostsPair = new Pair<>(hosts, 2); + Mockito.doReturn(hostsPair).when(spy).searchForServers( + Mockito.anyLong(), Mockito.anyLong(), Mockito.isNull(), Mockito.any(Type.class), + Mockito.isNull(), Mockito.anyLong(), Mockito.anyLong(), Mockito.isNull(), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), + Mockito.any(HypervisorType.class), Mockito.isNull(), Mockito.anyLong()); + + setupMigrationMocks(vm, srcHost, hosts, volume, true); + + Ternary, Integer>, List, Map> result = + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + + // Verify that storage motion capability was checked for system VM (VMware is in hypervisorTypes list) + Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.VMware, null); + + // Verify response structure + Assert.assertNotNull(result); + Assert.assertEquals("Should have 2 total hosts", Integer.valueOf(2), result.first().second()); + Assert.assertTrue("Should have suitable hosts", result.second().size() > 0); + } + + @Test + public void testListHostsForMigrationOfVMStorageMotionCheckForUserVM() { + // Test that storage motion capability is checked for User VMs + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.KVM); + Mockito.when(vm.getType()).thenReturn(VirtualMachine.Type.User); + + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) + .thenReturn(null); + + HostVO srcHost = mockHost(100L, 1L, 1L, 1L, HypervisorType.KVM); + Mockito.when(hostDao.findById(vm.getHostId())).thenReturn(srcHost); + + // Storage motion supported for User VM with KVM + Mockito.when(hypervisorCapabilitiesDao.isStorageMotionSupported(HypervisorType.KVM, "")) + .thenReturn(true); + + ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(offeringDao.findById(vm.getId(), vm.getServiceOfferingId())).thenReturn(offering); + + VolumeVO volume = mockVolume(1L, 1L); + Mockito.when(volumeDao.findCreatedByInstance(vm.getId())).thenReturn(List.of(volume)); + + DiskOfferingVO diskOffering = mockSharedDiskOffering(1L); + Mockito.when(diskOfferingDao.findById(volume.getDiskOfferingId())).thenReturn(diskOffering); + + // Mock searchForServers with zone-wide scope (storage motion enabled) + HostVO host1 = mockHost(101L, 1L, 1L, 1L, HypervisorType.KVM); + HostVO host2 = mockHost(102L, 1L, 1L, 1L, HypervisorType.KVM); + List hosts = List.of(host1, host2); + Pair, Integer> hostsPair = new Pair<>(hosts, 2); + Mockito.doReturn(hostsPair).when(spy).searchForServers( + Mockito.anyLong(), Mockito.anyLong(), Mockito.isNull(), Mockito.any(Type.class), + Mockito.isNull(), Mockito.anyLong(), Mockito.isNull(), Mockito.isNull(), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), + Mockito.any(HypervisorType.class), Mockito.isNull(), Mockito.anyLong()); + + setupMigrationMocks(vm, srcHost, hosts, volume, true); + + Ternary, Integer>, List, Map> result = + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + + // Verify User VM can migrate with storage (User VM type always checks) + Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.KVM, ""); + + // Verify response data + Assert.assertNotNull(result); + Assert.assertEquals("Should have 2 total hosts", Integer.valueOf(2), result.first().second()); + Assert.assertTrue("Should have suitable hosts", result.second().size() > 0); + } + + @Test + public void testListHostsForMigrationOfVMWithoutStorageMotionClusterScope() { + // When storage motion not supported, should search only in same cluster + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.XenServer); + Mockito.when(vm.getType()).thenReturn(VirtualMachine.Type.User); + + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) + .thenReturn(null); + + HostVO srcHost = mockHost(100L, 1L, 1L, 1L, HypervisorType.XenServer); + Mockito.when(hostDao.findById(vm.getHostId())).thenReturn(srcHost); + + // No storage motion support + Mockito.when(hypervisorCapabilitiesDao.isStorageMotionSupported(HypervisorType.XenServer, null)) + .thenReturn(false); + + ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(offeringDao.findById(vm.getId(), vm.getServiceOfferingId())).thenReturn(offering); + + VolumeVO volume = mockVolume(1L, 1L); + Mockito.when(volumeDao.findCreatedByInstance(vm.getId())).thenReturn(List.of(volume)); + + DiskOfferingVO diskOffering = mockSharedDiskOffering(1L); + Mockito.when(diskOfferingDao.findById(volume.getDiskOfferingId())).thenReturn(diskOffering); + + // Mock searchForServers - verify cluster scope is used + HostVO host1 = mockHost(101L, 1L, 1L, 1L, HypervisorType.XenServer); + HostVO host2 = mockHost(102L, 1L, 1L, 1L, HypervisorType.XenServer); + List hosts = List.of(host1, host2); + Pair, Integer> hostsPair = new Pair<>(hosts, 2); + Mockito.doReturn(hostsPair).when(spy).searchForServers( + Mockito.eq(0L), Mockito.eq(20L), Mockito.isNull(), Mockito.any(Type.class), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.eq(1L), // cluster=1L + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), + Mockito.isNull(), Mockito.isNull(), Mockito.eq(100L)); + + setupMigrationMocks(vm, srcHost, hosts, volume); + + Ternary, Integer>, List, Map> result = + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + + // Verify XenServer without storage motion was checked + Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.XenServer, null); + // Verify cluster-scoped search was used (not zone-wide) + Mockito.verify(spy).searchForServers( + Mockito.eq(0L), Mockito.eq(20L), Mockito.isNull(), Mockito.any(Type.class), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.eq(1L), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), + Mockito.isNull(), Mockito.isNull(), Mockito.eq(100L)); + + // Verify response data + Assert.assertNotNull(result); + Assert.assertEquals("Should return 2 total hosts", Integer.valueOf(2), result.first().second()); + Assert.assertEquals("Should have 2 suitable hosts", 2, result.second().size()); + Assert.assertTrue("Should contain host 101", + result.second().stream().anyMatch(h -> h.getId() == 101L)); + Assert.assertTrue("Should contain host 102", + result.second().stream().anyMatch(h -> h.getId() == 102L)); + } + + @Test + public void testListHostsForMigrationOfVMWithNoVolumes() { + // Edge case: VM with no volumes + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.KVM); + + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) + .thenReturn(null); + + HostVO srcHost = mockHost(100L, 1L, 1L, 1L, HypervisorType.KVM); + Mockito.when(hostDao.findById(vm.getHostId())).thenReturn(srcHost); + + Mockito.when(hypervisorCapabilitiesDao.isStorageMotionSupported(HypervisorType.KVM, "")) + .thenReturn(false); + + ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(offeringDao.findById(vm.getId(), vm.getServiceOfferingId())).thenReturn(offering); + + // No volumes + Mockito.when(volumeDao.findCreatedByInstance(vm.getId())).thenReturn(new ArrayList<>()); + + // Mock searchForServers for cluster-scoped search + HostVO host1 = mockHost(101L, 1L, 1L, 1L, HypervisorType.KVM); + HostVO host2 = mockHost(102L, 1L, 1L, 1L, HypervisorType.KVM); + List hosts = List.of(host1, host2); + Pair, Integer> hostsPair = new Pair<>(hosts, 2); + Mockito.doReturn(hostsPair).when(spy).searchForServers( + Mockito.anyLong(), Mockito.anyLong(), Mockito.isNull(), Mockito.any(Type.class), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.anyLong(), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), + Mockito.isNull(), Mockito.isNull(), Mockito.anyLong()); + + // Set up mocks without volume since there are no volumes + Pair> uefiResult = new Pair<>(true, hosts); + Mockito.doReturn(uefiResult).when(spy).filterUefiHostsForMigration( + Mockito.anyList(), Mockito.anyList(), Mockito.any()); + Mockito.when(dpdkHelper.isVMDpdkEnabled(vm.getId())).thenReturn(false); + Mockito.when(affinityGroupVMMapDao.countAffinityGroupsForVm(vm.getId())).thenReturn(0L); + DataCenterVO dc = Mockito.mock(DataCenterVO.class); + Mockito.when(dcDao.findById(1L)).thenReturn(dc); + Mockito.doNothing().when(dpMgr).checkForNonDedicatedResources(Mockito.any(), Mockito.any(), Mockito.any()); + Mockito.doNothing().when(dpMgr).reorderHostsByPriority(Mockito.any(), Mockito.anyList()); + + Mockito.when(hostAllocator.allocateTo(Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.anyList(), Mockito.anyInt(), Mockito.anyBoolean())) + .thenReturn(new ArrayList<>(hosts)); + + Ternary, Integer>, List, Map> result = + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + + // Should still process without throwing exception for usesLocal check + Mockito.verify(volumeDao).findCreatedByInstance(vm.getId()); + + // Verify response + Assert.assertNotNull(result); + Assert.assertEquals("Should have 2 total hosts", Integer.valueOf(2), result.first().second()); + Assert.assertEquals("Should have 2 suitable hosts even with no volumes", 2, result.second().size()); + Assert.assertTrue("Storage motion map should be empty for VM with no volumes", + result.third().isEmpty()); + } + + @Test + public void testListHostsForMigrationOfVMOverloadedMethod() { + // Test the overloaded method that takes vmId instead of vm object + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.KVM); + + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) + .thenReturn(null); + + HostVO srcHost = mockHost(100L, 1L, 1L, 1L, HypervisorType.KVM); + Mockito.when(hostDao.findById(vm.getHostId())).thenReturn(srcHost); + + Mockito.when(hypervisorCapabilitiesDao.isStorageMotionSupported(HypervisorType.KVM, "")) + .thenReturn(false); + + ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(offeringDao.findById(vm.getId(), vm.getServiceOfferingId())).thenReturn(offering); + + VolumeVO volume = mockVolume(1L, 1L); + Mockito.when(volumeDao.findCreatedByInstance(vm.getId())).thenReturn(List.of(volume)); + + DiskOfferingVO diskOffering = mockSharedDiskOffering(1L); + Mockito.when(diskOfferingDao.findById(volume.getDiskOfferingId())).thenReturn(diskOffering); + + // Mock searchForServers for cluster-scoped search with keyword + HostVO host1 = mockHost(101L, 1L, 1L, 1L, HypervisorType.KVM); + HostVO host2 = mockHost(102L, 1L, 1L, 1L, HypervisorType.KVM); + List hosts = List.of(host1, host2); + Pair, Integer> hostsPair = new Pair<>(hosts, 2); + Mockito.doReturn(hostsPair).when(spy).searchForServers( + Mockito.anyLong(), Mockito.anyLong(), Mockito.isNull(), Mockito.any(Type.class), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.anyLong(), + Mockito.isNull(), Mockito.eq("keyword-test"), Mockito.isNull(), Mockito.isNull(), + Mockito.isNull(), Mockito.isNull(), Mockito.anyLong()); + + setupMigrationMocks(vm, srcHost, hosts, volume); + + // Call overloaded method with vmId + Ternary, Integer>, List, Map> result = + spy.listHostsForMigrationOfVM(1L, 0L, 20L, "keyword-test"); + + // Verify VM was fetched by ID + Mockito.verify(vmInstanceDao).findById(1L); + + // Verify keyword was passed to searchForServers + Mockito.verify(spy).searchForServers( + Mockito.anyLong(), Mockito.anyLong(), Mockito.isNull(), Mockito.any(Type.class), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.anyLong(), + Mockito.isNull(), Mockito.eq("keyword-test"), Mockito.isNull(), Mockito.isNull(), + Mockito.isNull(), Mockito.isNull(), Mockito.anyLong()); + + // Verify response data + Assert.assertNotNull(result); + Assert.assertEquals("Should have 2 hosts", Integer.valueOf(2), result.first().second()); + Assert.assertEquals("Should have 2 suitable hosts", 2, result.second().size()); + } + + @Test + public void testListHostsForMigrationOfVMVmwareStorageMotionCheck() { + // VMware should check storage motion even for non-User VMs + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.VMware); + Mockito.when(vm.getType()).thenReturn(VirtualMachine.Type.DomainRouter); + + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) + .thenReturn(null); + + HostVO srcHost = mockHost(100L, 1L, 1L, 1L, HypervisorType.VMware); + Mockito.when(hostDao.findById(vm.getHostId())).thenReturn(srcHost); + + // VMware with DomainRouter should still check storage motion + Mockito.when(hypervisorCapabilitiesDao.isStorageMotionSupported(HypervisorType.VMware, null)) + .thenReturn(true); + + ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(offeringDao.findById(vm.getId(), vm.getServiceOfferingId())).thenReturn(offering); + + VolumeVO volume = mockVolume(1L, 1L); + Mockito.when(volumeDao.findCreatedByInstance(vm.getId())).thenReturn(List.of(volume)); + + DiskOfferingVO diskOffering = mockSharedDiskOffering(1L); + Mockito.when(diskOfferingDao.findById(volume.getDiskOfferingId())).thenReturn(diskOffering); + + // Mock searchForServers with zone-wide scope (storage motion enabled) + HostVO host1 = mockHost(101L, 1L, 1L, 1L, HypervisorType.VMware); + HostVO host2 = mockHost(102L, 1L, 1L, 1L, HypervisorType.VMware); + List hosts = List.of(host1, host2); + Pair, Integer> hostsPair = new Pair<>(hosts, 2); + Mockito.doReturn(hostsPair).when(spy).searchForServers( + Mockito.anyLong(), Mockito.anyLong(), Mockito.isNull(), Mockito.any(Type.class), + Mockito.isNull(), Mockito.anyLong(), Mockito.anyLong(), Mockito.isNull(), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), + Mockito.any(HypervisorType.class), Mockito.isNull(), Mockito.anyLong()); + + setupMigrationMocks(vm, srcHost, hosts, volume, true); + + Ternary, Integer>, List, Map> result = + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + + // Verify VMware always checks storage motion (hypervisorTypes list includes VMware) + Mockito.verify(hypervisorCapabilitiesDao).isStorageMotionSupported(HypervisorType.VMware, null); + + // Verify response + Assert.assertNotNull(result); + Assert.assertEquals("Should return 2 total hosts", Integer.valueOf(2), result.first().second()); + Assert.assertEquals("Should have 2 suitable hosts", 2, result.second().size()); + Assert.assertTrue("Host 101 should be in the list", + result.second().stream().anyMatch(h -> h.getId() == 101L)); + } + + + @Test + public void testListHostsForMigrationOfVMWithNullKeyword() { + // Test with null keyword parameter + VMInstanceVO vm = mockRunningVM(1L, HypervisorType.KVM); + + Account caller = mockRootAdminAccount(); + Mockito.doReturn(caller).when(spy).getCaller(); + Mockito.when(vmInstanceDao.findById(1L)).thenReturn(vm); + Mockito.when(serviceOfferingDetailsDao.findDetail(vm.getServiceOfferingId(), GPU.Keys.pciDevice.toString())) + .thenReturn(null); + + HostVO srcHost = mockHost(100L, 1L, 1L, 1L, HypervisorType.KVM); + Mockito.when(hostDao.findById(vm.getHostId())).thenReturn(srcHost); + + Mockito.when(hypervisorCapabilitiesDao.isStorageMotionSupported(HypervisorType.KVM, "")) + .thenReturn(false); + + ServiceOfferingVO offering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(offeringDao.findById(vm.getId(), vm.getServiceOfferingId())).thenReturn(offering); + + VolumeVO volume = mockVolume(1L, 1L); + Mockito.when(volumeDao.findCreatedByInstance(vm.getId())).thenReturn(List.of(volume)); + + DiskOfferingVO diskOffering = mockSharedDiskOffering(1L); + Mockito.when(diskOfferingDao.findById(volume.getDiskOfferingId())).thenReturn(diskOffering); + + // Mock searchForServers for cluster-scoped search + HostVO host1 = mockHost(101L, 1L, 1L, 1L, HypervisorType.KVM); + HostVO host2 = mockHost(102L, 1L, 1L, 1L, HypervisorType.KVM); + List hosts = List.of(host1, host2); + Pair, Integer> hostsPair = new Pair<>(hosts, 2); + Mockito.doReturn(hostsPair).when(spy).searchForServers( + Mockito.anyLong(), Mockito.anyLong(), Mockito.isNull(), Mockito.any(Type.class), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.anyLong(), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), + Mockito.isNull(), Mockito.isNull(), Mockito.anyLong()); + + setupMigrationMocks(vm, srcHost, hosts, volume); + + Ternary, Integer>, List, Map> result = + spy.listHostsForMigrationOfVM(1L, 0L, 20L, null); + + // Verify null keyword is handled + Mockito.verify(vmInstanceDao).findById(1L); + + // Verify searchForServers was called with null keyword + Mockito.verify(spy).searchForServers( + Mockito.anyLong(), Mockito.anyLong(), Mockito.isNull(), Mockito.any(Type.class), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.anyLong(), + Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), Mockito.isNull(), + Mockito.isNull(), Mockito.isNull(), Mockito.anyLong()); + + // Verify response data + Assert.assertNotNull(result); + Assert.assertEquals("Should return 2 total hosts", Integer.valueOf(2), result.first().second()); + Assert.assertEquals("Should have 2 suitable hosts", 2, result.second().size()); + Assert.assertTrue("Host 101 should be available", + result.second().stream().anyMatch(h -> h.getId() == 101L)); + Assert.assertTrue("Host 102 should be available", + result.second().stream().anyMatch(h -> h.getId() == 102L)); + } + + // Note: Tests for success scenarios with complex flows (managed storage, zone-wide volumes, + // DPDK exclusion, affinity groups, architecture filtering) require full setup with mocking + // private methods like hasSuitablePoolsForVolume(), excludeNonDPDKEnabledHosts(), and + // filterUefiHostsForMigration() which are better suited for integration tests. + + // ============= Helper methods for tests ============= + + /** + * Sets up common mocks for successful migration tests + * For storage motion tests, set forceStorageMotion=true to configure volume in same cluster + * (which avoids complex filtering logic for cross-cluster storage motion) + */ + private void setupMigrationMocks(VMInstanceVO vm, HostVO srcHost, + List targetHosts, VolumeVO volume) { + setupMigrationMocks(vm, srcHost, targetHosts, volume, false); + } + + private void setupMigrationMocks(VMInstanceVO vm, HostVO srcHost, + List targetHosts, VolumeVO volume, + boolean forceStorageMotion) { + // Mock dataStoreManager for volume pool lookup (lenient as not used in all paths) + // For storage motion tests, put volume in same cluster to avoid complex filtering + PrimaryDataStore primaryDataStore = Mockito.mock(PrimaryDataStore.class); + Mockito.when(dataStoreManager.getPrimaryDataStore(volume.getPoolId())).thenReturn(primaryDataStore); + // If not forceStorageMotion, volume is in same cluster (no storage motion needed) + // If forceStorageMotion, set volClusterId to null (zone-wide storage) + Mockito.when(primaryDataStore.getClusterId()).thenReturn(forceStorageMotion ? null : srcHost.getClusterId()); + + // Mock zoneWideVolumeRequiresStorageMotion for zone-wide volumes + if (forceStorageMotion) { + Mockito.doReturn(false).when(spy).zoneWideVolumeRequiresStorageMotion( + Mockito.any(), Mockito.any(), Mockito.any()); + } + + // Mock filterUefiHostsForMigration - must return hosts properly + Pair> uefiResult = new Pair<>(true, new ArrayList<>(targetHosts)); + Mockito.doReturn(uefiResult).when(spy).filterUefiHostsForMigration( + Mockito.anyList(), Mockito.anyList(), Mockito.any()); + + // Mock DPDK check + Mockito.when(dpdkHelper.isVMDpdkEnabled(vm.getId())).thenReturn(false); + + // Mock affinity group count + Mockito.when(affinityGroupVMMapDao.countAffinityGroupsForVm(vm.getId())).thenReturn(0L); + + // Mock datacenter + DataCenterVO dc = Mockito.mock(DataCenterVO.class); + Mockito.when(dcDao.findById(srcHost.getDataCenterId())).thenReturn(dc); + + // Mock dedicated resources check + Mockito.doNothing().when(dpMgr).checkForNonDedicatedResources( + Mockito.any(), Mockito.any(), Mockito.any()); + + // Mock priority reordering + Mockito.doNothing().when(dpMgr).reorderHostsByPriority(Mockito.any(), Mockito.anyList()); + + // Mock host allocators - both signatures + // 1. Version with filteredHosts list (used when canMigrateWithStorage = true) + Mockito.when(hostAllocator.allocateTo(Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.anyList(), Mockito.anyInt(), Mockito.anyBoolean())) + .thenReturn(new ArrayList<>(targetHosts)); + } + + private VMInstanceVO mockRunningVM(Long id, HypervisorType hypervisorType) { + return mockVM(id, hypervisorType, State.Running); + } + + private VMInstanceVO mockVM(Long id, HypervisorType hypervisorType, State state) { + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + when(vm.getId()).thenReturn(id); + when(vm.getState()).thenReturn(state); + when(vm.getHypervisorType()).thenReturn(hypervisorType); + when(vm.getHostId()).thenReturn(100L); + when(vm.getServiceOfferingId()).thenReturn(1L); + when(vm.getType()).thenReturn(VirtualMachine.Type.User); + when(vm.getUuid()).thenReturn("uuid-" + id); + when(vm.getDataCenterId()).thenReturn(1L); + return vm; + } + + private Account mockRootAdminAccount() { + Account account = Mockito.mock(Account.class); + Mockito.when(account.getId()).thenReturn(1L); + Mockito.when(_accountMgr.isRootAdmin(1L)).thenReturn(true); + return account; + } + + private HostVO mockHost(Long id, Long clusterId, Long podId, Long dcId, HypervisorType hypervisorType) { + HostVO host = new HostVO("guid-" + id); + ReflectionTestUtils.setField(host, "id", id); + ReflectionTestUtils.setField(host, "clusterId", clusterId); + ReflectionTestUtils.setField(host, "podId", podId); + ReflectionTestUtils.setField(host, "dataCenterId", dcId); + ReflectionTestUtils.setField(host, "hypervisorType", hypervisorType); + ReflectionTestUtils.setField(host, "type", Host.Type.Routing); + ReflectionTestUtils.setField(host, "hypervisorVersion", null); + return host; + } + + private VolumeVO mockVolume(Long id, Long poolId) { + VolumeVO volume = Mockito.mock(VolumeVO.class); + when(volume.getId()).thenReturn(id); + when(volume.getPoolId()).thenReturn(poolId); + when(volume.getDiskOfferingId()).thenReturn(1L); + when(volume.getVolumeType()).thenReturn(Volume.Type.ROOT); + return volume; + } + + private DiskOfferingVO mockLocalDiskOffering(Long id) { + DiskOfferingVO diskOffering = Mockito.mock(DiskOfferingVO.class); + Mockito.when(diskOffering.getId()).thenReturn(id); + Mockito.when(diskOffering.isUseLocalStorage()).thenReturn(true); + return diskOffering; + } + + private DiskOfferingVO mockSharedDiskOffering(Long id) { + DiskOfferingVO diskOffering = Mockito.mock(DiskOfferingVO.class); + Mockito.when(diskOffering.getId()).thenReturn(id); + Mockito.when(diskOffering.isUseLocalStorage()).thenReturn(false); + return diskOffering; + } } diff --git a/server/src/test/java/org/apache/cloudstack/cluster/ClusterDrsServiceImplTest.java b/server/src/test/java/org/apache/cloudstack/cluster/ClusterDrsServiceImplTest.java index 81aac9e4b54d..b76ef51c5234 100644 --- a/server/src/test/java/org/apache/cloudstack/cluster/ClusterDrsServiceImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/cluster/ClusterDrsServiceImplTest.java @@ -42,7 +42,9 @@ import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.VMInstanceVO; import com.cloud.vm.VirtualMachine; +import com.cloud.vm.VmDetailConstants; import com.cloud.vm.dao.VMInstanceDao; +import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao; import org.apache.cloudstack.api.command.admin.cluster.GenerateClusterDrsPlanCmd; import org.apache.cloudstack.api.response.ClusterDrsPlanMigrationResponse; import org.apache.cloudstack.api.response.ClusterDrsPlanResponse; @@ -116,6 +118,9 @@ public class ClusterDrsServiceImplTest { @Mock private VMInstanceDao vmInstanceDao; + @Mock + private AffinityGroupVMMapDao affinityGroupVMMapDao; + @Spy @InjectMocks private ClusterDrsServiceImpl clusterDrsService = new ClusterDrsServiceImpl(); @@ -168,9 +173,14 @@ public void testGetDrsPlan() throws ConfigurationException { VMInstanceVO vm1 = Mockito.mock(VMInstanceVO.class); Mockito.when(vm1.getId()).thenReturn(1L); Mockito.when(vm1.getHostId()).thenReturn(1L); + Mockito.when(vm1.getType()).thenReturn(VirtualMachine.Type.User); + Mockito.when(vm1.getState()).thenReturn(VirtualMachine.State.Running); VMInstanceVO vm2 = Mockito.mock(VMInstanceVO.class); Mockito.when(vm2.getHostId()).thenReturn(2L); + Mockito.when(vm2.getId()).thenReturn(2L); + Mockito.when(vm2.getType()).thenReturn(VirtualMachine.Type.User); + Mockito.when(vm2.getState()).thenReturn(VirtualMachine.State.Running); List hostList = new ArrayList<>(); hostList.add(host1); @@ -201,10 +211,11 @@ public void testGetDrsPlan() throws ConfigurationException { Mockito.when(vmInstanceDao.listByClusterId(1L)).thenReturn(vmList); Mockito.when(balancedAlgorithm.needsDrs(Mockito.any(), Mockito.anyList(), Mockito.anyList())).thenReturn( true, false); - Mockito.when( - clusterDrsService.getBestMigration(Mockito.any(Cluster.class), Mockito.any(ClusterDrsAlgorithm.class), - Mockito.anyList(), Mockito.anyMap(), Mockito.anyMap(), Mockito.anyMap())).thenReturn( - new Pair<>(vm1, host2)); + + Mockito.doReturn(new Pair<>(vm1, host2)).when(clusterDrsService).getBestMigration( + Mockito.any(Cluster.class), Mockito.any(ClusterDrsAlgorithm.class), + Mockito.anyList(), Mockito.anyMap(), Mockito.anyMap(), Mockito.anyMap(), + Mockito.anyMap(), Mockito.anyMap(), Mockito.anyMap()); Mockito.when(serviceOfferingDao.findByIdIncludingRemoved(Mockito.anyLong(), Mockito.anyLong())).thenReturn( serviceOffering); Mockito.when(hostJoinDao.searchByIds(host1.getId(), host2.getId())).thenReturn(List.of(hostJoin1, hostJoin2)); @@ -219,6 +230,445 @@ public void testGetDrsPlan() throws ConfigurationException { assertEquals(1, iterations.size()); } + @Test + public void testGetDrsPlanWithDisabledCluster() throws ConfigurationException { + ClusterVO cluster = Mockito.mock(ClusterVO.class); + Mockito.when(cluster.getAllocationState()).thenReturn(Grouping.AllocationState.Disabled); + + List> result = clusterDrsService.getDrsPlan(cluster, 5); + assertEquals(0, result.size()); + } + + @Test + public void testGetDrsPlanWithZeroMaxIterations() throws ConfigurationException { + ClusterVO cluster = Mockito.mock(ClusterVO.class); + Mockito.when(cluster.getAllocationState()).thenReturn(Grouping.AllocationState.Enabled); + + List> result = clusterDrsService.getDrsPlan(cluster, 0); + assertEquals(0, result.size()); + } + + @Test + public void testGetDrsPlanWithNegativeMaxIterations() throws ConfigurationException { + ClusterVO cluster = Mockito.mock(ClusterVO.class); + Mockito.when(cluster.getAllocationState()).thenReturn(Grouping.AllocationState.Enabled); + + List> result = clusterDrsService.getDrsPlan(cluster, -1); + assertEquals(0, result.size()); + } + + @Test + public void testGetDrsPlanWithSystemVMs() throws ConfigurationException { + ClusterVO cluster = Mockito.mock(ClusterVO.class); + Mockito.when(cluster.getId()).thenReturn(1L); + Mockito.when(cluster.getAllocationState()).thenReturn(Grouping.AllocationState.Enabled); + + HostVO host1 = Mockito.mock(HostVO.class); + Mockito.when(host1.getId()).thenReturn(1L); + + VMInstanceVO systemVm = Mockito.mock(VMInstanceVO.class); + Mockito.when(systemVm.getId()).thenReturn(1L); + Mockito.when(systemVm.getHostId()).thenReturn(1L); + Mockito.when(systemVm.getType()).thenReturn(VirtualMachine.Type.SecondaryStorageVm); + + List hostList = new ArrayList<>(); + hostList.add(host1); + + List vmList = new ArrayList<>(); + vmList.add(systemVm); + + HostJoinVO hostJoin1 = Mockito.mock(HostJoinVO.class); + Mockito.when(hostJoin1.getId()).thenReturn(1L); + Mockito.when(hostJoin1.getCpuUsedCapacity()).thenReturn(1000L); + Mockito.when(hostJoin1.getCpuReservedCapacity()).thenReturn(0L); + Mockito.when(hostJoin1.getCpus()).thenReturn(4); + Mockito.when(hostJoin1.getSpeed()).thenReturn(1000L); + Mockito.when(hostJoin1.getMemUsedCapacity()).thenReturn(1024L); + Mockito.when(hostJoin1.getMemReservedCapacity()).thenReturn(0L); + Mockito.when(hostJoin1.getTotalMemory()).thenReturn(8192L); + + Mockito.when(hostDao.findByClusterId(1L)).thenReturn(hostList); + Mockito.when(vmInstanceDao.listByClusterId(1L)).thenReturn(vmList); + Mockito.when(balancedAlgorithm.needsDrs(Mockito.any(), Mockito.anyList(), Mockito.anyList())).thenReturn(true); + Mockito.when(hostJoinDao.searchByIds(Mockito.any())).thenReturn(List.of(hostJoin1)); + + List> result = clusterDrsService.getDrsPlan(cluster, 5); + assertEquals(0, result.size()); + } + + @Test + public void testGetDrsPlanWithNonRunningVMs() throws ConfigurationException { + ClusterVO cluster = Mockito.mock(ClusterVO.class); + Mockito.when(cluster.getId()).thenReturn(1L); + Mockito.when(cluster.getAllocationState()).thenReturn(Grouping.AllocationState.Enabled); + + HostVO host1 = Mockito.mock(HostVO.class); + Mockito.when(host1.getId()).thenReturn(1L); + + VMInstanceVO stoppedVm = Mockito.mock(VMInstanceVO.class); + Mockito.when(stoppedVm.getId()).thenReturn(1L); + Mockito.when(stoppedVm.getHostId()).thenReturn(1L); + Mockito.when(stoppedVm.getType()).thenReturn(VirtualMachine.Type.User); + Mockito.when(stoppedVm.getState()).thenReturn(VirtualMachine.State.Stopped); + + List hostList = new ArrayList<>(); + hostList.add(host1); + + List vmList = new ArrayList<>(); + vmList.add(stoppedVm); + + HostJoinVO hostJoin1 = Mockito.mock(HostJoinVO.class); + Mockito.when(hostJoin1.getId()).thenReturn(1L); + Mockito.when(hostJoin1.getCpuUsedCapacity()).thenReturn(1000L); + Mockito.when(hostJoin1.getCpuReservedCapacity()).thenReturn(0L); + Mockito.when(hostJoin1.getCpus()).thenReturn(4); + Mockito.when(hostJoin1.getSpeed()).thenReturn(1000L); + Mockito.when(hostJoin1.getMemUsedCapacity()).thenReturn(1024L); + Mockito.when(hostJoin1.getMemReservedCapacity()).thenReturn(0L); + Mockito.when(hostJoin1.getTotalMemory()).thenReturn(8192L); + + Mockito.when(hostDao.findByClusterId(1L)).thenReturn(hostList); + Mockito.when(vmInstanceDao.listByClusterId(1L)).thenReturn(vmList); + Mockito.when(balancedAlgorithm.needsDrs(Mockito.any(), Mockito.anyList(), Mockito.anyList())).thenReturn(true); + Mockito.when(hostJoinDao.searchByIds(Mockito.any())).thenReturn(List.of(hostJoin1)); + + List> result = clusterDrsService.getDrsPlan(cluster, 5); + assertEquals(0, result.size()); + } + + @Test + public void testGetDrsPlanWithSkipDrsFlag() throws ConfigurationException { + ClusterVO cluster = Mockito.mock(ClusterVO.class); + Mockito.when(cluster.getId()).thenReturn(1L); + Mockito.when(cluster.getAllocationState()).thenReturn(Grouping.AllocationState.Enabled); + + HostVO host1 = Mockito.mock(HostVO.class); + Mockito.when(host1.getId()).thenReturn(1L); + + VMInstanceVO skippedVm = Mockito.mock(VMInstanceVO.class); + Mockito.when(skippedVm.getId()).thenReturn(1L); + Mockito.when(skippedVm.getHostId()).thenReturn(1L); + Mockito.when(skippedVm.getType()).thenReturn(VirtualMachine.Type.User); + Mockito.when(skippedVm.getState()).thenReturn(VirtualMachine.State.Running); + Map details = new HashMap<>(); + details.put(VmDetailConstants.SKIP_DRS, "true"); + Mockito.when(skippedVm.getDetails()).thenReturn(details); + + List hostList = new ArrayList<>(); + hostList.add(host1); + + List vmList = new ArrayList<>(); + vmList.add(skippedVm); + + HostJoinVO hostJoin1 = Mockito.mock(HostJoinVO.class); + Mockito.when(hostJoin1.getId()).thenReturn(1L); + Mockito.when(hostJoin1.getCpuUsedCapacity()).thenReturn(1000L); + Mockito.when(hostJoin1.getCpuReservedCapacity()).thenReturn(0L); + Mockito.when(hostJoin1.getCpus()).thenReturn(4); + Mockito.when(hostJoin1.getSpeed()).thenReturn(1000L); + Mockito.when(hostJoin1.getMemUsedCapacity()).thenReturn(1024L); + Mockito.when(hostJoin1.getMemReservedCapacity()).thenReturn(0L); + Mockito.when(hostJoin1.getTotalMemory()).thenReturn(8192L); + + Mockito.when(hostDao.findByClusterId(1L)).thenReturn(hostList); + Mockito.when(vmInstanceDao.listByClusterId(1L)).thenReturn(vmList); + Mockito.when(balancedAlgorithm.needsDrs(Mockito.any(), Mockito.anyList(), Mockito.anyList())).thenReturn(true); + Mockito.when(hostJoinDao.searchByIds(Mockito.any())).thenReturn(List.of(hostJoin1)); + + List> result = clusterDrsService.getDrsPlan(cluster, 5); + assertEquals(0, result.size()); + } + + @Test + public void testGetDrsPlanWithNoCompatibleHosts() throws ConfigurationException { + ClusterVO cluster = Mockito.mock(ClusterVO.class); + Mockito.when(cluster.getId()).thenReturn(1L); + Mockito.when(cluster.getAllocationState()).thenReturn(Grouping.AllocationState.Enabled); + + HostVO host1 = Mockito.mock(HostVO.class); + Mockito.when(host1.getId()).thenReturn(1L); + + VMInstanceVO vm1 = Mockito.mock(VMInstanceVO.class); + Mockito.when(vm1.getId()).thenReturn(1L); + Mockito.when(vm1.getHostId()).thenReturn(1L); + Mockito.when(vm1.getType()).thenReturn(VirtualMachine.Type.User); + Mockito.when(vm1.getState()).thenReturn(VirtualMachine.State.Running); + Mockito.when(vm1.getDetails()).thenReturn(Collections.emptyMap()); + + List hostList = new ArrayList<>(); + hostList.add(host1); + + List vmList = new ArrayList<>(); + vmList.add(vm1); + + HostJoinVO hostJoin1 = Mockito.mock(HostJoinVO.class); + Mockito.when(hostJoin1.getId()).thenReturn(1L); + Mockito.when(hostJoin1.getCpuUsedCapacity()).thenReturn(1000L); + Mockito.when(hostJoin1.getCpuReservedCapacity()).thenReturn(0L); + Mockito.when(hostJoin1.getCpus()).thenReturn(4); + Mockito.when(hostJoin1.getSpeed()).thenReturn(1000L); + Mockito.when(hostJoin1.getMemUsedCapacity()).thenReturn(1024L); + Mockito.when(hostJoin1.getMemReservedCapacity()).thenReturn(0L); + Mockito.when(hostJoin1.getTotalMemory()).thenReturn(8192L); + + ServiceOfferingVO serviceOffering = Mockito.mock(ServiceOfferingVO.class); + + Mockito.when(hostDao.findByClusterId(1L)).thenReturn(hostList); + Mockito.when(vmInstanceDao.listByClusterId(1L)).thenReturn(vmList); + Mockito.when(balancedAlgorithm.needsDrs(Mockito.any(), Mockito.anyList(), Mockito.anyList())).thenReturn(true); + Mockito.when(serviceOfferingDao.findByIdIncludingRemoved(Mockito.anyLong(), Mockito.anyLong())).thenReturn(serviceOffering); + Mockito.when(hostJoinDao.searchByIds(Mockito.any())).thenReturn(List.of(hostJoin1)); + + List> result = clusterDrsService.getDrsPlan(cluster, 5); + assertEquals(0, result.size()); + Mockito.verify(managementServer, Mockito.times(1)).listHostsForMigrationOfVM(Mockito.eq(vm1), Mockito.anyLong(), Mockito.anyLong(), Mockito.any(), Mockito.anyList()); + } + + @Test + public void testGetDrsPlanWithExceptionInCompatibilityCheck() throws ConfigurationException { + ClusterVO cluster = Mockito.mock(ClusterVO.class); + Mockito.when(cluster.getId()).thenReturn(1L); + Mockito.when(cluster.getAllocationState()).thenReturn(Grouping.AllocationState.Enabled); + + HostVO host1 = Mockito.mock(HostVO.class); + Mockito.when(host1.getId()).thenReturn(1L); + + VMInstanceVO vm1 = Mockito.mock(VMInstanceVO.class); + Mockito.when(vm1.getId()).thenReturn(1L); + Mockito.when(vm1.getHostId()).thenReturn(1L); + Mockito.when(vm1.getType()).thenReturn(VirtualMachine.Type.User); + Mockito.when(vm1.getState()).thenReturn(VirtualMachine.State.Running); + Mockito.when(vm1.getDetails()).thenReturn(Collections.emptyMap()); + + List hostList = new ArrayList<>(); + hostList.add(host1); + + List vmList = new ArrayList<>(); + vmList.add(vm1); + + HostJoinVO hostJoin1 = Mockito.mock(HostJoinVO.class); + Mockito.when(hostJoin1.getId()).thenReturn(1L); + Mockito.when(hostJoin1.getCpuUsedCapacity()).thenReturn(1000L); + Mockito.when(hostJoin1.getCpuReservedCapacity()).thenReturn(0L); + Mockito.when(hostJoin1.getCpus()).thenReturn(4); + Mockito.when(hostJoin1.getSpeed()).thenReturn(1000L); + Mockito.when(hostJoin1.getMemUsedCapacity()).thenReturn(1024L); + Mockito.when(hostJoin1.getMemReservedCapacity()).thenReturn(0L); + Mockito.when(hostJoin1.getTotalMemory()).thenReturn(8192L); + + ServiceOfferingVO serviceOffering = Mockito.mock(ServiceOfferingVO.class); + + Mockito.when(hostDao.findByClusterId(1L)).thenReturn(hostList); + Mockito.when(vmInstanceDao.listByClusterId(1L)).thenReturn(vmList); + Mockito.when(balancedAlgorithm.needsDrs(Mockito.any(), Mockito.anyList(), Mockito.anyList())).thenReturn(true); + Mockito.when(serviceOfferingDao.findByIdIncludingRemoved(Mockito.anyLong(), Mockito.anyLong())).thenReturn(serviceOffering); + Mockito.when(hostJoinDao.searchByIds(Mockito.any())).thenReturn(List.of(hostJoin1)); + + List> result = clusterDrsService.getDrsPlan(cluster, 5); + assertEquals(0, result.size()); + // Exception should be caught and logged, not propagated + Mockito.verify(managementServer, Mockito.times(1)).listHostsForMigrationOfVM(Mockito.eq(vm1), Mockito.anyLong(), Mockito.anyLong(), Mockito.any(), Mockito.anyList()); + } + + @Test + public void testGetDrsPlanWithNoBestMigration() throws ConfigurationException { + ClusterVO cluster = Mockito.mock(ClusterVO.class); + Mockito.when(cluster.getId()).thenReturn(1L); + Mockito.when(cluster.getAllocationState()).thenReturn(Grouping.AllocationState.Enabled); + + HostVO host1 = Mockito.mock(HostVO.class); + Mockito.when(host1.getId()).thenReturn(1L); + + VMInstanceVO vm1 = Mockito.mock(VMInstanceVO.class); + Mockito.when(vm1.getId()).thenReturn(1L); + Mockito.when(vm1.getHostId()).thenReturn(1L); + Mockito.when(vm1.getType()).thenReturn(VirtualMachine.Type.User); + Mockito.when(vm1.getState()).thenReturn(VirtualMachine.State.Running); + Mockito.when(vm1.getDetails()).thenReturn(Collections.emptyMap()); + + List hostList = new ArrayList<>(); + hostList.add(host1); + + List vmList = new ArrayList<>(); + vmList.add(vm1); + + HostJoinVO hostJoin1 = Mockito.mock(HostJoinVO.class); + Mockito.when(hostJoin1.getId()).thenReturn(1L); + Mockito.when(hostJoin1.getCpuUsedCapacity()).thenReturn(1000L); + Mockito.when(hostJoin1.getCpuReservedCapacity()).thenReturn(0L); + Mockito.when(hostJoin1.getCpus()).thenReturn(4); + Mockito.when(hostJoin1.getSpeed()).thenReturn(1000L); + Mockito.when(hostJoin1.getMemUsedCapacity()).thenReturn(1024L); + Mockito.when(hostJoin1.getMemReservedCapacity()).thenReturn(0L); + Mockito.when(hostJoin1.getTotalMemory()).thenReturn(8192L); + + ServiceOfferingVO serviceOffering = Mockito.mock(ServiceOfferingVO.class); + + Mockito.when(hostDao.findByClusterId(1L)).thenReturn(hostList); + Mockito.when(vmInstanceDao.listByClusterId(1L)).thenReturn(vmList); + Mockito.when(balancedAlgorithm.needsDrs(Mockito.any(), Mockito.anyList(), Mockito.anyList())).thenReturn(true); + Mockito.when(serviceOfferingDao.findByIdIncludingRemoved(Mockito.anyLong(), Mockito.anyLong())).thenReturn(serviceOffering); + Mockito.when(hostJoinDao.searchByIds(Mockito.any())).thenReturn(List.of(hostJoin1)); + + HostVO compatibleHost = Mockito.mock(HostVO.class); + + // Return null migration (no best migration found) + Mockito.doReturn(new Pair<>(null, null)).when(clusterDrsService).getBestMigration( + Mockito.any(Cluster.class), Mockito.any(ClusterDrsAlgorithm.class), + Mockito.anyList(), Mockito.anyMap(), Mockito.anyMap(), Mockito.anyMap(), + Mockito.anyMap(), Mockito.anyMap(), Mockito.anyMap()); + + List> result = clusterDrsService.getDrsPlan(cluster, 5); + assertEquals(0, result.size()); + } + + @Test + public void testGetDrsPlanWithMultipleIterations() throws ConfigurationException { + ClusterVO cluster = Mockito.mock(ClusterVO.class); + Mockito.when(cluster.getId()).thenReturn(1L); + Mockito.when(cluster.getAllocationState()).thenReturn(Grouping.AllocationState.Enabled); + + HostVO host1 = Mockito.mock(HostVO.class); + Mockito.when(host1.getId()).thenReturn(1L); + + HostVO host2 = Mockito.mock(HostVO.class); + Mockito.when(host2.getId()).thenReturn(2L); + + VMInstanceVO vm1 = Mockito.mock(VMInstanceVO.class); + Mockito.when(vm1.getId()).thenReturn(1L); + Mockito.when(vm1.getHostId()).thenReturn(1L); + Mockito.when(vm1.getType()).thenReturn(VirtualMachine.Type.User); + Mockito.when(vm1.getState()).thenReturn(VirtualMachine.State.Running); + Mockito.when(vm1.getDetails()).thenReturn(Collections.emptyMap()); + + VMInstanceVO vm2 = Mockito.mock(VMInstanceVO.class); + Mockito.when(vm2.getId()).thenReturn(2L); + Mockito.when(vm2.getHostId()).thenReturn(1L); + Mockito.when(vm2.getType()).thenReturn(VirtualMachine.Type.User); + Mockito.when(vm2.getState()).thenReturn(VirtualMachine.State.Running); + Mockito.when(vm2.getDetails()).thenReturn(Collections.emptyMap()); + + List hostList = new ArrayList<>(); + hostList.add(host1); + hostList.add(host2); + + List vmList = new ArrayList<>(); + vmList.add(vm1); + vmList.add(vm2); + + HostJoinVO hostJoin1 = Mockito.mock(HostJoinVO.class); + Mockito.when(hostJoin1.getId()).thenReturn(1L); + Mockito.when(hostJoin1.getCpuUsedCapacity()).thenReturn(1000L); + Mockito.when(hostJoin1.getCpuReservedCapacity()).thenReturn(0L); + Mockito.when(hostJoin1.getCpus()).thenReturn(4); + Mockito.when(hostJoin1.getSpeed()).thenReturn(1000L); + Mockito.when(hostJoin1.getMemUsedCapacity()).thenReturn(1024L); + Mockito.when(hostJoin1.getMemReservedCapacity()).thenReturn(0L); + Mockito.when(hostJoin1.getTotalMemory()).thenReturn(8192L); + + HostJoinVO hostJoin2 = Mockito.mock(HostJoinVO.class); + Mockito.when(hostJoin2.getId()).thenReturn(2L); + Mockito.when(hostJoin2.getCpuUsedCapacity()).thenReturn(1000L); + Mockito.when(hostJoin2.getCpuReservedCapacity()).thenReturn(0L); + Mockito.when(hostJoin2.getCpus()).thenReturn(4); + Mockito.when(hostJoin2.getSpeed()).thenReturn(1000L); + Mockito.when(hostJoin2.getMemUsedCapacity()).thenReturn(1024L); + Mockito.when(hostJoin2.getMemReservedCapacity()).thenReturn(0L); + Mockito.when(hostJoin2.getTotalMemory()).thenReturn(8192L); + + ServiceOfferingVO serviceOffering = Mockito.mock(ServiceOfferingVO.class); + Mockito.when(serviceOffering.getCpu()).thenReturn(1); + Mockito.when(serviceOffering.getRamSize()).thenReturn(1024); + Mockito.when(serviceOffering.getSpeed()).thenReturn(1000); + + Mockito.when(hostDao.findByClusterId(1L)).thenReturn(hostList); + Mockito.when(vmInstanceDao.listByClusterId(1L)).thenReturn(vmList); + Mockito.when(balancedAlgorithm.needsDrs(Mockito.any(), Mockito.anyList(), Mockito.anyList())).thenReturn( + true, true, false); + Mockito.when(serviceOfferingDao.findByIdIncludingRemoved(Mockito.anyLong(), Mockito.anyLong())).thenReturn(serviceOffering); + Mockito.when(hostJoinDao.searchByIds(Mockito.any())).thenReturn(List.of(hostJoin1, hostJoin2)); + + HostVO compatibleHost = Mockito.mock(HostVO.class); + + // Return migrations for first two iterations, then null + Mockito.doReturn(new Pair<>(vm1, host2), new Pair<>(vm2, host2), new Pair<>(null, null)) + .when(clusterDrsService).getBestMigration( + Mockito.any(Cluster.class), Mockito.any(ClusterDrsAlgorithm.class), + Mockito.anyList(), Mockito.anyMap(), Mockito.anyMap(), Mockito.anyMap(), + Mockito.anyMap(), Mockito.anyMap(), Mockito.anyMap()); + + List> result = clusterDrsService.getDrsPlan(cluster, 5); + assertEquals(2, result.size()); + Mockito.verify(balancedAlgorithm, Mockito.times(3)).needsDrs(Mockito.any(), Mockito.anyList(), Mockito.anyList()); + } + + @Test + public void testGetDrsPlanWithMigrationToOriginalHost() throws ConfigurationException { + ClusterVO cluster = Mockito.mock(ClusterVO.class); + Mockito.when(cluster.getId()).thenReturn(1L); + Mockito.when(cluster.getAllocationState()).thenReturn(Grouping.AllocationState.Enabled); + + HostVO host1 = Mockito.mock(HostVO.class); + Mockito.when(host1.getId()).thenReturn(1L); + + HostVO host2 = Mockito.mock(HostVO.class); + Mockito.when(host2.getId()).thenReturn(2L); + + VMInstanceVO vm1 = Mockito.mock(VMInstanceVO.class); + Mockito.when(vm1.getId()).thenReturn(1L); + Mockito.when(vm1.getHostId()).thenReturn(1L); + Mockito.when(vm1.getType()).thenReturn(VirtualMachine.Type.User); + Mockito.when(vm1.getState()).thenReturn(VirtualMachine.State.Running); + Mockito.when(vm1.getDetails()).thenReturn(Collections.emptyMap()); + + List hostList = new ArrayList<>(); + hostList.add(host1); + hostList.add(host2); + + List vmList = new ArrayList<>(); + vmList.add(vm1); + + HostJoinVO hostJoin1 = Mockito.mock(HostJoinVO.class); + Mockito.when(hostJoin1.getId()).thenReturn(1L); + Mockito.when(hostJoin1.getCpuUsedCapacity()).thenReturn(1000L); + Mockito.when(hostJoin1.getCpuReservedCapacity()).thenReturn(0L); + Mockito.when(hostJoin1.getCpus()).thenReturn(4); + Mockito.when(hostJoin1.getSpeed()).thenReturn(1000L); + Mockito.when(hostJoin1.getMemUsedCapacity()).thenReturn(1024L); + Mockito.when(hostJoin1.getMemReservedCapacity()).thenReturn(0L); + Mockito.when(hostJoin1.getTotalMemory()).thenReturn(8192L); + + HostJoinVO hostJoin2 = Mockito.mock(HostJoinVO.class); + Mockito.when(hostJoin2.getId()).thenReturn(2L); + Mockito.when(hostJoin2.getCpuUsedCapacity()).thenReturn(1000L); + Mockito.when(hostJoin2.getCpuReservedCapacity()).thenReturn(0L); + Mockito.when(hostJoin2.getCpus()).thenReturn(4); + Mockito.when(hostJoin2.getSpeed()).thenReturn(1000L); + Mockito.when(hostJoin2.getMemUsedCapacity()).thenReturn(1024L); + Mockito.when(hostJoin2.getMemReservedCapacity()).thenReturn(0L); + Mockito.when(hostJoin2.getTotalMemory()).thenReturn(8192L); + + ServiceOfferingVO serviceOffering = Mockito.mock(ServiceOfferingVO.class); + + Mockito.when(hostDao.findByClusterId(1L)).thenReturn(hostList); + Mockito.when(vmInstanceDao.listByClusterId(1L)).thenReturn(vmList); + Mockito.when(balancedAlgorithm.needsDrs(Mockito.any(), Mockito.anyList(), Mockito.anyList())).thenReturn(true); + Mockito.when(serviceOfferingDao.findByIdIncludingRemoved(Mockito.anyLong(), Mockito.anyLong())).thenReturn(serviceOffering); + Mockito.when(hostJoinDao.searchByIds(Mockito.any())).thenReturn(List.of(hostJoin1, hostJoin2)); + + HostVO compatibleHost = Mockito.mock(HostVO.class); + + // Return migration to original host (host1) - should break the loop + Mockito.doReturn(new Pair<>(vm1, host1)).when(clusterDrsService).getBestMigration( + Mockito.any(Cluster.class), Mockito.any(ClusterDrsAlgorithm.class), + Mockito.anyList(), Mockito.anyMap(), Mockito.anyMap(), Mockito.anyMap(), + Mockito.anyMap(), Mockito.anyMap(), Mockito.anyMap()); + + List> result = clusterDrsService.getDrsPlan(cluster, 5); + assertEquals(0, result.size()); + // Should break early when VM would migrate to original host + } + @Test(expected = InvalidParameterValueException.class) public void testGenerateDrsPlanClusterNotFound() { Mockito.when(clusterDao.findById(1L)).thenReturn(null); @@ -387,18 +837,41 @@ public void testGetBestMigration() throws ConfigurationException { vmIdServiceOfferingMap.put(vm.getId(), serviceOffering); } - Mockito.when(managementServer.listHostsForMigrationOfVM(vm1, 0L, 500L, null, vmList)).thenReturn( - new Ternary<>(new Pair<>(List.of(destHost), 1), List.of(destHost), Map.of(destHost, false))); - Mockito.when(managementServer.listHostsForMigrationOfVM(vm2, 0L, 500L, null, vmList)).thenReturn( - new Ternary<>(new Pair<>(List.of(destHost), 1), List.of(destHost), Map.of(destHost, false))); - Mockito.when(balancedAlgorithm.getMetrics(cluster, vm1, serviceOffering, destHost, new HashMap<>(), - new HashMap<>(), false)).thenReturn(new Ternary<>(1.0, 0.5, 1.5)); - - Mockito.when(balancedAlgorithm.getMetrics(cluster, vm2, serviceOffering, destHost, new HashMap<>(), - new HashMap<>(), false)).thenReturn(new Ternary<>(1.0, 2.5, 1.5)); - - Pair bestMigration = clusterDrsService.getBestMigration(cluster, balancedAlgorithm, - vmList, vmIdServiceOfferingMap, new HashMap<>(), new HashMap<>()); + // Create caches for the new method signature + Map> vmToCompatibleHostsCache = new HashMap<>(); + vmToCompatibleHostsCache.put(vm1.getId(), List.of(destHost)); + vmToCompatibleHostsCache.put(vm2.getId(), List.of(destHost)); + + Map> vmToStorageMotionCache = new HashMap<>(); + vmToStorageMotionCache.put(vm1.getId(), Map.of(destHost, false)); + vmToStorageMotionCache.put(vm2.getId(), Map.of(destHost, false)); + + Map vmToExcludesMap = new HashMap<>(); + vmToExcludesMap.put(vm1.getId(), Mockito.mock(com.cloud.deploy.DeploymentPlanner.ExcludeList.class)); + vmToExcludesMap.put(vm2.getId(), Mockito.mock(com.cloud.deploy.DeploymentPlanner.ExcludeList.class)); + + // Create capacity maps with dummy data for getClusterImbalance (include both source and dest hosts) + Map> hostCpuCapacityMap = new HashMap<>(); + hostCpuCapacityMap.put(host.getId(), new Ternary<>(2000L, 0L, 3000L)); // Source host + hostCpuCapacityMap.put(destHost.getId(), new Ternary<>(1000L, 0L, 2000L)); // Dest host + Map> hostMemoryCapacityMap = new HashMap<>(); + hostMemoryCapacityMap.put(host.getId(), new Ternary<>(2L * 1024L * 1024L * 1024L, 0L, 3L * 1024L * 1024L * 1024L)); // Source host + hostMemoryCapacityMap.put(destHost.getId(), new Ternary<>(1024L * 1024L * 1024L, 0L, 2L * 1024L * 1024L * 1024L)); // Dest host + + // Mock getMetrics for the optimized 10-parameter version used by getBestMigration + // Return better improvement for vm1, worse for vm2 + Mockito.doReturn(new Ternary<>(1.0, 0.5, 1.5)).when(balancedAlgorithm).getMetrics( + Mockito.eq(cluster), Mockito.eq(vm1), Mockito.any(ServiceOffering.class), + Mockito.eq(destHost), Mockito.eq(hostCpuCapacityMap), Mockito.eq(hostMemoryCapacityMap), Mockito.any(Boolean.class), + Mockito.any(Double.class), Mockito.any(double[].class), Mockito.any(Map.class)); + Mockito.doReturn(new Ternary<>(0.5, 2.5, 1.5)).when(balancedAlgorithm).getMetrics( + Mockito.eq(cluster), Mockito.eq(vm2), Mockito.any(ServiceOffering.class), + Mockito.eq(destHost), Mockito.eq(hostCpuCapacityMap), Mockito.eq(hostMemoryCapacityMap), Mockito.any(Boolean.class), + Mockito.any(Double.class), Mockito.any(double[].class), Mockito.any(Map.class)); + + Pair bestMigration = clusterDrsService.getBestMigration(cluster, balancedAlgorithm, + vmList, vmIdServiceOfferingMap, hostCpuCapacityMap, hostMemoryCapacityMap, + vmToCompatibleHostsCache, vmToStorageMotionCache, vmToExcludesMap); assertEquals(destHost, bestMigration.second()); assertEquals(vm1, bestMigration.first()); @@ -443,12 +916,28 @@ public void testGetBestMigrationDifferentCluster() throws ConfigurationException vmIdServiceOfferingMap.put(vm.getId(), serviceOffering); } - Mockito.when(managementServer.listHostsForMigrationOfVM(vm1, 0L, 500L, null, vmList)).thenReturn( - new Ternary<>(new Pair<>(List.of(destHost), 1), List.of(destHost), Map.of(destHost, false))); - Mockito.when(managementServer.listHostsForMigrationOfVM(vm2, 0L, 500L, null, vmList)).thenReturn( - new Ternary<>(new Pair<>(List.of(destHost), 1), List.of(destHost), Map.of(destHost, false))); + // Create caches for the new method signature + Map> vmToCompatibleHostsCache = new HashMap<>(); + vmToCompatibleHostsCache.put(vm1.getId(), List.of(destHost)); + vmToCompatibleHostsCache.put(vm2.getId(), List.of(destHost)); + + Map> vmToStorageMotionCache = new HashMap<>(); + vmToStorageMotionCache.put(vm1.getId(), Map.of(destHost, false)); + vmToStorageMotionCache.put(vm2.getId(), Map.of(destHost, false)); + + Map vmToExcludesMap = new HashMap<>(); + vmToExcludesMap.put(vm1.getId(), Mockito.mock(com.cloud.deploy.DeploymentPlanner.ExcludeList.class)); + vmToExcludesMap.put(vm2.getId(), Mockito.mock(com.cloud.deploy.DeploymentPlanner.ExcludeList.class)); + + // Create capacity maps with dummy data for getClusterImbalance + Map> hostCpuCapacityMap = new HashMap<>(); + hostCpuCapacityMap.put(destHost.getId(), new Ternary<>(1000L, 0L, 2000L)); + Map> hostMemoryCapacityMap = new HashMap<>(); + hostMemoryCapacityMap.put(destHost.getId(), new Ternary<>(1024L * 1024L * 1024L, 0L, 2L * 1024L * 1024L * 1024L)); + Pair bestMigration = clusterDrsService.getBestMigration(cluster, balancedAlgorithm, - vmList, vmIdServiceOfferingMap, new HashMap<>(), new HashMap<>()); + vmList, vmIdServiceOfferingMap, hostCpuCapacityMap, hostMemoryCapacityMap, + vmToCompatibleHostsCache, vmToStorageMotionCache, vmToExcludesMap); assertNull(bestMigration.second()); assertNull(bestMigration.first());