Copilot commented on code in PR #9564:
URL: https://github.com/apache/cloudstack/pull/9564#discussion_r4437693930
##########
server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java:
##########
@@ -2005,17 +2007,16 @@ private Optional<StoragePool>
getMatchingStoragePool(String preferredPoolId, Lis
.findFirst();
}
- private Optional<StoragePool> getPreferredStoragePool(List<StoragePool>
poolList, VirtualMachine vm) {
- String accountStoragePoolUuid =
StorageManager.PreferredStoragePool.valueIn(vm.getAccountId());
- Optional<StoragePool> storagePool =
getMatchingStoragePool(accountStoragePoolUuid, poolList);
-
- if (storagePool.isPresent()) {
- logger.debug("A storage pool is specified for this account, so we
will use this storage pool for allocation: {}", storagePool.get());
- } else {
- String globalStoragePoolUuid =
StorageManager.PreferredStoragePool.value();
- storagePool = getMatchingStoragePool(globalStoragePoolUuid,
poolList);
- storagePool.ifPresent(pool -> logger.debug("A storage pool is
specified in global setting, so we will use this storage pool for allocation:
{}", pool));
+ protected Optional<StoragePool> getPreferredStoragePool(List<StoragePool>
poolList, VirtualMachine vm, Long clusterId) {
+ Long accountId = vm != null ? vm.getAccountId() : null;
+ Long domainId = vm != null ? vm.getDomainId() : null;
+ Pair<ConfigKey.Scope, String> preferredStoragePool =
StorageManagerImpl.getPreferredStoragePoolUuid(accountId, domainId, clusterId);
+ if (preferredStoragePool == null) {
+ return Optional.empty();
}
+ Optional<StoragePool> storagePool =
getMatchingStoragePool(preferredStoragePool.second(), poolList);
Review Comment:
When the account pool is absent from the suitable pools, resolving a single
UUID now skips a usable global preference and leaves allocation to the
allocator's ordering, unlike the previous behavior. Check suitability at each
scope before stopping, and add a regression test with an unsuitable account
pool and a suitable global pool. Same issue in
`engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java:407`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
