Refactor of Allocator classes (#9074)

* Refactoring Allocator classes

* Break into smaller methods random and firfit allocators.

* Added unit tests for random and firstfit allocators

* Move random allocator from cloud-plugins to cloud-server

* Add BaseAllocator abstract class for duplicate code

* Add missing license

* Add missing license to unit test file

* Remove host allocator random dependency

* Change exception message on smoke tests

* Remove conditional as it was never actually reached in the original flow

* Fix tests

* Fix flipped parameters

* Fix NPE while listing hosts for migration when suitableHosts is null

* Remove unnecessary stubbings

* Fix checkstyle

* Remove unnecessary file

* Rename exception error messages

* Apply suggestions from code review

Co-authored-by: Fabricio Duarte <fabricio.duarte.jr@gmail.com>

* Rename UserVmDetailVO references to VMInstanceDetailVO

* Remove unused imports

* Add new line at EOF

* Remove unnecessary random allocator pom

* Fix GPU allocation mistake

* Fix failing tests

---------

Co-authored-by: Fabricio Duarte <fabricio.duarte@scclouds.com.br>
Co-authored-by: Fabricio Duarte <fabricio.duarte.jr@gmail.com>

Author: 3 people

api/src/main/java/com/cloud/agent/manager/allocator/HostAllocator.java

@@ -22,19 +22,11 @@
 import com.cloud.deploy.DeploymentPlanner.ExcludeList;
 import com.cloud.host.Host;
 import com.cloud.host.Host.Type;
-import com.cloud.offering.ServiceOffering;
 import com.cloud.utils.component.Adapter;
-import com.cloud.vm.VirtualMachine;
 import com.cloud.vm.VirtualMachineProfile;
 
 public interface HostAllocator extends Adapter {
 
-    /**
-     * @param UserVm vm
-     * @param ServiceOffering offering
-     **/
-    boolean isVirtualMachineUpgradable(final VirtualMachine vm, final ServiceOffering offering);
-
     /**
     * Determines which physical hosts are suitable to
     * allocate the guest virtual machines on
@@ -49,31 +41,6 @@ public interface HostAllocator extends Adapter {
 
     public List<Host> allocateTo(VirtualMachineProfile vmProfile, DeploymentPlan plan, Type type, ExcludeList avoid, int returnUpTo);
 
-    /**
-     * Determines which physical hosts are suitable to allocate the guest
-     * virtual machines on
-     *
-     * Allocators must set any other hosts not considered for allocation in the
-     * ExcludeList avoid. Thus the avoid set and the list of hosts suitable,
-     * together must cover the entire host set in the cluster.
-     *
-     * @param VirtualMachineProfile
-     *            vmProfile
-     * @param DeploymentPlan
-     *            plan
-     * @param GuestType
-     *            type
-     * @param ExcludeList
-     *            avoid
-     * @param int returnUpTo (use -1 to return all possible hosts)
-     * @param boolean considerReservedCapacity (default should be true, set to
-     *        false if host capacity calculation should not look at reserved
-     *        capacity)
-     * @return List<Host> List of hosts that are suitable for VM allocation
-     **/
-
-    public List<Host> allocateTo(VirtualMachineProfile vmProfile, DeploymentPlan plan, Type type, ExcludeList avoid, int returnUpTo, boolean considerReservedCapacity);
-
     /**
      * Determines which physical hosts are suitable to allocate the guest
      * virtual machines on

api/src/main/java/com/cloud/deploy/DeploymentPlanner.java

@@ -70,7 +70,7 @@ public interface DeploymentPlanner extends Adapter {
     boolean canHandle(VirtualMachineProfile vm, DeploymentPlan plan, ExcludeList avoid);
 
     public enum AllocationAlgorithm {
-        random, firstfit, userdispersing;
+        random, firstfit, userdispersing, firstfitleastconsumed;
     }
 
     public enum PlannerResourceUsage {

api/src/main/java/org/apache/cloudstack/api/command/admin/host/FindHostsForMigrationCmd.java

@@ -78,7 +78,7 @@ public void execute() {
         for (Host host : result.first()) {
             HostForMigrationResponse hostResponse = _responseGenerator.createHostForMigrationResponse(host);
             Boolean suitableForMigration = false;
-            if (hostsWithCapacity.contains(host)) {
+            if (hostsWithCapacity != null && hostsWithCapacity.contains(host)) {
                 suitableForMigration = true;
             }
             hostResponse.setSuitableForMigration(suitableForMigration);

api/src/main/java/org/apache/cloudstack/api/command/admin/host/ListHostsCmd.java

@@ -252,7 +252,7 @@ protected ListResponse<HostResponse> getHostResponses() {
             for (Host host : result.first()) {
                 HostResponse hostResponse = _responseGenerator.createHostResponse(host, getDetails());
                 Boolean suitableForMigration = false;
-                if (hostsWithCapacity.contains(host)) {
+                if (hostsWithCapacity != null && hostsWithCapacity.contains(host)) {
                     suitableForMigration = true;
                 }
                 hostResponse.setSuitableForMigration(suitableForMigration);

client/pom.xml

@@ -377,11 +377,6 @@
             <artifactId>cloud-plugin-explicit-dedication</artifactId>
             <version>${project.version}</version>
         </dependency>
-        <dependency>
-            <groupId>org.apache.cloudstack</groupId>
-            <artifactId>cloud-plugin-host-allocator-random</artifactId>
-            <version>${project.version}</version>
-        </dependency>
         <dependency>
             <groupId>org.apache.cloudstack</groupId>
             <artifactId>cloud-plugin-outofbandmanagement-driver-ipmitool</artifactId>

engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java

@@ -181,15 +181,6 @@ void orchestrateStart(String vmUuid, Map<VirtualMachineProfile.Param, Object> pa
     void advanceReboot(String vmUuid, Map<VirtualMachineProfile.Param, Object> params) throws InsufficientCapacityException, ResourceUnavailableException,
         ConcurrentOperationException, OperationTimedoutException;
 
-    /**
-     * Check to see if a virtual machine can be upgraded to the given service offering
-     *
-     * @param vm
-     * @param offering
-     * @return true if the host can handle the upgrade, false otherwise
-     */
-    boolean isVirtualMachineUpgradable(final VirtualMachine vm, final ServiceOffering offering);
-
     VirtualMachine findById(long vmId);
 
     void storageMigration(String vmUuid, Map<Long, Long> volumeToPool);

engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

@@ -4017,19 +4017,6 @@ protected void runInContext() {
         }
     }
 
-    @Override
-    public boolean isVirtualMachineUpgradable(final VirtualMachine vm, final ServiceOffering offering) {
-        boolean isMachineUpgradable = true;
-        for (final HostAllocator allocator : hostAllocators) {
-            isMachineUpgradable = allocator.isVirtualMachineUpgradable(vm, offering);
-            if (!isMachineUpgradable) {
-                break;
-            }
-        }
-
-        return isMachineUpgradable;
-    }
-
     @Override
     public void reboot(final String vmUuid, final Map<VirtualMachineProfile.Param, Object> params) throws InsufficientCapacityException, ResourceUnavailableException {
         try {
@@ -4469,11 +4456,6 @@ public void checkIfCanUpgrade(final VirtualMachine vmInstance, final ServiceOffe
             throw new InvalidParameterValueException("isSystem property is different for current service offering and new service offering");
         }
 
-        if (!isVirtualMachineUpgradable(vmInstance, newServiceOffering)) {
-            throw new InvalidParameterValueException("Unable to upgrade virtual machine, not enough resources available " + "for an offering of " +
-                    newServiceOffering.getCpu() + " cpu(s) at " + newServiceOffering.getSpeed() + " Mhz, and " + newServiceOffering.getRamSize() + " MB of memory");
-        }
-
         final List<String> currentTags = StringUtils.csvTagsToList(currentDiskOffering.getTags());
         final List<String> newTags = StringUtils.csvTagsToList(newDiskOffering.getTags());
         if (VolumeApiServiceImpl.MatchStoragePoolTagsWithDiskOffering.valueIn(vmInstance.getDataCenterId())) {

engine/schema/src/main/java/com/cloud/host/dao/HostDao.java

@@ -218,7 +218,7 @@ int countHostsByMsResourceStateTypeAndHypervisorType(long msId, List<ResourceSta
      */
     List<String> listOrderedHostsHypervisorVersionsInDatacenter(long datacenterId, HypervisorType hypervisorType);
 
-    List<HostVO> findHostsWithTagRuleThatMatchComputeOferringTags(String computeOfferingTags);
+    List<HostVO> findHostsWithTagRuleThatMatchComputeOfferingTags(String computeOfferingTags);
 
     List<Long> findClustersThatMatchHostTagRule(String computeOfferingTags);
 

engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java

@@ -1520,7 +1520,7 @@ private List<Long> findHostIdsByHostTags(String hostTags){
         }
     }
 
-    public List<HostVO> findHostsWithTagRuleThatMatchComputeOferringTags(String computeOfferingTags) {
+    public List<HostVO> findHostsWithTagRuleThatMatchComputeOfferingTags(String computeOfferingTags) {
         List<HostTagVO> hostTagVOList = _hostTagsDao.findHostRuleTags();
         List<HostVO> result = new ArrayList<>();
         for (HostTagVO rule: hostTagVOList) {
@@ -1534,7 +1534,7 @@ public List<HostVO> findHostsWithTagRuleThatMatchComputeOferringTags(String comp
 
     public List<Long> findClustersThatMatchHostTagRule(String computeOfferingTags) {
         Set<Long> result = new HashSet<>();
-        List<HostVO> hosts = findHostsWithTagRuleThatMatchComputeOferringTags(computeOfferingTags);
+        List<HostVO> hosts = findHostsWithTagRuleThatMatchComputeOfferingTags(computeOfferingTags);
         for (HostVO host: hosts) {
             result.add(host.getClusterId());
         }