fix(#3894): an unreadable power intent must reach the reconcile queue
The boot sweep's error arm substituted `Wanted::from_running(running)`, which is the one value for which `reconcile_action` returns `Noop` — both ways. An agent whose `agent_power` row could not be read therefore could never enter `drifted`, so on a fresh rev marker a corrupt row produced one `warn!` per boot and no other signal, indefinitely. Classify the unreadable case as its own outcome instead: the agent gets a boot `Reconcile`, whose `get_or_seed` fails as a per-agent node — a surface the dashboard already renders — and the failure stops at that one agent. The classification moved into `boot_action`, a pure function, because the loop had no tests at all. The first of the five asserts the unreadable case across every (fresh × running) combination, which is exactly the matrix the fabricated value made unreachable.
This commit is contained in:
parent
c90b999285
commit
9749e9324d
1 changed files with 130 additions and 19 deletions
|
|
@ -263,34 +263,36 @@ pub async fn run(coord: Arc<Coordinator>) -> Result<()> {
|
|||
for name in &logical_names {
|
||||
let running = lifecycle::is_running(name).await;
|
||||
let wanted = match coord.power.get_or_seed(name, running) {
|
||||
Ok(w) => w,
|
||||
Ok(w) => Some(w),
|
||||
Err(e) => {
|
||||
tracing::warn!(%name, error = ?e, "agent_power read failed — assuming observed");
|
||||
crate::power::Wanted::from_running(running)
|
||||
tracing::warn!(%name, error = ?e, "agent_power read failed — reconciling so it surfaces");
|
||||
None
|
||||
}
|
||||
};
|
||||
let fresh = current_rev.as_ref().is_some_and(|rev| {
|
||||
std::fs::read_to_string(crate::paths::applied_rev_marker(name))
|
||||
.is_ok_and(|stored| stored == rev.as_str())
|
||||
});
|
||||
if fresh {
|
||||
n_skipped += 1;
|
||||
} else {
|
||||
any_stale = true;
|
||||
if wanted == crate::power::Wanted::Up {
|
||||
// Rebuild against the post-bump lock; the DAG's tail
|
||||
// Reconcile brings the agent (back) up — covering both
|
||||
// the running-stale and stopped-but-wanted-up cases.
|
||||
match boot_action(wanted, fresh, running) {
|
||||
BootAction::Rebuild => {
|
||||
any_stale = true;
|
||||
fanout.push((name.clone(), running));
|
||||
continue;
|
||||
}
|
||||
// Stale but wanted offline: no boot-time nix work — the
|
||||
// start submit path upgrades a stale start to a rebuild.
|
||||
n_deferred += 1;
|
||||
tracing::debug!(%name, "boot reconcile: stale but offline — deferring rebuild to on-start");
|
||||
}
|
||||
if crate::power::reconcile_action(wanted, running) != crate::power::ReconcileAction::Noop {
|
||||
drifted.push(name.clone());
|
||||
BootAction::Defer { reconcile } => {
|
||||
any_stale = true;
|
||||
n_deferred += 1;
|
||||
tracing::debug!(%name, "boot reconcile: stale but offline — deferring rebuild to on-start");
|
||||
if reconcile {
|
||||
drifted.push(name.clone());
|
||||
}
|
||||
}
|
||||
BootAction::Skip { reconcile } => {
|
||||
n_skipped += 1;
|
||||
if reconcile {
|
||||
drifted.push(name.clone());
|
||||
}
|
||||
}
|
||||
BootAction::Unreadable => drifted.push(name.clone()),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -317,6 +319,45 @@ pub async fn run(coord: Arc<Coordinator>) -> Result<()> {
|
|||
Ok(())
|
||||
}
|
||||
|
||||
/// What the boot sweep does with one agent.
|
||||
#[derive(Debug, PartialEq, Eq)]
|
||||
enum BootAction {
|
||||
/// Stale ∧ wanted up: rebuild against the post-bump lock. The DAG's
|
||||
/// tail `Reconcile` brings the agent (back) up, covering both the
|
||||
/// running-stale and stopped-but-wanted-up cases.
|
||||
Rebuild,
|
||||
/// Stale ∧ wanted offline: no boot-time nix work — the start submit
|
||||
/// path upgrades a stale start to a rebuild.
|
||||
Defer { reconcile: bool },
|
||||
/// Already on the current rev.
|
||||
Skip { reconcile: bool },
|
||||
/// The intent could not be read.
|
||||
Unreadable,
|
||||
}
|
||||
|
||||
/// `wanted` is `None` when the store could not answer for this agent.
|
||||
///
|
||||
/// That case has no neutral stand-in, which is why it is a variant rather
|
||||
/// than a fallback value: `Wanted::from_running(running)` is precisely the
|
||||
/// value for which `reconcile_action` returns `Noop`, so guessing it here
|
||||
/// puts the agent beyond every check below — a corrupt row then leaves one
|
||||
/// `warn!` per boot and no other trace. The queued reconcile's own
|
||||
/// `get_or_seed` fails as a per-agent node instead.
|
||||
fn boot_action(wanted: Option<crate::power::Wanted>, fresh: bool, running: bool) -> BootAction {
|
||||
let Some(wanted) = wanted else {
|
||||
return BootAction::Unreadable;
|
||||
};
|
||||
let reconcile =
|
||||
crate::power::reconcile_action(wanted, running) != crate::power::ReconcileAction::Noop;
|
||||
if fresh {
|
||||
BootAction::Skip { reconcile }
|
||||
} else if wanted == crate::power::Wanted::Up {
|
||||
BootAction::Rebuild
|
||||
} else {
|
||||
BootAction::Defer { reconcile }
|
||||
}
|
||||
}
|
||||
|
||||
/// Submit the boot-time forge/matrix/webhook/knowledge/wanted-state sweeps as
|
||||
/// DAG nodes — `ForgeSweep`, `MatrixSweep`, `WebhookRegister`,
|
||||
/// `KnowledgePull`, `WantedPull`. Unlike
|
||||
|
|
@ -431,3 +472,73 @@ fn submit_boot_tree(
|
|||
}
|
||||
coord.emit_rebuild_queue_snapshot();
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{BootAction, boot_action};
|
||||
use crate::power::Wanted;
|
||||
|
||||
// The regression the `Unreadable` variant exists for: the arm used to
|
||||
// substitute `from_running(running)`, which is `Noop` against BOTH
|
||||
// observations — so no combination of inputs could reach a reconcile.
|
||||
#[test]
|
||||
fn an_unreadable_intent_is_never_settled_whatever_the_agent_is_doing() {
|
||||
for fresh in [true, false] {
|
||||
for running in [true, false] {
|
||||
assert_eq!(
|
||||
boot_action(None, fresh, running),
|
||||
BootAction::Unreadable,
|
||||
"fresh={fresh} running={running}"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_fresh_agent_doing_what_it_should_is_skipped_without_a_reconcile() {
|
||||
assert_eq!(
|
||||
boot_action(Some(Wanted::Up), true, true),
|
||||
BootAction::Skip { reconcile: false }
|
||||
);
|
||||
assert_eq!(
|
||||
boot_action(Some(Wanted::Offline), true, false),
|
||||
BootAction::Skip { reconcile: false }
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_fresh_agent_that_drifted_from_its_intent_is_reconciled() {
|
||||
assert_eq!(
|
||||
boot_action(Some(Wanted::Up), true, false),
|
||||
BootAction::Skip { reconcile: true }
|
||||
);
|
||||
assert_eq!(
|
||||
boot_action(Some(Wanted::Offline), true, true),
|
||||
BootAction::Skip { reconcile: true }
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_stale_agent_wanted_up_is_rebuilt_running_or_not() {
|
||||
assert_eq!(
|
||||
boot_action(Some(Wanted::Up), false, true),
|
||||
BootAction::Rebuild
|
||||
);
|
||||
assert_eq!(
|
||||
boot_action(Some(Wanted::Up), false, false),
|
||||
BootAction::Rebuild
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_stale_agent_wanted_offline_defers_and_still_reports_drift() {
|
||||
assert_eq!(
|
||||
boot_action(Some(Wanted::Offline), false, false),
|
||||
BootAction::Defer { reconcile: false }
|
||||
);
|
||||
assert_eq!(
|
||||
boot_action(Some(Wanted::Offline), false, true),
|
||||
BootAction::Defer { reconcile: true }
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue