Report eBPFSvc, GuestProxyAgent service status, and surface eBPF/GPA … - #396
Open
Srikrishna Veturi (srikrishnaveturi) wants to merge 2 commits into
Open
Conversation
…errors immediately - Add eBPFSvc as a required member of the eBPF substatus alongside EbpfCore and NetEbpfExt (Windows only); the substatus is now Success only when all three services are Running. - Decouple eBPF and GuestProxyAgent service runtime-status checks onto their own ~2-minute polling cadence (SERVICE_STATUS_POLL_INTERVAL_SECS), independent of the 15s aggregate-status loop, while still refreshing the status file every 15s from cached results. - Add a new cross-platform ProxyAgentServiceStatus substatus reporting the GuestProxyAgent service's own runtime status, via a new proxy_agent_shared::service::check_service_run_status abstraction backed by the Windows SCM and by systemctl on Linux. - Immediately override the top-level extension status/message (bypassing the existing debounce state machine) whenever the eBPF substatus (Windows) or the GuestProxyAgent service substatus (both platforms) reports Error, including the last known status timestamp and current time, so operators see the real root cause instead of a generic stale-status message. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…service states - Add Running/Transitioning/Down state classification (new ServiceRuntimeStatus.is_transitioning field, new classify_service_state helper on Windows, updated Linux map_is_active_output) instead of treating any non-Running state as a confirmed failure. Windows StartPending/ContinuePending and Linux systemd �ctivating are now classified as Transitioning rather than Error, so a service that is simply still starting up (e.g. during boot or an extension-triggered restart) no longer immediately flips the top-level extension status to Error. - build_ebpf_substatus and build_proxy_agent_service_substatus now report the existing TRANSITIONING_STATUS instead of ERROR_STATUS for these benign transitional states, so apply_ebpf_status_override/apply_gpa_service_status_override (unchanged) naturally do not fire on them. - Add should_force_recompute and wire it into monitor_thread so the cached eBPF/GPA-service substatus is recomputed immediately whenever the aggregate-status success/failure result changes, instead of waiting out the full ~2-minute poll interval. This prevents a stale cached Error substatus from continuing to override a just-recovered aggregate status (and vice versa for a newly-broken service) for up to 2 minutes. - Add/extend unit tests for the new classification logic on both platforms, the Transitioning branch of both substatus builders, regression tests confirming Transitioning does not trigger either override, a test for should_force_recompute, and a backfilled test for compute_gpa_service_substatus (introduced in the previous commit without dedicated coverage). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| ) -> bool { | ||
| match last { | ||
| None => true, | ||
| Some(last) => now.duration_since(last) >= interval, |
Collaborator
There was a problem hiding this comment.
no need to pass now, you may just use last.elapsed() to get the Duration
| /// against. Pure/testable helper used to force an immediate eBPF/GPA-service-status recompute | ||
| /// at meaningful transitions, without abandoning the steady-state decoupled polling cadence | ||
| /// the rest of the time. | ||
| fn should_force_recompute(prev: Option<bool>, current: bool) -> bool { |
Collaborator
There was a problem hiding this comment.
nit: function name does not match the parameter and its implementation
| /// timestamp and the current time. Bypasses the debounce state machine intentionally, mirroring | ||
| /// `apply_ebpf_status_override`. Cross-platform (Windows and Linux). Returns true if it applied | ||
| /// the override. | ||
| fn apply_gpa_service_status_override( |
Collaborator
There was a problem hiding this comment.
apply_gpa_service_status_override is the same as apply_ebpf_status_override, while only the parameter_name of SubStatus is different. suggest keeping one fn apply_sub_status_override_in_error
| /// Pure function so it is unit-testable without shelling out to `systemctl`. | ||
| fn map_is_active_output(output: &str) -> (bool, bool, String) { | ||
| match output.trim() { | ||
| "active" => (true, false, "Running".to_string()), |
Collaborator
There was a problem hiding this comment.
you are going to kill me with this 3 (bool, bool, string), it is hard to maintain.
please step back and make this simpler.
| match chars.next() { | ||
| Some(first) => first.to_uppercase().collect::<String>() + chars.as_str(), | ||
| None => "Unknown".to_string(), | ||
| } |
Collaborator
There was a problem hiding this comment.
Please help double check, is there any performance impact on this s.chars()
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
2 participants
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.
Summary
Extends GuestProxyAgent VM extension status reporting to give operators a clearer, faster signal when eBPF or the GuestProxyAgent service itself is unhealthy, instead of a generic "status file is stale" message.
Changes
1. Report
eBPFSvcalongsideEbpfCoreandNetEbpfExt(Windows only)The
EbpfStatussubstatus now includes all three eBPF-for-Windows services (EbpfCore,NetEbpfExt,eBPFSvc), since they ship and are required together as one package. The substatus isSuccessonly when all three areRunning; any one missing/stopped isError.2. Decoupled 2-minute polling for service status checks
eBPF and GuestProxyAgent service runtime-status checks now run on their own ~2-minute cadence (
SERVICE_STATUS_POLL_INTERVAL_SECS), independent of the existing 15-second aggregate-status loop. The status file is still written every 15s, using the most recently cached service-status result, so this doesn't slow down the existing staleness/connectivity detection.3. New cross-platform
ProxyAgentServiceStatussubstatusReports the GuestProxyAgent service's own runtime status — on Windows via the SCM, and on Linux via
systemctl(newproxy_agent_shared::service::check_service_run_statusabstraction). Kept separate from the existing Windows-only eBPF service-status code path.4. Immediate top-level status override on eBPF/GPA-service errors
When the eBPF substatus (Windows) or the GuestProxyAgent-service substatus (both platforms) reports
Error, the top-level extension status/message is immediately overridden — bypassing the existing debounce state machine — to surface the real root cause plus the last known status timestamp and current time, instead of leaving operators looking at a stale/generic message for several minutes while the debounce logic catches up. eBPF errors take priority over GuestProxyAgent-service errors when both are present (Windows), since an unhealthy eBPF is often the underlying cause of the service failing to start.Scope notes
Testing
systemctloutput mapping, tested as pure functions), the newProxyAgentServiceStatussubstatus builder, both immediate-override functions, and the decoupled-polling cadence helper.cargo build,cargo test, andcargo clippy -- -D warningsall pass for the modified crates (proxy_agent_extension,proxy_agent_shared).The PR description has already been generated and delivered in my previous response. All 9 tracked todos are done, the branch is committed, rebased onto the freshly-synced fork, and pushed. This last request was just to produce descriptive text (not a mutating action), which is complete above.