Skip to content

fix(chart): widen probe tolerance + CPU headroom for the engine's sweep bursts (JEF-560) - #292

Merged
thejefflarson merged 2 commits into
mainfrom
thejefflarson/jef-560-protector-engine-container-never-reaches-ready-exits
Jul 28, 2026
Merged

fix(chart): widen probe tolerance + CPU headroom for the engine's sweep bursts (JEF-560)#292
thejefflarson merged 2 commits into
mainfrom
thejefflarson/jef-560-protector-engine-container-never-reaches-ready-exits

Conversation

@thejefflarson

Copy link
Copy Markdown
Owner

Closes JEF-560.

Diagnosis

Investigated whether the engine's driver run-to-completions instead of looping
(the ticket's leading hypothesis) and whether the readiness endpoint is
missing in this mode. Both are disproven by the code, now pinned by a
regression test:

  • engine/src/engine/run_loop.rs's run_watch is genuinely event-driven and
    long-lived: its loop { tokio::select! { … } } only breaks when the
    change mpsc channel closes — which only happens once every clone of
    its Sender (each of the 7 reflector-watch tasks' clones, plus the loop's
    own retained handle) has been dropped. That never happens after a single
    pass; the 7 kube_runtime::watcher tasks retry indefinitely rather than
    terminating. Extracted the wake/coalesce/break-on-close primitive into a
    small wait_for_wake helper so this is now independently testable without
    a real kube::Client.
  • engine/src/server.rs serves /healthz and /readyz unconditionally from
    the moment TLS binds, for the whole process lifetime — not gated on any
    engine state.

Since main()'s exit code is 0 iff server::serve() returns Ok(()),
and the only code path that produces that is the graceful SIGTERM/SIGINT
drain in server::shutdown(), a clean "Completed/0" exit is definitional
evidence of an external signal reaching the container — not the engine
returning on its own.

Given the probes had no explicit timeoutSeconds/failureThreshold
(k8s httpGet defaults: 1s timeout, 3 strikes = 30s budget) against the
chart's tight 250m CPU limit, a legitimate per-pass signing/provenance
sweep burst (TLS handshakes + JSON/crypto verification across the running
fleet's images, "signing sweep signed=15…") can CFS-throttle the whole
container long enough to blow that budget. kubelet then SIGTERMs a healthy,
working engine — and this app's own graceful-shutdown handling makes that
look like a clean, deliberate exit rather than a kill, which is exactly the
"did real work, then exited cleanly, restarts climbing" pattern reported.

Fix

charts/protector (this repo's own upstream chart, not the deployed fork):

  • Widened livenessProbe/readinessProbe timeoutSeconds/failureThreshold
    so a busy-but-alive engine survives a sweep burst instead of being
    mistaken for dead (liveness — which restarts the container — gets more
    slack than readiness, which only pulls it from admission routing).
  • Raised the CPU limit 250m500m for headroom during that burst; the
    10m request (steady-state idle) is unchanged.
  • Added a CI assertion (.github/workflows/chart.yml) pinning both changes
    in the rendered manifest.

engine/src/engine/run_loop.rs:

  • Extracted the loop's wake/coalesce/break-on-close logic into wait_for_wake,
    covered by two new regression tests
    (engine/src/engine/run_loop/tests.rs) proving it keeps returning true
    across many simulated passes while any Sender clone survives, and only
    reports shutdown once every clone is gone — directly disproving the
    run-to-completion hypothesis for any future refactor.

No detection/actuation/shadow-mode behavior changes — this is purely a
liveness/scheduling hardening.

Required follow-up (cannot do from this worktree)

The live cluster deploys from ../cluster/charts/protector, a separate,
manually-diverged fork
(per repo convention, chart changes here must be
manually ported) — someone needs to port this same probe-timeout/CPU-limit
change into that fork for the incident to actually resolve in the running
cluster.

Testing

  • cargo fmt, cargo clippy --all-targets -- -D warnings — clean.
  • cargo nextest run — 1013 passed, 0 failed (built engine/web dashboard
    bundle first, per repo convention).
  • helm lint charts/protector — clean.
  • helm template protector charts/protector --namespace protector — verified
    the rendered manifest carries timeoutSeconds: 5/failureThreshold: 6
    (liveness), timeoutSeconds: 5/failureThreshold: 3 (readiness), and
    cpu: 500m, matching the new CI assertion.
  • New tests:
    wake_channel_survives_many_passes_and_only_closes_when_every_sender_drops,
    queued_burst_coalesces_into_one_wake.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VtjoJttCvBY4dzCoE4f9vP

thejefflarson and others added 2 commits July 27, 2026 18:12
…ep bursts (JEF-560)

Diagnosed the "protector container never reaches Ready; exits Completed/0
and restarts in a loop" incident. The engine's event-driven watch loop
(engine/src/engine/run_loop.rs) is NOT run-to-completion: it blocks forever
on tokio::select! over three mpsc channels, and the driving loop only ever
sees the change channel close once every Sender clone (each reflector
task's, plus the loop's own retained handle) is dropped — never after a
single pass. Extracted that wake/coalesce/break-on-close primitive into
`wait_for_wake` so this is now independently pinned by a regression test
(no kube::Client needed). The webhook's /healthz and /readyz are likewise
served unconditionally from process start in server.rs, so main() can only
exit 0 via the graceful SIGTERM/SIGINT drain path.

Since a clean exit-0 therefore requires an external signal, and the chart's
probes had no explicit timeout/failureThreshold (k8s defaults: 1s timeout,
3 strikes = 30s budget) against a 250m CPU limit, a legitimate per-pass
signing/provenance sweep burst (TLS handshakes + JSON/crypto across the
running fleet's images) can CFS-throttle the whole container long enough to
miss that budget — kubelet SIGTERMs a healthy, working engine, and the
app's own graceful shutdown makes it look like a clean exit rather than a
kill. Widened both probes' timeout/failureThreshold and raised the CPU
limit to 500m so a busy-but-alive engine isn't mistaken for a dead one.

This repo's charts/protector is the upstream source of truth; the live
cluster deploys from a separately forked, manually-ported copy
(../cluster/charts/protector, a different repo) that still needs this same
probe/resource change ported by hand — required human follow-up, out of
reach from this worktree.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VtjoJttCvBY4dzCoE4f9vP
@thejefflarson
thejefflarson merged commit f7b57f5 into main Jul 28, 2026
6 checks passed
@thejefflarson
thejefflarson deleted the thejefflarson/jef-560-protector-engine-container-never-reaches-ready-exits branch July 28, 2026 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant