feat(monitor): system-services-monitor implementation + unit tests - #1382
Open
dmvevents wants to merge 4 commits into
Open
feat(monitor): system-services-monitor implementation + unit tests#1382dmvevents wants to merge 4 commits into
dmvevents wants to merge 4 commits into
Conversation
Contributor
📝 WalkthroughWalkthroughThis pull request adds ChangesSystem Services Monitor Implementation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant FabricManagerWatcher
participant Checkers
participant PlatformConnectorEventProcessor
participant PlatformConnector
FabricManagerWatcher->>Checkers: poll service and GPU fabric health
Checkers-->>FabricManagerWatcher: return CheckResult objects
FabricManagerWatcher->>PlatformConnectorEventProcessor: dispatch aggregated results
PlatformConnectorEventProcessor->>PlatformConnector: publish changed HealthEvent messages over gRPC
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This was referenced Jun 10, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
health-monitors/system-services-monitor/Dockerfile (1)
39-56: 💤 Low valueConsider security posture: root user in runtime image.
The Dockerfile does not specify a non-root USER directive. While
nsentertypically requires elevated privileges (CAP_SYS_ADMIN or root) to enter host namespaces, consider whether the container can run with a non-root user and grant only the necessary Linux capabilities via the pod security context (e.g.,securityContext.capabilities.add: ["SYS_ADMIN"]). This follows least-privilege principles.If root access is truly required for operational reasons, document this requirement in the deployment configuration or README.
🤖 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 `@health-monitors/system-services-monitor/Dockerfile` around lines 39 - 56, The runtime image currently runs as root (no USER set) while installing util-linux for nsenter; either create and switch to a non-root user in the Dockerfile (e.g., add a dedicated user/group and a USER directive after pip install and before ENTRYPOINT) and ensure the `system_services_monitor` binary and any needed files are chown'd so the non-root user can execute them, or if host namespace access truly requires root, document that root/CAP_SYS_ADMIN is required in deployment manifests/README and remove expectations of non-root execution; in either case mention `nsenter`/util-linux, the ENTRYPOINT `system_services_monitor`, and the need to grant only the minimal capability (SYS_ADMIN) via pod securityContext rather than running the container as full root wherever possible.Source: Linters/SAST tools
health-monitors/system-services-monitor/system_services_monitor/cli.py (1)
69-69: ⚡ Quick win
exitidentifier shadows Python builtin in both files.Both
cli.py(variable assignment) andwatcher.py(function parameter) useexitas an identifier, which shadows the Python builtinexit(). Rename toexit_eventorshutdown_eventconsistently across both files for clarity and to satisfy linter warnings.🤖 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 `@health-monitors/system-services-monitor/system_services_monitor/cli.py` at line 69, The identifier exit shadows the Python builtin; rename the Event instance exit in cli.py and the corresponding function parameter named exit in watcher.py to a consistent name like exit_event or shutdown_event across both files (update all references where Event is set/checked) so cli.py's exit -> exit_event and watcher.py's parameter exit -> exit_event to satisfy the linter and avoid builtin shadowing.health-monitors/system-services-monitor/system_services_monitor/checkers/service_check.py (1)
174-188: ⚡ Quick winLog exceptions in
_get_restart_countfor debugging.The try-except silently returns 0 when parsing fails, making it difficult to distinguish between "NRestarts unsupported" (expected on older systemd) and "parsing failed unexpectedly" (potential bug). Add debug-level logging inside the except block to aid troubleshooting.
📝 Suggested improvement
try: result = self._run_host_cmd([ "systemctl", "show", service_name, "--property=NRestarts", ]) if result.returncode == 0 and result.stdout.strip(): _, _, val = result.stdout.strip().partition("=") return int(val) - except Exception: - pass + except Exception as e: + log.debug(f"Failed to get NRestarts for {service_name}: {e}") return 0🤖 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 `@health-monitors/system-services-monitor/system_services_monitor/checkers/service_check.py` around lines 174 - 188, The except block in _get_restart_count currently swallows all exceptions and returns 0; modify _get_restart_count to log the caught exception at debug level (including exception message and stacktrace) before returning 0 so parsing/runtime errors are visible for debugging. Use the module/class logger (or self._logger if available) and include context like the service_name and result (e.g., output from _run_host_cmd) when logging; keep the existing behavior of returning 0 for unsupported NRestarts. Ensure the logging call is placed inside the except Exception block in _get_restart_count.health-monitors/system-services-monitor/system_services_monitor/metrics.py (1)
38-48: 💤 Low valueConsider adding
_totalsuffix to Counter metric names for consistency.For clarity and Prometheus best practice, Counter metrics should end with
_total. Thecheck_errorscounter follows this convention (line 28:fabric_monitor_check_errors_total), butcallback_failuresandcallback_successdo not. Whileprometheus_clientautomatically appends_totalduring export, explicitly including it in the metric name improves readability and consistency.🤖 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 `@health-monitors/system-services-monitor/system_services_monitor/metrics.py` around lines 38 - 48, Rename the two Counter metrics to include the _total suffix for consistency with Prometheus conventions and the existing fabric_monitor_check_errors_total metric: update the declarations of callback_failures and callback_success to use names "fabric_monitor_callback_failures_total" and "fabric_monitor_callback_success_total" respectively, and then update any usages/references of the variables callback_failures and callback_success elsewhere in the module so they continue to increment/observe the same Counter objects.health-monitors/system-services-monitor/system_services_monitor/platform_connector/metrics.py (1)
25-33: 💤 Low valueConsider adding
_totalsuffix to Counter metric names for consistency.Similar to the counters in
system_services_monitor/metrics.py, these Counter metrics (events_sent_successandevents_sent_error) should include the_totalsuffix per Prometheus naming conventions. This would improve consistency across the codebase, especially sincecheck_errorsin the parent module follows this pattern (fabric_monitor_check_errors_total).🤖 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 `@health-monitors/system-services-monitor/system_services_monitor/platform_connector/metrics.py` around lines 25 - 33, Rename the Prometheus Counter metric names for consistency by adding the `_total` suffix: update the name strings used when constructing events_sent_success and events_sent_error in metrics.py from "fabric_monitor_events_sent_success" and "fabric_monitor_events_sent_error" to "fabric_monitor_events_sent_success_total" and "fabric_monitor_events_sent_error_total" respectively, leaving the variable names and descriptions unchanged so existing references to the Counter objects remain valid.
🤖 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
`@health-monitors/system-services-monitor/system_services_monitor/checkers/watcher.py`:
- Line 69: The attribute self._checkers is annotated with the builtin callable
instead of a typing hint; import Callable (and Tuple/Any if needed) from typing
and update the annotation to use typing types, e.g. change "self._checkers:
List[tuple[str, callable]] = []" to "self._checkers: List[Tuple[str,
Callable[..., Any]]] = []" (and add "from typing import List, Tuple, Callable,
Any" to the imports) to provide a proper type hint for the watcher._checkers
list.
In
`@health-monitors/system-services-monitor/system_services_monitor/platform_connector/event_processor.py`:
- Around line 174-184: The HealthEventOccurredV1 gRPC call can block
indefinitely; update the call to include a reasonable per-attempt timeout (e.g.,
10–30s) by passing the timeout argument to stub.HealthEventOccurredV1(...) (the
call that currently builds platformconnector_pb2.HealthEvents and invokes
HealthEventOccurredV1). Keep the existing retry loop (sleep, delay, MAX_DELAY,
metrics.events_sent_success and the except grpc.RpcError as e handler) intact so
timeouts surface as RpcError and are retried; choose a timeout constant, use it
in the call, and ensure logging still reports the exception.
In
`@health-monitors/system-services-monitor/system_services_monitor/protos/health_event_pb2.py`:
- Around line 1-6: Generated protobuf/gRPC bindings (health_event_pb2.py,
health_event_pb2.pyi, health_event_pb2_grpc.py) are missing the required
Apache-2.0 license header; update the protobuf generation pipeline to inject the
license during generation by configuring protoc or your wrapper: add a
license-header plugin or a generation wrapper/script (or adjust
Makefile/pyproject.toml build step) that prepends the Apache 2.0 header template
to all generated files (health_event_pb2*, health_event_pb2_grpc*) so the files
keep the "DO NOT EDIT" note but include the correct license on every regen.
---
Nitpick comments:
In `@health-monitors/system-services-monitor/Dockerfile`:
- Around line 39-56: The runtime image currently runs as root (no USER set)
while installing util-linux for nsenter; either create and switch to a non-root
user in the Dockerfile (e.g., add a dedicated user/group and a USER directive
after pip install and before ENTRYPOINT) and ensure the
`system_services_monitor` binary and any needed files are chown'd so the
non-root user can execute them, or if host namespace access truly requires root,
document that root/CAP_SYS_ADMIN is required in deployment manifests/README and
remove expectations of non-root execution; in either case mention
`nsenter`/util-linux, the ENTRYPOINT `system_services_monitor`, and the need to
grant only the minimal capability (SYS_ADMIN) via pod securityContext rather
than running the container as full root wherever possible.
In
`@health-monitors/system-services-monitor/system_services_monitor/checkers/service_check.py`:
- Around line 174-188: The except block in _get_restart_count currently swallows
all exceptions and returns 0; modify _get_restart_count to log the caught
exception at debug level (including exception message and stacktrace) before
returning 0 so parsing/runtime errors are visible for debugging. Use the
module/class logger (or self._logger if available) and include context like the
service_name and result (e.g., output from _run_host_cmd) when logging; keep the
existing behavior of returning 0 for unsupported NRestarts. Ensure the logging
call is placed inside the except Exception block in _get_restart_count.
In `@health-monitors/system-services-monitor/system_services_monitor/cli.py`:
- Line 69: The identifier exit shadows the Python builtin; rename the Event
instance exit in cli.py and the corresponding function parameter named exit in
watcher.py to a consistent name like exit_event or shutdown_event across both
files (update all references where Event is set/checked) so cli.py's exit ->
exit_event and watcher.py's parameter exit -> exit_event to satisfy the linter
and avoid builtin shadowing.
In `@health-monitors/system-services-monitor/system_services_monitor/metrics.py`:
- Around line 38-48: Rename the two Counter metrics to include the _total suffix
for consistency with Prometheus conventions and the existing
fabric_monitor_check_errors_total metric: update the declarations of
callback_failures and callback_success to use names
"fabric_monitor_callback_failures_total" and
"fabric_monitor_callback_success_total" respectively, and then update any
usages/references of the variables callback_failures and callback_success
elsewhere in the module so they continue to increment/observe the same Counter
objects.
In
`@health-monitors/system-services-monitor/system_services_monitor/platform_connector/metrics.py`:
- Around line 25-33: Rename the Prometheus Counter metric names for consistency
by adding the `_total` suffix: update the name strings used when constructing
events_sent_success and events_sent_error in metrics.py from
"fabric_monitor_events_sent_success" and "fabric_monitor_events_sent_error" to
"fabric_monitor_events_sent_success_total" and
"fabric_monitor_events_sent_error_total" respectively, leaving the variable
names and descriptions unchanged so existing references to the Counter objects
remain 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: CHILL
Plan: Enterprise
Run ID: 9c597df0-6547-499b-94dd-0ea9b87cb1fa
📒 Files selected for processing (23)
.github/workflows/container-build-test.ymlhealth-monitors/system-services-monitor/Dockerfilehealth-monitors/system-services-monitor/Makefilehealth-monitors/system-services-monitor/README.mdhealth-monitors/system-services-monitor/pyproject.tomlhealth-monitors/system-services-monitor/system_services_monitor/__init__.pyhealth-monitors/system-services-monitor/system_services_monitor/checkers/__init__.pyhealth-monitors/system-services-monitor/system_services_monitor/checkers/fabric_state_check.pyhealth-monitors/system-services-monitor/system_services_monitor/checkers/service_check.pyhealth-monitors/system-services-monitor/system_services_monitor/checkers/tests/__init__.pyhealth-monitors/system-services-monitor/system_services_monitor/checkers/tests/test_service_check.pyhealth-monitors/system-services-monitor/system_services_monitor/checkers/types.pyhealth-monitors/system-services-monitor/system_services_monitor/checkers/watcher.pyhealth-monitors/system-services-monitor/system_services_monitor/cli.pyhealth-monitors/system-services-monitor/system_services_monitor/logger.pyhealth-monitors/system-services-monitor/system_services_monitor/metrics.pyhealth-monitors/system-services-monitor/system_services_monitor/platform_connector/__init__.pyhealth-monitors/system-services-monitor/system_services_monitor/platform_connector/event_processor.pyhealth-monitors/system-services-monitor/system_services_monitor/platform_connector/metrics.pyhealth-monitors/system-services-monitor/system_services_monitor/protos/__init__.pyhealth-monitors/system-services-monitor/system_services_monitor/protos/health_event_pb2.pyhealth-monitors/system-services-monitor/system_services_monitor/protos/health_event_pb2.pyihealth-monitors/system-services-monitor/system_services_monitor/protos/health_event_pb2_grpc.py
| self._callback_thread_pool = ThreadPoolExecutor() | ||
|
|
||
| # Initialize checkers and build the check list based on enabled flags | ||
| self._checkers: List[tuple[str, callable]] = [] |
Contributor
There was a problem hiding this comment.
Use Callable from typing instead of lowercase callable.
callable is a builtin function, not a type annotation. For type hints, import and use Callable from the typing module.
Proposed fix
Add to imports:
-from typing import List
+from typing import Callable, ListThen update the type hint:
- self._checkers: List[tuple[str, callable]] = []
+ self._checkers: List[tuple[str, Callable[[], List[CheckResult]]]] = []🤖 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
`@health-monitors/system-services-monitor/system_services_monitor/checkers/watcher.py`
at line 69, The attribute self._checkers is annotated with the builtin callable
instead of a typing hint; import Callable (and Tuple/Any if needed) from typing
and update the annotation to use typing types, e.g. change "self._checkers:
List[tuple[str, callable]] = []" to "self._checkers: List[Tuple[str,
Callable[..., Any]]] = []" (and add "from typing import List, Tuple, Callable,
Any" to the imports) to provide a proper type hint for the watcher._checkers
list.
Source: Coding guidelines
Comment on lines
+174
to
+184
| with grpc.insecure_channel(f"unix://{self._socket_path}") as chan: | ||
| stub = platformconnector_pb2_grpc.PlatformConnectorStub(chan) | ||
| try: | ||
| stub.HealthEventOccurredV1(platformconnector_pb2.HealthEvents(events=health_events, version=1)) | ||
| metrics.events_sent_success.inc() | ||
| return True | ||
| except grpc.RpcError as e: | ||
| log.error(f"Failed to send health event to UDS: {e}") | ||
| sleep(delay) | ||
| delay = min(delay * 1.5, MAX_DELAY) | ||
| continue |
Contributor
There was a problem hiding this comment.
Add timeout to gRPC call to prevent indefinite blocking.
The gRPC call on line 177 lacks a timeout parameter, which means it can block indefinitely if the platform-connector is slow or unresponsive. This can exhaust the ThreadPoolExecutor's thread pool (as multiple callbacks block waiting), preventing health event delivery and defeating the purpose of the monitoring system.
🔧 Proposed fix to add gRPC timeout
with grpc.insecure_channel(f"unix://{self._socket_path}") as chan:
stub = platformconnector_pb2_grpc.PlatformConnectorStub(chan)
try:
- stub.HealthEventOccurredV1(platformconnector_pb2.HealthEvents(events=health_events, version=1))
+ stub.HealthEventOccurredV1(
+ platformconnector_pb2.HealthEvents(events=health_events, version=1),
+ timeout=30.0
+ )
metrics.events_sent_success.inc()
return True
except grpc.RpcError as e:Choose a timeout value appropriate for your environment (e.g., 10-30 seconds). Consider that the retry loop already provides resilience, so individual call timeouts can be shorter than the total retry budget.
🤖 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
`@health-monitors/system-services-monitor/system_services_monitor/platform_connector/event_processor.py`
around lines 174 - 184, The HealthEventOccurredV1 gRPC call can block
indefinitely; update the call to include a reasonable per-attempt timeout (e.g.,
10–30s) by passing the timeout argument to stub.HealthEventOccurredV1(...) (the
call that currently builds platformconnector_pb2.HealthEvents and invokes
HealthEventOccurredV1). Keep the existing retry loop (sleep, delay, MAX_DELAY,
metrics.events_sent_success and the except grpc.RpcError as e handler) intact so
timeouts surface as RpcError and are retried; choose a timeout constant, use it
in the call, and ensure logging still reports the exception.
Comment on lines
+1
to
+6
| # -*- coding: utf-8 -*- | ||
| # Generated by the protocol buffer compiler. DO NOT EDIT! | ||
| # NO CHECKED-IN PROTOBUF GENCODE | ||
| # source: health_event.proto | ||
| # Protobuf Python Version: 6.31.1 | ||
| """Generated protocol buffer code.""" |
Contributor
There was a problem hiding this comment.
All generated protobuf/gRPC files lack Apache 2.0 license headers (shared root cause).
All three autogenerated protobuf and gRPC binding files (health_event_pb2.py, health_event_pb2.pyi, health_event_pb2_grpc.py) are missing the required Apache 2.0 license header. Since these files carry "DO NOT EDIT" directives, the fix requires configuring the protobuf and gRPC code generation tooling to inject the license header template during generation. This is typically done via protoc plugins, wrapper scripts, or build system configuration (Makefile, pyproject.toml, or generation scripts).
🤖 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
`@health-monitors/system-services-monitor/system_services_monitor/protos/health_event_pb2.py`
around lines 1 - 6, Generated protobuf/gRPC bindings (health_event_pb2.py,
health_event_pb2.pyi, health_event_pb2_grpc.py) are missing the required
Apache-2.0 license header; update the protobuf generation pipeline to inject the
license during generation by configuring protoc or your wrapper: add a
license-header plugin or a generation wrapper/script (or adjust
Makefile/pyproject.toml build step) that prepends the Apache 2.0 header template
to all generated files (health_event_pb2*, health_event_pb2_grpc*) so the files
keep the "DO NOT EDIT" note but include the correct license on every regen.
Source: Coding guidelines
Contributor
|
@dmvevents this PR has been inactive for 14 days. Do you need help finishing it, or should we close it for now? Feel free to reopen anytime. |
dmvevents
added a commit
to dmvevents/NVSentinel
that referenced
this pull request
Jul 24, 2026
Adds the design doc for system-services-monitor — a health-monitor for the host services behind fabric health (nvidia-fabricmanager, nvidia-persistenced, NVSwitch registration state) that sit in a layer neither gpu-health-monitor nor syslog-health-monitor observes today. Numbered 049 (next free slot on main; 030/042/043 taken). Detection mechanism, HealthEvent schema, checkName/errorCode taxonomy, and cached-state semantics are grounded in the system-services-monitor implementation (PR NVIDIA#1382). Signed-off-by: Anton Alexander <dmvevents@users.noreply.github.com>
Lands the CI wiring as a focused PR ahead of the implementation (NVIDIA#891 split, 2 of 5). Adds the system-services-monitor matrix entry in container-build-test.yml so the implementation PR's first CI run will be on the real matrix row rather than backfilled afterwards. Includes a minimal stub Makefile with no-op targets (lint-test, docker-build, docker-publish) so the matrix row passes; the real Makefile (Poetry, make/python.mk + make/docker.mk includes, etc.) lands together with the Python implementation in PR NVIDIA#3. The original umbrella branch also touches lint-test.yml and publish.yml, but those diffs only contain unrelated actions/checkout SHA reverts (no actual module registration), so they are not included in this PR. cleanup-untagged-images.yml is similarly unmodified on the umbrella branch. Signed-off-by: Anton Alexander <dmvevents@users.noreply.github.com>
Implements the system-services-monitor package per ADR-030 (NVIDIA#1380): - service_check.py for systemd service health - watcher.py + event_processor.py with thread-safe entity_cache - cli.py with --verbose flag and version handling - logger.py with explicit warning on unknown log levels - Dockerfile (python:3.13 base, apt cache mount per CR review) - Makefile with real lint-test / test / docker-build / docker-publish targets - Unit tests under tests/ covering service_check + event_processor Lands as NVIDIA#891 split (3 of 5) on top of NVIDIA#1380 (ADR) and the CI PR. Excludes cuda_validation.py — that checker is being moved to preflight-checks/cuda-validation/ in a follow-up PR per @XRFXLP review on the umbrella PR. The runtime GPU-allocation concern raised in his review is resolved by removing it from the daemon-poll path entirely. Signed-off-by: Anton Alexander <dmvevents@users.noreply.github.com>
- Makefile: set MODULE_NAME := $(PYTHON_PACKAGE_NAME). common.mk defaults MODULE_NAME to the hyphenated dir basename, so python.mk's `coverage run --source=$(MODULE_NAME)` never matched the importable package and reported 0% coverage. Matches preflight-checks/nccl-allreduce. - pyproject.toml: relax python floor ^3.13 -> ^3.10 to match siblings (nccl-allreduce, dcgm-diag). Package uses no 3.11+/3.13-only syntax or stdlib (verified: no tomllib, PEP 695 type params, StrEnum, datetime.UTC, ExceptionGroup, etc.). - Dockerfile: pin poetry==2.3.3 + poetry-plugin-export==1.10.0 per .versions.yaml (was poetry==1.8.2 with no export plugin while running `poetry export`). Export invocation already matches the poetry-2.x sibling Dockerfiles. - cli.py: rename `exit = Event()` -> `stop_event` to stop shadowing the builtin (CodeRabbit flagged this on the prior NVIDIA#891). - platform_connector/event_processor.py: guard the UDS dial with _is_platform_connector_socket_present() before/between retries, mirroring gpu-health-monitor; log-and-skip instead of noisy gRPC stack traces when the socket isn't present yet. Adds events_sent_skipped_pc_unavailable counter. Signed-off-by: Anton Alexander <dmvevents@users.noreply.github.com>
…ounter, tests Builds on the review-findings base commit; adds the fixes it did not cover. - ADR-030 -> ADR-049 sweep (4 refs): README.md scope-doc link, watcher.py module + class docstrings, cli.py comment. Upstream renamed the ADR to 049-system-services-monitor-scope.md (030 is now an unrelated gRPC-TLS ADR). - cli.py: add Click envvar= fallbacks so the Helm chart's configmap (which passes config via env only, no args) satisfies the required --platform-connector-socket and the other tunables. Fixes the container CrashLoop. Names match the chart configmap: PLATFORM_CONNECTOR_SOCKET, METRICS_PORT, CHECK_INTERVAL, BOOT_GRACE_PERIOD, FLAP_WINDOW, FLAP_THRESHOLD, ENABLE_FABRIC_CHECK. CLI args still take precedence over env. - metrics.py + watcher.py: add fabric_manager_restarts_total counter and increment it by the systemd NRestarts delta each poll cycle. Backs the chart's FabricManagerFlapping alert (increase(fabric_manager_restarts_total[10m]) > 3). - tests: add pytest coverage for watcher.py, fabric_state_check.py, event_processor.py (incl. the socket-presence skip path), and cli.py (env-only, arg-overrides-env, and startup-validation failure paths), following the existing test_service_check.py mocking style. Full suite: 44 passed. Signed-off-by: Anton Alexander <dmvevents@users.noreply.github.com>
dmvevents
force-pushed
the
impl-system-services-monitor
branch
from
July 24, 2026 17:28
b6be122 to
8bfc1b4
Compare
dmvevents
added a commit
to dmvevents/NVSentinel
that referenced
this pull request
Jul 24, 2026
Lands the Helm chart for system-services-monitor (NVIDIA#891 split, 4 of 5). Subchart under distros/kubernetes/nvsentinel/charts/ + values.yaml + Chart.yaml dependency registration in the parent chart. The chart is aligned to the actual runtime contract of the app on the implementation branch (cli.py / metrics.py), not an assumed one: DaemonSet - The app's entrypoint is a Click CLI whose --platform-connector-socket option is required=True with no env fallback. The container now passes it (plus --port/--poll-interval/--boot-grace-period/--flap-window/ --flap-threshold/--enable-fabric-check/--processing-strategy) via args:, and mounts the platform-connector Unix socket (hostPath /var/run/nvsentinel at /var/run, mirroring the slurm/nic/csp siblings). The app prepends unix:// itself, so the flag value is a bare path. - Dropped the envFrom configMapRef and the unused /var/run/dbus mount -- the app reads host systemd state via nsenter into PID 1, not dbus. - Keeps NODE_NAME (fieldRef) and LOG_LEVEL, the only env the app reads. Metrics / alerts - metricsPort now binds global.metricsPort (2112), matching siblings and the --port flag the app actually honors. - PrometheusRule alerts only on metrics the monitor exports: fabric_manager_up, fabric_state_healthy, nvidia_service_up, and fabric_manager_restarts_total (added in NVIDIA#1382). The flapping alert fires on increase(fabric_manager_restarts_total[10m]) > 3. Removed the CUDAValidationFailed alert -- cuda validation is an exit-code-only init container (NVIDIA#1384), no cuda_validation_passed metric. - Alert names follow the ADR-049 check taxonomy: FabricManagerServiceDown, FabricStateUnhealthy, GpuServiceDown. Config - Deleted the ConfigMap: its keys were either dead or are real CLI flags, now templated into args: from values.yaml. LOG_LEVEL is a plain env var. - ServiceMonitor + PrometheusRule default enabled: false (no health-monitor sibling ships them enabled) and ServiceMonitor's release label is now driven by .Values.serviceMonitor.labels (empty default) instead of a hardcoded release: prometheus. Mirrors the sibling pattern (nic-health-monitor) for .Values.global references; the parent chart supplies globals, so validate by rendering the parent chart (helm template distros/kubernetes/nvsentinel --set global.systemServicesMonitor.enabled=true), not standalone lint. Signed-off-by: Anton Alexander <dmvevents@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
health-monitors/system-services-monitor/system_services_monitor/platform_connector/event_processor.py (1)
204-214: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winMissing timeout on gRPC call — can block indefinitely (still unaddressed).
stub.HealthEventOccurredV1(...)has notimeout, so a slow/unresponsive platform-connector can hang this call forever, tying up aThreadPoolExecutorworker inwatcher.py's_fire_callback_funcsand eventually exhausting the pool. This was flagged in a prior review and remains unfixed.🔧 Proposed fix
try: - stub.HealthEventOccurredV1(platformconnector_pb2.HealthEvents(events=health_events, version=1)) + stub.HealthEventOccurredV1( + platformconnector_pb2.HealthEvents(events=health_events, version=1), + timeout=30.0, + )🤖 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 `@health-monitors/system-services-monitor/system_services_monitor/platform_connector/event_processor.py` around lines 204 - 214, Update the HealthEventOccurredV1 call in the event-sending retry loop to provide a finite timeout, using the existing timeout configuration or an appropriate bounded value. Preserve the current RpcError logging, backoff, retry, and success behavior when the call times out.
🧹 Nitpick comments (5)
health-monitors/system-services-monitor/system_services_monitor/platform_connector/tests/test_event_processor.py (1)
100-131: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded retry count instead of referencing the constant.
call_count == 5 # MAX_RETRIESduplicates the constant as a magic number; ifMAX_RETRIESinevent_processor.pychanges, this assertion silently drifts from the comment's intent.♻️ Suggested fix
-from system_services_monitor.platform_connector.event_processor import PlatformConnectorEventProcessor +from system_services_monitor.platform_connector.event_processor import MAX_RETRIES, PlatformConnectorEventProcessor ... - assert mock_stub.HealthEventOccurredV1.call_count == 5 # MAX_RETRIES + assert mock_stub.HealthEventOccurredV1.call_count == MAX_RETRIES🤖 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 `@health-monitors/system-services-monitor/system_services_monitor/platform_connector/tests/test_event_processor.py` around lines 100 - 131, Update test_rpc_error_exhausts_retries_and_returns_false to assert the RPC call count against the MAX_RETRIES constant imported from event_processor rather than the hardcoded value 5, keeping the existing retry-exhaustion behavior unchanged.health-monitors/system-services-monitor/system_services_monitor/checkers/tests/test_watcher.py (1)
30-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrivate test helper functions omit return type hints. As per coding guidelines,
**/*.pyfiles must "Include type hints for all functions in Python code"; both helpers below are missing return annotations — same root cause, add the missing hints.
health-monitors/system-services-monitor/system_services_monitor/checkers/tests/test_watcher.py#L30-L30: annotate_make_watcheras-> FabricManagerWatcher.health-monitors/system-services-monitor/system_services_monitor/tests/test_cli.py#L27-L27: annotate_mock_runtimeas-> Generator[tuple[MagicMock, MagicMock], None, None].🤖 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 `@health-monitors/system-services-monitor/system_services_monitor/checkers/tests/test_watcher.py` at line 30, Add the missing return type hints to both private test helpers: annotate _make_watcher in health-monitors/system-services-monitor/system_services_monitor/checkers/tests/test_watcher.py (line 30) as FabricManagerWatcher, and annotate _mock_runtime in health-monitors/system-services-monitor/system_services_monitor/tests/test_cli.py (line 27) as Generator[tuple[MagicMock, MagicMock], None, None].Source: Coding guidelines
health-monitors/system-services-monitor/system_services_monitor/checkers/service_check.py (1)
262-262: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant exception type in tuple.
except (subprocess.TimeoutExpired, Exception)—TimeoutExpiredis already a subclass ofException, so the tuple is redundant;except Exceptionalone is equivalent and clearer.🤖 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 `@health-monitors/system-services-monitor/system_services_monitor/checkers/service_check.py` at line 262, Update the exception handler in the service-checking flow to catch only Exception; remove the redundant subprocess.TimeoutExpired entry from the tuple while preserving the existing handler behavior and alias.health-monitors/system-services-monitor/Makefile (1)
62-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
poetry shellrequires a plugin on Poetry 2.x.The Dockerfile pins
poetry==2.3.3. Poetry removed theshellcommand from core in 2.0; it now requires installingpoetry-plugin-shell, otherwisepoetry shellerrors with "command does not exist." If contributors are expected to use the same Poetry major version, this target will fail as-is.♻️ Proposed fix
shell: `@echo` "Opening Poetry shell for $(MODULE_NAME)..." - poetry shell + poetry env activate || (poetry self add poetry-plugin-shell && poetry shell)🤖 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 `@health-monitors/system-services-monitor/Makefile` around lines 62 - 64, Update the Makefile target named shell to work with the pinned Poetry 2.x setup by replacing the unsupported poetry shell invocation with the supported environment-entry command, or ensure the required shell plugin is installed before invoking it. Keep the existing user-facing message and MODULE_NAME behavior unchanged.health-monitors/system-services-monitor/system_services_monitor/protos/health_event_pb2_grpc.py (1)
20-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRuff F541 on generated code — exclude generated files from lint instead of editing them.
This file is marked "DO NOT EDIT!" (grpc_tools codegen output); the
f-string-without-placeholder on line 22 is a byproduct of the generated template, and hand-fixing it will be reverted on the nextprotoc/grpc_toolsregeneration. Consider excluding*_pb2.py/*_pb2_grpc.pyfrom Ruff (extend-excludeinpyproject.toml) instead.🤖 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 `@health-monitors/system-services-monitor/system_services_monitor/protos/health_event_pb2_grpc.py` around lines 20 - 26, Exclude generated protobuf files matching *_pb2.py and *_pb2_grpc.py from Ruff via the project’s Ruff configuration, such as extend-exclude in pyproject.toml. Do not modify the generated RuntimeError code in health_event_pb2_grpc.py, preserving it for future protoc/grpc_tools regeneration.Source: Linters/SAST tools
🤖 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
`@health-monitors/system-services-monitor/system_services_monitor/checkers/service_check.py`:
- Around line 174-188: Update _get_restart_count to log caught exceptions at
debug level before returning the existing fallback value of 0. Preserve the
current behavior for unsupported NRestarts queries while retaining the exception
details for troubleshooting.
In
`@health-monitors/system-services-monitor/system_services_monitor/tests/test_cli.py`:
- Around line 88-98: Remove the unused watcher binding from the _mock_runtime()
tuple unpacking in test_cli_arg_overrides_env, while preserving the processor
binding and all existing assertions.
---
Duplicate comments:
In
`@health-monitors/system-services-monitor/system_services_monitor/platform_connector/event_processor.py`:
- Around line 204-214: Update the HealthEventOccurredV1 call in the
event-sending retry loop to provide a finite timeout, using the existing timeout
configuration or an appropriate bounded value. Preserve the current RpcError
logging, backoff, retry, and success behavior when the call times out.
---
Nitpick comments:
In `@health-monitors/system-services-monitor/Makefile`:
- Around line 62-64: Update the Makefile target named shell to work with the
pinned Poetry 2.x setup by replacing the unsupported poetry shell invocation
with the supported environment-entry command, or ensure the required shell
plugin is installed before invoking it. Keep the existing user-facing message
and MODULE_NAME behavior unchanged.
In
`@health-monitors/system-services-monitor/system_services_monitor/checkers/service_check.py`:
- Line 262: Update the exception handler in the service-checking flow to catch
only Exception; remove the redundant subprocess.TimeoutExpired entry from the
tuple while preserving the existing handler behavior and alias.
In
`@health-monitors/system-services-monitor/system_services_monitor/checkers/tests/test_watcher.py`:
- Line 30: Add the missing return type hints to both private test helpers:
annotate _make_watcher in
health-monitors/system-services-monitor/system_services_monitor/checkers/tests/test_watcher.py
(line 30) as FabricManagerWatcher, and annotate _mock_runtime in
health-monitors/system-services-monitor/system_services_monitor/tests/test_cli.py
(line 27) as Generator[tuple[MagicMock, MagicMock], None, None].
In
`@health-monitors/system-services-monitor/system_services_monitor/platform_connector/tests/test_event_processor.py`:
- Around line 100-131: Update test_rpc_error_exhausts_retries_and_returns_false
to assert the RPC call count against the MAX_RETRIES constant imported from
event_processor rather than the hardcoded value 5, keeping the existing
retry-exhaustion behavior unchanged.
In
`@health-monitors/system-services-monitor/system_services_monitor/protos/health_event_pb2_grpc.py`:
- Around line 20-26: Exclude generated protobuf files matching *_pb2.py and
*_pb2_grpc.py from Ruff via the project’s Ruff configuration, such as
extend-exclude in pyproject.toml. Do not modify the generated RuntimeError code
in health_event_pb2_grpc.py, preserving it for future protoc/grpc_tools
regeneration.
🪄 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: CHILL
Plan: Enterprise
Run ID: 051c7b06-0802-483c-9e4b-f7d4b275d3b6
📒 Files selected for processing (29)
.github/workflows/container-build-test.ymlhealth-monitors/system-services-monitor/Dockerfilehealth-monitors/system-services-monitor/Makefilehealth-monitors/system-services-monitor/README.mdhealth-monitors/system-services-monitor/pyproject.tomlhealth-monitors/system-services-monitor/system_services_monitor/__init__.pyhealth-monitors/system-services-monitor/system_services_monitor/checkers/__init__.pyhealth-monitors/system-services-monitor/system_services_monitor/checkers/fabric_state_check.pyhealth-monitors/system-services-monitor/system_services_monitor/checkers/service_check.pyhealth-monitors/system-services-monitor/system_services_monitor/checkers/tests/__init__.pyhealth-monitors/system-services-monitor/system_services_monitor/checkers/tests/test_fabric_state_check.pyhealth-monitors/system-services-monitor/system_services_monitor/checkers/tests/test_service_check.pyhealth-monitors/system-services-monitor/system_services_monitor/checkers/tests/test_watcher.pyhealth-monitors/system-services-monitor/system_services_monitor/checkers/types.pyhealth-monitors/system-services-monitor/system_services_monitor/checkers/watcher.pyhealth-monitors/system-services-monitor/system_services_monitor/cli.pyhealth-monitors/system-services-monitor/system_services_monitor/logger.pyhealth-monitors/system-services-monitor/system_services_monitor/metrics.pyhealth-monitors/system-services-monitor/system_services_monitor/platform_connector/__init__.pyhealth-monitors/system-services-monitor/system_services_monitor/platform_connector/event_processor.pyhealth-monitors/system-services-monitor/system_services_monitor/platform_connector/metrics.pyhealth-monitors/system-services-monitor/system_services_monitor/platform_connector/tests/__init__.pyhealth-monitors/system-services-monitor/system_services_monitor/platform_connector/tests/test_event_processor.pyhealth-monitors/system-services-monitor/system_services_monitor/protos/__init__.pyhealth-monitors/system-services-monitor/system_services_monitor/protos/health_event_pb2.pyhealth-monitors/system-services-monitor/system_services_monitor/protos/health_event_pb2.pyihealth-monitors/system-services-monitor/system_services_monitor/protos/health_event_pb2_grpc.pyhealth-monitors/system-services-monitor/system_services_monitor/tests/__init__.pyhealth-monitors/system-services-monitor/system_services_monitor/tests/test_cli.py
🚧 Files skipped from review as they are similar to previous changes (16)
- health-monitors/system-services-monitor/system_services_monitor/checkers/tests/init.py
- health-monitors/system-services-monitor/system_services_monitor/checkers/init.py
- health-monitors/system-services-monitor/system_services_monitor/protos/init.py
- health-monitors/system-services-monitor/system_services_monitor/checkers/types.py
- health-monitors/system-services-monitor/system_services_monitor/platform_connector/init.py
- health-monitors/system-services-monitor/system_services_monitor/platform_connector/metrics.py
- health-monitors/system-services-monitor/README.md
- health-monitors/system-services-monitor/system_services_monitor/logger.py
- health-monitors/system-services-monitor/pyproject.toml
- .github/workflows/container-build-test.yml
- health-monitors/system-services-monitor/system_services_monitor/protos/health_event_pb2.pyi
- health-monitors/system-services-monitor/system_services_monitor/init.py
- health-monitors/system-services-monitor/system_services_monitor/checkers/tests/test_service_check.py
- health-monitors/system-services-monitor/system_services_monitor/protos/health_event_pb2.py
- health-monitors/system-services-monitor/system_services_monitor/metrics.py
- health-monitors/system-services-monitor/system_services_monitor/cli.py
Comment on lines
+174
to
+188
| def _get_restart_count(self, service_name: str) -> int: | ||
| """Get NRestarts from systemd, returning 0 if unsupported.""" | ||
| try: | ||
| result = self._run_host_cmd([ | ||
| "systemctl", | ||
| "show", | ||
| service_name, | ||
| "--property=NRestarts", | ||
| ]) | ||
| if result.returncode == 0 and result.stdout.strip(): | ||
| _, _, val = result.stdout.strip().partition("=") | ||
| return int(val) | ||
| except Exception: | ||
| pass | ||
| return 0 |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Silently swallowed exception loses the failure reason.
except Exception: pass discards why the NRestarts query failed, so "systemd doesn't support this property" (expected/benign) and "nsenter/systemctl actually broke" look identical — both just return 0. A debug-level log preserves the intended silent fallback while keeping the reason visible for troubleshooting.
🔧 Proposed fix
- except Exception:
- pass
+ except Exception as e:
+ log.debug(f"NRestarts query failed for {service_name} (likely unsupported): {e}")
return 0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def _get_restart_count(self, service_name: str) -> int: | |
| """Get NRestarts from systemd, returning 0 if unsupported.""" | |
| try: | |
| result = self._run_host_cmd([ | |
| "systemctl", | |
| "show", | |
| service_name, | |
| "--property=NRestarts", | |
| ]) | |
| if result.returncode == 0 and result.stdout.strip(): | |
| _, _, val = result.stdout.strip().partition("=") | |
| return int(val) | |
| except Exception: | |
| pass | |
| return 0 | |
| def _get_restart_count(self, service_name: str) -> int: | |
| """Get NRestarts from systemd, returning 0 if unsupported.""" | |
| try: | |
| result = self._run_host_cmd([ | |
| "systemctl", | |
| "show", | |
| service_name, | |
| "--property=NRestarts", | |
| ]) | |
| if result.returncode == 0 and result.stdout.strip(): | |
| _, _, val = result.stdout.strip().partition("=") | |
| return int(val) | |
| except Exception as e: | |
| log.debug( | |
| f"NRestarts query failed for {service_name} (likely unsupported): {e}" | |
| ) | |
| return 0 |
🧰 Tools
🪛 Ruff (0.15.21)
[error] 186-187: try-except-pass detected, consider logging the exception
(S110)
[warning] 186-186: Do not catch blind exception: Exception
(BLE001)
🤖 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
`@health-monitors/system-services-monitor/system_services_monitor/checkers/service_check.py`
around lines 174 - 188, Update _get_restart_count to log caught exceptions at
debug level before returning the existing fallback value of 0. Preserve the
current behavior for unsupported NRestarts queries while retaining the exception
details for troubleshooting.
Source: Linters/SAST tools
Comment on lines
+88
to
+98
| def test_cli_arg_overrides_env(self, runner: CliRunner) -> None: | ||
| """An explicit CLI flag wins over the corresponding env var.""" | ||
| with _mock_runtime() as (watcher, processor): | ||
| result = runner.invoke( | ||
| climod.cli, | ||
| ["--platform-connector-socket", "/run/from-cli.sock", "--node-name", "node-a"], | ||
| env={"PLATFORM_CONNECTOR_SOCKET": "/run/from-env.sock"}, | ||
| ) | ||
|
|
||
| assert result.exit_code == 0 | ||
| assert processor.call_args.kwargs["socket_path"] == "/run/from-cli.sock" |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Unused watcher binding.
watcher from _mock_runtime() is never referenced in this test; only processor is used. Matches the Ruff RUF059 hint.
🔧 Suggested fix
- with _mock_runtime() as (watcher, processor):
+ with _mock_runtime() as (_watcher, processor):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def test_cli_arg_overrides_env(self, runner: CliRunner) -> None: | |
| """An explicit CLI flag wins over the corresponding env var.""" | |
| with _mock_runtime() as (watcher, processor): | |
| result = runner.invoke( | |
| climod.cli, | |
| ["--platform-connector-socket", "/run/from-cli.sock", "--node-name", "node-a"], | |
| env={"PLATFORM_CONNECTOR_SOCKET": "/run/from-env.sock"}, | |
| ) | |
| assert result.exit_code == 0 | |
| assert processor.call_args.kwargs["socket_path"] == "/run/from-cli.sock" | |
| def test_cli_arg_overrides_env(self, runner: CliRunner) -> None: | |
| """An explicit CLI flag wins over the corresponding env var.""" | |
| with _mock_runtime() as (_watcher, processor): | |
| result = runner.invoke( | |
| climod.cli, | |
| ["--platform-connector-socket", "/run/from-cli.sock", "--node-name", "node-a"], | |
| env={"PLATFORM_CONNECTOR_SOCKET": "/run/from-env.sock"}, | |
| ) | |
| assert result.exit_code == 0 | |
| assert processor.call_args.kwargs["socket_path"] == "/run/from-cli.sock" |
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 90-90: Unpacked variable watcher is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
🤖 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
`@health-monitors/system-services-monitor/system_services_monitor/tests/test_cli.py`
around lines 88 - 98, Remove the unused watcher binding from the _mock_runtime()
tuple unpacking in test_cli_arg_overrides_env, while preserving the processor
binding and all existing assertions.
Source: Linters/SAST tools
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
1 participant
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.
Per #891 split (3 of 5). Core Python package + unit tests.
Series progress
ci-system-services-monitor)What this contains
health-monitors/system-services-monitor/Python package (nocuda_validation.py)system_services_monitor/checkers/tests/What this does NOT contain
preflight-checks/cuda-validation/in follow-up PR per @XRFXLP reviewCR-ignored findings carried over from #891
This PR addresses these findings in scope:
cli.py--verbose flag handling (is_flag=True, default=False)cli.pytry/except aroundget_package_version(PackageNotFoundError fallback)Dockerfileapt cache mount (--mount=type=cache,target=/var/cache/apt,sharing=locked)logger.pywarning on unknown log levelsthreading.Lockinevent_processor.pyCR-ignored findings deferred to PR 5 (demo)
demos/.../servicemonitor.yamlalert specdemos/.../daemonset.yamlimage pin + livenessProbecc @XRFXLP
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores