Enable backup framework when the config 'backup.framework.enabled' is set to true in Zone scope and not in Global setting #11567

sureshanaparti · 2025-09-03T12:53:25Z

Description

This PR enables backup framework when the config 'backup.framework.enabled' is set to true in Zone scope and not in Global setting. It checks backup framework enabled config is set to true or not in any of the zone scope setting when disabled in Global scope, thus supporting backup in a specific zone (and not entire cloud deployment).

Doc PR: apache/cloudstack-documentation#579

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
build/CI
test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Enable 'backup.framework.enabled' config in zone, Keep it disabled (by default) in Global scope.
Restart management server, notice the backup APIs discovered and relevant sections shown in UI.

BackupFrameworkEnabled_Config_Global_False

mysql> SELECT * FROM cloud.configuration WHERE name LIKE 'backup.framework.enabled'\G;
*************************** 1. row ***************************
     category: Advanced
     instance: DEFAULT
    component: BackupService
         name: backup.framework.enabled
        value: false
  description: Is backup and recovery framework enabled.
default_value: false
      updated: 2025-09-25 10:40:30
        scope: Zone
   is_dynamic: 0
     group_id: 1
  subgroup_id: 38
       parent: NULL
 display_text: Backup framework enabled
         kind: NULL
      options: NULL
1 row in set (0.00 sec)

ERROR: 
No query specified

mysql> SELECT * FROM cloud.data_center_details;
+----+-------+--------------------------+-------+---------+
| id | dc_id | name                     | value | display |
+----+-------+--------------------------+-------+---------+
|  2 |     1 | backup.framework.enabled | true  |       1 |
+----+-------+--------------------------+-------+---------+
1 row in set (0.00 sec)

How did you try to break this feature and the system with this change?

sureshanaparti · 2025-09-03T12:54:08Z

@blueorangutan package

codecov · 2025-09-03T13:00:56Z

Codecov Report

❌ Patch coverage is 0% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 16.17%. Comparing base (36cfd76) to head (2dffcda).

Files with missing lines	Patch %	Lines
...udstack/framework/config/impl/ConfigDepotImpl.java	0.00%	16 Missing ⚠️
.../apache/cloudstack/framework/config/ConfigKey.java	0.00%	5 Missing ⚠️
...udstack/resourcedetail/ResourceDetailsDaoBase.java	0.00%	4 Missing ⚠️
...oudstack/framework/config/ScopedConfigStorage.java	0.00%	3 Missing ⚠️
...rg/apache/cloudstack/backup/BackupManagerImpl.java	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               4.20   #11567      +/-   ##
============================================
- Coverage     16.17%   16.17%   -0.01%     
- Complexity    13296    13297       +1     
============================================
  Files          5656     5656              
  Lines        498219   498247      +28     
  Branches      60451    60455       +4     
============================================
+ Hits          80579    80580       +1     
- Misses       408672   408698      +26     
- Partials       8968     8969       +1

Flag	Coverage Δ
uitests	`4.00% <ø> (ø)`
unittests	`17.02% <0.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

abh1sar

We should remove this at line 983. Otherwise backup apis won't show if backup framework is enabled at zone but disabled globally.

        if (!BackupFrameworkEnabled.value()) {
            return cmdList;
        }

sureshanaparti · 2025-09-09T10:38:56Z

@blueorangutan package

Copilot

Pull Request Overview

This PR moves the backup framework enabled configuration from zone-scoped to global-scoped, simplifying the backup management by removing per-zone enablement checks. The change affects how the backup framework determines if it's enabled, removing the need to check individual zones.

Key Changes

Removed zone-specific backup framework configuration support
Simplified backup enablement logic to use global configuration only
Updated error messages to reflect global scope

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
api/src/main/java/org/apache/cloudstack/backup/BackupManager.java	Removes zone scope from backup framework configuration key
server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java	Updates backup enablement checks to use global configuration and removes zone-specific logic

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java

blueorangutan · 2025-09-09T10:40:06Z

@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2025-09-09T13:28:05Z

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 14918

sureshanaparti · 2025-09-24T07:10:34Z

@blueorangutan package

blueorangutan · 2025-09-24T07:12:04Z

@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2025-09-24T08:24:01Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15153

…cking in global scope)

This reverts commit b0b037f.

sureshanaparti · 2025-09-25T07:49:02Z

@blueorangutan package

blueorangutan · 2025-09-25T07:50:04Z

@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2025-09-25T09:16:08Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15163

sureshanaparti · 2025-09-26T08:39:45Z

@blueorangutan test

blueorangutan · 2025-09-26T08:42:05Z

@sureshanaparti a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

blueorangutan · 2025-09-27T01:27:35Z

[SF] Trillian test result (tid-14446)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 57034 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr11567-t14446-kvm-ol8.zip
Smoke tests completed. 141 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

weizhouapache · 2025-10-06T06:38:08Z

@sureshanaparti
I think it is not a bug. I suggest to move to 4.22.0

please check other zone-level settings too
a doc PR may be also needed to describe the behaviour

sureshanaparti · 2025-10-10T13:02:12Z

@sureshanaparti I think it is not a bug. I suggest to move to 4.22.0

please check other zone-level settings too a doc PR may be also needed to describe the behaviour

@weizhouapache There is no change in config scope, but seems minor doc changes required (I'll raise the doc PR). It fixes the issue when 'backup.framework.enabled' is set to true at zone level and set to false in global settings.

sureshanaparti · 2025-10-10T13:16:00Z

@sureshanaparti I think it is not a bug. I suggest to move to 4.22.0
please check other zone-level settings too a doc PR may be also needed to describe the behaviour

@weizhouapache There is no change in config scope, but seems minor doc changes required (I'll raise the doc PR). It fixes the issue when 'backup.framework.enabled' is set to true at zone level and set to false in global settings.

Raised doc PR - apache/cloudstack-documentation#579

weizhouapache · 2025-10-10T14:14:55Z

@sureshanaparti I think it is not a bug. I suggest to move to 4.22.0
please check other zone-level settings too a doc PR may be also needed to describe the behaviour

@weizhouapache There is no change in config scope, but seems minor doc changes required (I'll raise the doc PR). It fixes the issue when 'backup.framework.enabled' is set to true at zone level and set to false in global settings.

Raised doc PR - apache/cloudstack-documentation#579

thanks @sureshanaparti
moving back to 4.20.2 milestone

can you check other settings ? for example RoutedNetworkVpcEnabled (I can only find this)

weizhouapache · 2025-10-10T14:33:30Z

@rohityadavcloud
the last change on the global setting was introduced in the #4202 , what do you think about this PR ?
cc @sureshanaparti

sureshanaparti · 2025-10-13T10:54:19Z

thanks @sureshanaparti moving back to 4.20.2 milestone

can you check other settings ? for example RoutedNetworkVpcEnabled (I can only find this)

Hi @weizhouapache Yes, 'routed.network.vpc.enabled' is also Zone scoped, also has to be updated (at the below files/code) to work when 'routed.network.vpc.enabled' is set to true in Zone, and false in Global Settings. If it is set to true in Global Settings, all zones in the deployment are enabled with it by default when the respective Zone setting is not updated to false. Also, any relevant doc changes to be updated if required.

cloudstack/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java

Lines 171 to 174 in 86cad79

    
           if (!RoutedNetworkVpcEnabled.value()) { 
        
               return cmdList; 
        
           } 
        
           cmdList.add(CreateIpv4SubnetForZoneCmd.class);

cloudstack/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java

Lines 502 to 513 in 86cad79

    
           if (NetworkOffering.NetworkMode.ROUTED.equals(networkMode)) { 
        
               if (!RoutedIpv4Manager.RoutedNetworkVpcEnabled.value()) { 
        
                   throw new InvalidParameterValueException(String.format("Configuration %s needs to be enabled for Routed VPCs", RoutedIpv4Manager.RoutedNetworkVpcEnabled.key())); 
        
               } 
        
               if (zoneIds != null) { 
        
                   for (Long zoneId: zoneIds) { 
        
                       if (!RoutedIpv4Manager.RoutedNetworkVpcEnabled.valueIn(zoneId)) { 
        
                           throw new InvalidParameterValueException(String.format("Configuration %s needs to be enabled for Routed VPCs in zone (ID: %s)", RoutedIpv4Manager.RoutedNetworkVpcEnabled.key(), zoneId)); 
        
                       } 
        
                   } 
        
               } 
        
           }

cloudstack/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

Lines 6842 to 6851 in 86cad79

    
           if (NetworkOffering.NetworkMode.ROUTED.equals(networkMode)) { 
        
               if (!RoutedIpv4Manager.RoutedNetworkVpcEnabled.value()) { 
        
                   throw new InvalidParameterValueException(String.format("Configuration %s needs to be enabled for Routed networks", RoutedIpv4Manager.RoutedNetworkVpcEnabled.key())); 
        
               } 
        
               if (zoneIds != null) { 
        
                   for (Long zoneId: zoneIds) { 
        
                       if (!RoutedIpv4Manager.RoutedNetworkVpcEnabled.valueIn(zoneId)) { 
        
                           throw new InvalidParameterValueException(String.format("Configuration %s needs to be enabled for Routed networks in zone (ID: %s)", RoutedIpv4Manager.RoutedNetworkVpcEnabled.key(), zoneId)); 
        
                       } 
        
                   }

weizhouapache · 2025-10-13T12:12:16Z

Hi @weizhouapache Yes, 'routed.network.vpc.enabled' is also Zone scoped, also has to be updated (at the below files/code) to work when 'routed.network.vpc.enabled' is set to true in Zone, and false in Global Settings. If it is set to true in Global Settings, all zones in the deployment are enabled with it by default when the respective Zone setting is not updated to false. Also, any relevant doc changes to be updated if required.

thanks @sureshanaparti for the research

can you check the PR #4202 ?

sureshanaparti · 2025-10-13T12:21:49Z

Hi @weizhouapache Yes, 'routed.network.vpc.enabled' is also Zone scoped, also has to be updated (at the below files/code) to work when 'routed.network.vpc.enabled' is set to true in Zone, and false in Global Settings. If it is set to true in Global Settings, all zones in the deployment are enabled with it by default when the respective Zone setting is not updated to false. Also, any relevant doc changes to be updated if required.

thanks @sureshanaparti for the research

can you check the PR #4202 ?

Checked it @weizhouapache , B&R APIs are not exported when the setting is not enabled globally there because the config framework couldn't check whether any scoped level has the setting enabled or not. This PR introduces config framework to check any scoped level setting is set with any particular value set or not, which provides flexibility to check the value is set or not at the scope level.

DaanHoogland · 2025-10-13T14:16:26Z

@sureshanaparti , doesn’t ConfigKey.velueInScope(Scope, Long) already address this problem?

abh1sar

The approach is ok for 4.20 but this would need some major changes when merging to 4.21, as the _scope variable has been replaced with scopes to support multi-scope settings.

It would make more sense then to Look at all the parents of a given scope and see if any of them have the value set or not.

abh1sar · 2025-10-13T18:26:25Z

framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImpl.java

+        ConfigurationVO configurationVO = _configDao.findByName(key);
+        if (configurationVO != null) {


For Global scope value() is good enough, right?
why do we need this code?

@abh1sar some earlier config keys are not part of config framework, so these are fetched directly from the db. it's the same with the current method getConfigStringValueInternal() as well.

rajujith

LGTM based on my tests with the NAS plugin.

sureshanaparti · 2025-10-14T07:14:17Z

@sureshanaparti , doesn’t ConfigKey.velueInScope(Scope, Long) already address this problem?

@DaanHoogland No, ConfigKey.valueInScope(Scope, Long) fetches the value in that scope with id specified, if it doesn't exist, picks the global value.

DaanHoogland · 2025-10-14T07:21:33Z

@DaanHoogland No, ConfigKey.valueInScope(Scope, Long) fetches the value in that scope with id specified, if it doesn't exist, picks the global value.

reading the title of this PR that seems to be the intention?!?

sureshanaparti · 2025-10-14T11:48:52Z

@DaanHoogland No, ConfigKey.valueInScope(Scope, Long) fetches the value in that scope with id specified, if it doesn't exist, picks the global value.

reading the title of this PR that seems to be the intention?!?

updated title & description.

shwstppr · 2025-10-14T16:07:57Z

server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java

    public List<Class<?>> getCommands() {
        final List<Class<?>> cmdList = new ArrayList<Class<?>>();
-        if (!BackupFrameworkEnabled.value()) {
+        if (!BackupFrameworkEnabled.value() && !BackupFrameworkEnabled.hasValueInScope(Boolean.TRUE.toString())) {


We should not restrict/allow API access based on the zone config value. It might break UI/clients.

@shwstppr it doesn't restrict API access. it allows API access if it is enabled in global or in any zone (not mandatory to enable in global scope). previously, enabling at global level is mandatory for API access.

boring-cyborg bot added the component:backup label Sep 3, 2025

sureshanaparti requested a review from abh1sar September 3, 2025 12:54

abh1sar reviewed Sep 3, 2025

View reviewed changes

sureshanaparti changed the title ~~Check backup framework enabled config in zone scope first (before checking in global scope)~~ [WIP] Check backup framework enabled config in zone scope first (before checking in global scope) Sep 4, 2025

sureshanaparti requested a review from rajujith September 4, 2025 08:04

sureshanaparti force-pushed the check-backup-framework-disable-in-zone branch from 2f0ee6d to b0b037f Compare September 9, 2025 10:35

boring-cyborg bot added the component:api label Sep 9, 2025

sureshanaparti changed the title ~~[WIP] Check backup framework enabled config in zone scope first (before checking in global scope)~~ Update backup framework enabled config to global scope Sep 9, 2025

sureshanaparti requested review from abh1sar and Copilot September 9, 2025 10:38

Copilot AI reviewed Sep 9, 2025

View reviewed changes

server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java Show resolved Hide resolved

server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java Outdated Show resolved Hide resolved

sureshanaparti changed the title ~~Update backup framework enabled config to global scope~~ [WIP] Update backup framework enabled config to global scope Sep 10, 2025

boring-cyborg bot added the component:storage label Sep 24, 2025

sureshanaparti force-pushed the check-backup-framework-disable-in-zone branch 2 times, most recently from 233ce0d to 08aaaaf Compare September 24, 2025 07:09

sureshanaparti added 5 commits September 25, 2025 13:18

Check backup framework enabled config in zone scope first (before che…

fe996d6

…cking in global scope)

Update backup.framework.enabled config to Global scope

4af74e6

Revert "Update backup.framework.enabled config to Global scope"

1543511

This reverts commit b0b037f.

Check the config key and name exists in its scope (without scopie id)

878aa4c

code improvements

2dffcda

sureshanaparti force-pushed the check-backup-framework-disable-in-zone branch from 08aaaaf to 2dffcda Compare September 25, 2025 07:48

sureshanaparti changed the title ~~[WIP] Update backup framework enabled config to global scope~~ Check backup framework enabled config in zone scope first (before checking in global scope) Sep 26, 2025

sureshanaparti marked this pull request as ready for review September 26, 2025 08:42

DaanHoogland added this to the 4.20.2 milestone Sep 30, 2025

sureshanaparti requested a review from weizhouapache October 6, 2025 04:01

weizhouapache modified the milestones: 4.20.2, 4.22.0 Oct 8, 2025

weizhouapache modified the milestones: 4.22.0, 4.20.2 Oct 10, 2025

abh1sar reviewed Oct 13, 2025

View reviewed changes

rajujith approved these changes Oct 14, 2025

View reviewed changes

sureshanaparti changed the title ~~Check backup framework enabled config in zone scope first (before checking in global scope)~~ Enable backup framework when config 'backup.framework.enabled' is set to true in Zone scope only and not in Global setting Oct 14, 2025

sureshanaparti changed the title ~~Enable backup framework when config 'backup.framework.enabled' is set to true in Zone scope only and not in Global setting~~ Enable backup framework when the config 'backup.framework.enabled' is set to true in Zone scope and not in Global setting Oct 14, 2025

shwstppr reviewed Oct 14, 2025

View reviewed changes

		ConfigurationVO configurationVO = _configDao.findByName(key);
		if (configurationVO != null) {

Enable backup framework when the config 'backup.framework.enabled' is set to true in Zone scope and not in Global setting #11567

Are you sure you want to change the base?

Enable backup framework when the config 'backup.framework.enabled' is set to true in Zone scope and not in Global setting #11567

Uh oh!

Conversation

sureshanaparti commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

Uh oh!

sureshanaparti commented Sep 3, 2025

Uh oh!

codecov bot commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

abh1sar left a comment

Choose a reason for hiding this comment

Uh oh!

sureshanaparti commented Sep 9, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

blueorangutan commented Sep 9, 2025

Uh oh!

blueorangutan commented Sep 9, 2025

Uh oh!

sureshanaparti commented Sep 24, 2025

Uh oh!

blueorangutan commented Sep 24, 2025

Uh oh!

blueorangutan commented Sep 24, 2025

Uh oh!

sureshanaparti commented Sep 25, 2025

Uh oh!

blueorangutan commented Sep 25, 2025

Uh oh!

blueorangutan commented Sep 25, 2025

Uh oh!

sureshanaparti commented Sep 26, 2025

Uh oh!

blueorangutan commented Sep 26, 2025

Uh oh!

blueorangutan commented Sep 27, 2025

Uh oh!

weizhouapache commented Oct 6, 2025

Uh oh!

sureshanaparti commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sureshanaparti commented Oct 10, 2025

Uh oh!

weizhouapache commented Oct 10, 2025

Uh oh!

weizhouapache commented Oct 10, 2025

Uh oh!

sureshanaparti commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

weizhouapache commented Oct 13, 2025

Uh oh!

sureshanaparti commented Oct 13, 2025

Uh oh!

DaanHoogland commented Oct 13, 2025

Uh oh!

abh1sar left a comment

Choose a reason for hiding this comment

Uh oh!

sureshanaparti commented Sep 3, 2025 •

edited

Loading

codecov bot commented Sep 3, 2025 •

edited

Loading

sureshanaparti commented Oct 10, 2025 •

edited

Loading

sureshanaparti commented Oct 13, 2025 •

edited

Loading

shwstppr Oct 14, 2025 •

edited

Loading