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

.github/workflows/chart.yml

@@ -231,6 +231,33 @@ jobs:
             echo "engine auto-wired advisory despite feedSync.enabled=false"; exit 1
           fi
           echo "ok"
+      - name: "Assert liveness/readiness probes tolerate a sweep-CPU burst (JEF-560)"
+        run: |
+          # JEF-560: a protector container was crashlooping (Ready never latching, restart
+          # count climbing, clean exit 0) because the default 1s probe timeout / 3-strike
+          # (30s) budget was too tight for the CPU the per-pass signing/provenance sweep
+          # legitimately bursts under the chart's CPU limit — kubelet SIGTERM'd a healthy,
+          # working engine. Both probes must carry an explicit, more forgiving timeout +
+          # failure budget so a busy-but-alive process isn't mistaken for a dead one.
+          render=$(helm template protector charts/protector --namespace protector)
+          live=$(echo "$render" | awk '/livenessProbe:/,/readinessProbe:/')
+          echo "$live" | grep -q 'timeoutSeconds: 5' \
+            || { echo "livenessProbe missing the widened timeoutSeconds"; exit 1; }
+          echo "$live" | grep -q 'failureThreshold: 6' \
+            || { echo "livenessProbe missing the widened failureThreshold"; exit 1; }
+          # readinessProbe through the container's own resources block (bounded by the
+          # NEXT section, `env:`) — `resources:` alone isn't unique enough to scope on
+          # (RBAC rules and the feed-fetcher sidecar both render their own).
+          ready=$(echo "$render" | awk '/readinessProbe:/,/^          env:/')
+          echo "$ready" | grep -q 'timeoutSeconds: 5' \
+            || { echo "readinessProbe missing the widened timeoutSeconds"; exit 1; }
+          echo "$ready" | grep -q 'failureThreshold: 3' \
+            || { echo "readinessProbe missing an explicit failureThreshold"; exit 1; }
+          # The CPU limit that produced the throttling-induced probe misses also needs
+          # headroom (the RAM-tight request is unchanged; only the burst ceiling moves).
+          echo "$ready" | grep -q 'cpu: 500m' \
+            || { echo "cpu limit not raised past the JEF-560 250m ceiling"; exit 1; }
+          echo "ok"
       - name: Assert the feed-fetcher sidecar is unprivileged and has NO apiserver access
         run: |
           # JEF-238 egress boundary: the sidecar is the ONLY container with egress. It must

charts/protector/README.md

@@ -273,7 +273,7 @@ Requires the `protector-agent` image and probes load-tested on your kernel (see
 | `feedSync.epssUrl`           | FIRST.org EPSS scores CSV (gzipped)  | EPSS source (gzipped CSV, gunzipped in place). See feeds section. |
 | `feedSync.interval`          | `"12h"`                              | Re-fetch interval for the sidecar (a `sleep` arg, e.g. `6h`, `30m`). |
 | `webhook.enforcedFailurePolicy` | `Fail`                            | The fail-closed enforcing webhook's policy (its scope is derived from `enforceScope`). |
-| `resources`                  | 10m/64Mi → 250m/256Mi                | RAM-tight, arm64-friendly.                          |
+| `resources`                  | 10m/64Mi → 500m/256Mi                | RAM-tight, arm64-friendly; the CPU limit has headroom for signing-sweep bursts (JEF-560). |
 
 See [`values.yaml`](values.yaml) for the fully commented set.
 

charts/protector/values.yaml

@@ -118,25 +118,42 @@ service:
 
 # RAM-tight, arm64-friendly: a webhook that mostly idles with occasional signature
 # verification (network I/O to the registry + Rekor) plus the async engine loop.
+# The 250m limit (JEF-560) proved too tight: a per-pass signing/provenance sweep across a
+# fleet's worth of images does a burst of TLS handshakes + JSON/crypto work on the SAME cgroup
+# the probe HTTP server shares, and the CFS quota can throttle the whole container — including
+# the probe response — long enough to look dead. 500m gives that burst headroom without
+# meaningfully changing steady-state idle usage (still requests only 10m).
 resources:
   requests:
     cpu: 10m
     memory: 64Mi
   limits:
-    cpu: 250m
+    cpu: 500m
     memory: 256Mi
 
-# Probes hit the HTTPS server; kubelet doesn't verify the cert for probes.
+# Probes hit the HTTPS server; kubelet doesn't verify the cert for probes. `/healthz` and
+# `/readyz` are both served from process start (no engine-sweep dependency), so a probe
+# failure here means the process is either genuinely wedged or — the JEF-560 incident — was
+# merely CPU-throttled long enough to miss the default 1s timeout / 3-strike (30s) budget
+# during a legitimate sweep burst. Widened timeout + failure budget so a busy-but-alive engine
+# isn't mistaken for a dead one and SIGTERM'd into a restart loop; liveness (which restarts the
+# container) gets the most slack, readiness (which only pulls it from admission routing) less.
 livenessProbe:
   httpGet:
     path: /healthz
     port: https
     scheme: HTTPS
+  timeoutSeconds: 5
+  periodSeconds: 15
+  failureThreshold: 6
 readinessProbe:
   httpGet:
     path: /readyz
     port: https
     scheme: HTTPS
+  timeoutSeconds: 5
+  periodSeconds: 10
+  failureThreshold: 3
 
 nodeSelector: {}
 tolerations: []

engine/src/engine/run_loop.rs

@@ -299,6 +299,33 @@ fn build_adjudicator(
     }
 }
 
+/// Block until the driving loop has something to do, then drain any already-queued burst so
+/// it coalesces into one pass. Wakes on either a cluster change or a behavioral/audit report
+/// — the behavioral/audit channels only fire when the ingest actually changed the evidence
+/// store (a new observation, not a repeat), so mundane churn never reaches here.
+///
+/// Returns `true` to keep looping, `false` only once `change_rx` has PERMANENTLY closed —
+/// i.e. every clone of its `Sender` (each reflector task's, and the loop's own retained one)
+/// has been dropped, which happens only on total shutdown, never after a single pass (JEF-560:
+/// pinned by [`tests::wake_channel_survives_many_passes_and_only_closes_when_every_sender_drops`],
+/// a regression test for an operational incident where the engine container was suspected —
+/// wrongly, per that test — of running to completion instead of looping).
+async fn wait_for_wake(
+    change_rx: &mut tokio::sync::mpsc::Receiver<()>,
+    runtime_rx: &mut tokio::sync::mpsc::Receiver<()>,
+    audit_rx: &mut tokio::sync::mpsc::Receiver<()>,
+) -> bool {
+    tokio::select! {
+        next = change_rx.recv() => if next.is_none() { return false },
+        _ = runtime_rx.recv() => {},
+        _ = audit_rx.recv() => {},
+    }
+    while change_rx.try_recv().is_ok() {}
+    while runtime_rx.try_recv().is_ok() {}
+    while audit_rx.try_recv().is_ok() {}
+    true
+}
+
 /// Event-driven observer: the default. Reflectors keep an in-memory store of each
 /// watched resource current via `list`-then-`watch` (the periodic relist is the
 /// resync floor ADR-0004 calls for). The engine reacts to *events* — it sits quiet
@@ -691,21 +718,9 @@ pub async fn run_watch(
 
     tracing::info!("engine: watching cluster (event-driven)");
     loop {
-        // Wake on either a cluster change or a behavioral report. The behavioral channel
-        // only fires when the ingest actually changed the evidence store (a new
-        // observation, not a repeat) — see `ingest_behavior`. So a report that tells us
-        // nothing new never reaches here, and we don't burn a graph rebuild + CRD lists
-        // for it; mundane churn (the same connections, again) is dropped at ingest.
-        tokio::select! {
-            next = change_rx.recv() => if next.is_none() { break },
-            _ = runtime_rx.recv() => {},
-            _ = audit_rx.recv() => {},
+        if !wait_for_wake(&mut change_rx, &mut runtime_rx, &mut audit_rx).await {
+            break;
         }
-        // Coalesce an already-queued burst (a Deployment rollout, or several material
-        // reports) into one pass.
-        while change_rx.try_recv().is_ok() {}
-        while runtime_rx.try_recv().is_ok() {}
-        while audit_rx.try_recv().is_ok() {}
 
         let (linkerd_servers_now, linkerd_policies_now, linkerd_mtls_now) =
             observe::list_linkerd_authz(&client).await;

engine/src/engine/run_loop/tests.rs

@@ -118,6 +118,77 @@ fn tier_grants_config_fails_loud_but_serves_on_valid_or_absent() {
     clear();
 }
 
+// JEF-560: an operational incident reported the protector container exiting cleanly
+// (exit 0) and restart-looping, never reaching Ready — read from the tail of its logs as
+// "the engine is doing real work, then the process just stops" and hypothesized as a
+// run-to-completion bug (the driving loop returning after a single pass instead of
+// blocking for the next one). These tests exercise `wait_for_wake`, the actual primitive
+// `run_watch`'s `loop {}` calls each iteration, in isolation (no `kube::Client` needed) and
+// disprove that hypothesis: the loop only ever stops when the `change` channel's Sender has
+// been dropped EVERYWHERE it was cloned to (every reflector task, plus the loop's own
+// retained handle) — never merely because one pass finished.
+
+/// Many passes in a row: `wait_for_wake` must keep returning `true` — i.e. the driving loop
+/// keeps running — for as long as ANY clone of `change_tx` (mirroring a live reflector task)
+/// is still held, exactly like `run_watch` itself holds one for its entire body. A regression
+/// to "process the first pass and return" would fail this on the second iteration.
+#[tokio::test(flavor = "multi_thread")]
+async fn wake_channel_survives_many_passes_and_only_closes_when_every_sender_drops() {
+    let (change_tx, mut change_rx) = tokio::sync::mpsc::channel::<()>(64);
+    let (_runtime_tx, mut runtime_rx) = tokio::sync::mpsc::channel::<()>(64);
+    let (_audit_tx, mut audit_rx) = tokio::sync::mpsc::channel::<()>(64);
+
+    // Mirror `run_watch`'s reflector tasks: each holds its OWN clone, independent of the
+    // driving loop's retained `change_tx`, and sends one tick per simulated cluster change.
+    let reflector_tx = change_tx.clone();
+
+    const PASSES: usize = 5;
+    for pass in 0..PASSES {
+        reflector_tx
+            .send(())
+            .await
+            .expect("reflector clone can still send");
+        assert!(
+            super::wait_for_wake(&mut change_rx, &mut runtime_rx, &mut audit_rx).await,
+            "pass {pass}: the loop must keep running while a Sender clone is alive — a \
+             run-to-completion regression would return false after the very first pass"
+        );
+    }
+
+    // Drop every clone (the "reflector" one, then the loop's own retained original) — only
+    // now, with the Sender fully gone, may `wait_for_wake` report shutdown.
+    drop(reflector_tx);
+    drop(change_tx);
+    assert!(
+        !super::wait_for_wake(&mut change_rx, &mut runtime_rx, &mut audit_rx).await,
+        "once every Sender clone is dropped, the loop must be told to stop"
+    );
+}
+
+/// A burst of several already-queued ticks on the SAME pass coalesces into one wake, exactly
+/// as the comment on `wait_for_wake` documents (a Deployment rollout's several rapid changes
+/// must not fan out into a rebuild per tick).
+#[tokio::test(flavor = "multi_thread")]
+async fn queued_burst_coalesces_into_one_wake() {
+    let (change_tx, mut change_rx) = tokio::sync::mpsc::channel::<()>(64);
+    let (_runtime_tx, mut runtime_rx) = tokio::sync::mpsc::channel::<()>(64);
+    let (_audit_tx, mut audit_rx) = tokio::sync::mpsc::channel::<()>(64);
+
+    for _ in 0..4 {
+        change_tx.send(()).await.expect("send queues fine");
+    }
+    assert!(
+        super::wait_for_wake(&mut change_rx, &mut runtime_rx, &mut audit_rx).await,
+        "a queued burst still wakes the loop"
+    );
+    // The burst was fully drained by that one call — nothing left queued for a second wake
+    // without a fresh send.
+    assert!(
+        change_rx.try_recv().is_err(),
+        "wait_for_wake must drain the whole burst, not leave a residual tick queued"
+    );
+}
+
 /// The reflected element type asks the apiserver for metadata only. `metadata_api()`
 /// is what drives both `watcher(Api::<PartialObjectMeta<Secret>>, _)` and
 /// `Api::<Secret>::list_metadata` to issue `.../secrets` requests that return