Introduce volume allocation algorithm global configuration

engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java

@@ -89,7 +89,7 @@ public interface VolumeOrchestrationService {
             "Advanced",
             "random",
             "Order in which storage pool within a cluster will be considered for volume allocation. The value can be 'random', 'firstfit', 'userdispersing', 'userconcentratedpod_random', 'userconcentratedpod_firstfit', or 'firstfitleastconsumed'.",
-            false,
+            true,
             ConfigKey.Scope.Global, null, null, null, null, null,
             ConfigKey.Kind.Select,
             "random,firstfit,userdispersing,userconcentratedpod_random,userconcentratedpod_firstfit,firstfitleastconsumed");

engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java

@@ -67,7 +67,6 @@
 public abstract class AbstractStoragePoolAllocator extends AdapterBase implements StoragePoolAllocator {
 
     protected BigDecimal storageOverprovisioningFactor = new BigDecimal(1);
-    protected String volumeAllocationAlgorithm = "random";
     protected long extraBytesPerVolume = 0;
     @Inject protected DataStoreManager dataStoreMgr;
     @Inject protected PrimaryDataStoreDao storagePoolDao;
@@ -95,10 +94,6 @@
             String globalStorageOverprovisioningFactor = configs.get("storage.overprovisioning.factor");
             storageOverprovisioningFactor = new BigDecimal(NumbersUtil.parseFloat(globalStorageOverprovisioningFactor, 2.0f));
             extraBytesPerVolume = 0;
-            String volAllocationAlgorithm = VolumeOrchestrationService.VolumeAllocationAlgorithm.value();
-            if (volAllocationAlgorithm != null) {
-                this.volumeAllocationAlgorithm = volAllocationAlgorithm;
-            }
             return true;
         }
         return false;
@@ -209,7 +204,7 @@
             account = vmProfile.getOwner();
         }
 
-        pools = reorderStoragePoolsBasedOnAlgorithm(pools, plan, account);
+        pools = reorderStoragePoolsBasedOnAlgorithm(pools, plan, account, VolumeOrchestrationService.VolumeAllocationAlgorithm.value());
 
         if (vmProfile.getVirtualMachine() == null) {
             if (logger.isTraceEnabled()) {
@@ -226,13 +221,13 @@
         return pools;
     }
 
-    List<StoragePool> reorderStoragePoolsBasedOnAlgorithm(List<StoragePool> pools, DeploymentPlan plan, Account account) {
+    List<StoragePool> reorderStoragePoolsBasedOnAlgorithm(List<StoragePool> pools, DeploymentPlan plan, Account account, String volumeAllocationAlgorithm) {
         logger.debug("Using volume allocation algorithm {} to reorder pools.", volumeAllocationAlgorithm);
         if (volumeAllocationAlgorithm.equals("random") || volumeAllocationAlgorithm.equals("userconcentratedpod_random") || (account == null)) {
             reorderRandomPools(pools);
         } else if (StringUtils.equalsAny(volumeAllocationAlgorithm, "userdispersing", "firstfitleastconsumed")) {
             if (logger.isTraceEnabled()) {
                 logger.trace("Using reordering algorithm {}", volumeAllocationAlgorithm);
             }
 
             if (volumeAllocationAlgorithm.equals("userdispersing")) {
@@ -247,7 +242,7 @@
     void reorderRandomPools(List<StoragePool> pools) {
         StorageUtil.traceLogStoragePools(pools, logger, "pools to choose from: ");
         if (logger.isTraceEnabled()) {
-            logger.trace("Shuffle this so that we don't check the pools in the same order. Algorithm == {} (or no account?)", volumeAllocationAlgorithm);
+            logger.trace("Shuffle this so that we don't check the pools in the same order. Algorithm == 'random' (or no account?)");
         }
         StorageUtil.traceLogStoragePools(pools, logger, "pools to shuffle: ");
         Collections.shuffle(pools, secureRandom);

engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocatorTest.java

@@ -17,14 +17,17 @@
 package org.apache.cloudstack.storage.allocator;
 
 
-import static org.mockito.Mockito.when;
-
-import java.util.ArrayList;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
+import com.cloud.deploy.DeploymentPlan;
+import com.cloud.deploy.DeploymentPlanner;
+import com.cloud.storage.Storage;
+import com.cloud.storage.StoragePool;
+import com.cloud.storage.dao.VolumeDao;
+import com.cloud.user.Account;
+import com.cloud.vm.DiskProfile;
+import com.cloud.vm.VirtualMachineProfile;
 
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
@@ -34,14 +37,12 @@
 import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnitRunner;
 
-import com.cloud.deploy.DeploymentPlan;
-import com.cloud.deploy.DeploymentPlanner;
-import com.cloud.storage.Storage;
-import com.cloud.storage.StoragePool;
-import com.cloud.storage.dao.VolumeDao;
-import com.cloud.user.Account;
-import com.cloud.vm.DiskProfile;
-import com.cloud.vm.VirtualMachineProfile;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import static org.mockito.Mockito.when;
 
 @RunWith(MockitoJUnitRunner.class)
 public class AbstractStoragePoolAllocatorTest {
@@ -74,25 +75,23 @@ public void tearDown() {
 
     @Test
     public void reorderStoragePoolsBasedOnAlgorithm_random() {
-        allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account);
+        allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account, "random");
         Mockito.verify(allocator, Mockito.times(0)).reorderPoolsByCapacity(plan, pools);
         Mockito.verify(allocator, Mockito.times(0)).reorderPoolsByNumberOfVolumes(plan, pools, account);
         Mockito.verify(allocator, Mockito.times(1)).reorderRandomPools(pools);
     }
 
     @Test
     public void reorderStoragePoolsBasedOnAlgorithm_userdispersing() {
-        allocator.volumeAllocationAlgorithm = "userdispersing";
         Mockito.doReturn(pools).when(allocator).reorderPoolsByNumberOfVolumes(plan, pools, account);
-        allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account);
+        allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account,"userdispersing");
         Mockito.verify(allocator, Mockito.times(0)).reorderPoolsByCapacity(plan, pools);
         Mockito.verify(allocator, Mockito.times(1)).reorderPoolsByNumberOfVolumes(plan, pools, account);
         Mockito.verify(allocator, Mockito.times(0)).reorderRandomPools(pools);
     }
 
     @Test
     public void reorderStoragePoolsBasedOnAlgorithm_userdispersing_reorder_check() {
-        allocator.volumeAllocationAlgorithm = "userdispersing";
         allocator.volumeDao = volumeDao;
 
         when(plan.getDataCenterId()).thenReturn(1l);
@@ -104,7 +103,7 @@ public void reorderStoragePoolsBasedOnAlgorithm_userdispersing_reorder_check() {
         poolIds.add(9l);
         when(volumeDao.listPoolIdsByVolumeCount(1l,1l,1l,1l)).thenReturn(poolIds);
 
-        List<StoragePool> reorderedPools = allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account);
+        List<StoragePool> reorderedPools = allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account, "userdispersing");
         Assert.assertEquals(poolIds.size(),reorderedPools.size());
 
         Mockito.verify(allocator, Mockito.times(0)).reorderPoolsByCapacity(plan, pools);
@@ -115,9 +114,8 @@ public void reorderStoragePoolsBasedOnAlgorithm_userdispersing_reorder_check() {
 
     @Test
     public void reorderStoragePoolsBasedOnAlgorithm_firstfitleastconsumed() {
-        allocator.volumeAllocationAlgorithm = "firstfitleastconsumed";
         Mockito.doReturn(pools).when(allocator).reorderPoolsByCapacity(plan, pools);
-        allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account);
+        allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account, "firstfitleastconsumed");
         Mockito.verify(allocator, Mockito.times(1)).reorderPoolsByCapacity(plan, pools);
         Mockito.verify(allocator, Mockito.times(0)).reorderPoolsByNumberOfVolumes(plan, pools, account);
         Mockito.verify(allocator, Mockito.times(0)).reorderRandomPools(pools);

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

@@ -25,11 +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.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
-import org.springframework.stereotype.Component;
-
 import com.cloud.agent.manager.allocator.HostAllocator;
 import com.cloud.capacity.CapacityManager;
 import com.cloud.capacity.CapacityVO;
@@ -38,6 +33,7 @@
 import com.cloud.dc.ClusterDetailsDao;
 import com.cloud.dc.dao.ClusterDao;
 import com.cloud.deploy.DeploymentPlan;
+import com.cloud.deploy.DeploymentClusterPlanner;
 import com.cloud.deploy.DeploymentPlanner.ExcludeList;
 import com.cloud.gpu.GPU;
 import com.cloud.host.DetailVO;
@@ -58,12 +54,17 @@
 import com.cloud.user.Account;
 import com.cloud.utils.Pair;
 import com.cloud.utils.component.AdapterBase;
+import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.vm.UserVmDetailVO;
 import com.cloud.vm.VirtualMachine;
 import com.cloud.vm.VirtualMachineProfile;
 import com.cloud.vm.dao.UserVmDetailsDao;
 import com.cloud.vm.dao.VMInstanceDao;
 
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
+
+import org.springframework.stereotype.Component;
 
 /**
  * An allocator that tries to find a fit on a computing host.  This allocator does not care whether or not the host supports routing.
@@ -98,8 +99,6 @@
     UserVmDetailsDao _userVmDetailsDao;
 
     boolean _checkHvm = true;
-    protected String _allocationAlgorithm = "random";
-
 
     @Override
     public List<Host> allocateTo(VirtualMachineProfile vmProfile, DeploymentPlan plan, Type type, ExcludeList avoid, int returnUpTo) {
@@ -286,12 +285,13 @@
 
     protected List<Host> allocateTo(DeploymentPlan plan, ServiceOffering offering, VMTemplateVO template, ExcludeList avoid, List<? extends Host> hosts, int returnUpTo,
         boolean considerReservedCapacity, Account account) {
-        if (_allocationAlgorithm.equals("random") || _allocationAlgorithm.equals("userconcentratedpod_random")) {
+        String vmAllocationAlgorithm = DeploymentClusterPlanner.VmAllocationAlgorithm.value();
+        if (vmAllocationAlgorithm.equals("random") || vmAllocationAlgorithm.equals("userconcentratedpod_random")) {
             // Shuffle this so that we don't check the hosts in the same order.
             Collections.shuffle(hosts);
-        } else if (_allocationAlgorithm.equals("userdispersing")) {
+        } else if (vmAllocationAlgorithm.equals("userdispersing")) {
             hosts = reorderHostsByNumberOfVms(plan, hosts, account);
-        }else if(_allocationAlgorithm.equals("firstfitleastconsumed")){
+        }else if(vmAllocationAlgorithm.equals("firstfitleastconsumed")){
             hosts = reorderHostsByCapacity(plan, hosts);
         }
 
@@ -575,11 +575,6 @@
     public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
         if (_configDao != null) {
             Map<String, String> configs = _configDao.getConfiguration(params);
-
-            String allocationAlgorithm = configs.get("vm.allocation.algorithm");
-            if (allocationAlgorithm != null) {
-                _allocationAlgorithm = allocationAlgorithm;
-            }
             String value = configs.get("xenserver.check.hvm");
             _checkHvm = value == null ? true : Boolean.parseBoolean(value);
         }