From fe996d6d3657f65435c8c50b809debe90e75bd2e Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Wed, 3 Sep 2025 18:20:53 +0530 Subject: [PATCH 1/5] Check backup framework enabled config in zone scope first (before checking in global scope) --- .../java/org/apache/cloudstack/backup/BackupManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java index 1e6ef1a7852..58c3e892db4 100644 --- a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java @@ -947,7 +947,7 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { } public boolean isDisabled(final Long zoneId) { - return !(BackupFrameworkEnabled.value() && BackupFrameworkEnabled.valueIn(zoneId)); + return !(BackupFrameworkEnabled.valueIn(zoneId)); } private void validateForZone(final Long zoneId) { From 4af74e67d4f5c047b1c2480e1ffd112fef60a278 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Tue, 9 Sep 2025 16:05:08 +0530 Subject: [PATCH 2/5] Update backup.framework.enabled config to Global scope --- .../cloudstack/backup/BackupManager.java | 2 +- .../cloudstack/backup/BackupManagerImpl.java | 19 +++++++++---------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java b/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java index 78d189c3bf1..7aa0176f1a0 100644 --- a/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java +++ b/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java @@ -40,7 +40,7 @@ public interface BackupManager extends BackupService, Configurable, PluggableSer ConfigKey BackupFrameworkEnabled = new ConfigKey<>("Advanced", Boolean.class, "backup.framework.enabled", "false", - "Is backup and recovery framework enabled.", false, ConfigKey.Scope.Zone); + "Is backup and recovery framework enabled.", false); ConfigKey BackupProviderPlugin = new ConfigKey<>("Advanced", String.class, "backup.framework.provider.plugin", diff --git a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java index 58c3e892db4..58bb332f5d1 100644 --- a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java @@ -946,13 +946,13 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { return true; } - public boolean isDisabled(final Long zoneId) { - return !(BackupFrameworkEnabled.valueIn(zoneId)); + public boolean isDisabled() { + return !(BackupFrameworkEnabled.value()); } private void validateForZone(final Long zoneId) { - if (zoneId == null || isDisabled(zoneId)) { - throw new CloudRuntimeException("Backup and Recovery feature is disabled for the zone"); + if (zoneId == null || isDisabled()) { + throw new CloudRuntimeException("Backup and Recovery feature is disabled"); } } @@ -1127,7 +1127,7 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { continue; } - if (isDisabled(vm.getDataCenterId())) { + if (isDisabled()) { continue; } @@ -1236,12 +1236,11 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { if (logger.isTraceEnabled()) { logger.trace("Backup sync background task is running..."); } + if (isDisabled()) { + logger.debug("Backup Sync Task is not enabled. Skipping!"); + return; + } for (final DataCenter dataCenter : dataCenterDao.listAllZones()) { - if (dataCenter == null || isDisabled(dataCenter.getId())) { - logger.debug("Backup Sync Task is not enabled in zone [{}]. Skipping this zone!", dataCenter == null ? "NULL Zone!" : dataCenter); - continue; - } - final BackupProvider backupProvider = getBackupProvider(dataCenter.getId()); if (backupProvider == null) { logger.warn("Backup provider not available or configured for zone {}", dataCenter); From 1543511a087fa78b01376c2f15346cd1d4cf10b1 Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Tue, 23 Sep 2025 23:19:47 +0530 Subject: [PATCH 3/5] Revert "Update backup.framework.enabled config to Global scope" This reverts commit b0b037f2cf2296fbb4e06c9082b7a7057aa96d54. --- .../cloudstack/backup/BackupManager.java | 2 +- .../cloudstack/backup/BackupManagerImpl.java | 19 ++++++++++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java b/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java index 7aa0176f1a0..78d189c3bf1 100644 --- a/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java +++ b/api/src/main/java/org/apache/cloudstack/backup/BackupManager.java @@ -40,7 +40,7 @@ public interface BackupManager extends BackupService, Configurable, PluggableSer ConfigKey BackupFrameworkEnabled = new ConfigKey<>("Advanced", Boolean.class, "backup.framework.enabled", "false", - "Is backup and recovery framework enabled.", false); + "Is backup and recovery framework enabled.", false, ConfigKey.Scope.Zone); ConfigKey BackupProviderPlugin = new ConfigKey<>("Advanced", String.class, "backup.framework.provider.plugin", diff --git a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java index 58bb332f5d1..58c3e892db4 100644 --- a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java @@ -946,13 +946,13 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { return true; } - public boolean isDisabled() { - return !(BackupFrameworkEnabled.value()); + public boolean isDisabled(final Long zoneId) { + return !(BackupFrameworkEnabled.valueIn(zoneId)); } private void validateForZone(final Long zoneId) { - if (zoneId == null || isDisabled()) { - throw new CloudRuntimeException("Backup and Recovery feature is disabled"); + if (zoneId == null || isDisabled(zoneId)) { + throw new CloudRuntimeException("Backup and Recovery feature is disabled for the zone"); } } @@ -1127,7 +1127,7 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { continue; } - if (isDisabled()) { + if (isDisabled(vm.getDataCenterId())) { continue; } @@ -1236,11 +1236,12 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { if (logger.isTraceEnabled()) { logger.trace("Backup sync background task is running..."); } - if (isDisabled()) { - logger.debug("Backup Sync Task is not enabled. Skipping!"); - return; - } for (final DataCenter dataCenter : dataCenterDao.listAllZones()) { + if (dataCenter == null || isDisabled(dataCenter.getId())) { + logger.debug("Backup Sync Task is not enabled in zone [{}]. Skipping this zone!", dataCenter == null ? "NULL Zone!" : dataCenter); + continue; + } + final BackupProvider backupProvider = getBackupProvider(dataCenter.getId()); if (backupProvider == null) { logger.warn("Backup provider not available or configured for zone {}", dataCenter); From 878aa4ca1f83f3dd3a8890f01592934a84b351cb Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Wed, 24 Sep 2025 11:59:23 +0530 Subject: [PATCH 4/5] Check the config key and name exists in its scope (without scopie id) --- .../com/cloud/dc/ClusterDetailsDaoImpl.java | 5 +++++ .../dc/dao/DataCenterDetailsDaoImpl.java | 5 +++++ .../domain/dao/DomainDetailsDaoImpl.java | 5 +++++ .../dao/StoragePoolDetailsDaoImpl.java | 5 +++++ .../com/cloud/user/AccountDetailsDaoImpl.java | 5 +++++ .../resourcedetail/ResourceDetailsDao.java | 2 ++ .../ResourceDetailsDaoBase.java | 6 ++++++ .../db/ImageStoreDetailsDaoImpl.java | 5 +++++ .../framework/config/ConfigKey.java | 6 ++++++ .../framework/config/ScopedConfigStorage.java | 2 ++ .../config/impl/ConfigDepotImpl.java | 20 +++++++++++++++++++ .../cloudstack/backup/BackupManagerImpl.java | 2 +- 12 files changed, 67 insertions(+), 1 deletion(-) diff --git a/engine/schema/src/main/java/com/cloud/dc/ClusterDetailsDaoImpl.java b/engine/schema/src/main/java/com/cloud/dc/ClusterDetailsDaoImpl.java index 7650b40dd2f..7e83600c4e6 100644 --- a/engine/schema/src/main/java/com/cloud/dc/ClusterDetailsDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/dc/ClusterDetailsDaoImpl.java @@ -186,4 +186,9 @@ public class ClusterDetailsDaoImpl extends ResourceDetailsDaoBase } return vo == null ? null : getActualValue(vo); } + + @Override + public boolean doesConfigKeyAndValueExist(String key, String value) { + return doesKeyValuePairExist(key, value); + } } diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/StoragePoolDetailsDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/StoragePoolDetailsDaoImpl.java index 2d9656b5b0a..2a2ca428974 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/StoragePoolDetailsDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/StoragePoolDetailsDaoImpl.java @@ -57,4 +57,9 @@ public class StoragePoolDetailsDaoImpl extends ResourceDetailsDaoBase extends GenericDao long batchExpungeForResources(List ids, Long batchSize); String getActualValue(ResourceDetail resourceDetail); + + boolean doesKeyValuePairExist(String key, String value); } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/ResourceDetailsDaoBase.java b/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/ResourceDetailsDaoBase.java index 29d3f88fd90..c65a97de40b 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/ResourceDetailsDaoBase.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/ResourceDetailsDaoBase.java @@ -246,4 +246,10 @@ public abstract class ResourceDetailsDaoBase extends G } return resourceDetail.getValue(); } + + @Override + public boolean doesKeyValuePairExist(String key, String value) { + List details = findDetails(key, value, null); + return CollectionUtils.isNotEmpty(details); + } } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDetailsDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDetailsDaoImpl.java index 97e41949a57..22012664d99 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDetailsDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDetailsDaoImpl.java @@ -119,4 +119,9 @@ public class ImageStoreDetailsDaoImpl extends ResourceDetailsDaoBase { } } + public boolean hasValueInScope(String value) { + if (value != null && s_depot != null) { + return s_depot.doesConfigKeyAndValueExistsInScope(_name, value, _scope); + } + return false; + } } diff --git a/framework/config/src/main/java/org/apache/cloudstack/framework/config/ScopedConfigStorage.java b/framework/config/src/main/java/org/apache/cloudstack/framework/config/ScopedConfigStorage.java index 8126b9510a2..df53ee17121 100644 --- a/framework/config/src/main/java/org/apache/cloudstack/framework/config/ScopedConfigStorage.java +++ b/framework/config/src/main/java/org/apache/cloudstack/framework/config/ScopedConfigStorage.java @@ -31,4 +31,6 @@ public interface ScopedConfigStorage { default String getConfigValue(long id, ConfigKey key) { return getConfigValue(id, key.key()); } + + boolean doesConfigKeyAndValueExist(String key, String value); } diff --git a/framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java b/framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java index 8b8b6368527..a0768310def 100644 --- a/framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java +++ b/framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java @@ -296,6 +296,26 @@ public class ConfigDepotImpl implements ConfigDepot, ConfigDepotAdmin { return null; } + public boolean doesConfigKeyAndValueExistsInScope(String key, String value, ConfigKey.Scope scope) { + if (!ConfigKey.Scope.Global.equals(scope)) { + ScopedConfigStorage scopedConfigStorage = null; + for (ScopedConfigStorage storage : _scopedStorages) { + if (storage.getScope() == scope) { + scopedConfigStorage = storage; + } + } + if (scopedConfigStorage == null) { + throw new CloudRuntimeException("Unable to find config storage for this scope: " + scope + " for " + key); + } + return scopedConfigStorage.doesConfigKeyAndValueExist(key, value); + } + ConfigurationVO configurationVO = _configDao.findByName(key); + if (configurationVO != null) { + return configurationVO.getValue().equalsIgnoreCase(value); + } + return false; + } + protected Ternary getConfigCacheKey(String key, ConfigKey.Scope scope, Long scopeId) { return new Ternary<>(key, scope, scopeId); } diff --git a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java index 58c3e892db4..7cba74b6f40 100644 --- a/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java @@ -980,7 +980,7 @@ public class BackupManagerImpl extends ManagerBase implements BackupManager { @Override public List> getCommands() { final List> cmdList = new ArrayList>(); - if (!BackupFrameworkEnabled.value()) { + if (!BackupFrameworkEnabled.value() && !BackupFrameworkEnabled.hasValueInScope(Boolean.TRUE.toString())) { return cmdList; } From 2dffcda882085e5b2f8f3294ddfb4b6928c6892f Mon Sep 17 00:00:00 2001 From: Suresh Kumar Anaparti Date: Thu, 25 Sep 2025 11:27:55 +0530 Subject: [PATCH 5/5] code improvements --- .../src/main/java/com/cloud/dc/ClusterDetailsDaoImpl.java | 5 ----- .../java/com/cloud/dc/dao/DataCenterDetailsDaoImpl.java | 5 ----- .../java/com/cloud/domain/dao/DomainDetailsDaoImpl.java | 5 ----- .../com/cloud/storage/dao/StoragePoolDetailsDaoImpl.java | 5 ----- .../src/main/java/com/cloud/user/AccountDetailsDaoImpl.java | 5 ----- .../cloudstack/resourcedetail/ResourceDetailsDao.java | 2 -- .../cloudstack/resourcedetail/ResourceDetailsDaoBase.java | 1 - .../storage/datastore/db/ImageStoreDetailsDaoImpl.java | 5 ----- .../org/apache/cloudstack/framework/config/ConfigKey.java | 2 +- .../cloudstack/framework/config/ScopedConfigStorage.java | 6 +++++- .../cloudstack/framework/config/impl/ConfigDepotImpl.java | 5 +++-- 11 files changed, 9 insertions(+), 37 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/dc/ClusterDetailsDaoImpl.java b/engine/schema/src/main/java/com/cloud/dc/ClusterDetailsDaoImpl.java index 7e83600c4e6..7650b40dd2f 100644 --- a/engine/schema/src/main/java/com/cloud/dc/ClusterDetailsDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/dc/ClusterDetailsDaoImpl.java @@ -186,9 +186,4 @@ public class ClusterDetailsDaoImpl extends ResourceDetailsDaoBase } return vo == null ? null : getActualValue(vo); } - - @Override - public boolean doesConfigKeyAndValueExist(String key, String value) { - return doesKeyValuePairExist(key, value); - } } diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/StoragePoolDetailsDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/StoragePoolDetailsDaoImpl.java index 2a2ca428974..2d9656b5b0a 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/StoragePoolDetailsDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/StoragePoolDetailsDaoImpl.java @@ -57,9 +57,4 @@ public class StoragePoolDetailsDaoImpl extends ResourceDetailsDaoBase extends GenericDao long batchExpungeForResources(List ids, Long batchSize); String getActualValue(ResourceDetail resourceDetail); - - boolean doesKeyValuePairExist(String key, String value); } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/ResourceDetailsDaoBase.java b/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/ResourceDetailsDaoBase.java index c65a97de40b..9dd94a97d64 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/ResourceDetailsDaoBase.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/resourcedetail/ResourceDetailsDaoBase.java @@ -247,7 +247,6 @@ public abstract class ResourceDetailsDaoBase extends G return resourceDetail.getValue(); } - @Override public boolean doesKeyValuePairExist(String key, String value) { List details = findDetails(key, value, null); return CollectionUtils.isNotEmpty(details); diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDetailsDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDetailsDaoImpl.java index 22012664d99..97e41949a57 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDetailsDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDetailsDaoImpl.java @@ -119,9 +119,4 @@ public class ImageStoreDetailsDaoImpl extends ResourceDetailsDaoBase { public boolean hasValueInScope(String value) { if (value != null && s_depot != null) { - return s_depot.doesConfigKeyAndValueExistsInScope(_name, value, _scope); + return s_depot.doesConfigKeyAndValueExistInScope(_name, value, _scope); } return false; } diff --git a/framework/config/src/main/java/org/apache/cloudstack/framework/config/ScopedConfigStorage.java b/framework/config/src/main/java/org/apache/cloudstack/framework/config/ScopedConfigStorage.java index df53ee17121..d0b722b7df2 100644 --- a/framework/config/src/main/java/org/apache/cloudstack/framework/config/ScopedConfigStorage.java +++ b/framework/config/src/main/java/org/apache/cloudstack/framework/config/ScopedConfigStorage.java @@ -32,5 +32,9 @@ public interface ScopedConfigStorage { return getConfigValue(id, key.key()); } - boolean doesConfigKeyAndValueExist(String key, String value); + boolean doesKeyValuePairExist(String key, String value); + + default boolean doesConfigKeyAndValueExist(String key, String value) { + return doesKeyValuePairExist(key, value); + } } diff --git a/framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java b/framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java index a0768310def..adc7cb9345a 100644 --- a/framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java +++ b/framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java @@ -296,7 +296,7 @@ public class ConfigDepotImpl implements ConfigDepot, ConfigDepotAdmin { return null; } - public boolean doesConfigKeyAndValueExistsInScope(String key, String value, ConfigKey.Scope scope) { + public boolean doesConfigKeyAndValueExistInScope(String key, String value, ConfigKey.Scope scope) { if (!ConfigKey.Scope.Global.equals(scope)) { ScopedConfigStorage scopedConfigStorage = null; for (ScopedConfigStorage storage : _scopedStorages) { @@ -305,7 +305,8 @@ public class ConfigDepotImpl implements ConfigDepot, ConfigDepotAdmin { } } if (scopedConfigStorage == null) { - throw new CloudRuntimeException("Unable to find config storage for this scope: " + scope + " for " + key); + logger.warn("Unable to check existence of config key {} and value {} in scope: {}, couldn't find config storage for this scope", key, value, scope); + return false; } return scopedConfigStorage.doesConfigKeyAndValueExist(key, value); }