Add advance settings to fine tune DRS imbalance calculation (#8521)

* Use free/total instead of free metric to calculate imbalance

* Filter out hosts for condensed while checking imbalance

* Make DRS more configurable

* code refactor

* Add unit tests

* fixup

* Fix validation for drs.imbalance.condensed.skip.threshold

* Add logging and other minor changes for drs

* Add some logging for drs

* Change format for drs imbalance to string

* Show drs imbalance as percentage

* Fixup label for memorytotal in en.json

api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithm.java

@@ -33,6 +33,10 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 
+import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsMetric;
+import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsMetricType;
+import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsMetricUseRatio;
+
 public interface ClusterDrsAlgorithm extends Adapter {
 
     /**
@@ -42,16 +46,17 @@ public interface ClusterDrsAlgorithm extends Adapter {
      * @param clusterId
      *         the ID of the cluster to check
      * @param cpuList
-     *         a list of CPU allocated values for each host in the cluster
+     *         a list of Ternary of used, reserved & total CPU for each host in the cluster
      * @param memoryList
-     *         a list of memory allocated values for each host in the cluster
+     *         a list of Ternary of used, reserved & total memory values for each host in the cluster
      *
      * @return true if a DRS operation is needed, false otherwise
      *
      * @throws ConfigurationException
      *         if there is an error in the configuration
      */
-    boolean needsDrs(long clusterId, List<Long> cpuList, List<Long> memoryList) throws ConfigurationException;
+    boolean needsDrs(long clusterId, List<Ternary<Long, Long, Long>> cpuList,
+            List<Ternary<Long, Long, Long>> memoryList) throws ConfigurationException;
 
 
     /**
@@ -65,18 +70,19 @@ public interface ClusterDrsAlgorithm extends Adapter {
      *         the service offering for the virtual machine
      * @param destHost
      *         the destination host for the virtual machine
-     * @param hostCpuFreeMap
+     * @param hostCpuMap
-     *         a map of host IDs to the amount of CPU free on each host
+     *         a map of host IDs to the Ternary of used, reserved and total CPU on each host
-     * @param hostMemoryFreeMap
+     * @param hostMemoryMap
-     *         a map of host IDs to the amount of memory free on each host
+     *         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
      *
      * @return a ternary containing improvement, cost, benefit
      */
     Ternary<Double, Double, Double> getMetrics(long clusterId, VirtualMachine vm, ServiceOffering serviceOffering,
-                                               Host destHost, Map<Long, Long> hostCpuFreeMap,
+            Host destHost, Map<Long, Ternary<Long, Long, Long>> hostCpuMap,
-                                               Map<Long, Long> hostMemoryFreeMap, Boolean requiresStorageMotion);
+            Map<Long, Ternary<Long, Long, Long>> hostMemoryMap,
+            Boolean requiresStorageMotion) throws ConfigurationException;
 
     /**
      * Calculates the imbalance of the cluster after a virtual machine migration.
@@ -87,54 +93,93 @@ public interface ClusterDrsAlgorithm extends Adapter {
      *         the virtual machine being migrated
      * @param destHost
      *         the destination host for the virtual machine
-     * @param hostCpuFreeMap
+     * @param hostCpuMap
-     *         a map of host IDs to the amount of CPU free on each host
+     *         a map of host IDs to the Ternary of used, reserved and total CPU on each host
-     * @param hostMemoryFreeMap
+     * @param hostMemoryMap
-     *         a map of host IDs to the amount of memory free on each host
+     *         a map of host IDs to the Ternary of used, reserved and total memory on each host
      *
      * @return a pair containing the CPU and memory imbalance of the cluster after the migration
      */
-    default Pair<Double, Double> getImbalancePostMigration(ServiceOffering serviceOffering, VirtualMachine vm,
+    default Double getImbalancePostMigration(ServiceOffering serviceOffering, VirtualMachine vm,
-                                                           Host destHost, Map<Long, Long> hostCpuFreeMap,
+            Host destHost, Map<Long, Ternary<Long, Long, Long>> hostCpuMap,
-                                                           Map<Long, Long> hostMemoryFreeMap) {
+            Map<Long, Ternary<Long, Long, Long>> hostMemoryMap) throws ConfigurationException {
-        List<Long> postCpuList = new ArrayList<>();
+        Pair<Long, Map<Long, Ternary<Long, Long, Long>>> pair = getHostMetricsMapAndType(destHost.getClusterId(), serviceOffering, hostCpuMap, hostMemoryMap);
-        List<Long> postMemoryList = new ArrayList<>();
+        long vmMetric = pair.first();
-        final int vmCpu = serviceOffering.getCpu() * serviceOffering.getSpeed();
+        Map<Long, Ternary<Long, Long, Long>> hostMetricsMap = pair.second();
-        final long vmRam = serviceOffering.getRamSize() * 1024L * 1024L;
 
-        for (Long hostId : hostCpuFreeMap.keySet()) {
+        List<Double> list = new ArrayList<>();
-            long cpu = hostCpuFreeMap.get(hostId);
+        for (Long hostId : hostMetricsMap.keySet()) {
-            long memory = hostMemoryFreeMap.get(hostId);
+            list.add(getMetricValuePostMigration(destHost.getClusterId(), hostMetricsMap.get(hostId), vmMetric, hostId, destHost.getId(), vm.getHostId()));
-            if (hostId == destHost.getId()) {
-                postCpuList.add(cpu - vmCpu);
-                postMemoryList.add(memory - vmRam);
-            } else if (hostId.equals(vm.getHostId())) {
-                postCpuList.add(cpu + vmCpu);
-                postMemoryList.add(memory + vmRam);
-            } else {
-                postCpuList.add(cpu);
-                postMemoryList.add(memory);
         }
-        }
+        return getImbalance(list);
-        return new Pair<>(getClusterImbalance(postCpuList), getClusterImbalance(postMemoryList));
     }
 
-    /**
+    private Pair<Long, Map<Long, Ternary<Long, Long, Long>>> getHostMetricsMapAndType(Long clusterId,
-     * The cluster imbalance is defined as the percentage deviation from the mean
+            ServiceOffering serviceOffering, Map<Long, Ternary<Long, Long, Long>> hostCpuMap,
-     * for a configured metric of the cluster. The standard deviation is used as a
+            Map<Long, Ternary<Long, Long, Long>> hostMemoryMap) throws ConfigurationException {
-     * mathematical tool to normalize the metric data for all the resource and the
+        String metric = getClusterDrsMetric(clusterId);
-     * percentage deviation provides an easy tool to compare a cluster’s current
+        Pair<Long, Map<Long, Ternary<Long, Long, Long>>> pair;
-     * state against the defined imbalance threshold. Because this is essentially a
+        switch (metric) {
-     * percentage, the value is a number between 0.0 and 1.0.
+            case "cpu":
-     * Cluster Imbalance, Ic = σc / mavg , where σc is the standard deviation and
+                pair = new Pair<>((long) serviceOffering.getCpu() * serviceOffering.getSpeed(), hostCpuMap);
-     * mavg is the mean metric value for the cluster.
+                break;
-     */
+            case "memory":
-    default Double getClusterImbalance(List<Long> metricList) {
+                pair = new Pair<>(serviceOffering.getRamSize() * 1024L * 1024L, hostMemoryMap);
+                break;
+            default:
+                throw new ConfigurationException(
+                        String.format("Invalid metric: %s for cluster: %d", metric, clusterId));
+        }
+        return pair;
+    }
+
+    private Double getMetricValuePostMigration(Long clusterId, Ternary<Long, Long, Long> metrics, long vmMetric,
+            long hostId, long destHostId, long vmHostId) {
+        long used = metrics.first();
+        long actualTotal = metrics.third() - metrics.second();
+        long free = actualTotal - metrics.first();
+
+        if (hostId == destHostId) {
+            used += vmMetric;
+            free -= vmMetric;
+        } else if (hostId == vmHostId) {
+            used -= vmMetric;
+            free += vmMetric;
+        }
+        return getMetricValue(clusterId, used, free, actualTotal, null);
+    }
+
+    private static Double getImbalance(List<Double> metricList) {
         Double clusterMeanMetric = getClusterMeanMetric(metricList);
         Double clusterStandardDeviation = getClusterStandardDeviation(metricList, clusterMeanMetric);
         return clusterStandardDeviation / clusterMeanMetric;
     }
 
+    static String getClusterDrsMetric(long clusterId) {
+        return ClusterDrsMetric.valueIn(clusterId);
+    }
+
+    static Double getMetricValue(long clusterId, long used, long free, long total, Float skipThreshold) {
+        boolean useRatio = getDrsMetricUseRatio(clusterId);
+        switch (getDrsMetricType(clusterId)) {
+            case "free":
+                if (skipThreshold != null && free < skipThreshold * total) return null;
+                if (useRatio) {
+                    return (double) free / total;
+                } else {
+                    return (double) free;
+                }
+            case "used":
+                if (skipThreshold != null && used > skipThreshold * total) return null;
+                if (useRatio) {
+                    return (double) used / total;
+                } else {
+                    return (double) used;
+                }
+        }
+        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
@@ -142,7 +187,7 @@ public interface ClusterDrsAlgorithm extends Adapter {
      * 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.
      */
-    default Double getClusterMeanMetric(List<Long> metricList) {
+    static Double getClusterMeanMetric(List<Double> metricList) {
         return new Mean().evaluate(metricList.stream().mapToDouble(i -> i).toArray());
     }
 
@@ -157,11 +202,62 @@ public interface ClusterDrsAlgorithm extends Adapter {
      * mean metric value and mi is a measurable metric for some resource ‘i’ in the
      * cluster with total N number of resources.
      */
-    default Double getClusterStandardDeviation(List<Long> metricList, Double mean) {
+    static Double getClusterStandardDeviation(List<Double> 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);
+    }
+
+    static String getDrsMetricType(long clusterId) {
+        return ClusterDrsMetricType.valueIn(clusterId);
+    }
+
+    /**
+     * The cluster imbalance is defined as the percentage deviation from the mean
+     * for a configured metric of the cluster. The standard deviation is used as a
+     * mathematical tool to normalize the metric data for all the resource and the
+     * percentage deviation provides an easy tool to compare a cluster’s current
+     * state against the defined imbalance threshold. Because this is essentially a
+     * percentage, the value is a number between 0.0 and 1.0.
+     * Cluster Imbalance, Ic = σc / mavg , where σc is the standard deviation and
+     * mavg is the mean metric value for the cluster.
+     */
+    static Double getClusterImbalance(Long clusterId, List<Ternary<Long, Long, Long>> cpuList,
+            List<Ternary<Long, Long, Long>> memoryList, Float skipThreshold) throws ConfigurationException {
+        String metric = getClusterDrsMetric(clusterId);
+        List<Double> list;
+        switch (metric) {
+            case "cpu":
+                list = getMetricList(clusterId, cpuList, skipThreshold);
+                break;
+            case "memory":
+                list = getMetricList(clusterId, memoryList, skipThreshold);
+                break;
+            default:
+                throw new ConfigurationException(
+                        String.format("Invalid metric: %s for cluster: %d", metric, clusterId));
+        }
+        return getImbalance(list);
+    }
+
+    static List<Double> getMetricList(Long clusterId, List<Ternary<Long, Long, Long>> hostMetricsList,
+            Float skipThreshold) {
+        List<Double> list = new ArrayList<>();
+        for (Ternary<Long, Long, Long> ternary : hostMetricsList) {
+            long used = ternary.first();
+            long actualTotal = ternary.third() - ternary.second();
+            long free = actualTotal - ternary.first();
+            Double metricValue = getMetricValue(clusterId, used, free, actualTotal, skipThreshold);
+            if (metricValue != null) {
+                list.add(metricValue);
+            }
+        }
+        return list;
+    }
 }

api/src/main/java/org/apache/cloudstack/cluster/ClusterDrsService.java

@@ -66,6 +66,29 @@ public interface ClusterDrsService extends Manager, Configurable, Scheduler {
             true, ConfigKey.Scope.Cluster, null, "DRS metric", null, null, null, ConfigKey.Kind.Select,
             "memory,cpu");
 
+    ConfigKey<String> ClusterDrsMetricType = new ConfigKey<>(String.class, "drs.metric.type", ConfigKey.CATEGORY_ADVANCED,
+            "used",
+            "The metric type used to measure imbalance in a cluster. This can completely change the imbalance value. Possible values are free, used.",
+            true, ConfigKey.Scope.Cluster, null, "DRS metric type", null, null, null, ConfigKey.Kind.Select,
+            "free,used");
+
+    ConfigKey<Boolean> ClusterDrsMetricUseRatio = new ConfigKey<>(Boolean.class, "drs.metric.use.ratio", ConfigKey.CATEGORY_ADVANCED,
+            "true",
+            "Whether to use ratio of selected metric & total. Useful when the cluster has hosts with different capacities",
+            true, ConfigKey.Scope.Cluster, null, "DRS metric use ratio", null, null, null, ConfigKey.Kind.Select,
+            "true,false");
+
+    ConfigKey<Float> ClusterDrsImbalanceSkipThreshold = new ConfigKey<>(Float.class,
+            "drs.imbalance.condensed.skip.threshold", ConfigKey.CATEGORY_ADVANCED, "0.95",
+            "Threshold to ignore the metric for a host while calculating the imbalance to decide " +
+                    "whether DRS is required for a cluster.This is to avoid cases when the calculated imbalance" +
+                    " gets skewed due to a single host having a very high/low metric  value resulting in imbalance" +
+                    " being higher than 1. If " + ClusterDrsMetricType.key() + " is 'free', set a lower value and if it is 'used' " +
+                    "set a higher value. The value should be between 0.0 and 1.0",
+            true, ConfigKey.Scope.Cluster, null, "DRS imbalance skip threshold for Condensed algorithm",
+            null, null, null);
+
+
     /**
      * Generate a DRS plan for a cluster and save it as per the parameters
      *

api/src/test/java/org/apache/cloudstack/cluster/ClusterDrsAlgorithmTest.java

@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.cloudstack.cluster;
+
+import com.cloud.utils.Ternary;
+import junit.framework.TestCase;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.util.List;
+
+import static org.apache.cloudstack.cluster.ClusterDrsAlgorithm.getMetricValue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyFloat;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.when;
+
+@RunWith(MockitoJUnitRunner.class)
+public class ClusterDrsAlgorithmTest extends TestCase {
+
+    @Test
+    public void testGetMetricValue() {
+        List<Ternary<Boolean, String, Double>> testData = List.of(
+                new Ternary<>(true, "free", 0.4),
+                new Ternary<>(false, "free", 40.0),
+                new Ternary<>(true, "used", 0.3),
+                new Ternary<>(false, "used", 30.0)
+        );
+
+        long used = 30;
+        long free = 40;
+        long total = 100;
+
+        for (Ternary<Boolean, String, Double> data : testData) {
+            boolean useRatio = data.first();
+            String metricType = data.second();
+            double expectedValue = data.third();
+
+            try (MockedStatic<ClusterDrsAlgorithm> ignored = Mockito.mockStatic(ClusterDrsAlgorithm.class)) {
+                when(ClusterDrsAlgorithm.getDrsMetricUseRatio(1L)).thenReturn(useRatio);
+                when(ClusterDrsAlgorithm.getDrsMetricType(1L)).thenReturn(metricType);
+                when(ClusterDrsAlgorithm.getMetricValue(anyLong(), anyLong(), anyLong(), anyLong(), any())).thenCallRealMethod();
+
+                assertEquals(expectedValue, getMetricValue(1, used, free, total, null));
+            }
+        }
+    }
+
+    @Test
+    public void testGetMetricValueWithSkipThreshold() {
+        List<Ternary<Boolean, String, Double>> testData = List.of(
+                new Ternary<>(true, "free", 0.15),
+                new Ternary<>(false, "free", 15.0),
+                new Ternary<>(true, "used", null),
+                new Ternary<>(false, "used", null)
+        );
+
+        long used = 80;
+        long free = 15;
+        long total = 100;
+
+        for (Ternary<Boolean, String, Double> data : testData) {
+            boolean useRatio = data.first();
+            String metricType = data.second();
+            Double expectedValue = data.third();
+            float skipThreshold = metricType.equals("free") ? 0.1f : 0.7f;
+
+            try (MockedStatic<ClusterDrsAlgorithm> ignored = Mockito.mockStatic(ClusterDrsAlgorithm.class)) {
+                when(ClusterDrsAlgorithm.getDrsMetricUseRatio(1L)).thenReturn(useRatio);
+                when(ClusterDrsAlgorithm.getDrsMetricType(1L)).thenReturn(metricType);
+                when(ClusterDrsAlgorithm.getMetricValue(anyLong(), anyLong(), anyLong(), anyLong(), anyFloat())).thenCallRealMethod();
+
+                assertEquals(expectedValue, ClusterDrsAlgorithm.getMetricValue(1L, used, free, total, skipThreshold));
+            }
+        }
+    }
+}

plugins/drs/cluster/balanced/src/main/java/org/apache/cloudstack/cluster/Balanced.java

@@ -21,10 +21,10 @@ package org.apache.cloudstack.cluster;
 
 import com.cloud.host.Host;
 import com.cloud.offering.ServiceOffering;
-import com.cloud.utils.Pair;
 import com.cloud.utils.Ternary;
 import com.cloud.utils.component.AdapterBase;
 import com.cloud.vm.VirtualMachine;
+import org.apache.log4j.Logger;
 
 import javax.naming.ConfigurationException;
 import java.util.ArrayList;
@@ -32,68 +32,56 @@ import java.util.List;
 import java.util.Map;
 
 import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsImbalanceThreshold;
-import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsMetric;
 
 public class Balanced extends AdapterBase implements ClusterDrsAlgorithm {
 
+    private static final Logger logger = Logger.getLogger(Balanced.class);
+
+    @Override
+    public boolean needsDrs(long clusterId, List<Ternary<Long, Long, Long>> cpuList,
+            List<Ternary<Long, Long, Long>> memoryList) throws ConfigurationException {
+        double threshold = getThreshold(clusterId);
+        Double imbalance = ClusterDrsAlgorithm.getClusterImbalance(clusterId, cpuList, memoryList, null);
+        String drsMetric = ClusterDrsAlgorithm.getClusterDrsMetric(clusterId);
+        String metricType = ClusterDrsAlgorithm.getDrsMetricType(clusterId);
+        Boolean useRatio = ClusterDrsAlgorithm.getDrsMetricUseRatio(clusterId);
+        if (imbalance > threshold) {
+            logger.debug(String.format("Cluster %d needs DRS. Imbalance: %s Threshold: %s Algorithm: %s DRS metric: %s Metric Type: %s Use ratio: %s",
+                    clusterId, imbalance, threshold, getName(), drsMetric, metricType, useRatio));
+            return true;
+        } else {
+            logger.debug(String.format("Cluster %d does not need DRS. Imbalance: %s Threshold: %s Algorithm: %s DRS metric: %s Metric Type: %s Use ratio: %s",
+                    clusterId, imbalance, threshold, getName(), drsMetric, metricType, useRatio));
+            return false;
+        }
+    }
+
+    private double getThreshold(long clusterId) {
+        return 1.0 - ClusterDrsImbalanceThreshold.valueIn(clusterId);
+    }
+
     @Override
     public String getName() {
         return "balanced";
     }
 
-    @Override
-    public boolean needsDrs(long clusterId, List<Long> cpuList, List<Long> memoryList) throws ConfigurationException {
-        Double cpuImbalance = getClusterImbalance(cpuList);
-        Double memoryImbalance = getClusterImbalance(memoryList);
-        double threshold = getThreshold(clusterId);
-        String metric = ClusterDrsMetric.valueIn(clusterId);
-        switch (metric) {
-            case "cpu":
-                return cpuImbalance > threshold;
-            case "memory":
-                return memoryImbalance > threshold;
-            default:
-                throw new ConfigurationException(
-                        String.format("Invalid metric: %s for cluster: %d", metric, clusterId));
-        }
-    }
-
-    private double getThreshold(long clusterId) throws ConfigurationException {
-        return 1.0 - ClusterDrsImbalanceThreshold.valueIn(clusterId);
-    }
-
     @Override
     public Ternary<Double, Double, Double> getMetrics(long clusterId, VirtualMachine vm,
             ServiceOffering serviceOffering, Host destHost,
-                                                      Map<Long, Long> hostCpuUsedMap, Map<Long, Long> hostMemoryUsedMap,
+            Map<Long, Ternary<Long, Long, Long>> hostCpuMap, Map<Long, Ternary<Long, Long, Long>> hostMemoryMap,
-                                                      Boolean requiresStorageMotion) {
+            Boolean requiresStorageMotion) throws ConfigurationException {
-        Double preCpuImbalance = getClusterImbalance(new ArrayList<>(hostCpuUsedMap.values()));
+        Double preImbalance = ClusterDrsAlgorithm.getClusterImbalance(clusterId, new ArrayList<>(hostCpuMap.values()), new ArrayList<>(hostMemoryMap.values()), null);
-        Double preMemoryImbalance = getClusterImbalance(new ArrayList<>(hostMemoryUsedMap.values()));
+        Double postImbalance = getImbalancePostMigration(serviceOffering, vm, destHost, hostCpuMap, hostMemoryMap);
 
-        Pair<Double, Double> imbalancePair = getImbalancePostMigration(serviceOffering, vm, destHost, hostCpuUsedMap,
+        logger.debug(String.format("Cluster %d pre-imbalance: %s post-imbalance: %s Algorithm: %s VM: %s srcHost: %d destHost: %s",
-                hostMemoryUsedMap);
+                clusterId, preImbalance, postImbalance, getName(), vm.getUuid(), vm.getHostId(), destHost.getUuid()));
-        Double postCpuImbalance = imbalancePair.first();
-        Double postMemoryImbalance = imbalancePair.second();
 
         // 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
-        double cost = 0.0;
+        final double improvement = preImbalance - postImbalance;
-        double benefit = 1.0;
+        final double cost = 0.0;
-
+        final double benefit = 1.0;
-        String metric = ClusterDrsMetric.valueIn(clusterId);
-        final double improvement;
-        switch (metric) {
-            case "cpu":
-                improvement = preCpuImbalance - postCpuImbalance;
-                break;
-            case "memory":
-                improvement = preMemoryImbalance - postMemoryImbalance;
-                break;
-            default:
-                improvement = preCpuImbalance + preMemoryImbalance - postCpuImbalance - postMemoryImbalance;
-        }
-
         return new Ternary<>(improvement, cost, benefit);
     }
 }

plugins/drs/cluster/balanced/src/test/java/org/apache/cloudstack/cluster/BalancedTest.java

@@ -21,7 +21,6 @@ package org.apache.cloudstack.cluster;
 
 import com.cloud.host.Host;
 import com.cloud.service.ServiceOfferingVO;
-import com.cloud.service.dao.ServiceOfferingDao;
 import com.cloud.utils.Ternary;
 import com.cloud.vm.VirtualMachine;
 import org.apache.cloudstack.framework.config.ConfigKey;
@@ -30,13 +29,13 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.InjectMocks;
-import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
 import org.mockito.junit.MockitoJUnitRunner;
 
 import javax.naming.ConfigurationException;
 import java.lang.reflect.Field;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
@@ -66,14 +65,7 @@ public class BalancedTest {
 
     Map<Long, List<VirtualMachine>> hostVmMap;
 
-    List<Long> cpuList, memoryList;
+    Map<Long, Ternary<Long, Long, Long>> hostCpuFreeMap, hostMemoryFreeMap;
-
-    Map<Long, Long> hostCpuFreeMap, hostMemoryFreeMap;
-
-
-    @Mock
-    private ServiceOfferingDao serviceOfferingDao;
-
 
     private AutoCloseable closeable;
 
@@ -98,20 +90,17 @@ public class BalancedTest {
 
         Mockito.when(serviceOffering.getCpu()).thenReturn(1);
         Mockito.when(serviceOffering.getSpeed()).thenReturn(1000);
-        Mockito.when(serviceOffering.getRamSize()).thenReturn(512);
+        Mockito.when(serviceOffering.getRamSize()).thenReturn(1024);
 
         overrideDefaultConfigValue(ClusterDrsImbalanceThreshold, "_defaultValue", "0.5");
 
-        cpuList = Arrays.asList(1L, 2L);
-        memoryList = Arrays.asList(512L, 2048L);
-
         hostCpuFreeMap = new HashMap<>();
-        hostCpuFreeMap.put(1L, 2000L);
+        hostCpuFreeMap.put(1L, new Ternary<>(1000L, 0L, 10000L));
-        hostCpuFreeMap.put(2L, 1000L);
+        hostCpuFreeMap.put(2L, new Ternary<>(2000L, 0L, 10000L));
 
         hostMemoryFreeMap = new HashMap<>();
-        hostMemoryFreeMap.put(1L, 2048L * 1024L * 1024L);
+        hostMemoryFreeMap.put(1L, new Ternary<>(512L * 1024L * 1024L, 0L, 8192L * 1024L * 1024L));
-        hostMemoryFreeMap.put(2L, 512L * 1024L * 1024L);
+        hostMemoryFreeMap.put(2L, new Ternary<>(2048L * 1024L * 1024L, 0L, 8192L * 1024L * 1024L));
     }
 
     private void overrideDefaultConfigValue(final ConfigKey configKey, final String name,
@@ -144,7 +133,7 @@ public class BalancedTest {
     @Test
     public void needsDrsWithCpu() throws ConfigurationException, NoSuchFieldException, IllegalAccessException {
         overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "cpu");
-        assertFalse(balanced.needsDrs(clusterId, cpuList, memoryList));
+        assertFalse(balanced.needsDrs(clusterId, new ArrayList<>(hostCpuFreeMap.values()), new ArrayList<>(hostMemoryFreeMap.values())));
     }
 
     /*
@@ -154,14 +143,14 @@ public class BalancedTest {
     @Test
     public void needsDrsWithMemory() throws ConfigurationException, NoSuchFieldException, IllegalAccessException {
         overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "memory");
-        assertTrue(balanced.needsDrs(clusterId, cpuList, memoryList));
+        assertTrue(balanced.needsDrs(clusterId, new ArrayList<>(hostCpuFreeMap.values()), new ArrayList<>(hostMemoryFreeMap.values())));
     }
 
     /* 3. cluster with "unknown" metric */
     @Test
-    public void needsDrsWithUnknown() throws ConfigurationException, NoSuchFieldException, IllegalAccessException {
+    public void needsDrsWithUnknown() throws NoSuchFieldException, IllegalAccessException {
         overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "unknown");
-        assertThrows(ConfigurationException.class, () -> balanced.needsDrs(clusterId, cpuList, memoryList));
+        assertThrows(ConfigurationException.class, () -> balanced.needsDrs(clusterId, new ArrayList<>(hostCpuFreeMap.values()), new ArrayList<>(hostMemoryFreeMap.values())));
     }
 
     /**
@@ -188,7 +177,7 @@ public class BalancedTest {
      improvement = 0.3333 - 0.3333  = 0.0
     */
     @Test
-    public void getMetricsWithCpu() throws NoSuchFieldException, IllegalAccessException {
+    public void getMetricsWithCpu() throws NoSuchFieldException, IllegalAccessException, ConfigurationException {
         overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "cpu");
         Ternary<Double, Double, Double> result = balanced.getMetrics(clusterId, vm3, serviceOffering, destHost,
                 hostCpuFreeMap, hostMemoryFreeMap, false);
@@ -202,7 +191,7 @@ public class BalancedTest {
      improvement = 0.6 - 0.2 = 0.4
     */
     @Test
-    public void getMetricsWithMemory() throws NoSuchFieldException, IllegalAccessException {
+    public void getMetricsWithMemory() throws NoSuchFieldException, IllegalAccessException, ConfigurationException {
         overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "memory");
         Ternary<Double, Double, Double> result = balanced.getMetrics(clusterId, vm3, serviceOffering, destHost,
                 hostCpuFreeMap, hostMemoryFreeMap, false);
@@ -210,18 +199,4 @@ public class BalancedTest {
         assertEquals(0, result.second(), 0.0);
         assertEquals(1, result.third(), 0.0);
     }
-
-    /*
-     3. cluster with default metric
-     improvement = 0.3333 + 0.6 - 0.3333 - 0.2 = 0.4
-    */
-    @Test
-    public void getMetricsWithDefault() throws NoSuchFieldException, IllegalAccessException {
-        overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "both");
-        Ternary<Double, Double, Double> result = balanced.getMetrics(clusterId, vm3, serviceOffering, destHost,
-                hostCpuFreeMap, hostMemoryFreeMap, false);
-        assertEquals(0.4, result.first(), 0.01);
-        assertEquals(0, result.second(), 0.0);
-        assertEquals(1, result.third(), 0.0);
-    }
 }

plugins/drs/cluster/condensed/src/main/java/org/apache/cloudstack/cluster/Condensed.java

@@ -21,78 +21,71 @@ package org.apache.cloudstack.cluster;
 
 import com.cloud.host.Host;
 import com.cloud.offering.ServiceOffering;
-import com.cloud.utils.Pair;
 import com.cloud.utils.Ternary;
 import com.cloud.utils.component.AdapterBase;
 import com.cloud.vm.VirtualMachine;
+import org.apache.log4j.Logger;
 
 import javax.naming.ConfigurationException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 
+import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsImbalanceSkipThreshold;
 import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsImbalanceThreshold;
-import static org.apache.cloudstack.cluster.ClusterDrsService.ClusterDrsMetric;
 
 public class Condensed extends AdapterBase implements ClusterDrsAlgorithm {
 
+    private static final Logger logger = Logger.getLogger(Condensed.class);
+
+    @Override
+    public boolean needsDrs(long clusterId, List<Ternary<Long, Long, Long>> cpuList,
+            List<Ternary<Long, Long, Long>> memoryList) throws ConfigurationException {
+        double threshold = getThreshold(clusterId);
+        Float skipThreshold = ClusterDrsImbalanceSkipThreshold.valueIn(clusterId);
+        Double imbalance = ClusterDrsAlgorithm.getClusterImbalance(clusterId, cpuList, memoryList, skipThreshold);
+        String drsMetric = ClusterDrsAlgorithm.getClusterDrsMetric(clusterId);
+        String metricType = ClusterDrsAlgorithm.getDrsMetricType(clusterId);
+        Boolean useRatio = ClusterDrsAlgorithm.getDrsMetricUseRatio(clusterId);
+        if (imbalance < threshold) {
+
+            logger.debug(String.format("Cluster %d needs DRS. Imbalance: %s Threshold: %s Algorithm: %s DRS metric: %s Metric Type: %s Use ratio: %s SkipThreshold: %s",
+                    clusterId, imbalance, threshold, getName(), drsMetric, metricType, useRatio, skipThreshold));
+            return true;
+        } else {
+            logger.debug(String.format("Cluster %d does not need DRS. Imbalance: %s Threshold: %s Algorithm: %s DRS metric: %s Metric Type: %s Use ratio: %s SkipThreshold: %s",
+                    clusterId, imbalance, threshold, getName(), drsMetric, metricType, useRatio, skipThreshold));
+            return false;
+        }
+    }
+
+    private double getThreshold(long clusterId) {
+        return ClusterDrsImbalanceThreshold.valueIn(clusterId);
+    }
+
     @Override
     public String getName() {
         return "condensed";
     }
 
-    @Override
-    public boolean needsDrs(long clusterId, List<Long> cpuList, List<Long> memoryList) throws ConfigurationException {
-        Double cpuImbalance = getClusterImbalance(cpuList);
-        Double memoryImbalance = getClusterImbalance(memoryList);
-        double threshold = getThreshold(clusterId);
-        String metric = ClusterDrsMetric.valueIn(clusterId);
-        switch (metric) {
-            case "cpu":
-                return cpuImbalance < threshold;
-            case "memory":
-                return memoryImbalance < threshold;
-            default:
-                throw new ConfigurationException(
-                        String.format("Invalid metric: %s for cluster: %d", metric, clusterId));
-        }
-    }
-
-    private double getThreshold(long clusterId) throws ConfigurationException {
-        return ClusterDrsImbalanceThreshold.valueIn(clusterId);
-    }
-
     @Override
     public Ternary<Double, Double, Double> getMetrics(long clusterId, VirtualMachine vm,
             ServiceOffering serviceOffering, Host destHost,
-                                                      Map<Long, Long> hostCpuUsedMap, Map<Long, Long> hostMemoryUsedMap,
+            Map<Long, Ternary<Long, Long, Long>> hostCpuMap, Map<Long, Ternary<Long, Long, Long>> hostMemoryMap,
-                                                      Boolean requiresStorageMotion) {
+            Boolean requiresStorageMotion) throws ConfigurationException {
-        Double preCpuImbalance = getClusterImbalance(new ArrayList<>(hostCpuUsedMap.values()));
+        Double preImbalance = ClusterDrsAlgorithm.getClusterImbalance(clusterId, new ArrayList<>(hostCpuMap.values()),
-        Double preMemoryImbalance = getClusterImbalance(new ArrayList<>(hostMemoryUsedMap.values()));
+                new ArrayList<>(hostMemoryMap.values()), null);
+        Double postImbalance = getImbalancePostMigration(serviceOffering, vm, destHost, hostCpuMap, hostMemoryMap);
 
-        Pair<Double, Double> imbalancePair = getImbalancePostMigration(serviceOffering, vm, destHost, hostCpuUsedMap,
+        logger.debug(String.format("Cluster %d pre-imbalance: %s post-imbalance: %s Algorithm: %s VM: %s srcHost: %d destHost: %s",
-                hostMemoryUsedMap);
+                clusterId, preImbalance, postImbalance, getName(), vm.getUuid(), vm.getHostId(), destHost.getUuid()));
-        Double postCpuImbalance = imbalancePair.first();
-        Double postMemoryImbalance = imbalancePair.second();
 
         // 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
-        double cost = 0;
+        final double improvement = postImbalance - preImbalance;
-        double benefit = 1;
+        final double cost = 0;
-
+        final double benefit = 1;
-        String metric = ClusterDrsMetric.valueIn(clusterId);
-        double improvement;
-        switch (metric) {
-            case "cpu":
-                improvement = postCpuImbalance - preCpuImbalance;
-                break;
-            case "memory":
-                improvement = postMemoryImbalance - preMemoryImbalance;
-                break;
-            default:
-                improvement = postCpuImbalance + postMemoryImbalance - preCpuImbalance - preMemoryImbalance;
-        }
         return new Ternary<>(improvement, cost, benefit);
     }
 }

plugins/drs/cluster/condensed/src/test/java/org/apache/cloudstack/cluster/CondensedTest.java

@@ -35,6 +35,7 @@ import org.mockito.junit.MockitoJUnitRunner;
 
 import javax.naming.ConfigurationException;
 import java.lang.reflect.Field;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
@@ -64,9 +65,7 @@ public class CondensedTest {
 
     Map<Long, List<VirtualMachine>> hostVmMap;
 
-    List<Long> cpuList, memoryList;
+    Map<Long, Ternary<Long, Long, Long>> hostCpuFreeMap, hostMemoryFreeMap;
-
-    Map<Long, Long> hostCpuFreeMap, hostMemoryFreeMap;
 
 
     private AutoCloseable closeable;
@@ -95,16 +94,13 @@ public class CondensedTest {
 
         overrideDefaultConfigValue(ClusterDrsImbalanceThreshold, "_defaultValue", "0.5");
 
-        cpuList = Arrays.asList(1L, 2L);
-        memoryList = Arrays.asList(512L, 2048L);
-
         hostCpuFreeMap = new HashMap<>();
-        hostCpuFreeMap.put(1L, 2000L);
+        hostCpuFreeMap.put(1L, new Ternary<>(1000L, 0L, 10000L));
-        hostCpuFreeMap.put(2L, 1000L);
+        hostCpuFreeMap.put(2L, new Ternary<>(2000L, 0L, 10000L));
 
         hostMemoryFreeMap = new HashMap<>();
-        hostMemoryFreeMap.put(1L, 2048L * 1024L * 1024L);
+        hostMemoryFreeMap.put(1L, new Ternary<>(512L * 1024L * 1024L, 0L, 8192L * 1024L * 1024L));
-        hostMemoryFreeMap.put(2L, 512L * 1024L * 1024L);
+        hostMemoryFreeMap.put(2L, new Ternary<>(2048L * 1024L * 1024L, 0L, 8192L * 1024L * 1024L));
     }
 
     private void overrideDefaultConfigValue(final ConfigKey configKey,
@@ -138,7 +134,7 @@ public class CondensedTest {
     @Test
     public void needsDrsWithCpu() throws ConfigurationException, NoSuchFieldException, IllegalAccessException {
         overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "cpu");
-        assertTrue(condensed.needsDrs(clusterId, cpuList, memoryList));
+        assertTrue(condensed.needsDrs(clusterId, new ArrayList<>(hostCpuFreeMap.values()), new ArrayList<>(hostMemoryFreeMap.values())));
     }
 
     /*
@@ -148,14 +144,14 @@ public class CondensedTest {
     @Test
     public void needsDrsWithMemory() throws ConfigurationException, NoSuchFieldException, IllegalAccessException {
         overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "memory");
-        assertFalse(condensed.needsDrs(clusterId, cpuList, memoryList));
+        assertFalse(condensed.needsDrs(clusterId, new ArrayList<>(hostCpuFreeMap.values()), new ArrayList<>(hostMemoryFreeMap.values())));
     }
 
     /* 3. cluster with "unknown" metric */
     @Test
-    public void needsDrsWithUnknown() throws ConfigurationException, NoSuchFieldException, IllegalAccessException {
+    public void needsDrsWithUnknown() throws NoSuchFieldException, IllegalAccessException {
         overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "unknown");
-        assertThrows(ConfigurationException.class, () -> condensed.needsDrs(clusterId, cpuList, memoryList));
+        assertThrows(ConfigurationException.class, () -> condensed.needsDrs(clusterId, new ArrayList<>(hostCpuFreeMap.values()), new ArrayList<>(hostMemoryFreeMap.values())));
     }
 
     /**
@@ -182,7 +178,7 @@ public class CondensedTest {
      improvement = 0.3333 - 0.3333 = 0.0
     */
     @Test
-    public void getMetricsWithCpu() throws NoSuchFieldException, IllegalAccessException {
+    public void getMetricsWithCpu() throws NoSuchFieldException, IllegalAccessException, ConfigurationException {
         overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "cpu");
         Ternary<Double, Double, Double> result = condensed.getMetrics(clusterId, vm3, serviceOffering, destHost,
                 hostCpuFreeMap, hostMemoryFreeMap, false);
@@ -196,7 +192,7 @@ public class CondensedTest {
      improvement = 0.2 - 0.6 = -0.4
     */
     @Test
-    public void getMetricsWithMemory() throws NoSuchFieldException, IllegalAccessException {
+    public void getMetricsWithMemory() throws NoSuchFieldException, IllegalAccessException, ConfigurationException {
         overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "memory");
         Ternary<Double, Double, Double> result = condensed.getMetrics(clusterId, vm3, serviceOffering, destHost,
                 hostCpuFreeMap, hostMemoryFreeMap, false);
@@ -204,18 +200,4 @@ public class CondensedTest {
         assertEquals(0, result.second(), 0.0);
         assertEquals(1, result.third(), 0.0);
     }
-
-    /*
-     3. cluster with default metric
-     improvement = 0.3333 + 0.2 - 0.3333 - 0.6 = -0.4
-    */
-    @Test
-    public void getMetricsWithDefault() throws NoSuchFieldException, IllegalAccessException {
-        overrideDefaultConfigValue(ClusterDrsMetric, "_defaultValue", "both");
-        Ternary<Double, Double, Double> result = condensed.getMetrics(clusterId, vm3, serviceOffering, destHost,
-                hostCpuFreeMap, hostMemoryFreeMap, false);
-        assertEquals(-0.4, result.first(), 0.0001);
-        assertEquals(0, result.second(), 0.0);
-        assertEquals(1, result.third(), 0.0);
-    }
 }

plugins/metrics/src/main/java/org/apache/cloudstack/metrics/MetricsServiceImpl.java

@@ -29,7 +29,9 @@ import java.util.Map;
 import java.util.Properties;
 
 import javax.inject.Inject;
+import javax.naming.ConfigurationException;
 
+import com.cloud.utils.Ternary;
 import org.apache.cloudstack.api.ApiErrorCode;
 import org.apache.cloudstack.api.ListClustersMetricsCmd;
 import org.apache.cloudstack.api.ListDbMetricsCmd;
@@ -55,6 +57,7 @@ import org.apache.cloudstack.api.response.StoragePoolResponse;
 import org.apache.cloudstack.api.response.UserVmResponse;
 import org.apache.cloudstack.api.response.VolumeResponse;
 import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.cluster.ClusterDrsAlgorithm;
 import org.apache.cloudstack.context.CallContext;
 import org.apache.cloudstack.management.ManagementServerHost.State;
 import org.apache.cloudstack.response.ClusterMetricsResponse;
@@ -762,10 +765,13 @@ public class MetricsServiceImpl extends MutualExclusiveIdsManagerBase implements
             final Long clusterId = cluster.getId();
 
             // CPU and memory capacities
-            final CapacityDaoImpl.SummedCapacity cpuCapacity = getCapacity((int) Capacity.CAPACITY_TYPE_CPU, null, clusterId);
+            final CapacityDaoImpl.SummedCapacity cpuCapacity = getCapacity(Capacity.CAPACITY_TYPE_CPU, null, clusterId);
-            final CapacityDaoImpl.SummedCapacity memoryCapacity = getCapacity((int) Capacity.CAPACITY_TYPE_MEMORY, null, clusterId);
+            final CapacityDaoImpl.SummedCapacity memoryCapacity = getCapacity(Capacity.CAPACITY_TYPE_MEMORY, null, clusterId);
             final HostMetrics hostMetrics = new HostMetrics(cpuCapacity, memoryCapacity);
 
+            List<Ternary<Long, Long, Long>> cpuList = new ArrayList<>();
+            List<Ternary<Long, Long, Long>> memoryList = new ArrayList<>();
+
             for (final Host host: hostDao.findByClusterId(clusterId)) {
                 if (host == null || host.getType() != Host.Type.Routing) {
                     continue;
@@ -774,7 +780,18 @@ public class MetricsServiceImpl extends MutualExclusiveIdsManagerBase implements
                     hostMetrics.incrUpResources();
                 }
                 hostMetrics.incrTotalResources();
-                updateHostMetrics(hostMetrics, hostJoinDao.findById(host.getId()));
+                HostJoinVO hostJoin = hostJoinDao.findById(host.getId());
+                updateHostMetrics(hostMetrics, hostJoin);
+
+                cpuList.add(new Ternary<>(hostJoin.getCpuUsedCapacity(), hostJoin.getCpuReservedCapacity(), hostJoin.getCpus() * hostJoin.getSpeed()));
+                memoryList.add(new Ternary<>(hostJoin.getMemUsedCapacity(), hostJoin.getMemReservedCapacity(), hostJoin.getTotalMemory()));
+            }
+
+            try {
+                Double imbalance = ClusterDrsAlgorithm.getClusterImbalance(clusterId, cpuList, memoryList, null);
+                metricsResponse.setDrsImbalance(imbalance.isNaN() ? null : 100.0 * imbalance);
+            } catch (ConfigurationException e) {
+                LOGGER.warn("Failed to get cluster imbalance for cluster " + clusterId, e);
             }
 
             metricsResponse.setState(clusterResponse.getAllocationState(), clusterResponse.getManagedState());

plugins/metrics/src/main/java/org/apache/cloudstack/response/ClusterMetricsResponse.java

@@ -94,6 +94,10 @@ public class ClusterMetricsResponse extends ClusterResponse implements HostMetri
     @Param(description = "memory allocated disable threshold exceeded")
     private Boolean memoryAllocatedDisableThresholdExceeded;
 
+    @SerializedName("drsimbalance")
+    @Param(description = "DRS imbalance for the cluster")
+    private String drsImbalance;
+
     public void setState(final String allocationState, final String managedState) {
         this.state = allocationState;
         if (managedState.equals("Unmanaged")) {
@@ -208,4 +212,12 @@ public class ClusterMetricsResponse extends ClusterResponse implements HostMetri
             this.memoryAllocatedDisableThresholdExceeded = (1.0 * memAllocated / memTotal) > threshold;
         }
     }
+
+    public void setDrsImbalance(Double drsImbalance) {
+        if (drsImbalance != null) {
+            this.drsImbalance = String.format("%.2f%%", drsImbalance);
+        } else {
+            this.drsImbalance = null;
+        }
+    }
 }

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

@@ -577,6 +577,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
         weightBasedParametersForValidation.add(Config.VmUserDispersionWeight.key());
         weightBasedParametersForValidation.add(CapacityManager.SecondaryStorageCapacityThreshold.key());
         weightBasedParametersForValidation.add(ClusterDrsService.ClusterDrsImbalanceThreshold.key());
+        weightBasedParametersForValidation.add(ClusterDrsService.ClusterDrsImbalanceSkipThreshold.key());
 
     }
 

server/src/main/java/org/apache/cloudstack/cluster/ClusterDrsServiceImpl.java

@@ -353,10 +353,10 @@ public class ClusterDrsServiceImpl extends ManagerBase implements ClusterDrsServ
         List<HostJoinVO> hostJoinList = hostJoinDao.searchByIds(
                 hostList.stream().map(HostVO::getId).toArray(Long[]::new));
 
-        Map<Long, Long> hostCpuMap = hostJoinList.stream().collect(Collectors.toMap(HostJoinVO::getId,
+        Map<Long, Ternary<Long, Long, Long>> hostCpuMap = hostJoinList.stream().collect(Collectors.toMap(HostJoinVO::getId,
-                hostJoin -> hostJoin.getCpus() * hostJoin.getSpeed() - hostJoin.getCpuReservedCapacity() - hostJoin.getCpuUsedCapacity()));
+                hostJoin -> new Ternary<>(hostJoin.getCpuUsedCapacity(), hostJoin.getCpuReservedCapacity(), hostJoin.getCpus() * hostJoin.getSpeed())));
-        Map<Long, Long> hostMemoryMap = hostJoinList.stream().collect(Collectors.toMap(HostJoinVO::getId,
+        Map<Long, Ternary<Long, Long, Long>> hostMemoryMap = hostJoinList.stream().collect(Collectors.toMap(HostJoinVO::getId,
-                hostJoin -> hostJoin.getTotalMemory() - hostJoin.getMemUsedCapacity() - hostJoin.getMemReservedCapacity()));
+                hostJoin -> new Ternary<>(hostJoin.getMemUsedCapacity(), hostJoin.getMemReservedCapacity(), hostJoin.getTotalMemory())));
 
         Map<Long, ServiceOffering> vmIdServiceOfferingMap = new HashMap<>();
 
@@ -375,6 +375,8 @@ public class ClusterDrsServiceImpl extends ManagerBase implements ClusterDrsServ
                 logger.debug("VM migrating to it's original host or no host found for migration");
                 break;
             }
+            logger.debug(String.format("Plan for VM %s to migrate from host %s to host %s", vm.getUuid(),
+                    hostMap.get(vm.getHostId()).getUuid(), destHost.getUuid()));
 
             ServiceOffering serviceOffering = vmIdServiceOfferingMap.get(vm.getId());
             migrationPlan.add(new Ternary<>(vm, hostMap.get(vm.getHostId()), hostMap.get(destHost.getId())));
@@ -387,10 +389,11 @@ public class ClusterDrsServiceImpl extends ManagerBase implements ClusterDrsServ
             long vmCpu = (long) serviceOffering.getCpu() * serviceOffering.getSpeed();
             long vmMemory = serviceOffering.getRamSize() * 1024L * 1024L;
 
-            hostCpuMap.put(vm.getHostId(), hostCpuMap.get(vm.getHostId()) + vmCpu);
+            // Updating the map as per the migration
-            hostCpuMap.put(destHost.getId(), hostCpuMap.get(destHost.getId()) - vmCpu);
+            hostCpuMap.get(vm.getHostId()).first(hostCpuMap.get(vm.getHostId()).first() - vmCpu);
-            hostMemoryMap.put(vm.getHostId(), hostMemoryMap.get(vm.getHostId()) + vmMemory);
+            hostCpuMap.get(destHost.getId()).first(hostCpuMap.get(destHost.getId()).first() + vmCpu);
-            hostMemoryMap.put(destHost.getId(), hostMemoryMap.get(destHost.getId()) - vmMemory);
+            hostMemoryMap.get(vm.getHostId()).first(hostMemoryMap.get(vm.getHostId()).first() - vmMemory);
+            hostMemoryMap.get(destHost.getId()).first(hostMemoryMap.get(destHost.getId()).first() + vmMemory);
             vm.setHostId(destHost.getId());
             iteration++;
         }
@@ -443,8 +446,8 @@ public class ClusterDrsServiceImpl extends ManagerBase implements ClusterDrsServ
     Pair<VirtualMachine, Host> getBestMigration(Cluster cluster, ClusterDrsAlgorithm algorithm,
             List<VirtualMachine> vmList,
             Map<Long, ServiceOffering> vmIdServiceOfferingMap,
-            Map<Long, Long> hostCpuCapacityMap,
+            Map<Long, Ternary<Long, Long, Long>> hostCpuCapacityMap,
-            Map<Long, Long> hostMemoryCapacityMap) {
+            Map<Long, Ternary<Long, Long, Long>> hostMemoryCapacityMap) throws ConfigurationException {
         double improvement = 0;
         Pair<VirtualMachine, Host> bestMigration = new Pair<>(null, null);
 
@@ -627,8 +630,9 @@ public class ClusterDrsServiceImpl extends ManagerBase implements ClusterDrsServ
 
     @Override
     public ConfigKey<?>[] getConfigKeys() {
-        return new ConfigKey<?>[]{ClusterDrsPlanExpireInterval, ClusterDrsEnabled, ClusterDrsInterval,
+        return new ConfigKey<?>[]{ClusterDrsPlanExpireInterval, ClusterDrsEnabled, ClusterDrsInterval, ClusterDrsMaxMigrations,
-                ClusterDrsMaxMigrations, ClusterDrsAlgorithm, ClusterDrsImbalanceThreshold, ClusterDrsMetric};
+                ClusterDrsAlgorithm, ClusterDrsImbalanceThreshold, ClusterDrsMetric, ClusterDrsMetricType, ClusterDrsMetricUseRatio,
+                ClusterDrsImbalanceSkipThreshold};
     }
 
     @Override

server/src/test/java/org/apache/cloudstack/cluster/ClusterDrsServiceImplTest.java

@@ -180,14 +180,12 @@ public class ClusterDrsServiceImplTest {
         Mockito.when(hostJoin1.getCpuUsedCapacity()).thenReturn(1000L);
         Mockito.when(hostJoin1.getCpuReservedCapacity()).thenReturn(0L);
         Mockito.when(hostJoin1.getMemUsedCapacity()).thenReturn(1024L);
-        Mockito.when(hostJoin1.getMemReservedCapacity()).thenReturn(512L);
 
         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.getMemUsedCapacity()).thenReturn(1024L);
-        Mockito.when(hostJoin2.getMemReservedCapacity()).thenReturn(512L);
 
         List<VMInstanceVO> vmList = new ArrayList<>();
         vmList.add(vm1);
@@ -299,7 +297,7 @@ public class ClusterDrsServiceImplTest {
         Mockito.when(clusterDrsService.getResponseObjectForMigrations(Mockito.anyList())).thenReturn(
                 List.of(migrationResponse));
 
-        try(MockedStatic<ActionEventUtils> ignored = Mockito.mockStatic(ActionEventUtils.class)) {
+        try (MockedStatic<ActionEventUtils> ignored = Mockito.mockStatic(ActionEventUtils.class)) {
             Mockito.when(ActionEventUtils.onActionEvent(Mockito.anyLong(), Mockito.anyLong(),
                     Mockito.anyLong(),
                     Mockito.anyString(), Mockito.anyString(),
@@ -350,7 +348,7 @@ public class ClusterDrsServiceImplTest {
     }
 
     @Test
-    public void testGetBestMigration() {
+    public void testGetBestMigration() throws ConfigurationException {
         ClusterVO cluster = Mockito.mock(ClusterVO.class);
         Mockito.when(cluster.getId()).thenReturn(1L);
 

ui/public/locales/en.json

@@ -782,6 +782,7 @@
 "label.dpd": "Dead peer detection",
 "label.driver": "Driver",
 "label.drs": "DRS",
+"label.drsimbalance": "DRS imbalance",
 "label.drs.plan": "DRS Plan",
 "label.drs.generate.plan": "Generate DRS plan",
 "label.drs.no.plan.generated": "No DRS plan has been generated as the cluster is not imbalanced according to the threshold set",
@@ -1298,7 +1299,7 @@
 "label.memoryallocatedgb": "Memory allocated",
 "label.memorylimit": "Memory limits (MiB)",
 "label.memorymaxdeviation": "Deviation",
-"label.memorytotal": "Memory allocated",
+"label.memorytotal": "Memory total",
 "label.memorytotalgb": "Memory total",
 "label.memoryused": "Used memory",
 "label.memoryusedgb": "Memory used",

ui/src/components/view/ListView.vue

@@ -337,6 +337,9 @@
       <template v-if="column.key === 'templateversion'">
         <span>  {{ record.version }} </span>
       </template>
+      <template v-if="column.key === 'drsimbalance'">
+        <span>  {{ record.drsimbalance }} </span>
+      </template>
       <template v-if="column.key === 'softwareversion'">
         <span>  {{ record.softwareversion ? record.softwareversion : 'N/A' }} </span>
       </template>

ui/src/config/section/infra/clusters.js

@@ -26,7 +26,7 @@ export default {
   permission: ['listClustersMetrics'],
   columns: () => {
     const fields = ['name', 'state', 'allocationstate', 'clustertype', 'hypervisortype', 'hosts']
-    const metricsFields = ['cpuused', 'cpumaxdeviation', 'cpuallocated', 'cputotal', 'memoryused', 'memorymaxdeviation', 'memoryallocated', 'memorytotal']
+    const metricsFields = ['cpuused', 'cpumaxdeviation', 'cpuallocated', 'cputotal', 'memoryused', 'memorymaxdeviation', 'memoryallocated', 'memorytotal', 'drsimbalance']
     if (store.getters.metrics) {
       fields.push(...metricsFields)
     }
@@ -34,7 +34,7 @@ export default {
     fields.push('zonename')
     return fields
   },
-  details: ['name', 'id', 'allocationstate', 'clustertype', 'managedstate', 'hypervisortype', 'podname', 'zonename'],
+  details: ['name', 'id', 'allocationstate', 'clustertype', 'managedstate', 'hypervisortype', 'podname', 'zonename', 'drsimbalance'],
   related: [{
     name: 'host',
     title: 'label.hosts',

ui/src/views/infra/ClusterDRSTab.vue

@@ -57,7 +57,8 @@
         :columns="migrationColumns"
         :dataSource="record.migrations"
         :rowKey="(record, index) => index"
-        :pagination="{hideOnSinglePage: true, showSizeChanger: true}">
+        :pagination="{hideOnSinglePage: true, showSizeChanger: true}"
+        @resizeColumn="resizeColumn">
         <template #bodyCell="{ column, text, record }">
           <template v-if="column.key === 'vm'">
             <router-link :to="{ path: '/vm/' + record.virtualmachineid }">
@@ -117,7 +118,8 @@
       :columns="generatedPlanMigrationColumns"
       :dataSource="generatedMigrations"
       :rowKey="(record, index) => index"
-      :pagination="{ showTotal: (total, range) => [range[0], '-', range[1], $t('label.of'), total, $t('label.items')].join(' ') }" >
+      :pagination="{ showTotal: (total, range) => [range[0], '-', range[1], $t('label.of'), total, $t('label.items')].join(' ') }"
+      @resizeColumn="resizeColumn" >
       <template #bodyCell="{ column, text, record }">
         <template v-if="column.key === 'vm'">
           <router-link :to="{ path: '/vm/' + record.virtualmachineid }">
@@ -166,19 +168,22 @@ export default {
         key: 'vm',
         title: this.$t('label.vm'),
         dataIndex: 'vm',
-        ellipsis: true
+        ellipsis: true,
+        resizable: true
       },
       {
         key: 'sourcehost',
         title: this.$t('label.sourcehost'),
         dataIndex: 'sourcehost',
-        ellipsis: true
+        ellipsis: true,
+        resizable: true
       },
       {
         key: 'destinationhost',
         title: this.$t('label.desthost'),
         dataIndex: 'created',
-        ellipsis: true
+        ellipsis: true,
+        resizable: true
       }
     ]
     return {
@@ -291,6 +296,9 @@ export default {
     closeModal () {
       this.showModal = false
       this.generatedMigrations = reactive([])
+    },
+    resizeColumn (w, col) {
+      col.width = w
     }
   }
 }