#177#178
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a component maintenance mode feature across the backend, REST API, and UI, allowing operators to temporarily exclude specific components from health aggregation and alerting while clearly indicating the state in the dashboard.
Changes:
- Add a persisted (config-backed)
maintenanceModeflag to components and expose it via REST/UI beans. - Add REST endpoints to query/enable/disable maintenance mode and wire UI controls to call them.
- Exclude maintenance-mode components from worst-health calculations, status counters, and status-change event dispatching.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/main/webapp/js/main.js | Adds AJAX helpers to enable/disable maintenance mode via REST endpoints. |
| ui/src/main/webapp/css/common.css | Adds styling for maintenance icon and a maintenance visual treatment class. |
| ui/src/main/java/org/moskito/control/ui/restapi/control/ControlResource.java | Includes maintenance flag in the /control API component payload. |
| ui/src/main/java/org/moskito/control/ui/restapi/control/ComponentResource.java | Adds maintenance status endpoints and toggles maintenance mode server-side. |
| ui/src/main/java/org/moskito/control/ui/restapi/control/ComponentBean.java | Adds maintenanceMode field to REST API component bean. |
| ui/src/main/java/org/moskito/control/ui/jsp/MainView.jsp | Displays a wrench icon for maintenance components and adds UI buttons to toggle mode. |
| ui/src/main/java/org/moskito/control/ui/bean/ComponentCountAndStatusByCategoryBean.java | Skips maintenance components in category status aggregation. |
| ui/src/main/java/org/moskito/control/ui/bean/ComponentBean.java | Adds maintenanceMode to UI component bean used by JSP. |
| ui/src/main/java/org/moskito/control/ui/action/MainViewAction.java | Populates maintenance flag and excludes maintenance components from status counts. |
| core/src/main/java/org/moskito/control/core/Repository.java | Skips maintenance components when computing worst health status. |
| core/src/main/java/org/moskito/control/core/InternalComponentRepository.java | Same worst-health exclusion logic for internal repository variant. |
| core/src/main/java/org/moskito/control/core/EventsDispatcher.java | Suppresses status-change notifications/events for maintenance components. |
| core/src/main/java/org/moskito/control/core/Component.java | Adds maintenance flag and propagates changes to the underlying ComponentConfig. |
| config/src/main/java/org/moskito/control/config/ComponentConfig.java | Adds configurable/serializable maintenanceMode field. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public ReplyObject getMaintenanceMode(@PathParam("componentName") String componentName) { | ||
| Component component = Repository.getInstance().getComponent(componentName); | ||
| if (component == null) { | ||
| return ReplyObject.error("Component not found: " + componentName); | ||
| } |
There was a problem hiding this comment.
Repository.getInstance().getComponent(componentName) throws IllegalArgumentException when a component is missing (it doesn't return null), so the null-check here is ineffective and the endpoint will still 500. Handle the exception (or use a non-throwing lookup) and return a ReplyObject.error(...) instead.
| description = "Returns whether the component is currently in maintenance mode" | ||
| ) | ||
| @ApiResponse(description = "Maintenance mode status", | ||
| content = @Content(schema = @Schema(implementation = Boolean.class))) |
There was a problem hiding this comment.
The OpenAPI response schema for this endpoint is declared as Boolean, but the method returns a ReplyObject wrapper. Update the annotation to document the actual response shape (e.g., ReplyObject) so generated clients/docs are correct.
| content = @Content(schema = @Schema(implementation = Boolean.class))) | |
| content = @Content(schema = @Schema(implementation = ReplyObject.class))) |
| component.setMaintenanceMode(true); | ||
|
|
||
| // Update configuration for persistence | ||
| ComponentConfig config = component.getConfiguration(); | ||
| if (config != null) { | ||
| config.setMaintenanceMode(true); | ||
| MoskitoControlConfiguration.getConfiguration().addComponent(config); | ||
| } |
There was a problem hiding this comment.
component.setMaintenanceMode(true) already updates componentConfig via Component#setMaintenanceMode (see core change), so setting config.setMaintenanceMode(true) again and re-adding the component config is redundant. Also, addComponent only updates the in-memory config array, so the comment about “persistence” is misleading unless there is an explicit save step elsewhere.
| component.setMaintenanceMode(false); | ||
|
|
||
| // Update configuration for persistence | ||
| ComponentConfig config = component.getConfiguration(); | ||
| if (config != null) { | ||
| config.setMaintenanceMode(false); | ||
| MoskitoControlConfiguration.getConfiguration().addComponent(config); | ||
| } |
There was a problem hiding this comment.
Same as the enable endpoint: component.setMaintenanceMode(false) already updates the underlying ComponentConfig, so the extra config.setMaintenanceMode(false) + addComponent(config) is redundant, and the “persistence” comment is misleading unless you actually persist configuration changes to disk.
- Replace ineffective null checks with proper exception handling for Repository.getComponent() - Fix OpenAPI schema annotations to reflect actual ReplyObject return type - Remove redundant config.setMaintenanceMode() calls (already handled by component setter) - Clarify comments about configuration persistence 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
@copilot your review remands have been addressed, recheck. |
No description provided.