crash_watch: suppress crash event on recently-cleared transient (closes #425)

This commit is contained in:
damocles 2026-05-27 13:43:56 +02:00 committed by Mara
commit aba1d3153e
2 changed files with 124 additions and 12 deletions

View file

@ -67,6 +67,21 @@ pub struct Coordinator {
/// Read by the dashboard to render a spinner; cleared when the action
/// resolves (success or failure).
transient: Mutex<HashMap<String, TransientState>>,
/// Tombstone for transients that have JUST been cleared. The
/// crash watcher polls every 10s and would race the
/// drop-clears-immediately path of `TransientGuard`: an operator
/// kill / restart sets `Stopping` → runs `nixos-container stop` →
/// drop clears the transient → poll fires next tick and sees the
/// container missing-from-running with no active transient →
/// spurious "container stopped without an operator action"
/// message (closes #425).
///
/// `clear_transient` stamps the cleared kind here with an
/// `Instant`; `recent_transient_within(grace)` returns the set of
/// agents whose tombstone is still inside the grace window. Crash
/// watcher consults both this and the active map before declaring
/// a stop deliberate.
recent_transient: Mutex<HashMap<String, (TransientKind, std::time::Instant)>>,
/// Unified wire-facing event channel feeding the dashboard SSE
/// stream. Carries broker messages (mirrored from `broker.subscribe`
/// by the forwarder task in `main.rs`) and dashboard-only mutation
@ -211,6 +226,7 @@ impl Coordinator {
context_window_tokens,
agents: Mutex::new(HashMap::new()),
transient: Mutex::new(HashMap::new()),
recent_transient: Mutex::new(HashMap::new()),
dashboard_events,
event_seq: AtomicU64::new(0),
meta_updates_active: AtomicU64::new(0),
@ -563,8 +579,18 @@ impl Coordinator {
}
pub fn clear_transient(&self, name: &str) {
let removed = self.transient.lock().unwrap().remove(name).is_some();
if removed {
let removed = self.transient.lock().unwrap().remove(name);
if let Some(state) = removed {
// Stamp the tombstone so the crash watcher can still see
// "operator kicked this off recently" on its next 10s poll
// — without this, the clear-then-poll race produced a
// spurious ContainerCrash on every operator stop/restart
// (#425). Old entries get reaped lazily on read so the map
// doesn't grow unbounded.
self.recent_transient
.lock()
.unwrap()
.insert(name.to_owned(), (state.kind, std::time::Instant::now()));
self.emit_dashboard_event(DashboardEvent::TransientCleared {
seq: self.next_seq(),
name: name.to_owned(),
@ -572,6 +598,18 @@ impl Coordinator {
}
}
/// Set of agents whose transient was cleared within the last
/// `grace` seconds — i.e. agents the operator just acted on,
/// whose stop the crash watcher should NOT classify as a crash.
/// Lazily reaps entries older than `grace` so the map stays
/// bounded by the active agent count.
pub fn recent_transient_within(&self, grace: std::time::Duration) -> HashMap<String, TransientKind> {
let now = std::time::Instant::now();
let mut map = self.recent_transient.lock().unwrap();
map.retain(|_, (_, ts)| now.duration_since(*ts) <= grace);
map.iter().map(|(k, (kind, _))| (k.clone(), *kind)).collect()
}
/// Set a transient state and return a guard that clears it on drop.
/// Use this from any path where the surrounding future could be
/// cancelled or panic between set and clear (HTTP handlers, spawned

View file

@ -25,6 +25,13 @@ use crate::lifecycle::{self, AGENT_PREFIX, MANAGER_NAME};
const POLL_INTERVAL: Duration = Duration::from_secs(10);
/// How long an operator-initiated transient stays "recently cleared"
/// for the purpose of suppressing crash events. Three full
/// `POLL_INTERVAL`s gives the post-lifecycle path comfortable
/// breathing room — the watcher will have polled at least twice
/// inside the window even with worst-case timer skew (#425).
const RECENT_TRANSIENT_GRACE: Duration = Duration::from_secs(30);
pub fn spawn(coord: Arc<Coordinator>) {
let mut shutdown = coord.shutdown_rx();
tokio::spawn(async move {
@ -92,17 +99,14 @@ pub fn spawn(coord: Arc<Coordinator>) {
fn emit_crash_transitions(coord: &Coordinator, prev: &HashSet<String>, current: &HashSet<String>) {
let transients = coord.transient_snapshot();
// Operator actions whose RAII guard already cleared but only just;
// suppresses the race where `lifecycle::kill` returns + drops the
// guard between two crash-watch polls (closes #425).
let recent = coord.recent_transient_within(RECENT_TRANSIENT_GRACE);
for stopped in prev.difference(current) {
let deliberate = transients.get(stopped).is_some_and(|st| {
matches!(
st.kind,
TransientKind::Stopping
| TransientKind::Restarting
| TransientKind::Destroying
| TransientKind::Rebuilding
)
});
if deliberate {
let active = transients.get(stopped).map(|st| st.kind);
let recently_cleared = recent.get(stopped).copied();
if is_deliberate_stop(active, recently_cleared) {
continue;
}
tracing::warn!(agent = %stopped, "container crash detected");
@ -113,6 +117,76 @@ fn emit_crash_transitions(coord: &Coordinator, prev: &HashSet<String>, current:
}
}
/// Pure classifier: did the operator stop / restart / destroy /
/// rebuild this container, or did it crash? Splits the matcher out so
/// it has a focused unit test (#425) without needing a Coordinator
/// fixture. `active` is the currently-set transient (if any),
/// `recently_cleared` is one whose RAII guard dropped within the
/// grace window.
fn is_deliberate_stop(
active: Option<TransientKind>,
recently_cleared: Option<TransientKind>,
) -> bool {
let is_op_kind = |kind: TransientKind| {
matches!(
kind,
TransientKind::Stopping
| TransientKind::Restarting
| TransientKind::Destroying
| TransientKind::Rebuilding
)
};
active.is_some_and(is_op_kind) || recently_cleared.is_some_and(is_op_kind)
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn deliberate_when_active_transient_is_operator_kind() {
for kind in [
TransientKind::Stopping,
TransientKind::Restarting,
TransientKind::Destroying,
TransientKind::Rebuilding,
] {
assert!(is_deliberate_stop(Some(kind), None), "{kind:?}");
}
}
#[test]
fn deliberate_when_recent_transient_is_operator_kind() {
// Race the #425 bug repros: lifecycle action completes + drops
// the guard between two polls. recent_transient catches it.
for kind in [
TransientKind::Stopping,
TransientKind::Restarting,
TransientKind::Destroying,
TransientKind::Rebuilding,
] {
assert!(is_deliberate_stop(None, Some(kind)), "{kind:?}");
}
}
#[test]
fn not_deliberate_with_no_transient_at_all() {
// The real-crash case — fires the ContainerCrash event.
assert!(!is_deliberate_stop(None, None));
}
#[test]
fn not_deliberate_when_only_spawning_starting() {
// Spawning/Starting are never paired with a "stopped" transition
// — they're starts. If we see one alongside a stop, it's
// unrelated (e.g. just-started container died), still a crash.
for kind in [TransientKind::Spawning, TransientKind::Starting] {
assert!(!is_deliberate_stop(Some(kind), None), "{kind:?} active");
assert!(!is_deliberate_stop(None, Some(kind)), "{kind:?} recent");
}
}
}
fn emit_login_transitions(
coord: &Coordinator,
prev: &HashSet<String>,