Introduce volume allocation algorithm global configuration

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

@@ -62,7 +62,7 @@ public interface DeploymentClusterPlanner extends DeploymentPlanner {
             "vm.allocation.algorithm",
             "Advanced",
             "random",
-            "Order in which hosts within a cluster will be considered for VM/volume allocation. The value can be 'random', 'firstfit', 'userdispersing', 'userconcentratedpod_random', 'userconcentratedpod_firstfit', or 'firstfitleastconsumed'.",
+            "Order in which hosts within a cluster will be considered for VM allocation. The value can be 'random', 'firstfit', 'userdispersing', 'userconcentratedpod_random', 'userconcentratedpod_firstfit', or 'firstfitleastconsumed'.",
             true,
             ConfigKey.Scope.Global, null, null, null, null, null,
             ConfigKey.Kind.Select,

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

@@ -83,6 +83,17 @@ public interface VolumeOrchestrationService {
             "The maximum size for a volume (in GiB).",
             true);
 
+    ConfigKey<String> VolumeAllocationAlgorithm = new ConfigKey<>(
+            String.class,
+            "volume.allocation.algorithm",
+            "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'.",
+            true,
+            ConfigKey.Scope.Global, null, null, null, null, null,
+            ConfigKey.Kind.Select,
+            "random,firstfit,userdispersing,userconcentratedpod_random,userconcentratedpod_firstfit,firstfitleastconsumed");
+
     VolumeInfo moveVolume(VolumeInfo volume, long destPoolDcId, Long destPoolPodId, Long destPoolClusterId, HypervisorType dataDiskHyperType)
         throws ConcurrentOperationException, StorageUnavailableException;
 

engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java

@@ -38,6 +38,7 @@
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
+import com.cloud.deploy.DeploymentClusterPlanner;
 import com.cloud.exception.ResourceAllocationException;
 import com.cloud.storage.DiskOfferingVO;
 import com.cloud.storage.VMTemplateVO;
@@ -73,6 +74,7 @@
 import org.apache.cloudstack.framework.config.ConfigDepot;
 import org.apache.cloudstack.framework.config.ConfigKey;
 import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 import org.apache.cloudstack.framework.jobs.AsyncJobManager;
 import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO;
 import org.apache.cloudstack.resourcedetail.DiskOfferingDetailVO;
@@ -257,6 +259,10 @@
     StoragePoolHostDao storagePoolHostDao;
     @Inject
     DiskOfferingDao diskOfferingDao;
+    @Inject
+    ConfigDepot configDepot;
+    @Inject
+    ConfigurationDao configurationDao;
 
     @Inject
     protected SnapshotHelper snapshotHelper;
@@ -2018,7 +2024,9 @@
 
     @Override
     public ConfigKey<?>[] getConfigKeys() {
-        return new ConfigKey<?>[] {RecreatableSystemVmEnabled, MaxVolumeSize, StorageHAMigrationEnabled, StorageMigrationEnabled, CustomDiskOfferingMaxSize, CustomDiskOfferingMinSize, VolumeUrlCheck};
+        return new ConfigKey<?>[] {
+                RecreatableSystemVmEnabled, MaxVolumeSize, StorageHAMigrationEnabled, StorageMigrationEnabled,
+                CustomDiskOfferingMaxSize, CustomDiskOfferingMinSize, VolumeUrlCheck, VolumeAllocationAlgorithm};
     }
 
     @Override
@@ -2031,6 +2039,18 @@
         return true;
     }
 
+    @Override
+    public boolean start() {
+        if (configDepot.isNewConfig(VolumeOrchestrationService.VolumeAllocationAlgorithm)) {
+            String vmAllocationAlgo = DeploymentClusterPlanner.VmAllocationAlgorithm.value();
+            if (com.cloud.utils.StringUtils.isNotEmpty(vmAllocationAlgo) && !"random".equalsIgnoreCase(vmAllocationAlgo)) {
+                logger.debug("Updating value for configuration: {} to {}", VolumeOrchestrationService.VolumeAllocationAlgorithm.key(), vmAllocationAlgo);
+                configurationDao.update(VolumeOrchestrationService.VolumeAllocationAlgorithm.key(), vmAllocationAlgo);
+            }
+        }
+        return true;
+    }
+
     private void cleanupVolumeDuringAttachFailure(Long volumeId, Long vmId) {
         VolumeVO volume = _volsDao.findById(volumeId);
         if (volume == null) {

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

@@ -16,58 +16,57 @@
 // under the License.
 package org.apache.cloudstack.storage.allocator;
 
-import java.math.BigDecimal;
-import java.security.SecureRandom;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
-import javax.inject.Inject;
-import javax.naming.ConfigurationException;
-
 import com.cloud.api.query.dao.StoragePoolJoinDao;
-import com.cloud.exception.StorageUnavailableException;
-import com.cloud.storage.ScopeType;
-import com.cloud.storage.StoragePoolStatus;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailVO;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
-import org.apache.commons.lang3.StringUtils;
-import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
-import org.apache.commons.collections.CollectionUtils;
-
-import com.cloud.utils.Pair;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
-import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator;
-import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
-import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
-
 import com.cloud.capacity.Capacity;
 import com.cloud.capacity.dao.CapacityDao;
 import com.cloud.dc.ClusterVO;
 import com.cloud.dc.dao.ClusterDao;
 import com.cloud.deploy.DeploymentPlan;
 import com.cloud.deploy.DeploymentPlanner.ExcludeList;
+import com.cloud.exception.StorageUnavailableException;
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import com.cloud.storage.ScopeType;
 import com.cloud.storage.Storage;
 import com.cloud.storage.StorageManager;
 import com.cloud.storage.StoragePool;
+import com.cloud.storage.StoragePoolStatus;
 import com.cloud.storage.StorageUtil;
 import com.cloud.storage.Volume;
 import com.cloud.storage.dao.VolumeDao;
 import com.cloud.user.Account;
 import com.cloud.utils.NumbersUtil;
+import com.cloud.utils.Pair;
+import com.cloud.utils.StringUtils;
 import com.cloud.utils.component.AdapterBase;
 import com.cloud.vm.DiskProfile;
 import com.cloud.vm.VirtualMachineProfile;
 
+import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
+import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailVO;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
+
+import org.apache.commons.collections.CollectionUtils;
+
+import javax.inject.Inject;
+import javax.naming.ConfigurationException;
+import java.math.BigDecimal;
+import java.security.SecureRandom;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
 public abstract class AbstractStoragePoolAllocator extends AdapterBase implements StoragePoolAllocator {
 
     protected BigDecimal storageOverprovisioningFactor = new BigDecimal(1);
-    protected String allocationAlgorithm = "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 allocationAlgorithm = configs.get("vm.allocation.algorithm");
-            if (allocationAlgorithm != null) {
-                this.allocationAlgorithm = allocationAlgorithm;
-            }
             return true;
         }
         return false;
@@ -227,16 +222,16 @@
     }
 
     List<StoragePool> reorderStoragePoolsBasedOnAlgorithm(List<StoragePool> pools, DeploymentPlan plan, Account account) {
-        logger.debug(String.format("Using allocation algorithm [%s] to reorder pools.", allocationAlgorithm));
-
-        if (allocationAlgorithm.equals("random") || allocationAlgorithm.equals("userconcentratedpod_random") || (account == null)) {
+        String volumeAllocationAlgorithm = VolumeOrchestrationService.VolumeAllocationAlgorithm.value();
+        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(allocationAlgorithm, "userdispersing", "firstfitleastconsumed")) {
+        } else if (StringUtils.equalsAny(volumeAllocationAlgorithm, "userdispersing", "firstfitleastconsumed")) {
             if (logger.isTraceEnabled()) {
-                logger.trace(String.format("Using reordering algorithm [%s]", allocationAlgorithm));
+                logger.trace("Using reordering algorithm {}", volumeAllocationAlgorithm);
             }
 
-            if (allocationAlgorithm.equals("userdispersing")) {
+            if (volumeAllocationAlgorithm.equals("userdispersing")) {
                 pools = reorderPoolsByNumberOfVolumes(plan, pools, account);
             } else {
                 pools = reorderPoolsByCapacity(plan, pools);
@@ -248,7 +243,7 @@
     void reorderRandomPools(List<StoragePool> pools) {
         StorageUtil.traceLogStoragePools(pools, logger, "pools to choose from: ");
         if (logger.isTraceEnabled()) {
-            logger.trace(String.format("Shuffle this so that we don't check the pools in the same order. Algorithm == '%s' (or no account?)", allocationAlgorithm));
+            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/main/java/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java

@@ -16,27 +16,27 @@
 // under the License.
 package org.apache.cloudstack.storage.allocator;
 
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
-import java.util.Map;
-
-import javax.inject.Inject;
-import javax.naming.ConfigurationException;
-
-import com.cloud.storage.VolumeApiServiceImpl;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
-import org.springframework.stereotype.Component;
-
 import com.cloud.deploy.DeploymentPlan;
 import com.cloud.deploy.DeploymentPlanner.ExcludeList;
 import com.cloud.offering.ServiceOffering;
 import com.cloud.storage.ScopeType;
 import com.cloud.storage.StoragePool;
+import com.cloud.storage.VolumeApiServiceImpl;
 import com.cloud.storage.dao.DiskOfferingDao;
 import com.cloud.vm.DiskProfile;
 import com.cloud.vm.VirtualMachineProfile;
 
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+
+import org.springframework.stereotype.Component;
+
+import javax.inject.Inject;
+import javax.naming.ConfigurationException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+
 @Component
 public class ClusterScopeStoragePoolAllocator extends AbstractStoragePoolAllocator {
 
@@ -116,14 +116,6 @@ protected List<StoragePool> select(DiskProfile dskCh, VirtualMachineProfile vmPr
     @Override
     public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
         super.configure(name, params);
-
-        if (configDao != null) {
-            Map<String, String> configs = configDao.getConfiguration(params);
-            String allocationAlgorithm = configs.get("vm.allocation.algorithm");
-            if (allocationAlgorithm != null) {
-                this.allocationAlgorithm = allocationAlgorithm;
-            }
-        }
         return true;
     }
 }

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

@@ -17,14 +17,19 @@
 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.engine.orchestration.service.VolumeOrchestrationService;
+import org.apache.cloudstack.framework.config.ConfigKey;
 import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
@@ -34,14 +39,13 @@
 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.lang.reflect.Field;
+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 {
@@ -73,16 +77,17 @@ public void tearDown() {
     }
 
     @Test
-    public void reorderStoragePoolsBasedOnAlgorithm_random() {
+    public void reorderStoragePoolsBasedOnAlgorithm_random() throws Exception {
+        overrideDefaultConfigValue( VolumeOrchestrationService.VolumeAllocationAlgorithm, "random");
         allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account);
         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.allocationAlgorithm = "userdispersing";
+    public void reorderStoragePoolsBasedOnAlgorithm_userdispersing() throws Exception {
+        overrideDefaultConfigValue(VolumeOrchestrationService.VolumeAllocationAlgorithm, "userdispersing");
         Mockito.doReturn(pools).when(allocator).reorderPoolsByNumberOfVolumes(plan, pools, account);
         allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account);
         Mockito.verify(allocator, Mockito.times(0)).reorderPoolsByCapacity(plan, pools);
@@ -91,10 +96,9 @@ public void reorderStoragePoolsBasedOnAlgorithm_userdispersing() {
     }
 
     @Test
-    public void reorderStoragePoolsBasedOnAlgorithm_userdispersing_reorder_check() {
-        allocator.allocationAlgorithm = "userdispersing";
+    public void reorderStoragePoolsBasedOnAlgorithm_userdispersing_reorder_check() throws Exception {
+        overrideDefaultConfigValue(VolumeOrchestrationService.VolumeAllocationAlgorithm, "userdispersing");
         allocator.volumeDao = volumeDao;
-
         when(plan.getDataCenterId()).thenReturn(1l);
         when(plan.getPodId()).thenReturn(1l);
         when(plan.getClusterId()).thenReturn(1l);
@@ -114,8 +118,8 @@ public void reorderStoragePoolsBasedOnAlgorithm_userdispersing_reorder_check() {
     }
 
     @Test
-    public void reorderStoragePoolsBasedOnAlgorithm_firstfitleastconsumed() {
-        allocator.allocationAlgorithm = "firstfitleastconsumed";
+    public void reorderStoragePoolsBasedOnAlgorithm_firstfitleastconsumed() throws Exception {
+        overrideDefaultConfigValue(VolumeOrchestrationService.VolumeAllocationAlgorithm, "firstfitleastconsumed");
         Mockito.doReturn(pools).when(allocator).reorderPoolsByCapacity(plan, pools);
         allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account);
         Mockito.verify(allocator, Mockito.times(1)).reorderPoolsByCapacity(plan, pools);
@@ -132,6 +136,12 @@ public void reorderRandomPools() {
         }
         Assert.assertTrue(firstchoice.size() > 2);
     }
+
+    private void overrideDefaultConfigValue(final ConfigKey configKey, final String value) throws IllegalAccessException, NoSuchFieldException {
+        final Field f = ConfigKey.class.getDeclaredField("_defaultValue");
+        f.setAccessible(true);
+        f.set(configKey, value);
+    }
 }
 
 class MockStorapoolAllocater extends AbstractStoragePoolAllocator {