Repository navigation
declare and register ConfigKeys for all remaining Config enum entries - #14467
Open
DaanHoogland wants to merge 1 commit into
Open
DaanHoogland wants to merge 1 commit into
DaanHoogland wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #14467 +/- ##
=============================================
- Coverage 20.45% 3.70% -16.75%
=============================================
Files 6487 493 -5994
Lines 589329 42558 -546771
Branches 72246 8063 -64183
=============================================
- Hits 120554 1578 -118976
+ Misses 455642 40751 -414891
+ Partials 13133 229 -12904
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
3 open issues
admin1:AdMiN123) for a security-sensitive… · NewNetworkLBHaproxyStatsVisbilityis misspelled (should beVisibility). Since… · New
What changed in this PR
This PR continues the deprecation/migration away from the legacy Config enum by declaring ConfigKey constants for remaining entries and ensuring they get registered (via Configurable#getConfigKeys()) so ConfigDepot knows about them at startup.
Changes:
- Added/expanded
getConfigKeys()lists across multiple managers/services to register previously-unregistered configuration keys. - Introduced new
ConfigKeydeclarations on several public interfaces (API / components-api / plugin service interfaces). - Added
Configurableimplementations to a few classes to participate in config-key registration.
| File | Description |
|---|---|
| services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java | Registers many secondary storage related keys via getConfigKeys(). |
| server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java | Registers the VM snapshot create wait key. |
| server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | Registers several missing VM lifecycle/scaling keys. |
| server/src/main/java/com/cloud/vm/UserVmManager.java | Declares additional VM-related ConfigKeys. |
| server/src/main/java/com/cloud/user/AccountManagerImpl.java | Registers missing account/security keys. |
| server/src/main/java/com/cloud/user/AccountManager.java | Declares additional account/security ConfigKeys. |
| server/src/main/java/com/cloud/usage/UsageServiceImpl.java | Implements Configurable and registers usage-related keys. |
| api/src/main/java/org/apache/cloudstack/usage/UsageService.java | Declares usage-related ConfigKeys. |
| server/src/main/java/com/cloud/server/ManagementServerImpl.java | Expands getConfigKeys(); removes local ConfigKey declarations moved to interface. |
| server/src/main/java/com/cloud/server/ManagementServer.java | Declares many management-server ConfigKeys (including those mentioned in PR description). |
| engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java | Declares HAProxy stats-related ConfigKeys. |
| engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java | Registers the new HAProxy stats-related keys. |
| engine/components-api/src/main/java/com/cloud/storage/StorageManager.java | Declares additional storage-related ConfigKeys. |
| server/src/main/java/com/cloud/storage/StorageManagerImpl.java | Registers additional storage-related keys via getConfigKeys(). |
| plugins/network-elements/elastic-loadbalancer/src/main/java/com/cloud/network/lb/ElasticLoadBalancerManager.java | Declares ELB VM sizing/GC ConfigKeys. |
| plugins/network-elements/elastic-loadbalancer/src/main/java/com/cloud/network/lb/ElasticLoadBalancerManagerImpl.java | Implements Configurable and registers ELB keys. |
Review effort: Lite
Comment on lines
+150
to
+152
| ConfigKey<String> NetworkLBHaproxyStatsAuth = new ConfigKey<>("Secure", String.class, | ||
| "network.loadbalancer.haproxy.stats.auth", "admin1:AdMiN123", | ||
| "Load Balancer(haproxy) authentication string in the format username:password", true); |
| ConfigKey<Integer> VmNetworkThrottlingRate = new ConfigKey<Integer>("Network", Integer.class, "vm.network.throttling.rate", "200", | ||
| "Default data transfer rate in megabits per second allowed in User vm's default network.", true, ConfigKey.Scope.Zone); | ||
|
|
||
| ConfigKey<String> NetworkLBHaproxyStatsVisbility = new ConfigKey<>("Network", String.class, |
| ConfigKey<Integer> VMSnapshotMax = new ConfigKey<Integer>("Advanced", Integer.class, "vmsnapshot.max", "10", "Maximum VM snapshots for a single VM", true, ConfigKey.Scope.Account); | ||
|
|
||
| ConfigKey<Integer> VMSnapshotCreateWait = new ConfigKey<>("Advanced", Integer.class, "vmsnapshot.create.wait", "1800", | ||
| "In second, timeout for create vm snapshot", true); |
Contributor
|
@blueorangutan package |
|
@nvazquez a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✖️ debian ✔️ suse15. SL-JID 19690 |
Copilot stopped reviewing on behalf of
DaanHoogland due to an error
October 22, 2026 18:51
Contributor
|
@blueorangutan test |
|
@nvazquez a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Declares a
ConfigKeyfor every remaining entry of the deprecatedConfigenum and registers each one in its owner'sgetConfigKeys(). The enum entries stay in place and no call site changes; the only runtime effect is that the keys are registered with the ConfigDepot at startup. The follow-up PRs switch the call sites and the last one deletes the enum entries.Also registers three keys that #13884 moved without registering (
default.page.size,check.pod.cidrs,cluster.message.timeout.seconds). That omission is why #13884 failed the simulator CI:updateConfiguration default.page.sizereturned "doesn't exist".Part 1 of 12 of the split of #13884 (phase out the deprecated
Configenum, #10752). Base:main. Every other PR of the split builds on this one.Size: 78 files, +1085/-42.
Split overview: foundation → parallel PRs (account, vpn-vpc, snapshot-usage, event-bus, literals) + stacked core-1 … core-5 → enum-removal (last).
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?
Local verification: full reactor build (checkstyle included) and the unit tests of every touched module pass on this branch; merging all PRs of the split in any order gives no conflicts and reproduces #13884 plus its fixes.
🤖 Generated with Claude Code