From a84e65e41e124efb6335b42bfb81152d1cd9fc38 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Mon, 23 Sep 2024 10:39:30 +0200 Subject: [PATCH 1/8] server: add global settings for volume resize --- .../com/cloud/capacity/CapacityManager.java | 12 ++++++++++++ .../java/com/cloud/storage/StorageManager.java | 4 ++++ .../cloud/capacity/CapacityManagerImpl.java | 3 ++- .../ConfigurationManagerImpl.java | 1 + .../com/cloud/storage/StorageManagerImpl.java | 18 +++++++++++++++--- 5 files changed, 34 insertions(+), 4 deletions(-) diff --git a/engine/components-api/src/main/java/com/cloud/capacity/CapacityManager.java b/engine/components-api/src/main/java/com/cloud/capacity/CapacityManager.java index 1c3edad886bb..3be794e1cc51 100644 --- a/engine/components-api/src/main/java/com/cloud/capacity/CapacityManager.java +++ b/engine/components-api/src/main/java/com/cloud/capacity/CapacityManager.java @@ -40,6 +40,7 @@ public interface CapacityManager { static final String StorageCapacityDisableThresholdCK = "pool.storage.capacity.disablethreshold"; static final String StorageOverprovisioningFactorCK = "storage.overprovisioning.factor"; static final String StorageAllocatedCapacityDisableThresholdCK = "pool.storage.allocated.capacity.disablethreshold"; + static final String StorageAllocatedCapacityDisableThresholdForVolumeResizeCK = "pool.storage.allocated.resize.capacity.disablethreshold"; static final ConfigKey CpuOverprovisioningFactor = new ConfigKey<>( @@ -118,6 +119,17 @@ public interface CapacityManager { "Percentage (as a value between 0 and 1) of secondary storage capacity threshold.", true); + static final ConfigKey StorageAllocatedCapacityDisableThresholdForVolumeSize = + new ConfigKey<>( + ConfigKey.CATEGORY_ALERT, + Double.class, + StorageAllocatedCapacityDisableThresholdForVolumeResizeCK, + "0.85", + "Percentage (as a value between 0 and 1) of allocated storage utilization above which allocators will disable using the pool for volume resize. " + + "This is applicable only when volume.resize.allowed.beyond.allocation is set to true.", + true, + ConfigKey.Scope.Zone); + public boolean releaseVmCapacity(VirtualMachine vm, boolean moveFromReserved, boolean moveToReservered, Long hostId); void allocateVmCapacity(VirtualMachine vm, boolean fromLastHost); diff --git a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java index c3909bc56b0d..a73c0beb9174 100644 --- a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java +++ b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java @@ -209,6 +209,10 @@ public interface StorageManager extends StorageService { ConfigKey HEURISTICS_SCRIPT_TIMEOUT = new ConfigKey<>("Advanced", Long.class, "heuristics.script.timeout", "3000", "The maximum runtime, in milliseconds, to execute the heuristic rule; if it is reached, a timeout will happen.", true); + ConfigKey AllowVolumeReSizeBeyongAllocation = new ConfigKey("Advanced", Boolean.class, "volume.resize.allowed.beyond.allocation", "false", + "Determines whether volume size can exceed the pool capacity allocation disable threshold (pool.storage.allocated.capacity.disablethreshold) when resize a volume", + true, ConfigKey.Scope.StoragePool); + /** * should we execute in sequence not involving any storages? * @return tru if commands should execute in sequence diff --git a/server/src/main/java/com/cloud/capacity/CapacityManagerImpl.java b/server/src/main/java/com/cloud/capacity/CapacityManagerImpl.java index 421c980b2096..08f055ca3a35 100644 --- a/server/src/main/java/com/cloud/capacity/CapacityManagerImpl.java +++ b/server/src/main/java/com/cloud/capacity/CapacityManagerImpl.java @@ -1254,6 +1254,7 @@ public String getConfigComponentName() { @Override public ConfigKey[] getConfigKeys() { return new ConfigKey[] {CpuOverprovisioningFactor, MemOverprovisioningFactor, StorageCapacityDisableThreshold, StorageOverprovisioningFactor, - StorageAllocatedCapacityDisableThreshold, StorageOperationsExcludeCluster, ImageStoreNFSVersion, SecondaryStorageCapacityThreshold}; + StorageAllocatedCapacityDisableThreshold, StorageOperationsExcludeCluster, ImageStoreNFSVersion, SecondaryStorageCapacityThreshold, + StorageAllocatedCapacityDisableThresholdForVolumeSize }; } } diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 25cbd10de0a4..14855e17ce1a 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -590,6 +590,7 @@ protected void weightBasedParametersForValidation() { weightBasedParametersForValidation.add(Config.LocalStorageCapacityThreshold.key()); weightBasedParametersForValidation.add(CapacityManager.StorageAllocatedCapacityDisableThreshold.key()); weightBasedParametersForValidation.add(CapacityManager.StorageCapacityDisableThreshold.key()); + weightBasedParametersForValidation.add(CapacityManager.StorageAllocatedCapacityDisableThresholdForVolumeSize.key()); weightBasedParametersForValidation.add(DeploymentClusterPlanner.ClusterCPUCapacityDisableThreshold.key()); weightBasedParametersForValidation.add(DeploymentClusterPlanner.ClusterMemoryCapacityDisableThreshold.key()); weightBasedParametersForValidation.add(Config.AgentLoadThreshold.key()); diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 2ed6be39b543..9dab10442a88 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -3185,6 +3185,7 @@ protected boolean checkPoolforSpace(StoragePool pool, long allocatedSizeWithTemp logger.debug("Total capacity of the pool " + poolVO.getName() + " with ID " + pool.getId() + " is " + toHumanReadableSize(totalOverProvCapacity)); double storageAllocatedThreshold = CapacityManager.StorageAllocatedCapacityDisableThreshold.valueIn(pool.getDataCenterId()); + System.out.println("Wei storageAllocatedThreshold = " + storageAllocatedThreshold); if (logger.isDebugEnabled()) { logger.debug("Checking pool: " + pool.getId() + " for storage allocation , maxSize : " + toHumanReadableSize(totalOverProvCapacity) + ", totalAllocatedSize : " + toHumanReadableSize(allocatedSizeWithTemplate) @@ -3192,14 +3193,24 @@ protected boolean checkPoolforSpace(StoragePool pool, long allocatedSizeWithTemp } double usedPercentage = (allocatedSizeWithTemplate + totalAskingSize) / (double)(totalOverProvCapacity); + System.out.println("Wei usedPercentage = " + usedPercentage); if (usedPercentage > storageAllocatedThreshold) { if (logger.isDebugEnabled()) { logger.debug("Insufficient un-allocated capacity on: " + pool.getId() + " for storage allocation since its allocated percentage: " + usedPercentage - + " has crossed the allocated pool.storage.allocated.capacity.disablethreshold: " + storageAllocatedThreshold + ", skipping this pool"); + + " has crossed the allocated pool.storage.allocated.capacity.disablethreshold: " + storageAllocatedThreshold); + } + if (!AllowVolumeReSizeBeyongAllocation.valueIn(pool.getId())) { + logger.debug(String.format("Skipping the pool %s as %s is false", pool, AllowVolumeReSizeBeyongAllocation.key())); + return false; } - return false; + double storageAllocatedThresholdForResize = CapacityManager.StorageAllocatedCapacityDisableThresholdForVolumeSize.valueIn(pool.getDataCenterId()); + if (usedPercentage > storageAllocatedThresholdForResize) { + logger.debug(String.format("Skipping the pool %s since its allocated percentage: %s has crossed the allocated %s: %s", + pool, usedPercentage, CapacityManager.StorageAllocatedCapacityDisableThresholdForVolumeSize.key(), storageAllocatedThresholdForResize)); + return false; + } } if (totalOverProvCapacity < (allocatedSizeWithTemplate + totalAskingSize)) { @@ -4050,7 +4061,8 @@ public ConfigKey[] getConfigKeys() { MountDisabledStoragePool, VmwareCreateCloneFull, VmwareAllowParallelExecution, - DataStoreDownloadFollowRedirects + DataStoreDownloadFollowRedirects, + AllowVolumeReSizeBeyongAllocation }; } From 97a64ab534f35e095f48db1d9897d576c9e4ef6d Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Fri, 27 Sep 2024 09:58:07 +0200 Subject: [PATCH 2/8] resizeVolume: support automigrate --- .../command/user/volume/ResizeVolumeCmd.java | 8 +++ .../com/cloud/storage/StorageManager.java | 2 +- .../com/cloud/storage/StorageManagerImpl.java | 8 ++- .../cloud/storage/VolumeApiServiceImpl.java | 67 ++++++++++++++++--- 4 files changed, 73 insertions(+), 12 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/ResizeVolumeCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/ResizeVolumeCmd.java index 65a3d6a7063a..c806dc212fbd 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/ResizeVolumeCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/ResizeVolumeCmd.java @@ -73,6 +73,10 @@ public class ResizeVolumeCmd extends BaseAsyncCmd implements UserCmd { description = "new disk offering id") private Long newDiskOfferingId; + @Parameter(name = ApiConstants.AUTO_MIGRATE, type = CommandType.BOOLEAN, required = false, + description = "Flag for automatic migration of the volume whenever migration is required to apply the new size") + private Boolean autoMigrate; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -129,6 +133,10 @@ public Long getNewDiskOfferingId() { return newDiskOfferingId; } + public boolean getAutoMigrate() { + return autoMigrate == null ? false : autoMigrate; + } + ///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// diff --git a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java index a73c0beb9174..9e7e6984421a 100644 --- a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java +++ b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java @@ -211,7 +211,7 @@ public interface StorageManager extends StorageService { ConfigKey AllowVolumeReSizeBeyongAllocation = new ConfigKey("Advanced", Boolean.class, "volume.resize.allowed.beyond.allocation", "false", "Determines whether volume size can exceed the pool capacity allocation disable threshold (pool.storage.allocated.capacity.disablethreshold) when resize a volume", - true, ConfigKey.Scope.StoragePool); + true, ConfigKey.Scope.Zone); /** * should we execute in sequence not involving any storages? diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 9dab10442a88..0f941fdae250 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -3101,7 +3101,7 @@ public boolean storagePoolHasEnoughSpaceForResize(StoragePool pool, long current } else { final StoragePoolVO poolVO = _storagePoolDao.findById(pool.getId()); final long allocatedSizeWithTemplate = _capacityMgr.getAllocatedPoolCapacity(poolVO, null); - return checkPoolforSpace(pool, allocatedSizeWithTemplate, totalAskingSize); + return checkPoolforSpace(pool, allocatedSizeWithTemplate, totalAskingSize, true); } } @@ -3164,6 +3164,10 @@ public boolean isStoragePoolCompliantWithStoragePolicy(List, List> poolsPair = managementService.listStoragePoolsForSystemMigrationOfVolume(volume.getId(), newDiskOfferingId, currentSize, newMinIops, newMaxIops, true, false); + List suitableStoragePools = poolsPair.second(); + + boolean volumeMigrateRequired = false; + StoragePoolVO storagePool = _storagePoolDao.findById(volume.getPoolId()); + if (!storageMgr.storagePoolHasEnoughSpaceForResize(storagePool, currentSize, newSize)) { + volumeMigrateRequired = true; + if (!autoMigrateVolume) { + throw new CloudRuntimeException(String.format("Failed to resize volume %s since the storage pool does not have enough space to resize volume %s, automigrate is set to false but volume needs to migrated.", volume.getUuid(), volume.getName())); + } + } + + boolean volumeResizeRequired = false; + if (currentSize != newSize || !compareEqualsIncludingNullOrZero(newMaxIops, volume.getMaxIops()) || !compareEqualsIncludingNullOrZero(newMinIops, volume.getMinIops())) { + volumeResizeRequired = true; + } + if (!volumeMigrateRequired && !volumeResizeRequired && newDiskOffering != null) { + _volsDao.updateDiskOffering(volume.getId(), newDiskOffering.getId()); + volume = _volsDao.findById(volume.getId()); + updateStorageWithTheNewDiskOffering(volume, newDiskOffering); + + return volume; + } + + if (volumeMigrateRequired) { + if (CollectionUtils.isEmpty(poolsPair.first()) && CollectionUtils.isEmpty(poolsPair.second())) { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Volume resize operation failed for volume ID: %s as no suitable pool(s) found for migrating to support new disk offering or new size", volume.getUuid())); + } + final Long newSizeFinal = newSize; + List suitableStoragePoolsWithEnoughSpace = suitableStoragePools.stream().filter(pool -> storageMgr.storagePoolHasEnoughSpaceForResize(pool, 0L, newSizeFinal)).collect(Collectors.toList()); + if (CollectionUtils.isEmpty(suitableStoragePoolsWithEnoughSpace)) { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Volume resize operation failed for volume ID: %s as no suitable pool(s) with enough space found for volume migration.", volume.getUuid())); + } + Collections.shuffle(suitableStoragePoolsWithEnoughSpace); + MigrateVolumeCmd migrateVolumeCmd = new MigrateVolumeCmd(volume.getId(), suitableStoragePoolsWithEnoughSpace.get(0).getId(), newDiskOfferingId, true); + try { + Volume result = migrateVolume(migrateVolumeCmd); + volume = (result != null) ? _volsDao.findById(result.getId()) : null; + if (volume == null) { + throw new CloudRuntimeException(String.format("Volume resize operation failed for volume ID: %s migration failed to storage pool %s", volume.getUuid(), suitableStoragePools.get(0).getId())); + } + } catch (Exception e) { + throw new CloudRuntimeException(String.format("Volume resize operation failed for volume ID: %s migration failed to storage pool %s due to %s", volume.getUuid(), suitableStoragePools.get(0).getId(), e.getMessage())); + } + } + UserVmVO userVm = _userVmDao.findById(volume.getInstanceId()); if (userVm != null) { @@ -1973,6 +2017,7 @@ public Outcome checkAndRepairVolumeThroughJobQueue(final Long vmId, final public Volume changeDiskOfferingForVolume(ChangeOfferingForVolumeCmd cmd) throws ResourceAllocationException { Long newSize = cmd.getSize(); Long newMinIops = cmd.getMinIops(); + Long newMaxIops = cmd.getMaxIops(); Long newDiskOfferingId = cmd.getNewDiskOfferingId(); boolean shrinkOk = cmd.isShrinkOk(); @@ -2055,7 +2100,7 @@ public Volume changeDiskOfferingForVolumeInternal(Long volumeId, Long newDiskOff StoragePoolVO existingStoragePool = _storagePoolDao.findById(volume.getPoolId()); - Pair, List> poolsPair = managementService.listStoragePoolsForSystemMigrationOfVolume(volume.getId(), newDiskOffering.getId(), newSize, newMinIops, newMaxIops, true, false); + Pair, List> poolsPair = managementService.listStoragePoolsForSystemMigrationOfVolume(volume.getId(), newDiskOffering.getId(), currentSize, newMinIops, newMaxIops, true, false); List suitableStoragePools = poolsPair.second(); if (!suitableStoragePools.stream().anyMatch(p -> (p.getId() == existingStoragePool.getId()))) { @@ -2077,10 +2122,16 @@ public Volume changeDiskOfferingForVolumeInternal(Long volumeId, Long newDiskOff if (CollectionUtils.isEmpty(poolsPair.first()) && CollectionUtils.isEmpty(poolsPair.second())) { throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Volume change offering operation failed for volume ID: %s as no suitable pool(s) found for migrating to support new disk offering", volume.getUuid())); } - Collections.shuffle(suitableStoragePools); - MigrateVolumeCmd migrateVolumeCmd = new MigrateVolumeCmd(volume.getId(), suitableStoragePools.get(0).getId(), newDiskOffering.getId(), true); + final Long newSizeFinal = newSize; + List suitableStoragePoolsWithEnoughSpace = suitableStoragePools.stream().filter(pool -> storageMgr.storagePoolHasEnoughSpaceForResize(pool, 0L, newSizeFinal)).collect(Collectors.toList()); + if (CollectionUtils.isEmpty(suitableStoragePoolsWithEnoughSpace)) { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Volume change offering operation failed for volume ID: %s as no suitable pool(s) with enough space found for volume migration.", volume.getUuid())); + } + Collections.shuffle(suitableStoragePoolsWithEnoughSpace); + MigrateVolumeCmd migrateVolumeCmd = new MigrateVolumeCmd(volume.getId(), suitableStoragePoolsWithEnoughSpace.get(0).getId(), newDiskOffering.getId(), true); try { - volume = (VolumeVO) migrateVolume(migrateVolumeCmd); + Volume result = migrateVolume(migrateVolumeCmd); + volume = (result != null) ? _volsDao.findById(result.getId()) : null; if (volume == null) { throw new CloudRuntimeException(String.format("Volume change offering operation failed for volume ID: %s migration failed to storage pool %s", volume.getUuid(), suitableStoragePools.get(0).getId())); } From 0afa9cf0b71f3923e0a00b2f12b2682ddfb651e7 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Fri, 27 Sep 2024 11:28:31 +0200 Subject: [PATCH 3/8] Address Suresh's comments --- .../main/java/com/cloud/capacity/CapacityManager.java | 2 +- .../src/main/java/com/cloud/storage/StorageManager.java | 5 +++-- .../main/java/com/cloud/storage/StorageManagerImpl.java | 9 ++++++--- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/engine/components-api/src/main/java/com/cloud/capacity/CapacityManager.java b/engine/components-api/src/main/java/com/cloud/capacity/CapacityManager.java index 3be794e1cc51..e1bb10f5d268 100644 --- a/engine/components-api/src/main/java/com/cloud/capacity/CapacityManager.java +++ b/engine/components-api/src/main/java/com/cloud/capacity/CapacityManager.java @@ -124,7 +124,7 @@ public interface CapacityManager { ConfigKey.CATEGORY_ALERT, Double.class, StorageAllocatedCapacityDisableThresholdForVolumeResizeCK, - "0.85", + "0.90", "Percentage (as a value between 0 and 1) of allocated storage utilization above which allocators will disable using the pool for volume resize. " + "This is applicable only when volume.resize.allowed.beyond.allocation is set to true.", true, diff --git a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java index 9e7e6984421a..b51536688990 100644 --- a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java +++ b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java @@ -209,8 +209,9 @@ public interface StorageManager extends StorageService { ConfigKey HEURISTICS_SCRIPT_TIMEOUT = new ConfigKey<>("Advanced", Long.class, "heuristics.script.timeout", "3000", "The maximum runtime, in milliseconds, to execute the heuristic rule; if it is reached, a timeout will happen.", true); - ConfigKey AllowVolumeReSizeBeyongAllocation = new ConfigKey("Advanced", Boolean.class, "volume.resize.allowed.beyond.allocation", "false", - "Determines whether volume size can exceed the pool capacity allocation disable threshold (pool.storage.allocated.capacity.disablethreshold) when resize a volume", + ConfigKey AllowVolumeReSizeBeyondAllocation = new ConfigKey("Advanced", Boolean.class, "volume.resize.allowed.beyond.allocation", "false", + "Determines whether volume size can exceed the pool capacity allocation disable threshold (pool.storage.allocated.capacity.disablethreshold) " + + "when resize a volume upto resize capacity disable threshold (pool.storage.allocated.resize.capacity.disablethreshold)", true, ConfigKey.Scope.Zone); /** diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 0f941fdae250..d7c4ed4a28df 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -3202,8 +3202,11 @@ protected boolean checkPoolforSpace(StoragePool pool, long allocatedSizeWithTemp logger.debug("Insufficient un-allocated capacity on: " + pool.getId() + " for storage allocation since its allocated percentage: " + usedPercentage + " has crossed the allocated pool.storage.allocated.capacity.disablethreshold: " + storageAllocatedThreshold); } - if (!AllowVolumeReSizeBeyongAllocation.valueIn(pool.getId())) { - logger.debug(String.format("Skipping the pool %s as %s is false", pool, AllowVolumeReSizeBeyongAllocation.key())); + if (!forVolumeResize) { + return false; + } + if (!AllowVolumeReSizeBeyondAllocation.valueIn(pool.getDataCenterId())) { + s_logger.debug(String.format("Skipping the pool %s as %s is false", pool, AllowVolumeReSizeBeyondAllocation.key())); return false; } @@ -4064,7 +4067,7 @@ public ConfigKey[] getConfigKeys() { VmwareCreateCloneFull, VmwareAllowParallelExecution, DataStoreDownloadFollowRedirects, - AllowVolumeReSizeBeyongAllocation + AllowVolumeReSizeBeyondAllocation }; } From 30bdc3879b760683341b3a324b1697b1849880a6 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Wed, 2 Oct 2024 16:16:05 +0200 Subject: [PATCH 4/8] Update api/src/main/java/org/apache/cloudstack/api/command/user/volume/ResizeVolumeCmd.java Co-authored-by: Suresh Kumar Anaparti --- .../cloudstack/api/command/user/volume/ResizeVolumeCmd.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/ResizeVolumeCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/ResizeVolumeCmd.java index c806dc212fbd..eb89115cf464 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/ResizeVolumeCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/ResizeVolumeCmd.java @@ -74,7 +74,7 @@ public class ResizeVolumeCmd extends BaseAsyncCmd implements UserCmd { private Long newDiskOfferingId; @Parameter(name = ApiConstants.AUTO_MIGRATE, type = CommandType.BOOLEAN, required = false, - description = "Flag for automatic migration of the volume whenever migration is required to apply the new size") + description = "Flag to allow automatic migration of the volume to another suitable storage pool that accommodates the new size", since = "4.20.1") private Boolean autoMigrate; ///////////////////////////////////////////////////// From b07f591442b621163fed8ec68ecb28ef9ee39fa2 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Wed, 2 Oct 2024 16:20:41 +0200 Subject: [PATCH 5/8] address Suresh's comments --- .../com/cloud/storage/StorageManagerImpl.java | 2 +- .../cloud/storage/VolumeApiServiceImpl.java | 31 ++++++++++--------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index d7c4ed4a28df..f2e03cddb7cd 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -3206,7 +3206,7 @@ protected boolean checkPoolforSpace(StoragePool pool, long allocatedSizeWithTemp return false; } if (!AllowVolumeReSizeBeyondAllocation.valueIn(pool.getDataCenterId())) { - s_logger.debug(String.format("Skipping the pool %s as %s is false", pool, AllowVolumeReSizeBeyondAllocation.key())); + logger.debug(String.format("Skipping the pool %s as %s is false", pool, AllowVolumeReSizeBeyondAllocation.key())); return false; } diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index c49a42470cf5..b12575a8a088 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -1286,16 +1286,26 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep } Long newDiskOfferingId = newDiskOffering != null ? newDiskOffering.getId() : diskOffering.getId(); - Pair, List> poolsPair = managementService.listStoragePoolsForSystemMigrationOfVolume(volume.getId(), newDiskOfferingId, currentSize, newMinIops, newMaxIops, true, false); - List suitableStoragePools = poolsPair.second(); boolean volumeMigrateRequired = false; + List suitableStoragePoolsWithEnoughSpace = null; StoragePoolVO storagePool = _storagePoolDao.findById(volume.getPoolId()); if (!storageMgr.storagePoolHasEnoughSpaceForResize(storagePool, currentSize, newSize)) { - volumeMigrateRequired = true; if (!autoMigrateVolume) { - throw new CloudRuntimeException(String.format("Failed to resize volume %s since the storage pool does not have enough space to resize volume %s, automigrate is set to false but volume needs to migrated.", volume.getUuid(), volume.getName())); + throw new CloudRuntimeException(String.format("Failed to resize volume %s since the storage pool does not have enough space to accommodate new size for the volume %s, try with automigrate set to true in order to check in the other suitable pools for the new size and then migrate & resize volume there.", volume.getUuid(), volume.getName())); + } + Pair, List> poolsPair = managementService.listStoragePoolsForSystemMigrationOfVolume(volume.getId(), newDiskOfferingId, currentSize, newMinIops, newMaxIops, true, false); + List suitableStoragePools = poolsPair.second(); + if (CollectionUtils.isEmpty(poolsPair.first()) && CollectionUtils.isEmpty(poolsPair.second())) { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Volume resize failed for volume ID: %s as no suitable pool(s) found for migrating to support new disk offering or new size", volume.getUuid())); } + final Long newSizeFinal = newSize; + suitableStoragePoolsWithEnoughSpace = suitableStoragePools.stream().filter(pool -> storageMgr.storagePoolHasEnoughSpaceForResize(pool, 0L, newSizeFinal)).collect(Collectors.toList()); + if (CollectionUtils.isEmpty(suitableStoragePoolsWithEnoughSpace)) { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Volume resize failed for volume ID: %s as no suitable pool(s) with enough space found.", volume.getUuid())); + } + Collections.shuffle(suitableStoragePoolsWithEnoughSpace); + volumeMigrateRequired = true; } boolean volumeResizeRequired = false; @@ -1311,24 +1321,15 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep } if (volumeMigrateRequired) { - if (CollectionUtils.isEmpty(poolsPair.first()) && CollectionUtils.isEmpty(poolsPair.second())) { - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Volume resize operation failed for volume ID: %s as no suitable pool(s) found for migrating to support new disk offering or new size", volume.getUuid())); - } - final Long newSizeFinal = newSize; - List suitableStoragePoolsWithEnoughSpace = suitableStoragePools.stream().filter(pool -> storageMgr.storagePoolHasEnoughSpaceForResize(pool, 0L, newSizeFinal)).collect(Collectors.toList()); - if (CollectionUtils.isEmpty(suitableStoragePoolsWithEnoughSpace)) { - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Volume resize operation failed for volume ID: %s as no suitable pool(s) with enough space found for volume migration.", volume.getUuid())); - } - Collections.shuffle(suitableStoragePoolsWithEnoughSpace); MigrateVolumeCmd migrateVolumeCmd = new MigrateVolumeCmd(volume.getId(), suitableStoragePoolsWithEnoughSpace.get(0).getId(), newDiskOfferingId, true); try { Volume result = migrateVolume(migrateVolumeCmd); volume = (result != null) ? _volsDao.findById(result.getId()) : null; if (volume == null) { - throw new CloudRuntimeException(String.format("Volume resize operation failed for volume ID: %s migration failed to storage pool %s", volume.getUuid(), suitableStoragePools.get(0).getId())); + throw new CloudRuntimeException(String.format("Volume resize operation failed for volume ID: %s as migration failed to storage pool %s accommodating new size", volume.getUuid(), suitableStoragePoolsWithEnoughSpace.get(0).getId())); } } catch (Exception e) { - throw new CloudRuntimeException(String.format("Volume resize operation failed for volume ID: %s migration failed to storage pool %s due to %s", volume.getUuid(), suitableStoragePools.get(0).getId(), e.getMessage())); + throw new CloudRuntimeException(String.format("Volume resize operation failed for volume ID: %s as migration failed to storage pool %s accommodating new size", volume.getUuid(), suitableStoragePoolsWithEnoughSpace.get(0).getId())); } } From 2733d106ae11f509b93adb530c8eb909bab9675b Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Thu, 3 Oct 2024 13:14:13 +0200 Subject: [PATCH 6/8] UI: add autoMigrate to resizeVolume --- ui/src/views/storage/ResizeVolume.vue | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/ui/src/views/storage/ResizeVolume.vue b/ui/src/views/storage/ResizeVolume.vue index 38a7ea5cb23b..8d5bd28917a6 100644 --- a/ui/src/views/storage/ResizeVolume.vue +++ b/ui/src/views/storage/ResizeVolume.vue @@ -47,6 +47,15 @@ :checked="shrinkOk" @change="val => { shrinkOk = val }"/> + + + +
{{ $t('label.cancel') }} {{ $t('label.ok') }} @@ -58,9 +67,13 @@ import { ref, reactive, toRaw } from 'vue' import { api } from '@/api' import { mixinForm } from '@/utils/mixin' +import TooltipLabel from '@/components/widgets/TooltipLabel' export default { name: 'ResizeVolume', + components: { + TooltipLabel + }, mixins: [mixinForm], props: { resource: { From ae6acc140742c82955f0ac77309d0d84ce4739dd Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Fri, 4 Oct 2024 11:51:15 +0200 Subject: [PATCH 7/8] resizevolume: add unit tests --- .../cloud/storage/StorageManagerImplTest.java | 79 ++++++++++++++++ .../storage/VolumeApiServiceImplTest.java | 91 +++++++++++++++++++ 2 files changed, 170 insertions(+) diff --git a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java index fcbae4f339c7..98a6e203ed78 100644 --- a/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java +++ b/server/src/test/java/com/cloud/storage/StorageManagerImplTest.java @@ -16,6 +16,7 @@ // under the License. package com.cloud.storage; +import java.lang.reflect.Field; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; @@ -42,6 +43,7 @@ import org.apache.cloudstack.api.ApiConstants; 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.resourcedetail.dao.DiskOfferingDetailsDao; import org.apache.cloudstack.storage.command.CheckDataStoreStoragePolicyComplainceCommand; @@ -756,4 +758,81 @@ public void testGetStoragePoolMountFailureReason() { String failureReason = storageManagerImpl.getStoragePoolMountFailureReason(error); Assert.assertEquals(failureReason, "An incorrect mount option was specified"); } + + private void overrideDefaultConfigValue(final ConfigKey configKey, final String name, final Object o) throws IllegalAccessException, NoSuchFieldException { + Field f = ConfigKey.class.getDeclaredField(name); + f.setAccessible(true); + f.set(configKey, o); + } + + private Long testCheckPoolforSpaceForResizeSetup(StoragePoolVO pool, Long allocatedSizeWithTemplate) { + Long poolId = 10L; + Long zoneId = 2L; + + Long capacityBytes = (long) (allocatedSizeWithTemplate / Double.valueOf(CapacityManager.StorageAllocatedCapacityDisableThreshold.defaultValue()) + / Double.valueOf(CapacityManager.StorageOverprovisioningFactor.defaultValue())); + Long maxAllocatedSizeForResize = (long) (capacityBytes * Double.valueOf(CapacityManager.StorageOverprovisioningFactor.defaultValue()) + * Double.valueOf(CapacityManager.StorageAllocatedCapacityDisableThresholdForVolumeSize.defaultValue())); + + System.out.println("maxAllocatedSizeForResize = " + maxAllocatedSizeForResize); + System.out.println("allocatedSizeWithTemplate = " + allocatedSizeWithTemplate); + + Mockito.when(pool.getId()).thenReturn(poolId); + Mockito.when(pool.getCapacityBytes()).thenReturn(capacityBytes); + Mockito.when(pool.getDataCenterId()).thenReturn(zoneId); + Mockito.when(storagePoolDao.findById(poolId)).thenReturn(pool); + Mockito.when(pool.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + + return maxAllocatedSizeForResize - allocatedSizeWithTemplate; + } + + @Test + public void testCheckPoolforSpaceForResize1() { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + Long allocatedSizeWithTemplate = 100L * 1024 * 1024 * 1024; + + Long maxAskingSize = testCheckPoolforSpaceForResizeSetup(pool, allocatedSizeWithTemplate); + Long totalAskingSize = maxAskingSize / 2; + + boolean result = storageManagerImpl.checkPoolforSpace(pool, allocatedSizeWithTemplate, totalAskingSize, false); + Assert.assertFalse(result); + } + + @Test + public void testCheckPoolforSpaceForResize2() { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + Long allocatedSizeWithTemplate = 100L * 1024 * 1024 * 1024; + + Long maxAskingSize = testCheckPoolforSpaceForResizeSetup(pool, allocatedSizeWithTemplate); + Long totalAskingSize = maxAskingSize / 2; + + boolean result = storageManagerImpl.checkPoolforSpace(pool, allocatedSizeWithTemplate, totalAskingSize, true); + Assert.assertFalse(result); + } + + @Test + public void testCheckPoolforSpaceForResize3() throws NoSuchFieldException, IllegalAccessException { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + Long allocatedSizeWithTemplate = 100L * 1024 * 1024 * 1024; + + Long maxAskingSize = testCheckPoolforSpaceForResizeSetup(pool, allocatedSizeWithTemplate); + Long totalAskingSize = maxAskingSize + 1; + overrideDefaultConfigValue(StorageManagerImpl.AllowVolumeReSizeBeyondAllocation, "_defaultValue", "true"); + + boolean result = storageManagerImpl.checkPoolforSpace(pool, allocatedSizeWithTemplate, totalAskingSize, true); + Assert.assertFalse(result); + } + + @Test + public void testCheckPoolforSpaceForResize4() throws NoSuchFieldException, IllegalAccessException { + StoragePoolVO pool = Mockito.mock(StoragePoolVO.class); + Long allocatedSizeWithTemplate = 100L * 1024 * 1024 * 1024; + + Long maxAskingSize = testCheckPoolforSpaceForResizeSetup(pool, allocatedSizeWithTemplate); + Long totalAskingSize = maxAskingSize / 2; + overrideDefaultConfigValue(StorageManagerImpl.AllowVolumeReSizeBeyondAllocation, "_defaultValue", "true"); + + boolean result = storageManagerImpl.checkPoolforSpace(pool, allocatedSizeWithTemplate, totalAskingSize, true); + Assert.assertTrue(result); + } } diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java index 80835891327f..8e0902409dff 100644 --- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java @@ -19,10 +19,13 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; @@ -38,14 +41,17 @@ import java.util.UUID; import java.util.concurrent.ExecutionException; +import com.cloud.server.ManagementService; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.api.command.user.volume.CheckAndRepairVolumeCmd; import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd; import org.apache.cloudstack.api.command.user.volume.DetachVolumeCmd; import org.apache.cloudstack.api.command.user.volume.MigrateVolumeCmd; +import org.apache.cloudstack.api.command.user.volume.ResizeVolumeCmd; import org.apache.cloudstack.backup.dao.BackupDao; import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore; @@ -85,6 +91,7 @@ import org.springframework.test.util.ReflectionTestUtils; import com.cloud.api.query.dao.ServiceOfferingJoinDao; +import com.cloud.configuration.ConfigurationManager; import com.cloud.configuration.Resource; import com.cloud.configuration.Resource.ResourceType; import com.cloud.dc.DataCenterVO; @@ -210,6 +217,8 @@ public class VolumeApiServiceImplTest { private StoragePool storagePoolMock; private long storagePoolMockId = 1; @Mock + private DiskOfferingVO diskOfferingMock; + @Mock private DiskOfferingVO newDiskOfferingMock; @Mock @@ -238,10 +247,20 @@ public class VolumeApiServiceImplTest { @Mock private StorageManager storageMgr; + @Mock + private ConfigurationManager _configMgr; + + @Mock + private VolumeOrchestrationService _volumeMgr; + + @Mock + private ManagementService managementService; + private long accountMockId = 456l; private long volumeMockId = 12313l; private long vmInstanceMockId = 1123l; private long volumeSizeMock = 456789921939l; + private long newVolumeSizeMock = 456789930000l; private static long imageStoreId = 10L; private String projectMockUuid = "projectUuid"; @@ -250,6 +269,7 @@ public class VolumeApiServiceImplTest { private long projectMockAccountId = 132329390L; private long diskOfferingMockId = 100203L; + private long newDiskOfferingMockId = 100204L; private long offeringMockId = 31902L; @@ -1820,4 +1840,75 @@ public void testValidationsForCheckVolumeAPIWithInvalidVolumeFormat() { volumeApiServiceImpl.validationsForCheckVolumeOperation(volume); } + + private void testResizeVolumeSetup() throws ExecutionException, InterruptedException { + Long poolId = 11L; + + when(volumeDaoMock.findById(volumeMockId)).thenReturn(volumeVoMock); + when(volumeVoMock.getId()).thenReturn(volumeMockId); + when(volumeDaoMock.getHypervisorType(volumeMockId)).thenReturn(HypervisorType.KVM); + when(volumeVoMock.getState()).thenReturn(Volume.State.Ready); + when(volumeVoMock.getDiskOfferingId()).thenReturn(diskOfferingMockId); + when(_diskOfferingDao.findById(diskOfferingMockId)).thenReturn(diskOfferingMock); + when(_diskOfferingDao.findById(newDiskOfferingMockId)).thenReturn(newDiskOfferingMock); + when(newDiskOfferingMock.getRemoved()).thenReturn(null); + when(diskOfferingMock.getDiskSizeStrictness()).thenReturn(false); + when(newDiskOfferingMock.getDiskSizeStrictness()).thenReturn(false); + when(volumeVoMock.getInstanceId()).thenReturn(null); + when(volumeVoMock.getVolumeType()).thenReturn(Type.DATADISK); + when(newDiskOfferingMock.getDiskSize()).thenReturn(newVolumeSizeMock); + + VolumeInfo volInfo = Mockito.mock(VolumeInfo.class); + when(volumeDataFactoryMock.getVolume(volumeMockId)).thenReturn(volInfo); + DataStore dataStore = Mockito.mock(DataStore.class); + when((volInfo.getDataStore())).thenReturn(dataStore); + + when(volumeVoMock.getPoolId()).thenReturn(poolId); + StoragePoolVO storagePool = Mockito.mock(StoragePoolVO.class); + when(primaryDataStoreDaoMock.findById(poolId)).thenReturn(storagePool); + + Mockito.lenient().doReturn(asyncCallFutureVolumeapiResultMock).when(volumeServiceMock).resize(any(VolumeInfo.class)); + Mockito.doReturn(Mockito.mock(VolumeApiResult.class)).when(asyncCallFutureVolumeapiResultMock).get(); + } + + @Test + public void testResizeVolumeWithEnoughCapacity() throws ResourceAllocationException, ExecutionException, InterruptedException { + ResizeVolumeCmd cmd = new ResizeVolumeCmd(); + ReflectionTestUtils.setField(cmd, "id", volumeMockId); + ReflectionTestUtils.setField(cmd, "newDiskOfferingId", newDiskOfferingMockId); + + testResizeVolumeSetup(); + + when(storageMgr.storagePoolHasEnoughSpaceForResize(any(), nullable(Long.class), nullable(Long.class))).thenReturn(true); + + try (MockedStatic ignored = Mockito.mockStatic(UsageEventUtils.class)) { + volumeApiServiceImpl.resizeVolume(cmd); + + verify(volumeServiceMock).resize(any(VolumeInfo.class)); + } + } + + @Test + public void testResizeVolumeWithoutEnoughCapacity() throws ResourceAllocationException, ExecutionException, InterruptedException { + ResizeVolumeCmd cmd = new ResizeVolumeCmd(); + ReflectionTestUtils.setField(cmd, "id", volumeMockId); + ReflectionTestUtils.setField(cmd, "newDiskOfferingId", newDiskOfferingMockId); + ReflectionTestUtils.setField(cmd, "autoMigrate", true); + + testResizeVolumeSetup(); + + when(storageMgr.storagePoolHasEnoughSpaceForResize(any(), nullable(Long.class), nullable(Long.class))).thenReturn(false).thenReturn(true); + StoragePoolVO suitableStoragePool = Mockito.mock(StoragePoolVO.class); + Pair, List> poolsPair = new Pair<>(Arrays.asList(suitableStoragePool), Arrays.asList(suitableStoragePool)); + when(managementService.listStoragePoolsForSystemMigrationOfVolume(anyLong(), anyLong(), anyLong(), anyLong(), anyLong(), anyBoolean(), anyBoolean())).thenReturn(poolsPair); + doReturn(volumeInfoMock).when(volumeApiServiceImpl).migrateVolume(any()); + when(volumeInfoMock.getId()).thenReturn(volumeMockId); + + try (MockedStatic ignored = Mockito.mockStatic(UsageEventUtils.class)) { + volumeApiServiceImpl.resizeVolume(cmd); + + verify(volumeApiServiceImpl).migrateVolume(any()); + verify(volumeServiceMock).resize(any(VolumeInfo.class)); + } + } } From e92f38ecc68688dee310a3296aac6dd834c32714 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Fri, 4 Oct 2024 17:01:10 +0200 Subject: [PATCH 8/8] resizevolume: add unit test for Allocated volume --- .../cloud/storage/VolumeApiServiceImplTest.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java index 8e0902409dff..666324d4ed27 100644 --- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java @@ -1911,4 +1911,21 @@ public void testResizeVolumeWithoutEnoughCapacity() throws ResourceAllocationExc verify(volumeServiceMock).resize(any(VolumeInfo.class)); } } + + @Test + public void testResizeVolumeInAllocateState() throws ResourceAllocationException, ExecutionException, InterruptedException { + ResizeVolumeCmd cmd = new ResizeVolumeCmd(); + ReflectionTestUtils.setField(cmd, "id", volumeMockId); + ReflectionTestUtils.setField(cmd, "newDiskOfferingId", newDiskOfferingMockId); + + testResizeVolumeSetup(); + + when(volumeVoMock.getState()).thenReturn(Volume.State.Allocated); + + try (MockedStatic ignored = Mockito.mockStatic(UsageEventUtils.class)) { + volumeApiServiceImpl.resizeVolume(cmd); + + verify(volumeServiceMock, times(0)).resize(any(VolumeInfo.class)); + } + } }