Fix allocation of VMs with multiple clusters (#8611)

* Fix allocation of VMs with multiple clusters

* Readd debug guard

Author: BryanMLima

plugins/host-allocators/random/src/main/java/com/cloud/agent/manager/allocator/impl/RandomAllocator.java

@@ -22,7 +22,6 @@
 
 import javax.inject.Inject;
 
-import com.cloud.utils.exception.CloudRuntimeException;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.collections.ListUtils;
 import org.apache.log4j.Logger;
@@ -36,7 +35,6 @@
 import com.cloud.deploy.DeploymentPlanner.ExcludeList;
 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.offering.ServiceOffering;
 import com.cloud.resource.ResourceManager;
@@ -66,18 +64,20 @@ private List<Host> findSuitableHosts(VirtualMachineProfile vmProfile, Deployment
         Long podId = plan.getPodId();
         Long clusterId = plan.getClusterId();
         ServiceOffering offering = vmProfile.getServiceOffering();
-        List<? extends Host> hostsCopy = null;
-        List<Host> suitableHosts = new ArrayList<Host>();
+        List<Host> suitableHosts = new ArrayList<>();
 
         if (type == Host.Type.Storage) {
             return suitableHosts;
         }
+
         String hostTag = offering.getHostTag();
-        if (hostTag != null) {
-            s_logger.debug(String.format("Looking for hosts in dc [%s], pod [%s], cluster [%s] and complying with host tag [%s].", dcId, podId, clusterId, hostTag));
-        } else {
-            s_logger.debug("Looking for hosts in dc: " + dcId + "  pod:" + podId + "  cluster:" + clusterId);
-        }
+        String hostTagToLog = hostTag != null ? String.format("and complying with host tags [%s]", hostTag) : "";
+        String paramAsStringToLog = String.format("zone [%s], pod [%s], cluster [%s] %s", dcId, podId, clusterId, hostTagToLog);
+
+        s_logger.debug(String.format("Looking for hosts in %s.", paramAsStringToLog));
+
+        List<? extends Host> hostsCopy;
+
         if (hosts != null) {
             // retain all computing hosts, regardless of whether they support routing...it's random after all
             hostsCopy = new ArrayList<Host>(hosts);
@@ -88,7 +88,6 @@ private List<Host> findSuitableHosts(VirtualMachineProfile vmProfile, Deployment
             }
         } else {
             // list all computing hosts, regardless of whether they support routing...it's random after all
-            hostsCopy = new ArrayList<HostVO>();
             if (hostTag != null) {
                 hostsCopy = _hostDao.listByHostTag(type, clusterId, podId, dcId, hostTag);
             } else {
@@ -98,14 +97,15 @@ private List<Host> findSuitableHosts(VirtualMachineProfile vmProfile, Deployment
         hostsCopy = ListUtils.union(hostsCopy, _hostDao.findHostsWithTagRuleThatMatchComputeOferringTags(hostTag));
 
         if (hostsCopy.isEmpty()) {
-            s_logger.error(String.format("No suitable host found for vm [%s] with tags [%s].", vmProfile, hostTag));
-            throw new CloudRuntimeException(String.format("No suitable host found for vm [%s].", vmProfile));
+            s_logger.info(String.format("No suitable host found for VM [%s] in %s.", vmProfile, paramAsStringToLog));
+            return null;
         }
 
         s_logger.debug("Random Allocator found " + hostsCopy.size() + "  hosts");
-        if (hostsCopy.size() == 0) {
+        if (hostsCopy.isEmpty()) {
             return suitableHosts;
         }
+
         Collections.shuffle(hostsCopy);
         for (Host host : hostsCopy) {
             if (suitableHosts.size() == returnUpTo) {

server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java

@@ -25,7 +25,6 @@
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
-import com.cloud.utils.exception.CloudRuntimeException;
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
@@ -132,8 +131,10 @@ public List<Host> allocateTo(VirtualMachineProfile vmProfile, DeploymentPlan pla
             return new ArrayList<Host>();
         }
 
+        String paramAsStringToLog = String.format("zone [%s], pod [%s], cluster [%s]", dcId, podId, clusterId);
+
         if (s_logger.isDebugEnabled()) {
-            s_logger.debug("Looking for hosts in dc: " + dcId + "  pod:" + podId + "  cluster:" + clusterId);
+            s_logger.debug(String.format("Looking for hosts in %s.", paramAsStringToLog));
         }
 
         String hostTagOnOffering = offering.getHostTag();
@@ -206,8 +207,8 @@ public List<Host> allocateTo(VirtualMachineProfile vmProfile, DeploymentPlan pla
 
 
         if (clusterHosts.isEmpty()) {
-            s_logger.error(String.format("No suitable host found for vm [%s] with tags [%s].", vmProfile, hostTagOnOffering));
-            throw new CloudRuntimeException(String.format("No suitable host found for vm [%s].", vmProfile));
+            s_logger.info(String.format("No suitable host found for VM [%s] with tags [%s] in %s.", vmProfile, hostTagOnOffering, paramAsStringToLog));
+            return null;
         }
         // add all hosts that we are not considering to the avoid list
         List<HostVO> allhostsInCluster = _hostDao.listAllUpAndEnabledNonHAHosts(type, clusterId, podId, dcId, null);

server/src/main/java/com/cloud/agent/manager/allocator/impl/RecreateHostAllocator.java

@@ -26,6 +26,7 @@
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import org.apache.commons.collections.CollectionUtils;
 import org.apache.log4j.Logger;
 import org.springframework.stereotype.Component;
 
@@ -75,7 +76,7 @@ public class RecreateHostAllocator extends FirstFitRoutingAllocator {
     public List<Host> allocateTo(VirtualMachineProfile vm, DeploymentPlan plan, Type type, ExcludeList avoid, int returnUpTo) {
 
         List<Host> hosts = super.allocateTo(vm, plan, type, avoid, returnUpTo);
-        if (hosts != null && !hosts.isEmpty()) {
+        if (CollectionUtils.isNotEmpty(hosts)) {
             return hosts;
         }
 

server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java

@@ -1273,7 +1273,7 @@ private DeployDestination checkClustersforDestination(List<Long> clusterList, Vi
                 List<Host> suitableHosts = findSuitableHosts(vmProfile, potentialPlan, avoid, HostAllocator.RETURN_UPTO_ALL);
                 // if found suitable hosts in this cluster, find suitable storage
                 // pools for each volume of the VM
-                if (suitableHosts != null && !suitableHosts.isEmpty()) {
+                if (CollectionUtils.isNotEmpty(suitableHosts)) {
                     if (vmProfile.getHypervisorType() == HypervisorType.BareMetal) {
                         DeployDestination dest = new DeployDestination(dc, pod, clusterVO, suitableHosts.get(0));
                         return dest;
@@ -1621,18 +1621,17 @@ protected List<Host> findSuitableHosts(VirtualMachineProfile vmProfile, Deployme
         List<Host> suitableHosts = new ArrayList<Host>();
         for (HostAllocator allocator : _hostAllocators) {
             suitableHosts = allocator.allocateTo(vmProfile, plan, Host.Type.Routing, avoid, returnUpTo);
-            if (suitableHosts != null && !suitableHosts.isEmpty()) {
+            if (CollectionUtils.isNotEmpty(suitableHosts)) {
                 break;
             }
         }
 
-        if (suitableHosts.isEmpty()) {
-            s_logger.debug("No suitable hosts found");
+        if (CollectionUtils.isEmpty(suitableHosts)) {
+            s_logger.debug("No suitable hosts found.");
+        } else {
+            reorderHostsByPriority(plan.getHostPriorities(), suitableHosts);
         }
 
-        // re-order hosts by priority
-        reorderHostsByPriority(plan.getHostPriorities(), suitableHosts);
-
         return suitableHosts;
     }
 

server/src/main/java/com/cloud/deploy/FirstFitPlanner.java

@@ -525,7 +525,7 @@ private void removeClustersWithoutMatchingTag(List<Long> clusterListForVmAllocat
         matchingClusters.addAll(hostDao.findClustersThatMatchHostTagRule(hostTagOnOffering));
 
         if (matchingClusters.isEmpty()) {
-            s_logger.error(String.format("No suitable host found for the following compute offering tags [%s].", hostTagOnOffering));
+            s_logger.error(String.format("No suitable host found in any cluster for the following compute offering tags [%s].", hostTagOnOffering));
             throw new CloudRuntimeException("No suitable host found.");
         }
 

server/src/main/java/com/cloud/server/ManagementServerImpl.java

@@ -1590,20 +1590,17 @@ public Ternary<Pair<List<? extends Host>, Integer>, List<? extends Host>, Map<Ho
                 suitableHosts = allocator.allocateTo(vmProfile, plan, Host.Type.Routing, excludes, HostAllocator.RETURN_UPTO_ALL, false);
             }
 
-            if (suitableHosts != null && !suitableHosts.isEmpty()) {
+            if (CollectionUtils.isNotEmpty(suitableHosts)) {
                 break;
             }
         }
 
-        // re-order hosts by priority
         _dpMgr.reorderHostsByPriority(plan.getHostPriorities(), suitableHosts);
 
-        if (s_logger.isDebugEnabled()) {
-            if (suitableHosts.isEmpty()) {
-                s_logger.debug("No suitable hosts found");
-            } else {
-                s_logger.debug("Hosts having capacity and suitable for migration: " + suitableHosts);
-            }
+        if (suitableHosts.isEmpty()) {
+            s_logger.warn("No suitable hosts found.");
+        } else {
+            s_logger.debug("Hosts having capacity and suitable for migration: " + suitableHosts);
         }
 
         return new Ternary<>(otherHosts, suitableHosts, requiresStorageMotion);