feat(agent): sidecar manager attach mode for systemd-managed sidecars (#3425) #3482

Merged

mfreeman451 merged 2 commits from feat/netprobe-assignment-gated-lifecycle into staging

2026-06-01 02:10:37 +00:00

mfreeman451 commented

2026-06-01 02:04:50 +00:00

Owner

Summary

migrate-netprobe-to-native-addon §2.2 — building block. Teaches the agent sidecar supervisor to attach to an externally-(systemd-)managed sidecar socket without launching the process. This is the mechanism netprobe's systemd-service lifecycle connects through (config push + event ingest), and it lands first — additive, fully tested, zero production behavior change (no caller yet) — to de-risk the cutover, in the same spirit as #3481 shipping the inert unit.

Why attach mode

Under systemd-service, systemd owns the netprobe process (it binds the IPC socket). The agent must still get a connected client to push VisibilityConfig over IPC and ingest fingerprint/DPI/flow events. Ingest is already launch-decoupled (DrainFlowAttributionEvents drains a channel fed by any connected client), so the only thing the agent needs is a health/connect loop that wires the client in via OnHealthy — which is exactly what the supervisor's health loop already does. Attach mode reuses it, minus the exec/restart.

What changed (`go/pkg/agent/sidecar/manager.go`)

StartAttach(ctx) vs launch Start(ctx); both delegate to start(ctx, attach bool).
In attach mode supervise() → superviseAttached(): runs the existing healthLoop against the well-known socket — dial, health-check, OnHealthy (wires the client → config push + ingest), reconnect on loss — with no exec, no process wait, no restart/backoff/circuit-breaker. PID stays 0; status is driven to Running/Unhealthy/Stopped by connectivity.
Mode() (started, attach bool) so the forthcoming push-loop cutover can decide when a launch↔attach switch is needed.
attach is passed to supervise() as a parameter (not read from the field), so no new unsynchronized shared state; Mode() reads the mode field under the existing RLock.

Tests (`manager_attach_test.go`, registered in the Bazel `go_test` srcs)

connect-without-launch: the sidecar's binary path doesn't exist, yet it reaches Running with PID 0 and fires OnHealthy with a non-nil client — impossible on the launch path, so it proves no exec.
genuine reconnect: connect → Running → Ping starts failing (stale client Close()'d) → Unhealthy → Ping recovers → health loop re-dials → Running, with RestartCount==0 and PID==0 (no process restart).
Mode() across StartAttach/Stop/Start.

Validation

go test -race ./go/pkg/agent/sidecar/ green (full package, incl. existing launch tests — the supervise signature change broke nothing)
go vet + gofmt clean; agent packages build
An adversarial multi-agent review of the diff found the concurrency/lifecycle correct (no race/deadlock/leak) and surfaced two fixes I applied: registering the test file in the Bazel go_test srcs (this repo enumerates test srcs, no globbing), and making the reconnect test actually exercise the drop/recover path.

Next (the cutover, §2.2 remainder)

push_loop_config.go: detect the netprobe AddonAssignment (enabled systemd_service) → StartAttach + push config; absent → the existing launch path (unchanged). Plus apply-on-connect for the netprobe sidecar — required because applyVisibilityConfig runs before applyAddonAssignments installs the unit (a synchronous attach ApplyConfig would return false and abort the apply at push_loop_config.go:203), and the NotModified short-circuit means poll-driven re-push won't recover after a systemd restart.

🤖 Generated with Claude Code

## Summary `migrate-netprobe-to-native-addon` §2.2 — **building block**. Teaches the agent sidecar supervisor to **attach** to an externally-(systemd-)managed sidecar socket *without launching the process*. This is the mechanism netprobe's `systemd-service` lifecycle connects through (config push + event ingest), and it lands first — additive, fully tested, **zero production behavior change** (no caller yet) — to de-risk the cutover, in the same spirit as #3481 shipping the inert unit. ## Why attach mode Under `systemd-service`, systemd owns the netprobe process (it binds the IPC socket). The agent must still get a **connected client** to push `VisibilityConfig` over IPC and ingest fingerprint/DPI/flow events. Ingest is already **launch-decoupled** (`DrainFlowAttributionEvents` drains a channel fed by any connected client), so the only thing the agent needs is a health/connect loop that wires the client in via `OnHealthy` — which is exactly what the supervisor's health loop already does. Attach mode reuses it, minus the `exec`/restart. ## What changed (`go/pkg/agent/sidecar/manager.go`) - **`StartAttach(ctx)`** vs launch **`Start(ctx)`**; both delegate to `start(ctx, attach bool)`. - In attach mode `supervise()` → `superviseAttached()`: runs the existing `healthLoop` against the well-known socket — dial, health-check, `OnHealthy` (wires the client → config push + ingest), reconnect on loss — with **no `exec`, no process wait, no restart/backoff/circuit-breaker**. `PID` stays 0; status is driven to `Running`/`Unhealthy`/`Stopped` by connectivity. - **`Mode() (started, attach bool)`** so the forthcoming push-loop cutover can decide when a launch↔attach switch is needed. - `attach` is passed to `supervise()` as a **parameter** (not read from the field), so no new unsynchronized shared state; `Mode()` reads the mode field under the existing `RLock`. ## Tests (`manager_attach_test.go`, registered in the Bazel `go_test` srcs) - **connect-without-launch**: the sidecar's binary path doesn't exist, yet it reaches `Running` with `PID 0` and fires `OnHealthy` with a non-nil client — impossible on the launch path, so it proves no `exec`. - **genuine reconnect**: connect → `Running` → `Ping` starts failing (stale client `Close()`'d) → `Unhealthy` → `Ping` recovers → health loop **re-dials** → `Running`, with `RestartCount==0` and `PID==0` (no process restart). - **`Mode()`** across `StartAttach`/`Stop`/`Start`. ## Validation - `go test -race ./go/pkg/agent/sidecar/` green (full package, incl. existing launch tests — the `supervise` signature change broke nothing) - `go vet` + `gofmt` clean; agent packages build - An adversarial multi-agent review of the diff found the concurrency/lifecycle correct (no race/deadlock/leak) and surfaced two fixes I applied: registering the test file in the Bazel `go_test` srcs (this repo enumerates test srcs, no globbing), and making the reconnect test actually exercise the drop/recover path. ## Next (the cutover, §2.2 remainder) `push_loop_config.go`: detect the netprobe `AddonAssignment` (enabled `systemd_service`) → `StartAttach` + push config; absent → the existing launch path (unchanged). Plus **apply-on-connect** for the netprobe sidecar — required because `applyVisibilityConfig` runs before `applyAddonAssignments` installs the unit (a synchronous attach `ApplyConfig` would `return false` and abort the apply at `push_loop_config.go:203`), and the `NotModified` short-circuit means poll-driven re-push won't recover after a systemd restart. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Rows
Columns