Change configs 'consoleproxy.management.state' and 'consoleproxy.management.state.last' to dynamic and some improvements in Console Proxy Manager#13202
Open
sureshanaparti wants to merge 1 commit into
Conversation
sureshanaparti
changed the title
…gement.state.last' to dynamic and some improvements in Console Proxy Manager
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 4.22 #13202 +/- ##
============================================
- Coverage 17.67% 17.67% -0.01%
Complexity 15788 15788
============================================
Files 5922 5922
Lines 533121 533125 +4
Branches 65201 65201
============================================
Hits 94240 94240
- Misses 428236 428240 +4
Partials 10645 10645
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
sureshanaparti
force-pushed
the
console-proxy-manager-improvements
branch
from
64cc338 to
aac7a56
Compare
Contributor
Author
|
@blueorangutan package |
|
@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. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17945 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates Console Proxy Manager configuration handling so that the management state settings can be changed at runtime (dynamic configs), along with some small logging/cleanup improvements.
Changes:
- Mark
consoleproxy.management.stateandconsoleproxy.management.state.lastas dynamic ConfigKeys. - Update
ConsoleProxyManagerImplto use the renamed ConfigKeys and add additional debug/error logs around state transitions. - Minor logging/formatting cleanups in
ManagementServerImplandConfigurationVO.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
server/src/main/java/com/cloud/server/ManagementServerImpl.java |
Adds a debug log when rebooting SSVMs during certificate upload. |
server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManagerImpl.java |
Refactors management state update/read logic and adds logs; switches to the updated ConfigKeys. |
server/src/main/java/com/cloud/consoleproxy/ConsoleProxyManager.java |
Marks management state ConfigKeys as dynamic and renames the “last state” key constant. |
framework/config/src/main/java/org/apache/cloudstack/framework/config/impl/ConfigurationVO.java |
Formatting-only change (spacing). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1017
to
1028
| String stateConfigKey = ConsoleProxyServiceManagementState.key(); | ||
| String stateConfigValue = ConsoleProxyServiceManagementState.value(); | ||
|
|
||
| if (stateConfigValue != null) { | ||
| ConsoleProxyManagementState state = ConsoleProxyManagementState.valueOf(stateConfigValue); | ||
| if (state != null) { | ||
| return state; | ||
| } | ||
| } | ||
|
|
||
| logger.error(String.format("Value [%s] set in global configuration [%s] is not a valid console proxy management state.", value, configKey)); | ||
| logger.error("Console proxy management state value is null in the global configuration [{}].", stateConfigKey); | ||
| return null; |
Comment on lines
+1051
to
1062
| String lastStateConfigKey = ConsoleProxyServiceManagementLastState.key(); | ||
| String lastStateConfigValue = ConsoleProxyServiceManagementLastState.value(); | ||
|
|
||
| if (value != null) { | ||
| ConsoleProxyManagementState state = ConsoleProxyManagementState.valueOf(value); | ||
|
|
||
| if (state != null) { | ||
| return state; | ||
| if (lastStateConfigValue != null) { | ||
| ConsoleProxyManagementState lastState = ConsoleProxyManagementState.valueOf(lastStateConfigValue); | ||
| if (lastState != null) { | ||
| return lastState; | ||
| } | ||
| } | ||
|
|
||
| logger.error(String.format("Value [%s] set in global configuration [%s] is not a valid console proxy management state.", value, configKey)); | ||
| logger.error("Console proxy last management state value is null in the global configuration [{}].", lastStateConfigKey); | ||
| return null; |
Pearl1594
reviewed
May 21, 2026
Comment on lines
+92
to
+96
| ConfigKey<String> ConsoleProxyServiceManagementState = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, String.class, "consoleproxy.management.state", com.cloud.consoleproxy.ConsoleProxyManagementState.Auto.toString(), | ||
| "console proxy service management state", true, ConfigKey.Kind.Select, consoleProxyServiceManagementStates); | ||
|
|
||
| ConfigKey<String> ConsoleProxyServiceManagementLastState = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, String.class, "consoleproxy.management.state.last", com.cloud.consoleproxy.ConsoleProxyManagementState.Auto.toString(), | ||
| "last console proxy service management state", true, ConfigKey.Kind.Select, consoleProxyServiceManagementStates); |
Contributor
There was a problem hiding this comment.
Should we move these to Zone scope as we have done with most of the Console proxy related settings?
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
This PR changes the configurations 'consoleproxy.management.state' and 'consoleproxy.management.state.last' to dynamic, add few logs, and has some improvements in Console Proxy Manager.
The configurations 'consoleproxy.management.state' and 'consoleproxy.management.state.last' were updated in the runtime, and Console Proxy Manager is not able to access the updated values as these configurations are marked as not dynamic here , previously the values were directly accessed from the configuration table using the dao, which was changed here: https://github.com/apache/cloudstack/pull/11415/changes#diff-83d8d4dd27757aa0ff1364c3d156e2396e79f043073acffdddfeb37b1790a8c7.
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?