Hosting Integration and Health Model - #230
Conversation
Complete P1.1 by introducing a typed, secret-free watch-backed health snapshot and publisher. This gives later gRPC, systemd, datastore, listener, and upstream integrations one maintainable source of startup, liveness, readiness, and degradation semantics.
Complete P1.2 by declaring datastore load and notification policies, retrying SONiC startup with cancellable jittered backoff, refreshing and resubscribing continuous sources, retaining known-good configuration after rejected candidates, and running one preferred recovery probe across zero-server reloads.
Complete P1.3 by centralizing SIGTERM and Ctrl-C handling, broadcasting shutdown only after health withdrawal, publishing listener binding/bound/stopped transitions with guards, awaiting sibling cancellation on listener failure, preserving request draining, and cleaning Unix sockets before return.
Complete P1.4 by sequencing aggregate connection observations per server-set generation, reporting available after any successful warm-up or request bind, reporting unavailable only after full candidate exhaustion, ignoring preferred-probe failures, and resetting to unknown after material reloads without changing readiness.
Complete P1.5 by adding tonic-health 0.14.6 beside the existing Client API, mapping stable startup, liveness, readiness, overall, and business-service names from the common runtime snapshot, supporting Check and Watch, and withdrawing every status during coordinated shutdown without changing the application proto.
Complete P1.6 by sharing UDS and TCP channel construction, adding a typed standard health client, packaging tacacsrs-agent-health, defining stable serving/invocation/check exit codes, enforcing total timeouts, redacting diagnostics, and testing the built probe over TCP and Unix sockets.
Complete P1.7 by adding auto, none, and strict systemd modes, consuming common health snapshots, sending READY once and STOPPING during drain, publishing sanitized degraded status, injecting command execution for tests, preserving auto warn-and-continue behavior, and making strict notification failures fatal.
Complete P1.8 by proving health Watch closure, readiness withdrawal before active-request drain, retry-after-refresh-failure ordering, and redaction of snapshots, public errors, systemd arguments, and probe diagnostics. The existing matrix also covers listener races, failover, host modes, probes, and dynamic reload behavior.
Complete P1.9 with a reusable WSL/Linux process test that starts before Redis, recovers readiness after ConfigDB appears, validates UDS and proxy listeners, replaces stale sockets, drains an active proxy connection on SIGTERM, releases endpoints, and exercises none, auto, and strict systemd modes.
Complete P1.10 by documenting standard health names and probe exits, none/auto/systemd behavior, Redis startup and resubscription supervision, coordinated drain, container guidance, and the independent downstream proxy secret model. The SONiC unit now permits supervised database recovery and requests strict systemd integration.
There was a problem hiding this comment.
Pull request overview
This pull request adds a protocol-neutral runtime health model to tacacsrs-agentd and associated crates, then uses it to drive standard gRPC health checks, supervised SONiC ConfigDB loading/reloading, and optional host integration (systemd notifications) suitable for both systemd units and container deployments.
Changes:
- Introduces typed runtime health publication (watch-backed snapshots), listener lifecycle registration, and coordinated shutdown/drain semantics in
tacacsrs-agent. - Updates datastore contracts to include declared runtime policy (initial-load + notification mode) and to emit typed change events (changed vs candidate rejected), enabling resilient ConfigDB/file reload supervision.
- Adds a standalone
tacacsrs-agent-healthexecutable plus deployment/docs updates for exec probes, systemdType=notify, and container-friendly operation.
Reviewed changes
Copilot reviewed 49 out of 50 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| libraries/tacacsrs_sonic/src/mapping.rs | Uses shared TacacsPlus::empty() helper for empty SONiC table mapping. |
| libraries/tacacsrs_sonic/src/lib.rs | Implements new datastore runtime policy + typed change stream for SONiC ConfigDB. |
| libraries/tacacsrs_sonic/examples/configdb_watch.rs | Updates example to handle ConfigChangeEvent variants. |
| libraries/tacacsrs_datastore/src/lib.rs | Introduces ConfigChangeEvent + datastore runtime policy contract and updates helpers/tests. |
| libraries/tacacsrs_config/src/builders.rs | Adds TacacsPlus::empty() and reuses it in the builder. |
| libraries/tacacsrs_config/Cargo.toml | Bumps base64 dependency. |
| libraries/tacacsrs_cli_datastore/src/datastore.rs | Implements runtime policy and emits typed change events (including candidate rejection). |
| libraries/tacacsrs_cli_datastore/Cargo.toml | Bumps base64 dependency. |
| libraries/tacacsrs_agent/src/upstream/manager/mod.rs | Wires upstream availability tracking into runtime health and resets on material reloads. |
| libraries/tacacsrs_agent/src/upstream/manager/availability.rs | Adds race-safe aggregate upstream availability publisher + tests. |
| libraries/tacacsrs_agent/src/services/tacacs_proxy/service.rs | Passes shutdown + listener registration into proxy listener serving. |
| libraries/tacacsrs_agent/src/services/tacacs_proxy/listener/unix.rs | Marks listener bound in health and respects shared shutdown receiver. |
| libraries/tacacsrs_agent/src/services/tacacs_proxy/listener/tcp.rs | Marks listener bound in health and respects shared shutdown receiver. |
| libraries/tacacsrs_agent/src/services/tacacs_proxy/listener/mod.rs | Refactors accept loop to use ShutdownReceiver and listener registration. |
| libraries/tacacsrs_agent/src/services/client_api/upstream_bridge/mod.rs | Updates tests to construct upstream manager with health publisher. |
| libraries/tacacsrs_agent/src/services/client_api/service.rs | Adds shutdown + health integration and provides health snapshot receiver to listener. |
| libraries/tacacsrs_agent/src/services/client_api/mod.rs | Introduces client_api::health module. |
| libraries/tacacsrs_agent/src/services/client_api/listener/unix.rs | Serves standard gRPC health service and coordinates shutdown/drain. |
| libraries/tacacsrs_agent/src/services/client_api/listener/tcp.rs | Serves standard gRPC health service and coordinates shutdown/drain. |
| libraries/tacacsrs_agent/src/services/client_api/listener/mod.rs | Threads shutdown/registration/health receiver through listener selection. |
| libraries/tacacsrs_agent/src/services/client_api/health.rs | Implements standard grpc.health.v1.Health mapping from runtime snapshots + tests. |
| libraries/tacacsrs_agent/src/runtime/shutdown.rs | Adds shared shutdown coordination + listener registration guard + tests. |
| libraries/tacacsrs_agent/src/runtime/mod.rs | Re-exports new runtime health/shutdown types. |
| libraries/tacacsrs_agent/src/runtime/health.rs | Adds typed health model, publisher, readiness/liveness/startup semantics + tests. |
| libraries/tacacsrs_agent/src/runtime/client_service.rs | Integrates health + shutdown coordinator into serving lifecycle and reload behavior. |
| libraries/tacacsrs_agent/src/lib.rs | Re-exports runtime health types publicly. |
| libraries/tacacsrs_agent/README.md | Updates runtime lifecycle/shutdown and health model documentation. |
| libraries/tacacsrs_agent/Cargo.toml | Adds tonic-health and test-util tokio dev-dependency. |
| libraries/tacacsrs_agent_ipc_emulator/Cargo.toml | Updates regorus dependency version. |
| libraries/tacacsrs_agent_client/src/lib.rs | Exposes new health module and re-exports HealthClient. |
| libraries/tacacsrs_agent_client/src/health.rs | Adds stable health service names and a standard gRPC health client. |
| libraries/tacacsrs_agent_client/src/endpoint.rs | Extracts reusable connect_channel() for both service + health clients. |
| libraries/tacacsrs_agent_client/src/client.rs | Refactors ServiceClient to reuse shared channel connection helper. |
| libraries/tacacsrs_agent_client/Cargo.toml | Adds tonic-health dependency. |
| lde/run-p1-health-process-smoke.sh | Adds a process-level smoke script covering UDS replacement, pre-Redis startup, shutdown cleanup, and health recovery. |
| executables/tacacsrs_agentd/tests/health_probe.rs | Adds integration tests for probe exit codes and health protocol behavior. |
| executables/tacacsrs_agentd/src/systemd_notify.rs | Removes legacy systemd notification shim (replaced by host integration). |
| executables/tacacsrs_agentd/src/main.rs | Introduces config supervision + host integration and starts service in “waiting for config” mode. |
| executables/tacacsrs_agentd/src/host_integration.rs | Adds --host-integration handling and systemd notification bridge from health snapshots. |
| executables/tacacsrs_agentd/src/config_supervisor.rs | Adds cancellable load/retry/subscribe supervision with backoff and health state updates. |
| executables/tacacsrs_agentd/src/cli.rs | Adds --host-integration flag and HostIntegrationMode enum. |
| executables/tacacsrs_agentd/src/bin/tacacsrs-agent-health.rs | Adds tacacsrs-agent-health exec probe binary with stable exit codes. |
| executables/tacacsrs_agentd/sonic/tacacsrs-agentd.service | Updates SONiC unit dependencies and enables strict systemd host integration. |
| executables/tacacsrs_agentd/README.md | Documents health endpoint/probe usage and host integration modes. |
| executables/tacacsrs_agentd/Cargo.toml | Adds deps for supervision/health and packages the new probe executable. |
| docs/tacacsrs-agentd.md | Documents health model, probe contract, supervised ConfigDB behavior, and systemd notify settings. |
| docs/tacacs-plus-tls-transition.md | Updates downstream shared-secret guidance for proxy mode under new behavior. |
| docs/sonic-configdb-integration.md | Updates SONiC hot-reload behavior to describe atomic in-process reloads and retry semantics. |
| docs/sonic-agentd-container.md | Updates container deployment guidance (mounts, host integration none, and exec probe usage). |
| Cargo.lock | Updates lockfile for new/bumped dependencies (including tonic-health, base64, etc.). |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 49 out of 50 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
executables/tacacsrs_agentd/src/host_integration.rs:153
- When systemd-notify fails in non-strict (auto) mode, the warning log drops the underlying error, which makes diagnosing why notifications aren’t being published unnecessarily hard (e.g., missing helper vs. exit status vs. execution failure). Include the error details in the log message (it should not contain secrets).
if let Err(error) = self.command.execute(&arguments) {
if self.strict {
return Err(error);
}
log::warn!("Failed to publish automatic systemd notification");
}
The tacacsrs-agentd package has a second binary (tacacsrs-agent-health) defined in src/bin/ and referenced in the Debian package assets. The build-binary action was using --bin which only built the primary artifact, causing cargo-deb to fail with 'Static file asset has not been built'. - build-binary: use --bins to build all binaries in the package - prepare-debian-package-assets: copy additional src/bin/*.rs binaries from the target dir to target/release/ so cargo-deb can find them
Head branch was pushed to by a user without write access
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 51 out of 52 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
executables/tacacsrs_agentd/src/host_integration.rs:120
- The
tokio::select!is markedbiased;with thehealth.changed()branch first. If health updates arrive continuously, this can starve the cancellation branch and delay (or potentially prevent) host integration shutdown, especially during noisy health transitions.
tokio::select! {
biased;
result = health.changed() => {
if result.is_err() {
return Ok(());
executables/tacacsrs_agentd/src/host_integration.rs:152
- In auto mode, notification failures are intentionally non-fatal, but the warning currently drops the underlying error. Including the error context here will make diagnosing missing
systemd-notifyor permission issues much easier.
if let Err(error) = self.command.execute(&arguments) {
if self.strict {
return Err(error);
}
log::warn!("Failed to publish automatic systemd notification");
This pull request introduces several important improvements to the
tacacsrs-agentdproject, focusing on enhanced health checking, improved host integration, and more robust SONiC ConfigDB handling. The changes include a new health probe executable, expanded documentation, and updates to deployment/service files to support systemd notification and containerized environments.Health checking and host integration:
tacacsrs-agent-healthexecutable for checking the agent's gRPC health endpoints, with clear exit codes for use in Kubernetes exec probes and diagnostics (executables/tacacsrs_agentd/src/bin/tacacsrs-agent-health.rs,Cargo.toml,README.md). [1] [2] [3]--host-integrationflag to control systemd notification, with modes forauto,systemd, andnone(container-friendly). Documentation and service files updated accordingly (docs/tacacsrs-agentd.md,sonic-agentd-container.md,sonic/tacacsrs-agentd.service). [1] [2] [3]SONiC and ConfigDB improvements:
docs/sonic-agentd-container.md,sonic-configdb-integration.md,tacacsrs-agentd.md). [1] [2] [3]Shared secret and proxy mode behavior:
--proxy-shared-secret(tacacsrs-agentd.md,tacacs-plus-tls-transition.md). [1] [2]Packaging and deployment:
Type=notifyand enable systemd notification integration (Cargo.toml,tacacsrs-agentd.service). [1] [2] [3]Dependency updates:
Cargo.toml). [1] [2]These changes improve observability, reliability, and operational flexibility for both containerized and systemd-based deployments.