From b28db5daff8ad6c23a73dd38c6c08957c59a398a Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Fri, 11 Apr 2025 10:42:30 +0530 Subject: [PATCH 01/10] Introduce volume allocation algorithm global configuration. Volume allocation has been using vm.allocation.algorithm config value. This change will allow them to be configured in isolation. --- .../deploy/DeploymentClusterPlanner.java | 2 +- .../service/VolumeOrchestrationService.java | 11 ++++ .../orchestration/VolumeOrchestrator.java | 4 +- .../AbstractStoragePoolAllocator.java | 63 +++++++++---------- .../ClusterScopeStoragePoolAllocator.java | 30 +++------ 5 files changed, 55 insertions(+), 55 deletions(-) diff --git a/api/src/main/java/com/cloud/deploy/DeploymentClusterPlanner.java b/api/src/main/java/com/cloud/deploy/DeploymentClusterPlanner.java index 2697311d2b94..d127e4bdd660 100644 --- a/api/src/main/java/com/cloud/deploy/DeploymentClusterPlanner.java +++ b/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, diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java index 7950dda4d68e..56016d76357d 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java +++ b/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 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'.", + false, + 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; diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java index db0119febde7..66799c9a5770 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java @@ -2018,7 +2018,9 @@ public boolean canVmRestartOnAnotherServer(long vmId) { @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 diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java index 2c034d8429a5..b4ed23f5d73c 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java @@ -16,53 +16,51 @@ // 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.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 org.apache.commons.lang3.StringUtils; + +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 { @@ -95,7 +93,7 @@ public boolean configure(String name, Map params) throws Configu String globalStorageOverprovisioningFactor = configs.get("storage.overprovisioning.factor"); storageOverprovisioningFactor = new BigDecimal(NumbersUtil.parseFloat(globalStorageOverprovisioningFactor, 2.0f)); extraBytesPerVolume = 0; - String allocationAlgorithm = configs.get("vm.allocation.algorithm"); + String allocationAlgorithm = VolumeOrchestrationService.VolumeAllocationAlgorithm.value(); if (allocationAlgorithm != null) { this.allocationAlgorithm = allocationAlgorithm; } @@ -227,13 +225,12 @@ public List reorderPools(List pools, VirtualMachinePro } List reorderStoragePoolsBasedOnAlgorithm(List pools, DeploymentPlan plan, Account account) { - logger.debug(String.format("Using allocation algorithm [%s] to reorder pools.", allocationAlgorithm)); - + logger.debug("Using volume allocation algorithm {} to reorder pools.", allocationAlgorithm); if (allocationAlgorithm.equals("random") || allocationAlgorithm.equals("userconcentratedpod_random") || (account == null)) { reorderRandomPools(pools); } else if (StringUtils.equalsAny(allocationAlgorithm, "userdispersing", "firstfitleastconsumed")) { if (logger.isTraceEnabled()) { - logger.trace(String.format("Using reordering algorithm [%s]", allocationAlgorithm)); + logger.trace("Using reordering algorithm {}", allocationAlgorithm); } if (allocationAlgorithm.equals("userdispersing")) { @@ -248,7 +245,7 @@ List reorderStoragePoolsBasedOnAlgorithm(List pools, D void reorderRandomPools(List 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 == {} (or no account?)", allocationAlgorithm); } StorageUtil.traceLogStoragePools(pools, logger, "pools to shuffle: "); Collections.shuffle(pools, secureRandom); diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java index a52372fb3cad..6d5f082664f8 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java @@ -16,26 +16,24 @@ // 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 +114,6 @@ protected List select(DiskProfile dskCh, VirtualMachineProfile vmPr @Override public boolean configure(String name, Map params) throws ConfigurationException { super.configure(name, params); - - if (configDao != null) { - Map configs = configDao.getConfiguration(params); - String allocationAlgorithm = configs.get("vm.allocation.algorithm"); - if (allocationAlgorithm != null) { - this.allocationAlgorithm = allocationAlgorithm; - } - } return true; } } From f6e7fa6b798039c7f3c1692200f9400744c2f090 Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Fri, 11 Apr 2025 14:44:55 +0530 Subject: [PATCH 02/10] Address review comments --- .../AbstractStoragePoolAllocator.java | 24 ++++++++++--------- .../ClusterScopeStoragePoolAllocator.java | 1 + .../AbstractStoragePoolAllocatorTest.java | 6 ++--- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java index b4ed23f5d73c..ab8a5b99cefd 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java +++ b/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 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 reorderPools(List pools, VirtualMachinePro } List reorderStoragePoolsBasedOnAlgorithm(List 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 reorderStoragePoolsBasedOnAlgorithm(List pools, D void reorderRandomPools(List 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); diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java index 6d5f082664f8..d4982e6c25dc 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java +++ b/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; diff --git a/engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocatorTest.java b/engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocatorTest.java index cddbc9e93cb5..913db6b9c6e9 100644 --- a/engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocatorTest.java +++ b/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); From 9c3d38133cce5c46aa0d8d0ad036f4f96a1661a6 Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Fri, 11 Apr 2025 15:09:49 +0530 Subject: [PATCH 03/10] format import statements --- .../storage/allocator/ClusterScopeStoragePoolAllocator.java | 1 + 1 file changed, 1 insertion(+) diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java index d4982e6c25dc..a80e003a139e 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ClusterScopeStoragePoolAllocator.java @@ -27,6 +27,7 @@ import com.cloud.vm.VirtualMachineProfile; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; + import org.springframework.stereotype.Component; import javax.inject.Inject; From 33f9e6fa7e2856ae00369faca5047047e776c625 Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Mon, 14 Apr 2025 13:42:55 +0530 Subject: [PATCH 04/10] make config dynamic in nature to avoid need of mgmt server restart --- .../service/VolumeOrchestrationService.java | 2 +- .../AbstractStoragePoolAllocator.java | 11 ++--- .../AbstractStoragePoolAllocatorTest.java | 40 +++++++++---------- .../allocator/impl/FirstFitAllocator.java | 25 +++++------- 4 files changed, 33 insertions(+), 45 deletions(-) diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java index 56016d76357d..28dda571df81 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java +++ b/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"); diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java index ab8a5b99cefd..2555ea66d20c 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java +++ b/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 @@ public boolean configure(String name, Map params) throws Configu 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 @@ public List reorderPools(List pools, VirtualMachinePro account = vmProfile.getOwner(); } - pools = reorderStoragePoolsBasedOnAlgorithm(pools, plan, account); + pools = reorderStoragePoolsBasedOnAlgorithm(pools, plan, account, VolumeOrchestrationService.VolumeAllocationAlgorithm.value()); if (vmProfile.getVirtualMachine() == null) { if (logger.isTraceEnabled()) { @@ -226,7 +221,7 @@ public List reorderPools(List pools, VirtualMachinePro return pools; } - List reorderStoragePoolsBasedOnAlgorithm(List pools, DeploymentPlan plan, Account account) { + List reorderStoragePoolsBasedOnAlgorithm(List 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); @@ -247,7 +242,7 @@ List reorderStoragePoolsBasedOnAlgorithm(List pools, D void reorderRandomPools(List 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); diff --git a/engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocatorTest.java b/engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocatorTest.java index 913db6b9c6e9..07120cde2b9c 100644 --- a/engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocatorTest.java +++ b/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,7 +75,7 @@ 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); @@ -82,9 +83,8 @@ public void reorderStoragePoolsBasedOnAlgorithm_random() { @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); @@ -92,7 +92,6 @@ public void reorderStoragePoolsBasedOnAlgorithm_userdispersing() { @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 reorderedPools = allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account); + List 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); diff --git a/server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java b/server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java index 4a5f80571ae7..2ccec4772cd9 100644 --- a/server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java +++ b/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 @@ public class FirstFitAllocator extends AdapterBase implements HostAllocator { UserVmDetailsDao _userVmDetailsDao; boolean _checkHvm = true; - protected String _allocationAlgorithm = "random"; - @Override public List allocateTo(VirtualMachineProfile vmProfile, DeploymentPlan plan, Type type, ExcludeList avoid, int returnUpTo) { @@ -286,12 +285,13 @@ public List allocateTo(VirtualMachineProfile vmProfile, DeploymentPlan pla protected List allocateTo(DeploymentPlan plan, ServiceOffering offering, VMTemplateVO template, ExcludeList avoid, List 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 @@ protected String getTemplateGuestOSCategory(VMTemplateVO template) { public boolean configure(String name, Map params) throws ConfigurationException { if (_configDao != null) { Map 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); } From c3006bfd3bf9d29f5b09929a130dbdaab88fc5b0 Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Mon, 14 Apr 2025 17:13:46 +0530 Subject: [PATCH 05/10] Revert method signature change done for unit test --- .../AbstractStoragePoolAllocator.java | 5 ++-- .../AbstractStoragePoolAllocatorTest.java | 30 +++++++++++++------ 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java index 2555ea66d20c..eccbb0d6dd96 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java @@ -204,7 +204,7 @@ public List reorderPools(List pools, VirtualMachinePro account = vmProfile.getOwner(); } - pools = reorderStoragePoolsBasedOnAlgorithm(pools, plan, account, VolumeOrchestrationService.VolumeAllocationAlgorithm.value()); + pools = reorderStoragePoolsBasedOnAlgorithm(pools, plan, account); if (vmProfile.getVirtualMachine() == null) { if (logger.isTraceEnabled()) { @@ -221,7 +221,8 @@ public List reorderPools(List pools, VirtualMachinePro return pools; } - List reorderStoragePoolsBasedOnAlgorithm(List pools, DeploymentPlan plan, Account account, String volumeAllocationAlgorithm) { + List reorderStoragePoolsBasedOnAlgorithm(List pools, DeploymentPlan plan, Account account) { + 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); diff --git a/engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocatorTest.java b/engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocatorTest.java index 07120cde2b9c..1b4b96d71e53 100644 --- a/engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocatorTest.java +++ b/engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocatorTest.java @@ -26,6 +26,8 @@ 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; @@ -37,6 +39,7 @@ import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; +import java.lang.reflect.Field; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -74,26 +77,28 @@ public void tearDown() { } @Test - public void reorderStoragePoolsBasedOnAlgorithm_random() { - allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account, "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() { + public void reorderStoragePoolsBasedOnAlgorithm_userdispersing() throws Exception { + overrideDefaultConfigValue(VolumeOrchestrationService.VolumeAllocationAlgorithm, "userdispersing"); Mockito.doReturn(pools).when(allocator).reorderPoolsByNumberOfVolumes(plan, pools, account); - allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account,"userdispersing"); + allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account); 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() { + 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); @@ -103,7 +108,7 @@ public void reorderStoragePoolsBasedOnAlgorithm_userdispersing_reorder_check() { poolIds.add(9l); when(volumeDao.listPoolIdsByVolumeCount(1l,1l,1l,1l)).thenReturn(poolIds); - List reorderedPools = allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account, "userdispersing"); + List reorderedPools = allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account); Assert.assertEquals(poolIds.size(),reorderedPools.size()); Mockito.verify(allocator, Mockito.times(0)).reorderPoolsByCapacity(plan, pools); @@ -113,9 +118,10 @@ public void reorderStoragePoolsBasedOnAlgorithm_userdispersing_reorder_check() { } @Test - public void reorderStoragePoolsBasedOnAlgorithm_firstfitleastconsumed() { + public void reorderStoragePoolsBasedOnAlgorithm_firstfitleastconsumed() throws Exception { + overrideDefaultConfigValue(VolumeOrchestrationService.VolumeAllocationAlgorithm, "firstfitleastconsumed"); Mockito.doReturn(pools).when(allocator).reorderPoolsByCapacity(plan, pools); - allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account, "firstfitleastconsumed"); + allocator.reorderStoragePoolsBasedOnAlgorithm(pools, plan, account); 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); @@ -130,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 { From 5c100367740a842eadc5281f3956366f535f5a9d Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Wed, 16 Apr 2025 15:58:58 +0530 Subject: [PATCH 06/10] Add upgrade patch for volume.allocation.algorithm --- .../upgrade/dao/Upgrade42010to42100.java | 36 +++++++++++++++++++ .../upgrade/dao/Upgrade42010to42100Test.java | 31 ++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42010to42100.java b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42010to42100.java index d6dc85dbb9aa..00c8f0fe68c2 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42010to42100.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42010to42100.java @@ -23,6 +23,7 @@ import java.io.InputStream; import java.sql.Connection; import java.sql.PreparedStatement; +import java.sql.ResultSet; import java.sql.SQLException; import java.util.List; @@ -60,6 +61,7 @@ public InputStream[] getPrepareScripts() { @Override public void performDataMigration(Connection conn) { migrateConfigurationScopeToBitmask(conn); + migrateVolumeAllocationAlgorithm(conn); } @Override @@ -118,4 +120,38 @@ protected void migrateExistingConfigurationScopeValues(Connection conn) { throw new CloudRuntimeException(String.format("Failed to migrate existing configuration scope values to bitmask due to: %s", e.getMessage())); } } + + void migrateVolumeAllocationAlgorithm(Connection conn) { + ResultSet rs = null; + PreparedStatement pstmt = null; + try { + String vmAllocationAlgo = "random"; + pstmt = conn.prepareStatement("SELECT value FROM `cloud`.`configuration` where name = 'vm.allocation.algorithm'"); + rs = pstmt.executeQuery(); + if (rs.next()) { + vmAllocationAlgo = rs.getString(1); + } + rs.close(); + pstmt.close(); + + if (!"random".equalsIgnoreCase(vmAllocationAlgo)) { + pstmt = conn.prepareStatement("UPDATE `cloud`.`configuration` SET value = ? WHERE name = 'volume.allocation.algorithm'"); + pstmt.setString(1, vmAllocationAlgo); + pstmt.executeUpdate(); + } + } catch (SQLException e) { + throw new CloudRuntimeException("Unable to migrate the volume.allocation.algorithm", e); + } finally { + try { + if (rs != null) { + rs.close(); + } + if (pstmt != null) { + pstmt.close(); + } + } catch (SQLException e) { + logger.info("[ignored]",e); + } + } + } } diff --git a/engine/schema/src/test/java/com/cloud/upgrade/dao/Upgrade42010to42100Test.java b/engine/schema/src/test/java/com/cloud/upgrade/dao/Upgrade42010to42100Test.java index 035790f0716a..740e736daa9a 100644 --- a/engine/schema/src/test/java/com/cloud/upgrade/dao/Upgrade42010to42100Test.java +++ b/engine/schema/src/test/java/com/cloud/upgrade/dao/Upgrade42010to42100Test.java @@ -16,12 +16,15 @@ // under the License. package com.cloud.upgrade.dao; +import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.when; import java.sql.Connection; import java.sql.PreparedStatement; +import java.sql.ResultSet; import java.sql.SQLException; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; @@ -37,9 +40,24 @@ public class Upgrade42010to42100Test { @Spy Upgrade42010to42100 upgrade; + @Mock + private PreparedStatement mockPreparedStatement; + @Mock + private ResultSet mockResultSet; @Mock private Connection conn; + + @Before + public void setup() throws Exception { + when(conn.prepareStatement("SELECT value FROM `cloud`.`configuration` where name = 'vm.allocation.algorithm'")) + .thenReturn(mockPreparedStatement); + when(mockPreparedStatement.executeQuery()).thenReturn(mockResultSet); + when(mockResultSet.next()).thenReturn(true); + when(mockResultSet.getString(1)).thenReturn("random"); + } + + @Test public void testPerformDataMigration() throws SQLException { try (MockedStatic ignored = Mockito.mockStatic(DbUpgradeUtils.class)) { @@ -70,4 +88,17 @@ public void testPerformDataMigration() throws SQLException { } } } + + @Test + public void testPerformDataMigrationShouldUpdateVolumeAlgorithm() throws SQLException { + when(mockResultSet.getString(1)).thenReturn("userdispersing"); + + PreparedStatement updateStmt = Mockito.mock(PreparedStatement.class); + when(conn.prepareStatement("UPDATE `cloud`.`configuration` SET value = ? WHERE name = 'volume.allocation.algorithm'")) + .thenReturn(updateStmt); + doNothing().when(upgrade).migrateConfigurationScopeToBitmask(conn); + upgrade.performDataMigration(conn); + Mockito.verify(updateStmt).setString(1, "userdispersing"); + Mockito.verify(updateStmt).executeUpdate(); + } } From 29342956ab546f580ee7df0e9c4c2d3d47b907a8 Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Wed, 16 Apr 2025 22:10:59 +0530 Subject: [PATCH 07/10] Use configDepot.isNewConfig() to populate volume.allocation.algorithm --- .../orchestration/VolumeOrchestrator.java | 18 ++++++++++ .../upgrade/dao/Upgrade42010to42100.java | 36 ------------------- .../upgrade/dao/Upgrade42010to42100Test.java | 31 ---------------- 3 files changed, 18 insertions(+), 67 deletions(-) diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java index 66799c9a5770..3de74cdaa9f3 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java +++ b/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 @@ public enum UserVmCloneType { StoragePoolHostDao storagePoolHostDao; @Inject DiskOfferingDao diskOfferingDao; + @Inject + ConfigDepot configDepot; + @Inject + ConfigurationDao configurationDao; @Inject protected SnapshotHelper snapshotHelper; @@ -2033,6 +2039,18 @@ public boolean configure(String name, Map params) throws Configu 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) { diff --git a/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42010to42100.java b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42010to42100.java index 00c8f0fe68c2..d6dc85dbb9aa 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42010to42100.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42010to42100.java @@ -23,7 +23,6 @@ import java.io.InputStream; import java.sql.Connection; import java.sql.PreparedStatement; -import java.sql.ResultSet; import java.sql.SQLException; import java.util.List; @@ -61,7 +60,6 @@ public InputStream[] getPrepareScripts() { @Override public void performDataMigration(Connection conn) { migrateConfigurationScopeToBitmask(conn); - migrateVolumeAllocationAlgorithm(conn); } @Override @@ -120,38 +118,4 @@ protected void migrateExistingConfigurationScopeValues(Connection conn) { throw new CloudRuntimeException(String.format("Failed to migrate existing configuration scope values to bitmask due to: %s", e.getMessage())); } } - - void migrateVolumeAllocationAlgorithm(Connection conn) { - ResultSet rs = null; - PreparedStatement pstmt = null; - try { - String vmAllocationAlgo = "random"; - pstmt = conn.prepareStatement("SELECT value FROM `cloud`.`configuration` where name = 'vm.allocation.algorithm'"); - rs = pstmt.executeQuery(); - if (rs.next()) { - vmAllocationAlgo = rs.getString(1); - } - rs.close(); - pstmt.close(); - - if (!"random".equalsIgnoreCase(vmAllocationAlgo)) { - pstmt = conn.prepareStatement("UPDATE `cloud`.`configuration` SET value = ? WHERE name = 'volume.allocation.algorithm'"); - pstmt.setString(1, vmAllocationAlgo); - pstmt.executeUpdate(); - } - } catch (SQLException e) { - throw new CloudRuntimeException("Unable to migrate the volume.allocation.algorithm", e); - } finally { - try { - if (rs != null) { - rs.close(); - } - if (pstmt != null) { - pstmt.close(); - } - } catch (SQLException e) { - logger.info("[ignored]",e); - } - } - } } diff --git a/engine/schema/src/test/java/com/cloud/upgrade/dao/Upgrade42010to42100Test.java b/engine/schema/src/test/java/com/cloud/upgrade/dao/Upgrade42010to42100Test.java index 740e736daa9a..035790f0716a 100644 --- a/engine/schema/src/test/java/com/cloud/upgrade/dao/Upgrade42010to42100Test.java +++ b/engine/schema/src/test/java/com/cloud/upgrade/dao/Upgrade42010to42100Test.java @@ -16,15 +16,12 @@ // under the License. package com.cloud.upgrade.dao; -import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.when; import java.sql.Connection; import java.sql.PreparedStatement; -import java.sql.ResultSet; import java.sql.SQLException; -import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; @@ -40,24 +37,9 @@ public class Upgrade42010to42100Test { @Spy Upgrade42010to42100 upgrade; - @Mock - private PreparedStatement mockPreparedStatement; - @Mock - private ResultSet mockResultSet; @Mock private Connection conn; - - @Before - public void setup() throws Exception { - when(conn.prepareStatement("SELECT value FROM `cloud`.`configuration` where name = 'vm.allocation.algorithm'")) - .thenReturn(mockPreparedStatement); - when(mockPreparedStatement.executeQuery()).thenReturn(mockResultSet); - when(mockResultSet.next()).thenReturn(true); - when(mockResultSet.getString(1)).thenReturn("random"); - } - - @Test public void testPerformDataMigration() throws SQLException { try (MockedStatic ignored = Mockito.mockStatic(DbUpgradeUtils.class)) { @@ -88,17 +70,4 @@ public void testPerformDataMigration() throws SQLException { } } } - - @Test - public void testPerformDataMigrationShouldUpdateVolumeAlgorithm() throws SQLException { - when(mockResultSet.getString(1)).thenReturn("userdispersing"); - - PreparedStatement updateStmt = Mockito.mock(PreparedStatement.class); - when(conn.prepareStatement("UPDATE `cloud`.`configuration` SET value = ? WHERE name = 'volume.allocation.algorithm'")) - .thenReturn(updateStmt); - doNothing().when(upgrade).migrateConfigurationScopeToBitmask(conn); - upgrade.performDataMigration(conn); - Mockito.verify(updateStmt).setString(1, "userdispersing"); - Mockito.verify(updateStmt).executeUpdate(); - } } From cf65404aa16040cabe37edaef393c0005795cee7 Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Thu, 17 Apr 2025 10:28:28 +0530 Subject: [PATCH 08/10] replace 'random' with VolumeAllocationAlgorithm.getDefaultValue() --- .../engine/orchestration/VolumeOrchestrator.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java index 3de74cdaa9f3..d9d325f012cb 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java @@ -2041,11 +2041,11 @@ public boolean configure(String name, Map params) throws Configu @Override public boolean start() { - if (configDepot.isNewConfig(VolumeOrchestrationService.VolumeAllocationAlgorithm)) { + if (configDepot.isNewConfig(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); + if (com.cloud.utils.StringUtils.isNotEmpty(vmAllocationAlgo) && !VolumeAllocationAlgorithm.defaultValue().equalsIgnoreCase(vmAllocationAlgo)) { + logger.debug("Updating value for configuration: {} to {}", VolumeAllocationAlgorithm.key(), vmAllocationAlgo); + configurationDao.update(VolumeAllocationAlgorithm.key(), vmAllocationAlgo); } } return true; From 979386d4fab0fdfeaaa7e503dd188e07cedd9c6e Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Fri, 25 Apr 2025 17:32:44 +0530 Subject: [PATCH 09/10] log capacity alongwith host/storagaepool ids --- .../com/cloud/capacity/dao/CapacityDao.java | 2 +- .../cloud/capacity/dao/CapacityDaoImpl.java | 11 +++++++---- .../AbstractStoragePoolAllocator.java | 19 ++++++++++++++----- .../ZoneWideStoragePoolAllocator.java | 15 +++++++++++++-- .../allocator/impl/FirstFitAllocator.java | 16 ++++++++++++++-- 5 files changed, 49 insertions(+), 14 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDao.java b/engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDao.java index 1bb79ce417aa..7a4c96c6e1f4 100644 --- a/engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDao.java +++ b/engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDao.java @@ -64,5 +64,5 @@ List listCapacitiesGroupedByLevelAndType(Integer capacityType, L float findClusterConsumption(Long clusterId, short capacityType, long computeRequested); - List orderHostsByFreeCapacity(Long zoneId, Long clusterId, short capacityType); + Pair, Map> orderHostsByFreeCapacity(Long zoneId, Long clusterId, short capacityType); } diff --git a/engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDaoImpl.java b/engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDaoImpl.java index 5e7eee4566c1..0860f14518f6 100644 --- a/engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDaoImpl.java @@ -1028,10 +1028,11 @@ public Pair, Map> orderClustersByAggregateCapacity(long } @Override - public List orderHostsByFreeCapacity(Long zoneId, Long clusterId, short capacityTypeForOrdering){ + public Pair, Map> orderHostsByFreeCapacity(Long zoneId, Long clusterId, short capacityTypeForOrdering){ TransactionLegacy txn = TransactionLegacy.currentTxn(); PreparedStatement pstmt = null; - List result = new ArrayList(); + List result = new ArrayList<>(); + Map hostCapacityMap = new HashMap<>(); StringBuilder sql = new StringBuilder(ORDER_HOSTS_BY_FREE_CAPACITY_PART1); if (zoneId != null) { sql.append(" AND data_center_id = ?"); @@ -1054,9 +1055,11 @@ public List orderHostsByFreeCapacity(Long zoneId, Long clusterId, short ca ResultSet rs = pstmt.executeQuery(); while (rs.next()) { - result.add(rs.getLong(1)); + Long hostId = rs.getLong(1); + result.add(hostId); + hostCapacityMap.put(hostId, rs.getDouble(2)); } - return result; + return new Pair<>(result, hostCapacityMap); } catch (SQLException e) { throw new CloudRuntimeException("DB Exception on: " + sql, e); } catch (Throwable e) { diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java index eccbb0d6dd96..e0e9b2526f60 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java @@ -57,17 +57,22 @@ import javax.naming.ConfigurationException; import java.math.BigDecimal; import java.security.SecureRandom; +import java.text.DecimalFormat; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; public abstract class AbstractStoragePoolAllocator extends AdapterBase implements StoragePoolAllocator { protected BigDecimal storageOverprovisioningFactor = new BigDecimal(1); protected long extraBytesPerVolume = 0; + static DecimalFormat decimalFormat = new DecimalFormat("#.##"); @Inject protected DataStoreManager dataStoreMgr; @Inject protected PrimaryDataStoreDao storagePoolDao; @Inject protected VolumeDao volumeDao; @@ -137,12 +142,16 @@ protected List reorderPoolsByCapacity(DeploymentPlan plan, List poolIdsByCapacity = capacityDao.orderHostsByFreeCapacity(zoneId, clusterId, capacityType); + Pair, Map> result = capacityDao.orderHostsByFreeCapacity(zoneId, clusterId, capacityType); + List poolIdsByCapacity = result.first(); + Map sortedHostByCapacity = result.second().entrySet() + .stream() + .sorted(Map.Entry.comparingByValue(Comparator.reverseOrder())) + .collect(Collectors.toMap(Map.Entry::getKey, entry -> decimalFormat.format(entry.getValue() * 100) + "%", (e1, e2) -> e1, LinkedHashMap::new)); + logger.debug("List of pools in descending order of hostId: [{}] available capacity (percentage): {}", + poolIdsByCapacity, sortedHostByCapacity); - logger.debug(String.format("List of pools in descending order of available capacity [%s].", poolIdsByCapacity)); - - - //now filter the given list of Pools by this ordered list + // now filter the given list of Pools by this ordered list Map poolMap = new HashMap<>(); for (StoragePool pool : pools) { poolMap.put(pool.getId(), pool); diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java index 0c59cf24fc5b..970b063eb9cc 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java @@ -18,12 +18,16 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Comparator; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import javax.inject.Inject; +import com.cloud.utils.Pair; import org.springframework.stereotype.Component; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; @@ -122,9 +126,16 @@ protected List reorderPoolsByCapacity(DeploymentPlan plan, return null; } - List poolIdsByCapacity = capacityDao.orderHostsByFreeCapacity(zoneId, null, capacityType); + Pair, Map> result = capacityDao.orderHostsByFreeCapacity(zoneId, null, capacityType); + List poolIdsByCapacity = result.first(); + Map sortedHostByCapacity = result.second().entrySet() + .stream() + .sorted(Map.Entry.comparingByValue(Comparator.reverseOrder())) + .collect(Collectors.toMap(Map.Entry::getKey, entry -> decimalFormat.format(entry.getValue() * 100) + "%", + (e1, e2) -> e1, LinkedHashMap::new)); if (logger.isDebugEnabled()) { - logger.debug("List of zone-wide storage pools in descending order of free capacity: "+ poolIdsByCapacity); + logger.debug("List of zone-wide storage pools: [{}] in descending order of free capacity (percentage): {}", + poolIdsByCapacity, sortedHostByCapacity); } //now filter the given list of Pools by this ordered list diff --git a/server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java b/server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java index d3d6e24d1aff..68a901a68a20 100644 --- a/server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java +++ b/server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java @@ -16,11 +16,15 @@ // under the License. package com.cloud.agent.manager.allocator.impl; +import java.text.DecimalFormat; import java.util.ArrayList; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import javax.inject.Inject; import javax.naming.ConfigurationException; @@ -98,6 +102,7 @@ public class FirstFitAllocator extends AdapterBase implements HostAllocator { UserVmDetailsDao _userVmDetailsDao; boolean _checkHvm = true; + static DecimalFormat decimalFormat = new DecimalFormat("#.##"); @Override public List allocateTo(VirtualMachineProfile vmProfile, DeploymentPlan plan, Type type, ExcludeList avoid, int returnUpTo) { @@ -373,9 +378,16 @@ private List reorderHostsByCapacity(DeploymentPlan plan, List hostIdsByFreeCapacity = _capacityDao.orderHostsByFreeCapacity(zoneId, clusterId, capacityType); + Pair, Map> result = _capacityDao.orderHostsByFreeCapacity(zoneId, clusterId, capacityType); + List hostIdsByFreeCapacity = result.first(); + Map sortedHostByCapacity = result.second().entrySet() + .stream() + .sorted(Map.Entry.comparingByValue(Comparator.reverseOrder())) + .collect(Collectors.toMap(Map.Entry::getKey, entry -> decimalFormat.format(entry.getValue() * 100) + "%", + (e1, e2) -> e1, LinkedHashMap::new)); if (logger.isDebugEnabled()) { - logger.debug("List of hosts in descending order of free capacity in the cluster: "+ hostIdsByFreeCapacity); + logger.debug("List of hosts: [{}] in descending order of free capacity (percentage) in the cluster: {}", + hostIdsByFreeCapacity, sortedHostByCapacity); } //now filter the given list of Hosts by this ordered list From 9721484c869aa48c56b9420c797af5a8ca76f25d Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Wed, 30 Apr 2025 09:02:20 +0530 Subject: [PATCH 10/10] add unit tests to increase code coverage --- .../orchestration/VolumeOrchestratorTest.java | 410 +++++++++++++++++- .../capacity/dao/CapacityDaoImplTest.java | 266 +++++++++++- .../AbstractStoragePoolAllocator.java | 2 +- .../ZoneWideStoragePoolAllocator.java | 2 +- .../AbstractStoragePoolAllocatorTest.java | 36 +- .../ZoneWideStoragePoolAllocatorTest.java | 71 +++ .../allocator/impl/FirstFitAllocatorTest.java | 159 +++++++ 7 files changed, 915 insertions(+), 31 deletions(-) create mode 100644 engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocatorTest.java create mode 100644 server/src/test/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocatorTest.java diff --git a/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java b/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java index a37845c86c2f..5136fc9a8301 100644 --- a/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java +++ b/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestratorTest.java @@ -16,15 +16,30 @@ // under the License. package org.apache.cloudstack.engine.orchestration; -import java.util.ArrayList; -import java.util.Date; - +import com.cloud.configuration.Resource; +import com.cloud.deploy.DeploymentClusterPlanner; +import com.cloud.exception.InvalidParameterValueException; +import com.cloud.exception.StorageAccessException; +import com.cloud.host.Host; +import com.cloud.host.HostVO; import com.cloud.hypervisor.Hypervisor; import com.cloud.offering.DiskOffering; +import com.cloud.storage.ScopeType; import com.cloud.storage.Storage; import com.cloud.storage.Volume; +import com.cloud.storage.Volume.Type; +import com.cloud.storage.VolumeApiService; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.VolumeDao; import com.cloud.user.Account; +import com.cloud.user.ResourceLimitService; +import com.cloud.uservm.UserVm; +import com.cloud.utils.db.EntityManager; +import com.cloud.utils.db.UUIDManager; +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.vm.VirtualMachine; +import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore; @@ -32,6 +47,13 @@ import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeService; +import org.apache.cloudstack.framework.config.ConfigDepot; +import org.apache.cloudstack.framework.config.ConfigKey; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.cloudstack.secret.PassphraseVO; +import org.apache.cloudstack.secret.dao.PassphraseDao; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.commons.lang3.ObjectUtils; import org.junit.Assert; import org.junit.Before; @@ -45,14 +67,14 @@ import org.mockito.junit.MockitoJUnitRunner; import org.mockito.stubbing.Answer; -import com.cloud.configuration.Resource; -import com.cloud.exception.StorageAccessException; -import com.cloud.host.Host; -import com.cloud.host.HostVO; -import com.cloud.storage.VolumeVO; -import com.cloud.storage.Volume.Type; -import com.cloud.user.ResourceLimitService; -import com.cloud.utils.exception.CloudRuntimeException; +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.Date; +import java.util.List; + +import static org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService.VolumeAllocationAlgorithm; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; @RunWith(MockitoJUnitRunner.class) public class VolumeOrchestratorTest { @@ -65,13 +87,36 @@ public class VolumeOrchestratorTest { protected VolumeDataFactory volumeDataFactory; @Mock protected VolumeDao volumeDao; + @Mock + protected PassphraseDao passphraseDao; + @Mock + protected PrimaryDataStoreDao storagePoolDao; + @Mock + protected VolumeApiService volumeApiService; + @Mock + protected UUIDManager uuidMgr; + @Mock + protected EntityManager entityMgr; + @Mock + protected DiskOfferingDao diskOfferingDao; + @Mock + protected CallContext mockCallContext; + @Mock + ConfigDepot configDepot; + @Mock + ConfigurationDao configurationDao; + @Spy @InjectMocks private VolumeOrchestrator volumeOrchestrator = new VolumeOrchestrator(); + private static final Long DEFAULT_ACCOUNT_PS_RESOURCE_COUNT = 100L; private Long accountPSResourceCount; + private static final long MOCK_VM_ID = 202L; + private static final long MOCK_POOL_ID = 303L; + private static final String MOCK_VM_NAME = "Test-VM"; @Before public void setUp() throws Exception { @@ -208,4 +253,347 @@ public void testImportVolume() { Mockito.verify(volume, Mockito.times(1)).setChainInfo(chainInfo); Mockito.verify(volume, Mockito.times(1)).setState(Volume.State.Ready); } + + @Test + public void testAllocateDuplicateVolumeVOBasic() { + Volume oldVol = Mockito.mock(Volume.class); + Mockito.when(oldVol.getVolumeType()).thenReturn(Volume.Type.ROOT); + Mockito.when(oldVol.getName()).thenReturn("testVol"); + Mockito.when(oldVol.getDataCenterId()).thenReturn(1L); + Mockito.when(oldVol.getDomainId()).thenReturn(2L); + Mockito.when(oldVol.getAccountId()).thenReturn(3L); + Mockito.when(oldVol.getDiskOfferingId()).thenReturn(4L); + Mockito.when(oldVol.getProvisioningType()).thenReturn(Storage.ProvisioningType.THIN); + Mockito.when(oldVol.getSize()).thenReturn(10L); + Mockito.when(oldVol.getMinIops()).thenReturn(100L); + Mockito.when(oldVol.getMaxIops()).thenReturn(200L); + Mockito.when(oldVol.get_iScsiName()).thenReturn("iqn.test"); + Mockito.when(oldVol.getTemplateId()).thenReturn(5L); + Mockito.when(oldVol.getDeviceId()).thenReturn(1L); + Mockito.when(oldVol.getInstanceId()).thenReturn(6L); + Mockito.when(oldVol.isRecreatable()).thenReturn(false); + Mockito.when(oldVol.getFormat()).thenReturn(Storage.ImageFormat.QCOW2); + Mockito.when(oldVol.getPassphraseId()).thenReturn(null); // no encryption + + VolumeVO persistedVol = Mockito.mock(VolumeVO.class); + Mockito.when(volumeDao.persist(Mockito.any(VolumeVO.class))).thenReturn(persistedVol); + + VolumeVO result = volumeOrchestrator.allocateDuplicateVolumeVO(oldVol, null, null); + assertNotNull(result); + Mockito.verify(volumeDao, Mockito.times(1)).persist(Mockito.any(VolumeVO.class)); + } + + @Test + public void testAllocateDuplicateVolumeVOWithEncryption() { + Volume oldVol = Mockito.mock(Volume.class); + Mockito.when(oldVol.getVolumeType()).thenReturn(Volume.Type.ROOT); + Mockito.when(oldVol.getName()).thenReturn("secureVol"); + Mockito.when(oldVol.getDataCenterId()).thenReturn(1L); + Mockito.when(oldVol.getDomainId()).thenReturn(2L); + Mockito.when(oldVol.getAccountId()).thenReturn(3L); + Mockito.when(oldVol.getDiskOfferingId()).thenReturn(4L); + Mockito.when(oldVol.getProvisioningType()).thenReturn(Storage.ProvisioningType.THIN); + Mockito.when(oldVol.getSize()).thenReturn(10L); + Mockito.when(oldVol.getMinIops()).thenReturn(100L); + Mockito.when(oldVol.getMaxIops()).thenReturn(200L); + Mockito.when(oldVol.get_iScsiName()).thenReturn("iqn.secure"); + Mockito.when(oldVol.getTemplateId()).thenReturn(5L); + Mockito.when(oldVol.getDeviceId()).thenReturn(2L); + Mockito.when(oldVol.getInstanceId()).thenReturn(7L); + Mockito.when(oldVol.isRecreatable()).thenReturn(true); + Mockito.when(oldVol.getFormat()).thenReturn(Storage.ImageFormat.RAW); + Mockito.when(oldVol.getPassphraseId()).thenReturn(42L); + + PassphraseVO passphrase = Mockito.mock(PassphraseVO.class); + Mockito.when(passphrase.getId()).thenReturn(999L); + Mockito.when(passphraseDao.persist(Mockito.any())).thenReturn(passphrase); + + VolumeVO persistedVol = Mockito.mock(VolumeVO.class); + Mockito.when(volumeDao.persist(Mockito.any())).thenReturn(persistedVol); + + VolumeVO result = volumeOrchestrator.allocateDuplicateVolumeVO(oldVol, null, null); + assertNotNull(result); + Mockito.verify(passphraseDao).persist(Mockito.any(PassphraseVO.class)); + Mockito.verify(volumeDao).persist(Mockito.any()); + } + + @Test + public void testAllocateDuplicateVolumeVOWithTemplateOverride() { + Volume oldVol = Mockito.mock(Volume.class); + Mockito.when(oldVol.getVolumeType()).thenReturn(Volume.Type.ROOT); + Mockito.when(oldVol.getName()).thenReturn("tmplVol"); + Mockito.when(oldVol.getDataCenterId()).thenReturn(1L); + Mockito.when(oldVol.getDomainId()).thenReturn(2L); + Mockito.when(oldVol.getAccountId()).thenReturn(3L); + Mockito.when(oldVol.getDiskOfferingId()).thenReturn(4L); + Mockito.when(oldVol.getProvisioningType()).thenReturn(Storage.ProvisioningType.THIN); + Mockito.when(oldVol.getSize()).thenReturn(20L); + Mockito.when(oldVol.getMinIops()).thenReturn(50L); + Mockito.when(oldVol.getMaxIops()).thenReturn(250L); + Mockito.when(oldVol.get_iScsiName()).thenReturn("iqn.tmpl"); + + VolumeVO persistedVol = Mockito.mock(VolumeVO.class); + Mockito.when(volumeDao.persist(Mockito.any())).thenReturn(persistedVol); + + PassphraseVO mockPassPhrase = Mockito.mock(PassphraseVO.class); + Mockito.when(passphraseDao.persist(Mockito.any())).thenReturn(mockPassPhrase); + + VolumeVO result = volumeOrchestrator.allocateDuplicateVolumeVO(oldVol, null, 222L); + assertNotNull(result); + } + + @Test + public void testAllocateDuplicateVolumeVOEncryptionFromOldVolumeOnly() { + Volume oldVol = Mockito.mock(Volume.class); + Mockito.when(oldVol.getVolumeType()).thenReturn(Volume.Type.ROOT); + Mockito.when(oldVol.getName()).thenReturn("vol-old"); + Mockito.when(oldVol.getDataCenterId()).thenReturn(1L); + Mockito.when(oldVol.getDomainId()).thenReturn(2L); + Mockito.when(oldVol.getAccountId()).thenReturn(3L); + Mockito.when(oldVol.getDiskOfferingId()).thenReturn(4L); + Mockito.when(oldVol.getProvisioningType()).thenReturn(Storage.ProvisioningType.SPARSE); + Mockito.when(oldVol.getSize()).thenReturn(30L); + Mockito.when(oldVol.getMinIops()).thenReturn(10L); + Mockito.when(oldVol.getMaxIops()).thenReturn(500L); + Mockito.when(oldVol.get_iScsiName()).thenReturn("iqn.old"); + Mockito.when(oldVol.getTemplateId()).thenReturn(123L); + Mockito.when(oldVol.getDeviceId()).thenReturn(1L); + Mockito.when(oldVol.getInstanceId()).thenReturn(100L); + Mockito.when(oldVol.isRecreatable()).thenReturn(false); + Mockito.when(oldVol.getFormat()).thenReturn(Storage.ImageFormat.RAW); + + DiskOffering diskOffering = Mockito.mock(DiskOffering.class); + Mockito.when(diskOffering.getEncrypt()).thenReturn(false); // explicitly disables encryption + + VolumeVO persistedVol = Mockito.mock(VolumeVO.class); + Mockito.when(volumeDao.persist(Mockito.any())).thenReturn(persistedVol); + + VolumeVO result = volumeOrchestrator.allocateDuplicateVolumeVO(oldVol, diskOffering, null); + assertNotNull(result); + Mockito.verify(volumeDao).persist(Mockito.any()); + } + + @Test + public void testVolumeOnSharedStoragePoolTrue() { + VolumeVO volume = Mockito.mock(VolumeVO.class); + Mockito.when(volume.getPoolId()).thenReturn(MOCK_POOL_ID); + + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + Mockito.when(pool.getScope()).thenReturn(ScopeType.CLUSTER); // Shared scope + Mockito.when(storagePoolDao.findById(MOCK_POOL_ID)).thenReturn(pool); + + assertTrue(volumeOrchestrator.volumeOnSharedStoragePool(volume)); + } + + @Test + public void testVolumeOnSharedStoragePoolFalseHostScope() { + VolumeVO volume = Mockito.mock(VolumeVO.class); + Mockito.when(volume.getPoolId()).thenReturn(MOCK_POOL_ID); + + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + Mockito.when(pool.getScope()).thenReturn(ScopeType.HOST); // Local scope + Mockito.when(storagePoolDao.findById(MOCK_POOL_ID)).thenReturn(pool); + + Assert.assertFalse(volumeOrchestrator.volumeOnSharedStoragePool(volume)); + } + + @Test + public void testVolumeOnSharedStoragePoolFalseNoPool() { + VolumeVO volume = Mockito.mock(VolumeVO.class); + Mockito.when(volume.getPoolId()).thenReturn(null); // No pool associated + + Assert.assertFalse(volumeOrchestrator.volumeOnSharedStoragePool(volume)); + Mockito.verify(storagePoolDao, Mockito.never()).findById(Mockito.anyLong()); + } + + @Test + public void testVolumeOnSharedStoragePoolFalsePoolNotFound() { + VolumeVO volume = Mockito.mock(VolumeVO.class); + Mockito.when(volume.getPoolId()).thenReturn(MOCK_POOL_ID); + + Mockito.when(storagePoolDao.findById(MOCK_POOL_ID)).thenReturn(null); // Pool not found in DB + + Assert.assertFalse(volumeOrchestrator.volumeOnSharedStoragePool(volume)); + } + + + @Test + public void testVolumeInactiveNoVmId() { + VolumeVO volume = Mockito.mock(VolumeVO.class); + Mockito.when(volume.getInstanceId()).thenReturn(null); + assertTrue(volumeOrchestrator.volumeInactive(volume)); + Mockito.verify(entityMgr, Mockito.never()).findById(Mockito.eq(UserVm.class), Mockito.anyLong()); + } + + @Test + public void testVolumeInactiveVmNotFound() { + VolumeVO volume = Mockito.mock(VolumeVO.class); + Mockito.when(volume.getInstanceId()).thenReturn(MOCK_VM_ID); + Mockito.when(entityMgr.findById(UserVm.class, MOCK_VM_ID)).thenReturn(null); + assertTrue(volumeOrchestrator.volumeInactive(volume)); + } + + @Test + public void testVolumeInactiveVmStopped() { + VolumeVO volume = Mockito.mock(VolumeVO.class); + Mockito.when(volume.getInstanceId()).thenReturn(MOCK_VM_ID); + UserVm vm = Mockito.mock(UserVm.class); + Mockito.when(vm.getState()).thenReturn(VirtualMachine.State.Stopped); + Mockito.when(entityMgr.findById(UserVm.class, MOCK_VM_ID)).thenReturn(vm); + assertTrue(volumeOrchestrator.volumeInactive(volume)); + } + + @Test + public void testVolumeInactiveVmDestroyed() { + VolumeVO volume = Mockito.mock(VolumeVO.class); + Mockito.when(volume.getInstanceId()).thenReturn(MOCK_VM_ID); + UserVm vm = Mockito.mock(UserVm.class); + Mockito.when(vm.getState()).thenReturn(VirtualMachine.State.Destroyed); + Mockito.when(entityMgr.findById(UserVm.class, MOCK_VM_ID)).thenReturn(vm); + assertTrue(volumeOrchestrator.volumeInactive(volume)); + } + + @Test + public void testVolumeInactiveVmRunning() { + VolumeVO volume = Mockito.mock(VolumeVO.class); + Mockito.when(volume.getInstanceId()).thenReturn(MOCK_VM_ID); + UserVm vm = Mockito.mock(UserVm.class); + Mockito.when(vm.getState()).thenReturn(VirtualMachine.State.Running); // Active state + Mockito.when(entityMgr.findById(UserVm.class, MOCK_VM_ID)).thenReturn(vm); + Assert.assertFalse(volumeOrchestrator.volumeInactive(volume)); + } + + @Test + public void testGetVmNameOnVolumeNoVmId() { + VolumeVO volume = Mockito.mock(VolumeVO.class); + Mockito.when(volume.getInstanceId()).thenReturn(null); + Assert.assertNull(volumeOrchestrator.getVmNameOnVolume(volume)); + Mockito.verify(entityMgr, Mockito.never()).findById(Mockito.eq(VirtualMachine.class), Mockito.anyLong()); + } + + @Test + public void testGetVmNameOnVolumeVmNotFound() { + VolumeVO volume = Mockito.mock(VolumeVO.class); + Mockito.when(volume.getInstanceId()).thenReturn(MOCK_VM_ID); + Mockito.when(entityMgr.findById(VirtualMachine.class, MOCK_VM_ID)).thenReturn(null); + Assert.assertNull(volumeOrchestrator.getVmNameOnVolume(volume)); + } + + @Test + public void testGetVmNameOnVolumeSuccess() { + VolumeVO volume = Mockito.mock(VolumeVO.class); + Mockito.when(volume.getInstanceId()).thenReturn(MOCK_VM_ID); + VirtualMachine vm = Mockito.mock(VirtualMachine.class); + Mockito.when(vm.getInstanceName()).thenReturn(MOCK_VM_NAME); + Mockito.when(entityMgr.findById(VirtualMachine.class, MOCK_VM_ID)).thenReturn(vm); + Assert.assertEquals(MOCK_VM_NAME, volumeOrchestrator.getVmNameOnVolume(volume)); + } + + @Test + public void testValidateVolumeSizeRangeValid() throws Exception { + overrideDefaultConfigValue(VolumeOrchestrator.MaxVolumeSize, "2000"); + assertTrue(volumeOrchestrator.validateVolumeSizeRange(1024 * 1024 * 1024)); // 1 GiB + assertTrue(volumeOrchestrator.validateVolumeSizeRange(2000 * 1024 * 1024 * 1024)); // 2 TiB + } + + @Test(expected = InvalidParameterValueException.class) + public void testValidateVolumeSizeRangeTooSmall() { + volumeOrchestrator.validateVolumeSizeRange(1024L); // Less than 1GiB + } + + @Test(expected = InvalidParameterValueException.class) + public void testValidateVolumeSizeRangeNegative() { + volumeOrchestrator.validateVolumeSizeRange(-10); // Negative size + } + + @Test(expected = InvalidParameterValueException.class) + public void testValidateVolumeSizeRangeTooLarge() throws Exception { + overrideDefaultConfigValue(VolumeOrchestrator.MaxVolumeSize, "100L"); + volumeOrchestrator.validateVolumeSizeRange(101); + } + + @Test + public void testCanVmRestartOnAnotherServerAllShared() { + VolumeVO vol1 = Mockito.mock(VolumeVO.class); + VolumeVO vol2 = Mockito.mock(VolumeVO.class); + Mockito.when(vol1.getPoolId()).thenReturn(10L); + Mockito.when(vol2.getPoolId()).thenReturn(20L); + Mockito.when(vol1.isRecreatable()).thenReturn(false); + Mockito.when(vol2.isRecreatable()).thenReturn(false); + + + StoragePoolVO pool1 = Mockito.mock(StoragePoolVO.class); + StoragePoolVO pool2 = Mockito.mock(StoragePoolVO.class); + Mockito.when(pool1.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); // Shared + Mockito.when(pool2.getPoolType()).thenReturn(Storage.StoragePoolType.RBD); // Shared + + Mockito.when(volumeDao.findCreatedByInstance(MOCK_VM_ID)).thenReturn(List.of(vol1, vol2)); + Mockito.when(storagePoolDao.findById(10L)).thenReturn(pool1); + Mockito.when(storagePoolDao.findById(20L)).thenReturn(pool2); + + + assertTrue(volumeOrchestrator.canVmRestartOnAnotherServer(MOCK_VM_ID)); + } + + @Test + public void testCanVmRestartOnAnotherServerOneLocalNotRecreatable() { + VolumeVO vol1 = Mockito.mock(VolumeVO.class); + VolumeVO vol2 = Mockito.mock(VolumeVO.class); // Local, not recreatable + Mockito.when(vol1.getPoolId()).thenReturn(10L); + Mockito.when(vol2.getPoolId()).thenReturn(30L); + Mockito.when(vol1.isRecreatable()).thenReturn(false); + Mockito.when(vol2.isRecreatable()).thenReturn(false); // Not recreatable + + StoragePoolVO pool1 = Mockito.mock(StoragePoolVO.class); + StoragePoolVO pool2 = Mockito.mock(StoragePoolVO.class); + Mockito.when(pool1.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); // Shared + Mockito.when(pool2.getPoolType()).thenReturn(Storage.StoragePoolType.LVM); // Local + + Mockito.when(volumeDao.findCreatedByInstance(MOCK_VM_ID)).thenReturn(List.of(vol1, vol2)); + Mockito.when(storagePoolDao.findById(10L)).thenReturn(pool1); + Mockito.when(storagePoolDao.findById(30L)).thenReturn(pool2); + + Assert.assertFalse("VM restart should be false if a non-recreatable local disk exists", + volumeOrchestrator.canVmRestartOnAnotherServer(MOCK_VM_ID)); + } + + @Test + public void testCanVmRestartOnAnotherServerOneLocalRecreatable() { + VolumeVO vol1 = Mockito.mock(VolumeVO.class); + VolumeVO vol2 = Mockito.mock(VolumeVO.class); // Local, but recreatable + Mockito.when(vol1.getPoolId()).thenReturn(10L); + Mockito.when(vol2.getPoolId()).thenReturn(30L); + Mockito.when(vol1.isRecreatable()).thenReturn(false); + Mockito.when(vol2.isRecreatable()).thenReturn(true); // Recreatable + + StoragePoolVO pool1 = Mockito.mock(StoragePoolVO.class); + StoragePoolVO pool2 = Mockito.mock(StoragePoolVO.class); + Mockito.when(pool1.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); // Shared + + Mockito.when(volumeDao.findCreatedByInstance(MOCK_VM_ID)).thenReturn(List.of(vol1, vol2)); + Mockito.when(storagePoolDao.findById(10L)).thenReturn(pool1); + Mockito.when(storagePoolDao.findById(30L)).thenReturn(pool2); + + assertTrue("VM restart should be true if local disk is recreatable", + volumeOrchestrator.canVmRestartOnAnotherServer(MOCK_VM_ID)); + } + + 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); + } + + @Test + public void testStart() throws Exception { + Mockito.when(configDepot.isNewConfig(VolumeAllocationAlgorithm)).thenReturn(true); + overrideDefaultConfigValue(DeploymentClusterPlanner.VmAllocationAlgorithm, "firstfit"); + Mockito.when(configurationDao.update(Mockito.anyString(), Mockito.anyString())).thenReturn(true); + volumeOrchestrator.start(); + } + + @Test + public void testConfigKeys() { + assertTrue(volumeOrchestrator.getConfigKeys().length > 0); + } } diff --git a/engine/schema/src/test/java/com/cloud/capacity/dao/CapacityDaoImplTest.java b/engine/schema/src/test/java/com/cloud/capacity/dao/CapacityDaoImplTest.java index 76c1092546a6..8e9a0bd34c77 100644 --- a/engine/schema/src/test/java/com/cloud/capacity/dao/CapacityDaoImplTest.java +++ b/engine/schema/src/test/java/com/cloud/capacity/dao/CapacityDaoImplTest.java @@ -16,38 +16,59 @@ // under the License. package com.cloud.capacity.dao; +import com.cloud.capacity.CapacityVO; +import com.cloud.host.Host; +import com.cloud.utils.Pair; +import com.cloud.utils.Ternary; +import com.cloud.utils.db.SearchBuilder; +import com.cloud.utils.db.SearchCriteria; +import com.cloud.utils.db.TransactionLegacy; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnitRunner; + +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; + import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.any; +import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; - -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.InjectMocks; -import org.mockito.Mockito; -import org.mockito.Spy; -import org.mockito.junit.MockitoJUnitRunner; - -import com.cloud.capacity.CapacityVO; -import com.cloud.utils.db.SearchBuilder; -import com.cloud.utils.db.SearchCriteria; - @RunWith(MockitoJUnitRunner.class) public class CapacityDaoImplTest { @Spy @InjectMocks CapacityDaoImpl capacityDao = new CapacityDaoImpl(); + @Mock + private TransactionLegacy txn; + @Mock + private PreparedStatement pstmt; + @Mock + private ResultSet resultSet; + private MockedStatic mockedTransactionLegacy; + private SearchBuilder searchBuilder; private SearchCriteria searchCriteria; @@ -59,6 +80,16 @@ public void setUp() { searchCriteria = mock(SearchCriteria.class); doReturn(searchBuilder).when(capacityDao).createSearchBuilder(); when(searchBuilder.create()).thenReturn(searchCriteria); + + mockedTransactionLegacy = Mockito.mockStatic(TransactionLegacy.class); + mockedTransactionLegacy.when(TransactionLegacy::currentTxn).thenReturn(txn); + } + + @After + public void tearDown() { + if (mockedTransactionLegacy != null) { + mockedTransactionLegacy.close(); + } } @Test @@ -96,4 +127,207 @@ public void testListByHostIdTypesEmptyResult() { verify(capacityDao).listBy(searchCriteria); assertTrue(result.isEmpty()); } + + @Test + public void testListClustersCrossingThresholdEmptyResult() throws Exception { + when(txn.prepareAutoCloseStatement(anyString())).thenReturn(pstmt); + when(pstmt.executeQuery()).thenReturn(resultSet); + when(resultSet.next()).thenReturn(false); + List result = capacityDao.listClustersCrossingThreshold((short)1, 1L, "cpu.threshold", 5000L); + assertNotNull(result); + assertTrue(result.isEmpty()); + } + + @Test + public void testFindCapacityByZoneAndHostTagNoResults() throws Exception { + when(txn.prepareAutoCloseStatement(anyString())).thenReturn(pstmt); + when(pstmt.executeQuery()).thenReturn(resultSet); + when(resultSet.next()).thenReturn(false); + + Ternary result = capacityDao.findCapacityByZoneAndHostTag(1L, "host-tag"); + assertNotNull(result); + assertEquals(Long.valueOf(0L), result.first()); + assertEquals(Long.valueOf(0L), result.second()); + assertEquals(Long.valueOf(0L), result.third()); + } + @Test + public void testFindByHostIdType() { + CapacityVO capacity = new CapacityVO(); + capacity.setHostId(1L); + capacity.setCapacityType((short) 1); + + doReturn(capacity).when(capacityDao).findOneBy(any()); + + CapacityVO found = capacityDao.findByHostIdType(1L, (short) 1); + assertNotNull(found); + assertEquals(Long.valueOf(1L), found.getHostOrPoolId()); + } + + @Test + public void testUpdateAllocatedAddition() throws Exception { + when(txn.prepareAutoCloseStatement(anyString())).thenReturn(pstmt); + doNothing().when(txn).start(); + when(txn.commit()).thenReturn(true); + + capacityDao.updateAllocated(1L, 1000L, (short)1, true); + + verify(txn, times(1)).start(); + verify(txn, times(1)).commit(); + verify(pstmt, times(1)).executeUpdate(); + } + + @Test + public void testUpdateAllocatedSubtraction() throws Exception { + when(txn.prepareAutoCloseStatement(anyString())).thenReturn(pstmt); + doNothing().when(txn).start(); + when(txn.commit()).thenReturn(true); + + capacityDao.updateAllocated(1L, 500L, (short)1, false); + + verify(txn, times(1)).start(); + verify(txn, times(1)).commit(); + verify(pstmt, times(1)).executeUpdate(); + } + + @Test + public void testFindFilteredCapacityByEmptyResult() throws Exception { + when(txn.prepareAutoCloseStatement(anyString())).thenReturn(pstmt); + when(pstmt.executeQuery()).thenReturn(resultSet); + when(resultSet.next()).thenReturn(false); + List result = capacityDao.findFilteredCapacityBy(null, null, null, null, Collections.emptyList(), Collections.emptyList()); + assertNotNull(result); + assertTrue(result.isEmpty()); + } + + @Test + public void testListClustersInZoneOrPodByHostCapacitiesEmpty() throws Exception { + when(txn.prepareAutoCloseStatement(anyString())).thenReturn(pstmt); + when(pstmt.executeQuery()).thenReturn(resultSet); + when(resultSet.next()).thenReturn(false); + + List resultZone = capacityDao.listClustersInZoneOrPodByHostCapacities(1L, 123L, 2, 2048L, (short)0, true); + assertNotNull(resultZone); + assertTrue(resultZone.isEmpty()); + + List resultPod = capacityDao.listClustersInZoneOrPodByHostCapacities(1L, 123L, 2, 2048L, (short)0, false); + assertNotNull(resultPod); + assertTrue(resultPod.isEmpty()); + } + + + @Test + public void testListHostsWithEnoughCapacityEmptyResult() throws Exception { + when(txn.prepareAutoCloseStatement(anyString())).thenReturn(pstmt); + when(pstmt.executeQuery()).thenReturn(resultSet); + when(resultSet.next()).thenReturn(false); + + List result = capacityDao.listHostsWithEnoughCapacity(1, 100L, 200L, Host.Type.Routing.toString()); + assertNotNull(result); + assertTrue(result.isEmpty()); + } + + + @Test + public void testOrderClustersByAggregateCapacityEmptyResult() throws Exception { + when(txn.prepareAutoCloseStatement(anyString())).thenReturn(pstmt); + when(pstmt.executeQuery()).thenReturn(resultSet); + when(resultSet.next()).thenReturn(false); + + Pair, Map> result = capacityDao.orderClustersByAggregateCapacity(1L, 1L, (short) 1, true); + assertNotNull(result); + assertTrue(result.first().isEmpty()); + assertTrue(result.second().isEmpty()); + } + + + @Test + public void testOrderPodsByAggregateCapacityEmptyResult() throws Exception { + when(txn.prepareAutoCloseStatement(anyString())).thenReturn(pstmt); + when(pstmt.executeQuery()).thenReturn(resultSet); + when(resultSet.next()).thenReturn(false); + + Pair, Map> result = capacityDao.orderPodsByAggregateCapacity(1L, (short) 1); + assertNotNull(result); + assertTrue(result.first().isEmpty()); + assertTrue(result.second().isEmpty()); + } + + + @Test + public void testUpdateCapacityState() throws Exception { + when(txn.prepareAutoCloseStatement(anyString())).thenReturn(pstmt); + when(pstmt.executeUpdate()).thenReturn(1); + + capacityDao.updateCapacityState(1L, 1L, 1L, 1L, "Enabled", new short[]{1}); + + verify(pstmt, times(1)).executeUpdate(); + } + + + @Test + public void testFindClusterConsumption() throws Exception { + when(txn.prepareAutoCloseStatement(anyString())).thenReturn(pstmt); + when(pstmt.executeQuery()).thenReturn(resultSet); + when(resultSet.next()).thenReturn(true); + when(resultSet.getFloat(1)).thenReturn(0.5f); + + float result = capacityDao.findClusterConsumption(1L, (short) 1, 1000L); + assertEquals(0.5f, result, 0.0f); + } + + @Test + public void testListPodsByHostCapacitiesEmptyResult() throws Exception { + when(txn.prepareAutoCloseStatement(anyString())).thenReturn(pstmt); + when(pstmt.executeQuery()).thenReturn(resultSet); + when(resultSet.next()).thenReturn(false); + + List result = capacityDao.listPodsByHostCapacities(1L, 2, 1024L, (short)0); + assertNotNull(result); + assertTrue(result.isEmpty()); + } + + @Test + public void testOrderHostsByFreeCapacityEmptyResult() throws Exception { + when(txn.prepareAutoCloseStatement(anyString())).thenReturn(pstmt); + when(pstmt.executeQuery()).thenReturn(resultSet); + when(resultSet.next()).thenReturn(false); + + Pair, Map> result = capacityDao.orderHostsByFreeCapacity(1L, 1L, (short) 0); + assertNotNull(result); + assertTrue(result.first().isEmpty()); + } + + @Test + public void testFindByClusterPodZoneEmptyResult() throws Exception { + when(txn.prepareAutoCloseStatement(anyString())).thenReturn(pstmt); + when(pstmt.executeQuery()).thenReturn(resultSet); + when(resultSet.next()).thenReturn(false); + + List result = capacityDao.findByClusterPodZone(1L, 1L, 1L); + assertNotNull(result); + assertTrue(result.isEmpty()); + } + + @Test + public void testListCapacitiesGroupedByLevelAndTypeEmptyResult() throws Exception { + when(txn.prepareAutoCloseStatement(anyString())).thenReturn(pstmt); + when(pstmt.executeQuery()).thenReturn(resultSet); + when(resultSet.next()).thenReturn(false); + + List result = capacityDao.listCapacitiesGroupedByLevelAndType(0, 1L, + 1L, 1L, 0, Collections.emptyList(), Collections.emptyList(), 1L); + assertNotNull(result); + assertTrue(result.isEmpty()); + } + + @Test + public void testFindCapacityByEmptyResult() throws Exception { + when(txn.prepareAutoCloseStatement(anyString())).thenReturn(pstmt); + when(pstmt.executeQuery()).thenReturn(resultSet); + when(resultSet.next()).thenReturn(false); + + List result = capacityDao.findCapacityBy(1, 1L, 1L, 1L); + assertNotNull(result); + assertTrue(result.isEmpty()); + } } diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java index e0e9b2526f60..cde635b80499 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java @@ -77,7 +77,7 @@ public abstract class AbstractStoragePoolAllocator extends AdapterBase implement @Inject protected PrimaryDataStoreDao storagePoolDao; @Inject protected VolumeDao volumeDao; @Inject protected ConfigurationDao configDao; - @Inject private CapacityDao capacityDao; + @Inject protected CapacityDao capacityDao; @Inject private ClusterDao clusterDao; @Inject private StorageManager storageMgr; @Inject private StorageUtil storageUtil; diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java index 970b063eb9cc..f6712ce46b1f 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocator.java @@ -49,7 +49,7 @@ public class ZoneWideStoragePoolAllocator extends AbstractStoragePoolAllocator { @Inject private DataStoreManager dataStoreMgr; @Inject - private CapacityDao capacityDao; + protected CapacityDao capacityDao; @Override protected List select(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo, boolean bypassStorageTypeCheck, String keyword) { diff --git a/engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocatorTest.java b/engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocatorTest.java index 1b4b96d71e53..9bc4d197c630 100644 --- a/engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocatorTest.java +++ b/engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocatorTest.java @@ -17,19 +17,20 @@ package org.apache.cloudstack.storage.allocator; +import com.cloud.capacity.Capacity; +import com.cloud.capacity.dao.CapacityDao; 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.utils.Pair; 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; @@ -41,10 +42,15 @@ import java.lang.reflect.Field; import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) @@ -57,6 +63,10 @@ public class AbstractStoragePoolAllocatorTest { @Mock Account account; + + @Mock + CapacityDao capacityDao; + private List pools; @Mock @@ -137,6 +147,28 @@ public void reorderRandomPools() { Assert.assertTrue(firstchoice.size() > 2); } + @Test + public void reorderStoragePoolsBasedOnAlgorithmFirstFitLeastConsumed() throws Exception { + overrideDefaultConfigValue(VolumeOrchestrationService.VolumeAllocationAlgorithm, "firstfitleastconsumed"); + when(plan.getDataCenterId()).thenReturn(1L); + when(plan.getClusterId()).thenReturn(1L); + StoragePool pool1 = mock(StoragePool.class); + StoragePool pool2 = mock(StoragePool.class); + when(pool1.getId()).thenReturn(1L); + when(pool2.getId()).thenReturn(2L); + List pools = Arrays.asList(pool1, pool2); + List poolIds = Arrays.asList(2L, 1L); + Map hostCapacityMap = new HashMap<>(); + hostCapacityMap.put(1L, 8.0); + hostCapacityMap.put(2L, 8.5); + Pair, Map> poolsOrderedByCapacity = new Pair<>(poolIds, hostCapacityMap); + + allocator.capacityDao = capacityDao; + Mockito.when(capacityDao.orderHostsByFreeCapacity(1L, 1L, Capacity.CAPACITY_TYPE_LOCAL_STORAGE)).thenReturn(poolsOrderedByCapacity); + List result = allocator.reorderPoolsByCapacity(plan, pools); + assertEquals(Arrays.asList(pool2, pool1), result); + } + private void overrideDefaultConfigValue(final ConfigKey configKey, final String value) throws IllegalAccessException, NoSuchFieldException { final Field f = ConfigKey.class.getDeclaredField("_defaultValue"); f.setAccessible(true); diff --git a/engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocatorTest.java b/engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocatorTest.java new file mode 100644 index 000000000000..b81e57f91579 --- /dev/null +++ b/engine/storage/src/test/java/org/apache/cloudstack/storage/allocator/ZoneWideStoragePoolAllocatorTest.java @@ -0,0 +1,71 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cloudstack.storage.allocator; + +import com.cloud.capacity.Capacity; +import com.cloud.capacity.dao.CapacityDao; +import com.cloud.deploy.DeploymentPlan; +import com.cloud.storage.Storage; +import com.cloud.storage.StoragePool; +import com.cloud.utils.Pair; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mockito; + +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class ZoneWideStoragePoolAllocatorTest { + private ZoneWideStoragePoolAllocator allocator; + private DeploymentPlan plan; + + @Before + public void setUp() { + allocator = new ZoneWideStoragePoolAllocator(); + plan = mock(DeploymentPlan.class); + } + + @Test + public void testReorderPoolsByCapacity() { + when(plan.getDataCenterId()).thenReturn(1L); + when(plan.getClusterId()).thenReturn(null); + StoragePool pool1 = mock(StoragePool.class); + StoragePool pool2 = mock(StoragePool.class); + when(pool1.getPoolType()).thenReturn(Storage.StoragePoolType.Filesystem); + when(pool1.getId()).thenReturn(1L); + when(pool2.getId()).thenReturn(2L); + List pools = Arrays.asList(pool1, pool2); + List poolIds = Arrays.asList(2L, 1L); + Map hostCapacityMap = new HashMap<>(); + hostCapacityMap.put(1L, 8.0); + hostCapacityMap.put(2L, 8.5); + Pair, Map> poolsOrderedByCapacity = new Pair<>(poolIds, hostCapacityMap); + CapacityDao capacityDao = mock(CapacityDao.class); + Mockito.when(capacityDao.orderHostsByFreeCapacity(1L, null, Capacity.CAPACITY_TYPE_LOCAL_STORAGE)).thenReturn(poolsOrderedByCapacity); + allocator.capacityDao = capacityDao; + List result = allocator.reorderPoolsByCapacity(plan, pools); + assertEquals(Arrays.asList(pool2, pool1), result); + } +} diff --git a/server/src/test/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocatorTest.java b/server/src/test/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocatorTest.java new file mode 100644 index 000000000000..4524943db38d --- /dev/null +++ b/server/src/test/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocatorTest.java @@ -0,0 +1,159 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.cloud.agent.manager.allocator.impl; + +import com.cloud.capacity.CapacityManager; +import com.cloud.deploy.DeploymentPlan; +import com.cloud.deploy.DeploymentPlanner; +import com.cloud.host.Host; +import com.cloud.offering.ServiceOffering; +import com.cloud.resource.ResourceManager; +import com.cloud.service.dao.ServiceOfferingDetailsDao; +import com.cloud.user.Account; +import com.cloud.utils.Pair; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.junit.Before; +import org.junit.Test; + +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyMap; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + + +public class FirstFitAllocatorTest { + private FirstFitAllocator allocator; + private CapacityManager capacityMgr; + private ServiceOfferingDetailsDao offeringDetailsDao; + private ResourceManager resourceMgr; + + private DeploymentPlan plan; + private ServiceOffering offering; + private DeploymentPlanner.ExcludeList avoid; + private Account account; + + private Host host1; + private Host host2; + ConfigurationDao configDao; + + @Before + public void setUp() { + allocator = new FirstFitAllocator(); + capacityMgr = mock(CapacityManager.class); + offeringDetailsDao = mock(ServiceOfferingDetailsDao.class); + resourceMgr = mock(ResourceManager.class); + configDao = mock(ConfigurationDao.class); + + allocator._capacityMgr = capacityMgr; + allocator._serviceOfferingDetailsDao = offeringDetailsDao; + allocator._resourceMgr = resourceMgr; + allocator._configDao = configDao; + + plan = mock(DeploymentPlan.class); + offering = mock(ServiceOffering.class); + avoid = mock(DeploymentPlanner.ExcludeList.class); + account = mock(Account.class); + + host1 = mock(Host.class); + host2 = mock(Host.class); + + when(plan.getDataCenterId()).thenReturn(1L); + when(offering.getCpu()).thenReturn(2); + when(offering.getSpeed()).thenReturn(1000); + when(offering.getRamSize()).thenReturn(2048); + when(offering.getId()).thenReturn(123L); + when(offering.getHostTag()).thenReturn(null); + } + + @Test + public void testConfigure() throws Exception { + when(configDao.getConfiguration(anyMap())).thenReturn(new HashMap<>()); + assertTrue(allocator._checkHvm); + assertTrue(allocator.configure("test", new HashMap<>())); + } + + @Test + public void testAllocateTo_SuccessfulMatch() { + List inputHosts = Arrays.asList(host1, host2); + + // All hosts are allowed + when(avoid.shouldAvoid(host1)).thenReturn(false); + when(avoid.shouldAvoid(host2)).thenReturn(false); + + // No GPU requirement + when(offeringDetailsDao.findDetail(eq(123L), anyString())).thenReturn(null); + + // CPU capability and capacity is met + when(capacityMgr.checkIfHostReachMaxGuestLimit(any())).thenReturn(false); + when(capacityMgr.checkIfHostHasCpuCapabilityAndCapacity(eq(host1), eq(offering), eq(true))) + .thenReturn(new Pair<>(true, true)); + when(capacityMgr.checkIfHostHasCpuCapabilityAndCapacity(eq(host2), eq(offering), eq(true))) + .thenReturn(new Pair<>(true, false)); + + List result = allocator.allocateTo(plan, offering, null, avoid, inputHosts, 2, true, account); + + // Only host1 should be returned + assertEquals(1, result.size()); + assertTrue(result.contains(host1)); + assertFalse(result.contains(host2)); + } + + @Test + public void testAllocateTo_AvoidSetAndGuestLimit() { + List inputHosts = Arrays.asList(host1, host2); + + when(avoid.shouldAvoid(host1)).thenReturn(true); // Avoided + when(avoid.shouldAvoid(host2)).thenReturn(false); + + when(capacityMgr.checkIfHostReachMaxGuestLimit(host2)).thenReturn(true); // Reached limit + + List result = allocator.allocateTo(plan, offering, null, avoid, inputHosts, 2, true, account); + + assertTrue(result.isEmpty()); + } + + @Test + public void testAllocateTo_GPUNotAvailable() { + List inputHosts = Arrays.asList(host1); + when(avoid.shouldAvoid(host1)).thenReturn(false); + + // GPU required but not available + var vgpuDetail = mock(com.cloud.service.ServiceOfferingDetailsVO.class); + var pciDetail = mock(com.cloud.service.ServiceOfferingDetailsVO.class); + when(offeringDetailsDao.findDetail(eq(123L), eq("vgpuType"))).thenReturn(vgpuDetail); + when(offeringDetailsDao.findDetail(eq(123L), eq("pciDevice"))).thenReturn(pciDetail); + when(pciDetail.getValue()).thenReturn("NVIDIA"); + when(vgpuDetail.getValue()).thenReturn("GRID"); + + when(resourceMgr.isGPUDeviceAvailable(eq(host1), eq("NVIDIA"), eq("GRID"))).thenReturn(false); + + List result = allocator.allocateTo(plan, offering, null, avoid, inputHosts, 1, true, account); + + assertTrue(result.isEmpty()); + } +}