Address review comments

Author: sudo87

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

@@ -36,9 +36,11 @@
 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;
@@ -48,8 +50,8 @@
 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 org.apache.commons.lang3.StringUtils;
 
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
@@ -65,7 +67,7 @@
 public abstract class AbstractStoragePoolAllocator extends AdapterBase implements StoragePoolAllocator {
 
     protected BigDecimal storageOverprovisioningFactor = new BigDecimal(1);
-    protected String allocationAlgorithm = "random";
+    protected String volumeAllocationAlgorithm = "random";
     protected long extraBytesPerVolume = 0;
     @Inject protected DataStoreManager dataStoreMgr;
     @Inject protected PrimaryDataStoreDao storagePoolDao;
@@ -93,9 +95,9 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
             String globalStorageOverprovisioningFactor = configs.get("storage.overprovisioning.factor");
             storageOverprovisioningFactor = new BigDecimal(NumbersUtil.parseFloat(globalStorageOverprovisioningFactor, 2.0f));
             extraBytesPerVolume = 0;
-            String allocationAlgorithm = VolumeOrchestrationService.VolumeAllocationAlgorithm.value();
-            if (allocationAlgorithm != null) {
-                this.allocationAlgorithm = allocationAlgorithm;
+            String volAllocationAlgorithm = VolumeOrchestrationService.VolumeAllocationAlgorithm.value();
+            if (volAllocationAlgorithm != null) {
+                this.volumeAllocationAlgorithm = volAllocationAlgorithm;
             }
             return true;
         }
@@ -225,15 +227,15 @@ public List<StoragePool> reorderPools(List<StoragePool> pools, VirtualMachinePro
     }
 
     List<StoragePool> reorderStoragePoolsBasedOnAlgorithm(List<StoragePool> pools, DeploymentPlan plan, Account account) {
-        logger.debug("Using volume allocation algorithm {} to reorder pools.", allocationAlgorithm);
-        if (allocationAlgorithm.equals("random") || allocationAlgorithm.equals("userconcentratedpod_random") || (account == null)) {
+        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("Using reordering algorithm {}", 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);
@@ -245,7 +247,7 @@ List<StoragePool> reorderStoragePoolsBasedOnAlgorithm(List<StoragePool> pools, D
     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?)", allocationAlgorithm);
+            logger.trace("Shuffle this so that we don't check the pools in the same order. Algorithm == {} (or no account?)", volumeAllocationAlgorithm);
         }
         StorageUtil.traceLogStoragePools(pools, logger, "pools to shuffle: ");
         Collections.shuffle(pools, secureRandom);

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

@@ -25,6 +25,7 @@
 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;
 

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

@@ -82,7 +82,7 @@ public void reorderStoragePoolsBasedOnAlgorithm_random() {
 
     @Test
     public void reorderStoragePoolsBasedOnAlgorithm_userdispersing() {
-        allocator.allocationAlgorithm = "userdispersing";
+        allocator.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);
@@ -92,7 +92,7 @@ public void reorderStoragePoolsBasedOnAlgorithm_userdispersing() {
 
     @Test
     public void reorderStoragePoolsBasedOnAlgorithm_userdispersing_reorder_check() {
-        allocator.allocationAlgorithm = "userdispersing";
+        allocator.volumeAllocationAlgorithm = "userdispersing";
         allocator.volumeDao = volumeDao;
 
         when(plan.getDataCenterId()).thenReturn(1l);
@@ -115,7 +115,7 @@ public void reorderStoragePoolsBasedOnAlgorithm_userdispersing_reorder_check() {
 
     @Test
     public void reorderStoragePoolsBasedOnAlgorithm_firstfitleastconsumed() {
-        allocator.allocationAlgorithm = "firstfitleastconsumed";
+        allocator.volumeAllocationAlgorithm = "firstfitleastconsumed";
         Mockito.doReturn(pools).when(allocator).reorderPoolsByCapacity(plan, pools);
         allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account);
         Mockito.verify(allocator, Mockito.times(1)).reorderPoolsByCapacity(plan, pools);