[Feat] Service status updates with polling & vito agent#1200
[Feat] Service status updates with polling & vito agent#1200RichardAnderson wants to merge 7 commits into
Conversation
📝 WalkthroughWalkthroughChangesThe PR adds service-status synchronisation from agent metrics and systemd polling, cached transition confirmation, status events, and Vito Agent configuration updates after successful lifecycle operations. It also adds scheduling, API, SSH, and WebSocket safeguards. Monitoring synchronisation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MetricsCommand
participant CheckServiceStatuses
participant Systemd
participant SyncServiceStatus
participant ServiceDatabase
MetricsCommand->>CheckServiceStatuses: check server services
CheckServiceStatuses->>Systemd: request active states
Systemd-->>CheckServiceStatuses: ordered unit states
CheckServiceStatuses->>SyncServiceStatus: synchronise each service
SyncServiceStatus->>ServiceDatabase: conditionally update status
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds end-to-end “service status” synchronization across both the periodic metrics:get polling path and the vito-agent metrics ingestion path, while keeping the existing agent payload format backwards-compatible and improving secret handling.
Changes:
- Introduces service-status polling via
systemctl is-activeduringmetrics:get, plus status sync + event/broadcast plumbing. - Adds vito-agent config generation/update (including service unit list) and triggers config refresh after service install/uninstall.
- Redacts
type_data.secretfromServiceResourceresponses and hardens agent secret lookup when a service has no handler.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Unit/Jobs/UpdateVitoAgentConfigJobTest.php | Adds coverage for agent config updates and restart behavior. |
| tests/Unit/Commands/GetMetricsCommandTest.php | Adds coverage ensuring metrics:get checks and updates service statuses + dispatches events. |
| tests/Unit/Actions/Service/UninstallTest.php | Asserts uninstall dispatches (or does not dispatch) agent config update job appropriately. |
| tests/Unit/Actions/Service/InstallTest.php | Asserts install dispatch behavior and verifies agent config contents on agent install. |
| tests/Unit/Actions/Service/CheckServiceStatusesTest.php | Adds comprehensive coverage for polling-based status sync and flap/confirmation behavior. |
| tests/Feature/API/ServicesTest.php | Verifies API service listing redacts secrets from type_data. |
| tests/Feature/API/AgentControllerTest.php | Adds coverage for agent-reported service statuses and validation/ignore rules. |
| app/SSH/OS/Systemd.php | Adds batched is-active helper to fetch states for multiple units efficiently. |
| app/Services/Monitoring/VitoAgent/VitoAgent.php | Writes/updates agent config (with managed service units) and restarts agent. |
| app/Models/Service.php | Updates phpdoc to reflect nullable type_data. |
| app/Jobs/Service/UpdateVitoAgentConfigJob.php | New job to regenerate agent config when managed services change. |
| app/Jobs/Service/UninstallJob.php | Dispatches agent config update after successful uninstall. |
| app/Jobs/Service/InstallJob.php | Dispatches agent config update after successful install. |
| app/Http/Resources/ServiceResource.php | Redacts secret from type_data in API responses. |
| app/Http/Controllers/API/AgentController.php | Avoids calling handler when a service has no handler. |
| app/Helpers/SSH.php | Cleans up temporary upload file after successful remote write. |
| app/Events/ServiceStatusChanged.php | New event emitted when a service status changes. |
| app/Console/Kernel.php | Prevents overlapping metrics:get runs via scheduler lock. |
| app/Console/Commands/GetMetricsCommand.php | Invokes service-status polling as part of metrics collection. |
| app/Actions/Service/SyncServiceStatus.php | Centralizes “state -> status” mapping, confirmation logic, and broadcasting/event emission. |
| app/Actions/Service/CheckServiceStatuses.php | Polls checkable services and syncs status updates based on systemd states. |
| app/Actions/Monitoring/StoreAgentMetric.php | Accepts optional services payload from agent and syncs service statuses accordingly. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/Services/Monitoring/VitoAgent/VitoAgent.php`:
- Around line 112-132: Update json_encode in VitoAgent::updateConfig to include
JSON_THROW_ON_ERROR alongside the existing JSON_PRETTY_PRINT and
JSON_UNESCAPED_SLASHES flags, ensuring encoding failures throw before the
configuration file is written.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0ac92c4c-422a-4039-b33d-a22b6fea5acc
📒 Files selected for processing (22)
app/Actions/Monitoring/StoreAgentMetric.phpapp/Actions/Service/CheckServiceStatuses.phpapp/Actions/Service/SyncServiceStatus.phpapp/Console/Commands/GetMetricsCommand.phpapp/Console/Kernel.phpapp/Events/ServiceStatusChanged.phpapp/Helpers/SSH.phpapp/Http/Controllers/API/AgentController.phpapp/Http/Resources/ServiceResource.phpapp/Jobs/Service/InstallJob.phpapp/Jobs/Service/UninstallJob.phpapp/Jobs/Service/UpdateVitoAgentConfigJob.phpapp/Models/Service.phpapp/SSH/OS/Systemd.phpapp/Services/Monitoring/VitoAgent/VitoAgent.phptests/Feature/API/AgentControllerTest.phptests/Feature/API/ServicesTest.phptests/Unit/Actions/Service/CheckServiceStatusesTest.phptests/Unit/Actions/Service/InstallTest.phptests/Unit/Actions/Service/UninstallTest.phptests/Unit/Commands/GetMetricsCommandTest.phptests/Unit/Jobs/UpdateVitoAgentConfigJobTest.php
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/Jobs/Service/UpdateVitoAgentConfigJob.php`:
- Around line 25-45: Remove the try/catch and warning log from dispatchFor in
app/Jobs/Service/UpdateVitoAgentConfigJob.php around lines 25-45 so provider and
service Throwables bubble up unchanged; retain the existing guard checks and
dispatch behavior. Delete the exception-swallowing test in
tests/Unit/Jobs/UpdateVitoAgentConfigJobTest.php around lines 68-87 because it
asserts behavior that is no longer valid.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f49b9e94-559a-4154-8095-af37e998f28e
📒 Files selected for processing (5)
app/Actions/Monitoring/StoreAgentMetric.phpapp/Jobs/Service/UpdateVitoAgentConfigJob.phpapp/Services/Monitoring/VitoAgent/VitoAgent.phptests/Feature/API/AgentControllerTest.phptests/Unit/Jobs/UpdateVitoAgentConfigJobTest.php
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Services/Monitoring/VitoAgent/VitoAgent.php (1)
104-104: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUpdate the PHPDoc to reflect the newly thrown exception.
Since the
$this->updateConfig()call here can throw aJsonException, consider updating the@throwsannotation in theinstall()method's PHPDoc (around line 73) to include\JsonException. As per the path instructions,@throwsannotations must accurately reflect reality to prevent hiding bugs./** * `@throws` SSHError * `@throws` ServiceInstallationFailed * `@throws` ConnectionException * `@throws` JsonException */ public function install(): void🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/Services/Monitoring/VitoAgent/VitoAgent.php` at line 104, Update the PHPDoc for VitoAgent::install() to include \JsonException in its `@throws` annotations, reflecting the exception propagated by updateConfig().Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@app/Services/Monitoring/VitoAgent/VitoAgent.php`:
- Line 104: Update the PHPDoc for VitoAgent::install() to include \JsonException
in its `@throws` annotations, reflecting the exception propagated by
updateConfig().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d865cec5-390a-429d-8430-0edf88ef8983
📒 Files selected for processing (6)
app/Actions/Service/CheckServiceStatuses.phpapp/Http/Controllers/ConsoleController.phpapp/Http/Controllers/EventsController.phpapp/Services/Monitoring/VitoAgent/VitoAgent.phptests/Unit/Actions/Service/CheckServiceStatusesTest.phptests/Unit/Jobs/UpdateVitoAgentConfigJobTest.php
Service status updates are a cheap addition (very minor overhead) that can be added to the metrics:get polling and vito-agent stats collection process. This PR ensures full backwards compatibility with the current vito agent.
Companion PR: vitodeploy/agent#4
You can test by updating
app/Services/Monitoring/VitoAgent/VitoAgent.phpwith the following;Summary by CodeRabbit
Summary of changes