From 99d2ed5b9eb0bb1c1364a56626cb648abb135b19 Mon Sep 17 00:00:00 2001 From: atlas Date: Mon, 26 Oct 2026 17:53:39 +0100 Subject: [PATCH] build a failed meta build once more, and treat a timeout or missing grant as no verdict --- docs/swarm/builder.md | 40 ++++-- nix/host-modules/swarm-grafana.nix | 13 ++ nix/module-eval/grafana.nix | 26 ++++ swarm-builder/src/nix_build.rs | 78 ++++++++--- swarm-controller/src/held_deploys.rs | 179 ++++++++++++++++++++++++ swarm-controller/src/jobq.rs | 200 +++++++++++++++++++++------ swarm-controller/src/main.rs | 2 + swarm-controller/src/meta_build.rs | 180 ++++++++++++++++++++---- swarm-wire/src/builder.rs | 10 +- 9 files changed, 619 insertions(+), 109 deletions(-) create mode 100644 swarm-controller/src/held_deploys.rs diff --git a/docs/swarm/builder.md b/docs/swarm/builder.md index 87388f79..2f932ccd 100644 --- a/docs/swarm/builder.md +++ b/docs/swarm/builder.md @@ -194,7 +194,7 @@ runs only when the build is green. The `build_meta` node reads `internal/meta`'s head commit when it runs, because most paths declare it before the update that commits that head. It sends the -agent and that commit on `.builder.build-meta` and waits up to 61 +agent and that commit on `$SWARM.builder.build-meta` and waits up to 61 minutes. The builder takes the same one-build slot as PR checks, and refuses a request that waited 30 seconds for it. It runs `nix build` of `nixosConfigurations..config.system.build.toplevel` from @@ -205,22 +205,35 @@ the builder's platform. The hive doesn't reuse the result yet: it still builds locally, from whatever head it reads at deploy time, which can be newer than the one the swarm built. -The node ends one of three ways: +The node ends one of three ways, and the error text decides what the ten-minute +timer does: -| Builder's answer | Node | Deploy | Timer | -| ----------------------------------------------------------------------------------------------------------------- | ------------------------------------ | ------- | --------------------------------- | -| built | done, closure recorded | runs | | -| `nix` failed or timed out (a red build) | failed, with `nix`'s last lines | skipped | declares nothing | -| no builder listening, no reply in time, or the builder couldn't start `nix` (busy for 30 seconds, no forge token) | failed, `not built, will be retried` | skipped | declares a build and deploy again | +| Builder's answer | Node | Deploy | Timer | +| ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------- | ------- | ----------------------------------- | +| built | done, closure recorded | runs | | +| `nix` exited with an error, first time | failed, `build failed, will be built once more: …` and `nix`'s last lines | skipped | builds again, as the second attempt | +| `nix` exited with an error, second time (red) | failed, with `nix`'s last lines | skipped | declares nothing | +| no builder listening, no reply in 61 minutes, a reply that doesn't decode, or the builder never ran `nix` (busy for 30 seconds, no forge token, `nix` couldn't start, `nix` hit its one-hour deadline) | failed, `not built, will be retried: …` | skipped | builds again, same attempt | +| the controller couldn't read the `internal/meta` head, find a commit there, grant the builder read on the repo, or write the closure | failed, `not built, will be retried: …` | skipped | builds again, same attempt | -A red build is a verdict on the config: a config merge queues its own build and -deploy. A build that gave no verdict isn't, so the ten-minute timer declares -it again, without a meta update when the latest update succeeded, until the -builder answers. That covers a controller that updates before its builder +Only `nix` exiting with an error twice is a verdict on the config. The first +error may be a fetch hiccup inside `nix`, so the controller builds once more +without reading `nix`'s output. A red build holds the deploy until a config +merge queues its own build and deploy, and the pin fan-out still records the +pin, so one red agent doesn't resend the healthy ones. Anything else gave no +verdict, so the timer declares a build and deploy again every ten minutes, +without a meta update when the latest update succeeded, and the attempt count +stays where it was. That covers a controller that updates before its builder host does: the old builder has no subscriber for the new subject, so the controller gets no responders at once, and the deploys go out after the builder updates. Each retry adds a build and deploy node per affected agent while the -builder stays away. +builder stays away. The build node carries the attempt count as `attempt`. + +The controller exports the gauge `swarm.deploys.held`: the agents whose latest +build failed, with the label `reason` set to `red` or `retrying`. The Grafana +alert `hh-deploys-held` fires when the sum stays above zero for 30 minutes, so +a dead builder or a red config shows in Grafana before anyone notices an agent +not updating. On a built reply the controller writes the agent's key in the `agent-closures` KV bucket: the `internal/meta` commit, the closure's store path, the builder's @@ -230,7 +243,8 @@ Nothing reads the bucket yet. The closure stays unrooted in the builder host's store. The controller gives the `builder` forge account read on `internal/meta` when it -updates the flake, since the repo is private. +updates the flake, and again before each build, since the repo is private. A +grant the forge refuses counts as no verdict. ## Credentials diff --git a/nix/host-modules/swarm-grafana.nix b/nix/host-modules/swarm-grafana.nix index cdc934b7..7edec4a0 100644 --- a/nix/host-modules/swarm-grafana.nix +++ b/nix/host-modules/swarm-grafana.nix @@ -1725,6 +1725,19 @@ in # renew at half their life, so a fraction over 0.8 means # the renewal has been stuck for days. `hive` and `agent` # stay in the grouping so the alert names the holder. + # Agents whose deploy the build gate holds back, from the gauge + # swarm-controller exports (`held_deploys.rs`): a build that + # is red, or that keeps getting no answer from the builder. + # Thirty minutes covers a builder restart and the controller's + # ten-minute retries. + (metricThresholdRule { + uid = "hh-deploys-held"; + title = "deploys held by the build gate"; + expr = ''sum({__name__="swarm.deploys.held"})''; + threshold = 0; + for = "30m"; + summary = "swarm-controller is holding back the deploy of one or more agents because the builder's build of internal/meta is red or gets no answer; the failed `build_meta` node on the Jobs page says which"; + }) (metricThresholdRule { uid = "hh-bao-client-cert-lifetime"; title = "bao client certificate past 80% of its lifetime"; diff --git a/nix/module-eval/grafana.nix b/nix/module-eval/grafana.nix index 9f00a1b7..d732fa94 100644 --- a/nix/module-eval/grafana.nix +++ b/nix/module-eval/grafana.nix @@ -606,6 +606,32 @@ let ] && query.instant; } + { + # The held-deploys alert fires on any agent held for 30 minutes and + # selects the gauge name `held_deploys.rs` spells in Rust. The disk rule + # is the presence control: it goes through the same lookup. + # 🔬 Invert-proof: change `for = "30m"` or the selector on + # `hh-deploys-held` in swarm-grafana.nix, or `METRIC` in held_deploys.rs. + name = "the held-deploys alert fires after 30 minutes on the gauge swarm-controller exports"; + ok = + let + groups = + grafanaHere.containers.swarm-grafana.config.services.grafana.provision.alerting.rules.settings.groups; + rules = lib.concatMap (g: g.rules) groups; + source = builtins.readFile ../../swarm-controller/src/held_deploys.rs; + rule = uid: lib.findFirst (r: r.uid == uid) null rules; + held = rule "hh-deploys-held"; + threshold = + r: + lib.head + (lib.head (lib.findFirst (q: q.refId == "C") null r.data).model.conditions).evaluator.params; + in + threshold held == 0 + && held.for == "30m" + && threshold (rule "hh-host-disk-high") == 0.85 + && lib.hasInfix ''{__name__="swarm.deploys.held"}'' (lib.head held.data).model.expr + && lib.hasInfix ''"swarm.deploys.held"'' source; + } { # 🩸 The regression this guards: moving bao's metrics scrape into its # own in-container collector made that collector stamp every exported diff --git a/swarm-builder/src/nix_build.rs b/swarm-builder/src/nix_build.rs index 71f52f67..5aff769d 100644 --- a/swarm-builder/src/nix_build.rs +++ b/swarm-builder/src/nix_build.rs @@ -80,12 +80,18 @@ impl Builder { } let mut cmd = self.nix_command(token); cmd.args(BUILD_FLAGS).arg(self.meta_attr(job)); - match run(cmd, self.deadline).await { - Reply::Built { out_path } => FromMetaReply::Built { - out_path, - system: self.system.clone(), + // `nix` not finishing (deadline, spawn or wait error) says nothing about + // the config, so it is `Unavailable`; only an exit of `nix` itself is a + // verdict. + match run_raw(cmd, self.deadline).await { + Err(reason) => FromMetaReply::Unavailable { reason }, + Ok((status, out, err)) => match verdict(status, &out, &err) { + Reply::Built { out_path } => FromMetaReply::Built { + out_path, + system: self.system.clone(), + }, + Reply::Failed { error_tail } => FromMetaReply::Failed { error_tail }, }, - Reply::Failed { error_tail } => FromMetaReply::Failed { error_tail }, } } @@ -274,18 +280,22 @@ fn check_agent_and_rev(agent: &str, rev: &str) -> Result<(), String> { async fn run(cmd: tokio::process::Command, deadline: Duration) -> Reply { match run_raw(cmd, deadline).await { Err(error_tail) => Reply::Failed { error_tail }, - Ok((status, out, err)) => { - let out_path = String::from_utf8_lossy(&out) - .lines() - .rfind(|l| !l.trim().is_empty()) - .map(|l| l.trim().to_owned()); - match out_path { - Some(out_path) if status.success() => Reply::Built { out_path }, - _ => Reply::Failed { - error_tail: tail(&String::from_utf8_lossy(&err), status), - }, - } - } + Ok((status, out, err)) => verdict(status, &out, &err), + } +} + +/// What a `nix build` that exited said: the store path it printed, or the tail +/// of what it said. +fn verdict(status: std::process::ExitStatus, out: &[u8], err: &[u8]) -> Reply { + let out_path = String::from_utf8_lossy(out) + .lines() + .rfind(|l| !l.trim().is_empty()) + .map(|l| l.trim().to_owned()); + match out_path { + Some(out_path) if status.success() => Reply::Built { out_path }, + _ => Reply::Failed { + error_tail: tail(&String::from_utf8_lossy(err), status), + }, } } @@ -815,6 +825,40 @@ mod tests { (Builder { nix, ..builder() }, dir) } + /// `nix` not finishing says nothing about the config: a build past its + /// deadline and a `nix` that cannot start are `Unavailable`. A `nix` that + /// exits non-zero is `Failed`, the control for the same builder. + #[tokio::test] + async fn a_meta_build_that_does_not_finish_is_unavailable_and_one_that_exits_is_failed() { + let (slow, _guard) = scripted("sleep 5"); + let slow = Builder { + deadline: Duration::from_millis(200), + ..slow + }; + let reply = slow.build_from_meta(&meta_job(), "t").await; + assert!( + matches!(&reply, FromMetaReply::Unavailable { reason } if reason.contains("did not finish")), + "{reply:?}" + ); + + let missing = Builder { + nix: PathBuf::from("/nonexistent/nix"), + ..builder() + }; + let reply = missing.build_from_meta(&meta_job(), "t").await; + assert!( + matches!(&reply, FromMetaReply::Unavailable { reason } if reason.contains("starting nix")), + "{reply:?}" + ); + + let (failing, _guard) = scripted("echo 'error: attribute missing' >&2; exit 1"); + let reply = failing.build_from_meta(&meta_job(), "t").await; + assert!( + matches!(&reply, FromMetaReply::Failed { error_tail } if error_tail.ends_with("attribute missing")), + "{reply:?}" + ); + } + fn lock_job() -> LockMeta { LockMeta { flake_nix: "{ outputs = _: { }; }".to_owned(), diff --git a/swarm-controller/src/held_deploys.rs b/swarm-controller/src/held_deploys.rs new file mode 100644 index 00000000..1de9c3dc --- /dev/null +++ b/swarm-controller/src/held_deploys.rs @@ -0,0 +1,179 @@ +//! The `swarm.deploys.held` gauge: how many agents have a deploy held back by +//! the build gate, exported on the jobq exporter. Grafana alerts when it stays +//! above zero (`hh-deploys-held`, `nix/host-modules/swarm-grafana.nix`). +//! +//! An agent counts when its latest `BuildMeta` node is `Failed`, once per agent +//! under one of two `reason` labels: +//! - `red`: `nix` exited with an error twice. Nothing retries it; a config merge +//! builds again. +//! - `retrying`: the build gave no verdict or met its first `nix` error, so the +//! ten-minute timer builds it again. +//! +//! A pending, running or finished build does not count, so the value falls to +//! zero when the builder answers. + +use std::sync::{Arc, Mutex}; + +use hive_jobq::State; +use opentelemetry::KeyValue; +use opentelemetry::metrics::MeterProvider as _; +use opentelemetry_sdk::metrics::SdkMeterProvider; + +use crate::jobq::{SwarmNodeKind, SwarmResourceKind}; +use crate::meta_build; + +/// The gauge's name. The Grafana rule selects it by this name. +pub(crate) const METRIC: &str = "swarm.deploys.held"; + +/// Held agents as `(red, retrying)`. +fn count(graph: &hive_jobq::Graph) -> (u64, u64) { + let mut latest = std::collections::BTreeMap::new(); + for n in graph.nodes() { + if let SwarmNodeKind::BuildMeta { agent, .. } = &n.payload + && latest + .get(agent) + .is_none_or(|l: &&hive_jobq::Node<_, _>| l.id < n.id) + { + latest.insert(agent, n); + } + } + latest + .into_values() + .filter(|n| n.state == State::Failed) + .fold((0, 0), |(red, retrying), n| { + if n.error.as_deref().is_some_and(meta_build::is_retried) { + (red, retrying + 1) + } else { + (red + 1, retrying) + } + }) +} + +/// Register the gauge on `provider`, reading `jobq` at each export. Nothing when +/// no exporter is configured. +pub(crate) fn register( + provider: Option<&SdkMeterProvider>, + jobq: &Arc>>, +) { + let Some(provider) = provider else { + return; + }; + let jobq = Arc::clone(jobq); + provider + .meter("hyperhive.deploy_gate") + .u64_observable_gauge(METRIC) + .with_description("agents whose deploy the build gate is holding back, by reason") + .with_callback(move |obs| { + let (red, retrying) = count( + jobq.lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .graph(), + ); + obs.observe(red, &[KeyValue::new("reason", "red")]); + obs.observe(retrying, &[KeyValue::new("reason", "retrying")]); + }) + .build(); +} + +#[cfg(test)] +mod tests { + use hive_jobq::scheduler::{Outcome, Scheduler}; + use opentelemetry_sdk::metrics::data::{AggregatedMetrics, MetricData, ResourceMetrics}; + use opentelemetry_sdk::metrics::{InMemoryMetricExporter, PeriodicReader}; + + use super::*; + + type Jobq = Arc>>; + + /// A scheduler where each of `builds` (agent, outcome) ran once, in order. + async fn after(builds: &[(&str, Outcome)]) -> Jobq { + let jobq: Jobq = Arc::new(Mutex::new(Scheduler::new( + hive_jobq::Graph::new(), + hive_jobq::resources::ResourceTable::new(), + ))); + for (agent, outcome) in builds { + jobq.lock() + .unwrap() + .append( + SwarmNodeKind::BuildMeta { + agent: (*agent).to_owned(), + attempt: 0, + }, + Vec::new(), + None, + ) + .expect("insert"); + let outcome = outcome.clone(); + Scheduler::claim_next(&jobq, move |_, _, b| async move { (b, outcome) }) + .expect("runnable") + .await + .1 + .expect("no growth"); + } + jobq + } + + fn red() -> Outcome { + Outcome::Failed("error: attribute missing".to_owned()) + } + + fn retrying() -> Outcome { + Outcome::Failed(format!("{}no builder", meta_build::RETRY_PREFIX)) + } + + #[tokio::test] + async fn a_failed_latest_build_holds_its_agent_by_reason() { + let jobq = after(&[ + ("atlas", red()), + ("argus", retrying()), + ("iris", Outcome::Done), + ]) + .await; + assert_eq!(count(jobq.lock().unwrap().graph()), (1, 1)); + } + + /// Only an agent's latest build counts: a green build after a red one + /// releases it, and a red one after a green one holds it. + #[tokio::test] + async fn only_the_latest_build_of_an_agent_counts() { + let released = after(&[("atlas", red()), ("atlas", Outcome::Done)]).await; + assert_eq!(count(released.lock().unwrap().graph()), (0, 0)); + let held = after(&[("atlas", Outcome::Done), ("atlas", red())]).await; + assert_eq!(count(held.lock().unwrap().graph()), (1, 0)); + } + + /// The gauge reads the graph at export time and labels both reasons, zero + /// included, so a rule sees the series before anything is held. + #[tokio::test] + async fn the_gauge_exports_both_reasons() { + let jobq = after(&[("atlas", red())]).await; + let exporter = InMemoryMetricExporter::default(); + let reader = PeriodicReader::builder(exporter.clone()).build(); + let provider = SdkMeterProvider::builder().with_reader(reader).build(); + register(Some(&provider), &jobq); + provider.force_flush().expect("flush"); + let all = exporter.get_finished_metrics().expect("metrics"); + let mut points = Vec::new(); + for m in all + .last() + .into_iter() + .flat_map(ResourceMetrics::scope_metrics) + .flat_map(opentelemetry_sdk::metrics::data::ScopeMetrics::metrics) + .filter(|m| m.name() == METRIC) + { + let AggregatedMetrics::U64(MetricData::Gauge(g)) = m.data() else { + panic!("not a u64 gauge: {:?}", m.data()); + }; + for p in g.data_points() { + let reason = p + .attributes() + .find(|kv| kv.key.as_str() == "reason") + .map(|kv| kv.value.to_string()) + .expect("a reason"); + points.push((reason, p.value())); + } + } + points.sort(); + assert_eq!(points, [("red".to_owned(), 1), ("retrying".to_owned(), 0)]); + } +} diff --git a/swarm-controller/src/jobq.rs b/swarm-controller/src/jobq.rs index 5c39b793..6f6e55a1 100644 --- a/swarm-controller/src/jobq.rs +++ b/swarm-controller/src/jobq.rs @@ -115,8 +115,10 @@ pub(crate) enum SwarmNodeKind { /// runs only when the build is green. See `meta_build`. /// /// Carries no rev: it reads the head when it runs, which in most DAGs is - /// after the `UpdateMeta` that commits it. - BuildMeta { agent: String }, + /// after the `UpdateMeta` that commits it. `attempt` is how many builds of + /// this deploy already ended in an error from `nix`; the build after the + /// second such error is red. + BuildMeta { agent: String, attempt: u32 }, /// Re-mint `agent`'s queue secret if it is still stored and old enough. /// See `agent_renewal`. /// @@ -202,7 +204,6 @@ impl hive_jobq_wire::WireNode for SwarmNodeKind { | SwarmNodeKind::MintAgentIdentity { agent } | SwarmNodeKind::MintAgentForgeToken { agent } | SwarmNodeKind::MintAgentMatrixAccount { agent } - | SwarmNodeKind::BuildMeta { agent } | SwarmNodeKind::RenewAgentQueueCredential { agent } => { serde_json::json!({ "agent": agent }) } @@ -215,6 +216,9 @@ impl hive_jobq_wire::WireNode for SwarmNodeKind { SwarmNodeKind::BuildAgent { agent, rev, pr } => { serde_json::json!({ "agent": agent, "rev": rev, "pr": pr }) } + SwarmNodeKind::BuildMeta { agent, attempt } => { + serde_json::json!({ "agent": agent, "attempt": attempt }) + } SwarmNodeKind::TriggerDeploy { hive, agent, rev } => { serde_json::json!({ "agent": agent, "hive": hive, "rev": rev }) } @@ -414,16 +418,13 @@ async fn run_node_work( SwarmNodeKind::BuildAgent { agent, rev, pr } => { build_agent(&deps, forge, agent, rev, pr).await } - SwarmNodeKind::BuildMeta { agent } => build_meta(&deps, forge, agent).await, + SwarmNodeKind::BuildMeta { agent, attempt } => { + build_meta(&deps, forge, agent, attempt).await + } SwarmNodeKind::RenewAgentQueueCredential { agent } => renew_queue_credential(&agent).await, - SwarmNodeKind::SetAgentWanted { hive, agent } => match deps.wanted { - None => Outcome::Failed( - "no swarm queue is configured on this host, so no wanted-state \ - declaration can be written" - .to_owned(), - ), - Some(writer) => declare_new_agent(&writer, &hive, &agent).await, - }, + SwarmNodeKind::SetAgentWanted { hive, agent } => { + set_agent_wanted(deps.wanted.as_deref(), &hive, &agent).await + } SwarmNodeKind::TriggerDeploy { hive, agent, rev } => match deps.queue { None => Outcome::Failed( "no swarm queue is configured on this host, so no hive can be told to deploy" @@ -607,8 +608,9 @@ async fn build_meta( deps: &WorkerDeps, forge: Option>, agent: String, + attempt: u32, ) -> hive_jobq::scheduler::Outcome { - meta_build::run(forge.as_deref(), deps.queue.as_ref(), agent).await + meta_build::run(forge.as_deref(), deps.queue.as_ref(), agent, attempt).await } /// What an `UpdateMeta` pass decided: the deploys to send, and the placements @@ -746,16 +748,21 @@ async fn record_fanout( /// parent. The build waits on it, since the build reads the head that update /// commits. The deploy keeps its own edge to it too: `skipped_deploys` finds /// the deploys a failed update skipped by that edge. +/// +/// `attempt` is the build's `BuildMeta::attempt`: 0 for a deploy no build has +/// failed for yet. fn gated_deploy<'a>( b: &'a hive_jobq::builder::JobBuilder, hive: &str, agent: &str, rev: Option, update: Option, + attempt: u32, ) -> hive_jobq::builder::NodeRef<'a, SwarmNodeKind, SwarmResourceKind> { let mut build = b .node(SwarmNodeKind::BuildMeta { agent: agent.to_owned(), + attempt, }) .needs(SwarmResourceKind::Builder); if let Some(update) = update { @@ -790,7 +797,7 @@ fn declare_fan_out( } let deploys: Vec = placements .iter() - .map(|(hive, agent)| gated_deploy(grown, hive, agent, None, None).guid()) + .map(|(hive, agent)| gated_deploy(grown, hive, agent, None, None, 0).guid()) .collect(); if let FanOut::Record { pin } = fan_out { let mut record = grown.node(SwarmNodeKind::RecordFanout { pin: pin.clone() }); @@ -967,6 +974,23 @@ async fn mint_hive_sender(homeserver: Option<&str>, hive: &str) -> hive_jobq::sc } } +/// The `SetAgentWanted` arm, lifted out so `run_node_work` stays under the line +/// limit: it fails without a wanted-state writer. +async fn set_agent_wanted( + wanted: Option<&wanted::WantedWriter>, + hive: &str, + agent: &str, +) -> hive_jobq::scheduler::Outcome { + match wanted { + None => hive_jobq::scheduler::Outcome::Failed( + "no swarm queue is configured on this host, so no wanted-state \ + declaration can be written" + .to_owned(), + ), + Some(writer) => declare_new_agent(writer, hive, agent).await, + } +} + /// Declare a brand-new agent at [`NEW_AGENT_WANTED_STATE`] — unless it turns /// out not to be new: an agent that already has a declaration (other than /// `Destroyed`, which this treats as reusable) is left alone, so a retried @@ -1211,7 +1235,7 @@ fn merged_deploy_job( .node(SwarmNodeKind::UpdateMeta) .needs(SwarmResourceKind::Meta); let root = update_meta.guid(); - let _deploy = gated_deploy(b, hive, agent, Some(rev.to_owned()), Some(root)); + let _deploy = gated_deploy(b, hive, agent, Some(rev.to_owned()), Some(root), 0); vec![root] } @@ -1335,7 +1359,7 @@ pub(crate) fn declare_agent_job( // queue at all fails this node — but `TriggerDeploy` has nothing to // publish to on such a host either, so the strict edge cancels a node // that could not have succeeded anyway. - let trigger_deploy = gated_deploy(b, hive, agent, None, Some(update_meta.guid())) + let trigger_deploy = gated_deploy(b, hive, agent, None, Some(update_meta.guid()), 0) .after_any(mint_identity) .after_any(mint_forge_token) // `after_any` for the same reason: the container reads this token @@ -1540,7 +1564,8 @@ pub(crate) fn spawn_meta_update_timer( enum SkipCause { /// It waited on an `UpdateMeta` that `Failed`. FailedUpdate, - /// It waited on a `BuildMeta` that gave no verdict ([`meta_build::is_retried`]). + /// It waited on a `BuildMeta` that gave no verdict or whose first `nix` + /// error asks for another build ([`meta_build::is_retried`]). UnfinishedBuild, } @@ -1550,16 +1575,18 @@ struct SkippedDeploy { agent: String, rev: Option, cause: SkipCause, + /// The `BuildMeta::attempt` of the build that replaces the failed one. + attempt: u32, } /// The deploys the timer declares again: for each agent, its latest /// `TriggerDeploy` when the scheduler skipped it because an `UpdateMeta` it -/// waited on `Failed`, or because the `BuildMeta` it waited on gave no verdict. -/// A deploy skipped for another reason is left alone. A red build is a verdict -/// on the config, which a config merge answers with a build of its own. The -/// pause declaration or the config seeding failing means the update that would -/// unblock the deploy was skipped too, and deploying past those is the race -/// those edges close. +/// waited on `Failed`, or because the `BuildMeta` it waited on gave no verdict +/// or met its first `nix` error. A deploy skipped for another reason is left +/// alone. A red build, the second `nix` error, is a verdict on the config, which +/// a config merge answers with a build of its own. The pause declaration or the +/// config seeding failing means the update that would unblock the deploy was +/// skipped too, and deploying past those is the race those edges close. fn skipped_deploys( graph: &hive_jobq::Graph, ) -> Vec { @@ -1581,24 +1608,28 @@ fn skipped_deploys( let SwarmNodeKind::TriggerDeploy { hive, agent, rev } = &n.payload else { return None; }; - let waited_on = - |wanted: fn(&hive_jobq::Node) -> bool| { - n.deps.iter().any(|d| match d { - hive_jobq::Dep::Node { id, .. } => graph.node(*id).is_some_and(wanted), - hive_jobq::Dep::Resource { .. } => false, - }) - }; - let cause = if waited_on(|dep| { + let deps = || { + n.deps.iter().filter_map(|d| match d { + hive_jobq::Dep::Node { id, .. } => graph.node(*id), + hive_jobq::Dep::Resource { .. } => None, + }) + }; + let failed_update = deps().any(|dep| { matches!(dep.payload, SwarmNodeKind::UpdateMeta) && dep.state == hive_jobq::State::Failed - }) { - SkipCause::FailedUpdate - } else if waited_on(|dep| { - matches!(dep.payload, SwarmNodeKind::BuildMeta { .. }) - && dep.state == hive_jobq::State::Failed - && dep.error.as_deref().is_some_and(meta_build::is_retried) - }) { - SkipCause::UnfinishedBuild + }); + let unfinished_build = deps().find_map(|dep| match (&dep.payload, &dep.error) { + (SwarmNodeKind::BuildMeta { attempt, .. }, Some(error)) + if dep.state == hive_jobq::State::Failed && meta_build::is_retried(error) => + { + Some(meta_build::next_attempt(*attempt, error)) + } + _ => None, + }); + let (cause, attempt) = if failed_update { + (SkipCause::FailedUpdate, 0) + } else if let Some(attempt) = unfinished_build { + (SkipCause::UnfinishedBuild, attempt) } else { return None; }; @@ -1607,6 +1638,7 @@ fn skipped_deploys( agent: agent.clone(), rev: rev.clone(), cause, + attempt, }) }) .collect() @@ -1662,7 +1694,7 @@ fn queue_meta_update( }); let deploys: Vec<_> = skipped .into_iter() - .map(|s| gated_deploy(b, &s.hive, &s.agent, s.rev, update).guid()) + .map(|s| gated_deploy(b, &s.hive, &s.agent, s.rev, update, s.attempt).guid()) .collect(); update.map_or(deploys, |update| vec![update]) }) @@ -2357,7 +2389,7 @@ mod tests { hive_jobq::Dep::Node { id, when } => graph .node(*id) .filter(|n| { - matches!(&n.payload, SwarmNodeKind::BuildMeta { agent: a } if a == agent) + matches!(&n.payload, SwarmNodeKind::BuildMeta { agent: a, .. } if a == agent) }) .map(|n| (n, *when)), hive_jobq::Dep::Resource { .. } => None, @@ -2641,7 +2673,7 @@ mod tests { { Outcome::Failed("the hive did not take it".to_owned()) } - SwarmNodeKind::BuildMeta { agent } + SwarmNodeKind::BuildMeta { agent, .. } if failing_build.as_ref().is_some_and(|(a, _)| *a == agent) => { Outcome::Failed(failing_build.map(|(_, error)| error).expect("matched")) @@ -2902,7 +2934,7 @@ mod tests { .filter(|n| n.payload.label() == label) .filter(|n| n.state == State::Pending) .map(|n| match &n.payload { - SwarmNodeKind::BuildMeta { agent } + SwarmNodeKind::BuildMeta { agent, .. } | SwarmNodeKind::TriggerDeploy { agent, .. } => agent.clone(), other => panic!("not a build or a deploy: {other:?}"), }) @@ -3786,7 +3818,9 @@ mod tests { let error = error.clone(); move |_id, kind, builder| async move { let outcome = match kind { - SwarmNodeKind::BuildMeta { agent: a } if a == agent => Outcome::Failed(error), + SwarmNodeKind::BuildMeta { agent: a, .. } if a == agent => { + Outcome::Failed(error) + } _ => Outcome::Done, }; (builder, outcome) @@ -3885,6 +3919,84 @@ mod tests { assert!(!queue_meta_update(&sched).expect("tick")); assert_eq!(deploys_of(&sched, "atlas"), [State::Skipped]); } + + /// The `attempt` of `agent`'s builds, oldest first. + fn build_attempts(sched: &SharedSched, agent: &str) -> Vec { + let guard = sched.lock().unwrap(); + let mut nodes: Vec<_> = guard + .graph() + .nodes() + .filter_map(|n| match &n.payload { + SwarmNodeKind::BuildMeta { agent: a, attempt } if a == agent => { + Some((n.id, *attempt)) + } + _ => None, + }) + .collect(); + nodes.sort_by_key(|(id, _)| *id); + nodes.into_iter().map(|(_, attempt)| attempt).collect() + } + + fn merged_atlas() -> SharedSched { + let sched = fresh_sched(); + sched + .lock() + .unwrap() + .insert_job(None, |b| merged_deploy_job(b, "pr1ma", "atlas", "abc")) + .expect("the graph inserts"); + sched + } + + /// `nix` failing once is built again on the next tick, as the second + /// attempt; failing again is red, and the tick declares nothing more. + #[tokio::test] + async fn a_first_nix_error_is_built_once_more_and_the_second_is_red() { + use hive_jobq::State; + + let sched = merged_atlas(); + let first = format!("{}error: fetch failed", meta_build::FIRST_FAILURE_PREFIX); + drain_failing_build(&sched, "atlas", first).await; + assert_eq!(deploys_of(&sched, "atlas"), [State::Skipped]); + assert_eq!(build_attempts(&sched, "atlas"), [0]); + + assert!(queue_meta_update(&sched).expect("tick")); + assert_eq!(build_attempts(&sched, "atlas"), [0, 1]); + drain_failing_build(&sched, "atlas", "error: attribute missing".to_owned()).await; + assert_eq!( + deploys_of(&sched, "atlas"), + [State::Skipped, State::Skipped] + ); + + assert!(!queue_meta_update(&sched).expect("tick")); + assert_eq!(build_attempts(&sched, "atlas"), [0, 1]); + } + + /// A build that passes after a first `nix` error sends the deploy, once. + #[tokio::test] + async fn a_build_that_passes_after_a_first_nix_error_deploys() { + use hive_jobq::State; + + let sched = merged_atlas(); + let first = format!("{}error: fetch failed", meta_build::FIRST_FAILURE_PREFIX); + drain_failing_build(&sched, "atlas", first).await; + assert!(queue_meta_update(&sched).expect("tick")); + drain(&sched, false, false).await; + assert_eq!(deploys_of(&sched, "atlas"), [State::Skipped, State::Done]); + assert!(!queue_meta_update(&sched).expect("tick")); + } + + /// A build with no verdict keeps its attempt, however many ticks it takes, + /// so a builder that is away does not use up the second `nix` attempt. + #[tokio::test] + async fn a_build_with_no_verdict_keeps_its_attempt() { + let sched = merged_atlas(); + let away = format!("{}no builder", meta_build::RETRY_PREFIX); + drain_failing_build(&sched, "atlas", away.clone()).await; + assert!(queue_meta_update(&sched).expect("tick")); + drain_failing_build(&sched, "atlas", away).await; + assert!(queue_meta_update(&sched).expect("tick")); + assert_eq!(build_attempts(&sched, "atlas"), [0, 0, 0]); + } } #[cfg(test)] diff --git a/swarm-controller/src/main.rs b/swarm-controller/src/main.rs index dcde78ed..ad714199 100644 --- a/swarm-controller/src/main.rs +++ b/swarm-controller/src/main.rs @@ -45,6 +45,7 @@ mod forge; mod forge_account; mod forge_events; mod github_account; +mod held_deploys; mod inference; mod issue_report; mod jobq; @@ -176,6 +177,7 @@ async fn main() -> Result<()> { let jobq_metrics_provider = hive_jobq_metrics::spawn_exporter(Arc::clone(&jobq), "swarm-controller", jobq_http_client); register_cert_gauges(jobq_metrics_provider.as_ref()); + held_deploys::register(jobq_metrics_provider.as_ref(), &jobq); let webhook_secret = load_webhook_secret(); diff --git a/swarm-controller/src/meta_build.rs b/swarm-controller/src/meta_build.rs index d41e6ccc..06d832b1 100644 --- a/swarm-controller/src/meta_build.rs +++ b/swarm-controller/src/meta_build.rs @@ -8,11 +8,17 @@ //! key itself: the builder holds no grant on the bucket. A `Built` reply and //! nothing else writes, so a red build leaves the previous record. //! -//! The node ends one of three ways. `Done` is a green build. A red build, the -//! builder's `Failed` reply, is `Failed` with `nix`'s tail: the deploy behind it -//! is skipped and the Jobs page shows why. Anything that stopped the build from -//! giving a verdict is `Failed` with [`RETRY_PREFIX`], which -//! `jobq::queue_meta_update` retries on the meta timer. +//! The node ends one of three ways. `Done` is a green build. Every `Failed` skips +//! the deploy behind it, and the Jobs page shows why. What decides whether the +//! timer builds again (`jobq::queue_meta_update`) is the error text: +//! - No verdict (no builder, a timeout, a forge error, no `internal/meta` head, +//! a missing read grant, a failed closure write): [`RETRY_PREFIX`]. The next +//! build has the same `attempt`. +//! - `nix` exited with an error on the first attempt: [`FIRST_FAILURE_PREFIX`] +//! and `nix`'s tail. The next build is `attempt + 1`, which catches a fetch +//! hiccup inside `nix` without reading its output. +//! - `nix` exited with an error again: `nix`'s tail alone. That is a red build: +//! the deploy stays held until a config merge, and nothing retries it. use std::fmt::Display; use std::future::Future; @@ -29,28 +35,54 @@ use crate::meta_flake::{META_REPO, MetaForge}; /// What a `Failed` outcome that is not a verdict on the config starts with. pub(crate) const RETRY_PREFIX: &str = "not built, will be retried: "; -/// Whether a `Failed` node's `error` says the build gave no verdict. +/// What the `Failed` outcome of a first `nix` error starts with. +pub(crate) const FIRST_FAILURE_PREFIX: &str = "build failed, will be built once more: "; + +/// How many times `nix` may exit with an error before the build is red. +const NIX_ATTEMPTS: u32 = 2; + +/// Whether a `Failed` node's `error` asks for another build. pub(crate) fn is_retried(error: &str) -> bool { - error.starts_with(RETRY_PREFIX) + error.starts_with(RETRY_PREFIX) || error.starts_with(FIRST_FAILURE_PREFIX) +} + +/// The `attempt` of the build that follows a `Failed` build of `attempt` +/// with `error`: one more after an error `nix` gave, the same after none. +pub(crate) fn next_attempt(attempt: u32, error: &str) -> u32 { + if error.starts_with(FIRST_FAILURE_PREFIX) { + attempt + 1 + } else { + attempt + } } fn not_built(why: impl Display) -> Outcome { Outcome::Failed(format!("{RETRY_PREFIX}{why}")) } -/// Where `internal/meta`'s head is read: the forge in production, a fake in -/// the tests. -pub(crate) trait MetaHead { +/// Where `internal/meta` is read and shared with the builder: the forge in +/// production, a fake in the tests. +pub(crate) trait MetaRepo { /// The head commit; `None` when the repo has none. fn head(&self) -> impl Future>> + Send; + /// Make sure the builder's forge account can read the repo. + fn grant_builder_read(&self) -> impl Future> + Send; } -impl MetaHead for forge::Client { +impl MetaRepo for forge::Client { async fn head(&self) -> Result> { let read = self.head_files(SHARED_ORG, META_REPO, None, &[]); let head = crate::jobq::forge_node("build_meta", read).await?; Ok(head.map(|h| h.commit)) } + + async fn grant_builder_read(&self) -> Result<()> { + crate::jobq::forge_node( + "build_meta", + self.grant_meta_read(forge::builder_account::LOGIN), + ) + .await + } } /// Where a build is sent and its closure recorded: the queue in production, a @@ -104,10 +136,13 @@ impl MetaBuilds for async_nats::Client { } /// Build `agent` from `internal/meta`'s head and record the closure. +/// `attempt` is how many times `nix` has already exited with an error for this +/// deploy. pub(crate) async fn run( - head: Option<&impl MetaHead>, + head: Option<&impl MetaRepo>, queue: Option<&impl MetaBuilds>, agent: String, + attempt: u32, ) -> Outcome { let Some(head) = head else { return not_built( @@ -118,6 +153,12 @@ pub(crate) async fn run( let Some(queue) = queue else { return not_built("no swarm queue is configured on this host, so no build can be sent"); }; + // The repo is private, and the grant is otherwise made only by a meta update. + if let Err(e) = head.grant_builder_read().await { + return not_built(format!( + "the builder's read on internal/meta could not be granted: {e:#}" + )); + } let meta_rev = match head.head().await { Ok(Some(rev)) => rev, Ok(None) => return not_built("the swarm has not published internal/meta yet"), @@ -133,6 +174,9 @@ pub(crate) async fn run( match queue.build(request).await { Err(why) => not_built(why), Ok(FromMetaReply::Unavailable { reason }) => not_built(reason), + Ok(FromMetaReply::Failed { error_tail }) if attempt + 1 < NIX_ATTEMPTS => { + Outcome::Failed(format!("{FIRST_FAILURE_PREFIX}{error_tail}")) + } Ok(FromMetaReply::Failed { error_tail }) => Outcome::Failed(error_tail), Ok(FromMetaReply::Built { out_path, system }) => { let closure = AgentClosure { @@ -157,15 +201,34 @@ mod tests { const REV: &str = "0123456789abcdef0123456789abcdef01234567"; - struct FakeHead(Result, &'static str>); + /// `internal/meta` as the forge answers: the head read, and whether the + /// builder's read grant can be made. + struct FakeHead { + head: Result, &'static str>, + grant_fails: bool, + } - impl MetaHead for FakeHead { + impl FakeHead { + fn reading(head: Result, &'static str>) -> Self { + Self { + head, + grant_fails: false, + } + } + } + + impl MetaRepo for FakeHead { async fn head(&self) -> Result> { - match self.0 { + match self.head { Ok(rev) => Ok(rev.map(str::to_owned)), Err(e) => Err(anyhow::anyhow!(e)), } } + + async fn grant_builder_read(&self) -> Result<()> { + anyhow::ensure!(!self.grant_fails, "the forge refused the collaborator"); + Ok(()) + } } /// A queue that answers every build with `reply`, and keeps what it was @@ -215,13 +278,17 @@ mod tests { } async fn run_with(head: &FakeHead, queue: &FakeQueue) -> Outcome { - run(Some(head), Some(queue), "atlas".to_owned()).await + run_at(head, queue, 0).await + } + + async fn run_at(head: &FakeHead, queue: &FakeQueue, attempt: u32) -> Outcome { + run(Some(head), Some(queue), "atlas".to_owned(), attempt).await } #[tokio::test] async fn a_green_build_of_the_head_is_recorded_for_the_agent() { let queue = FakeQueue::built(); - let outcome = run_with(&FakeHead(Ok(Some(REV))), &queue).await; + let outcome = run_with(&FakeHead::reading(Ok(Some(REV))), &queue).await; assert_eq!(outcome, Outcome::Done); assert_eq!( *queue.sent.lock().unwrap(), @@ -241,22 +308,69 @@ mod tests { assert!(closure.built_unix > 0); } - /// The builder said the build is red: the node fails with `nix`'s tail, - /// which is not a retried failure, and nothing is recorded. The green test - /// above is the control that a record is written when the build is green. - #[tokio::test] - async fn a_red_build_fails_with_its_tail_and_records_nothing() { - let queue = FakeQueue::answering(Ok(FromMetaReply::Failed { + fn nix_error() -> FakeQueue { + FakeQueue::answering(Ok(FromMetaReply::Failed { error_tail: "error: builder for foo failed".to_owned(), - })); - let outcome = run_with(&FakeHead(Ok(Some(REV))), &queue).await; + })) + } + + /// `nix` exiting with an error the first time is asked for once more: the + /// failure carries the tail and the retry marker, and the next build is the + /// second attempt. Nothing is recorded. + #[tokio::test] + async fn the_first_nix_error_is_built_once_more() { + let queue = nix_error(); + let Outcome::Failed(error) = run_with(&FakeHead::reading(Ok(Some(REV))), &queue).await + else { + panic!("not failed") + }; + assert!(is_retried(&error), "{error}"); + assert!(error.ends_with("error: builder for foo failed"), "{error}"); + assert_eq!(next_attempt(0, &error), 1); + assert!(queue.recorded.lock().unwrap().is_empty()); + } + + /// The second `nix` error is a red build: the tail alone, nothing retries + /// it, nothing is recorded. The green test above is the control that a + /// record is written when the build is green. + #[tokio::test] + async fn the_second_nix_error_is_red_and_records_nothing() { + let queue = nix_error(); + let outcome = run_at(&FakeHead::reading(Ok(Some(REV))), &queue, 1).await; assert_eq!( outcome, Outcome::Failed("error: builder for foo failed".to_owned()) ); + assert!(!is_retried("error: builder for foo failed")); assert!(queue.recorded.lock().unwrap().is_empty()); } + /// A build with no verdict keeps its attempt, so a builder that is away for + /// hours does not use up the retry that a nix error is allowed. + #[test] + fn only_a_nix_error_uses_up_an_attempt() { + assert_eq!(next_attempt(0, &format!("{RETRY_PREFIX}no builder")), 0); + assert_eq!(next_attempt(1, &format!("{RETRY_PREFIX}no builder")), 1); + assert_eq!(next_attempt(0, &format!("{FIRST_FAILURE_PREFIX}tail")), 1); + } + + /// A read grant the forge will not make is the controller's precondition, + /// not the config's: nothing is sent and the failure is retried. + #[tokio::test] + async fn a_read_grant_that_cannot_be_made_is_retried_before_anything_is_sent() { + let queue = FakeQueue::built(); + let head = FakeHead { + grant_fails: true, + ..FakeHead::reading(Ok(Some(REV))) + }; + let Outcome::Failed(error) = run_with(&head, &queue).await else { + panic!("not failed") + }; + assert!(is_retried(&error), "{error}"); + assert!(error.contains("read on internal/meta"), "{error}"); + assert!(queue.sent.lock().unwrap().is_empty()); + } + #[tokio::test] async fn a_builder_that_gave_no_verdict_is_a_retried_failure_and_records_nothing() { for (name, reply) in [ @@ -272,7 +386,7 @@ mod tests { ), ] { let queue = FakeQueue::answering(reply); - let outcome = run_with(&FakeHead(Ok(Some(REV))), &queue).await; + let outcome = run_with(&FakeHead::reading(Ok(Some(REV))), &queue).await; let Outcome::Failed(error) = outcome else { panic!("{name}: not failed") }; @@ -287,7 +401,8 @@ mod tests { record_fails: true, ..FakeQueue::built() }; - let Outcome::Failed(error) = run_with(&FakeHead(Ok(Some(REV))), &queue).await else { + let Outcome::Failed(error) = run_with(&FakeHead::reading(Ok(Some(REV))), &queue).await + else { panic!("not failed") }; assert!(is_retried(&error), "{error}"); @@ -297,16 +412,21 @@ mod tests { #[tokio::test] async fn without_a_head_or_a_queue_nothing_is_sent_and_the_failure_is_retried() { for (name, head, with_queue) in [ - ("no repo yet", Some(FakeHead(Ok(None))), true), - ("forge error", Some(FakeHead(Err("forge down"))), true), + ("no repo yet", Some(FakeHead::reading(Ok(None))), true), + ( + "forge error", + Some(FakeHead::reading(Err("forge down"))), + true, + ), ("no forge", None, true), - ("no queue", Some(FakeHead(Ok(Some(REV)))), false), + ("no queue", Some(FakeHead::reading(Ok(Some(REV)))), false), ] { let queue = FakeQueue::built(); let outcome = run( head.as_ref(), with_queue.then_some(&queue), "atlas".to_owned(), + 0, ) .await; let Outcome::Failed(error) = outcome else { diff --git a/swarm-wire/src/builder.rs b/swarm-wire/src/builder.rs index 30692cc3..8ea62271 100644 --- a/swarm-wire/src/builder.rs +++ b/swarm-wire/src/builder.rs @@ -142,12 +142,12 @@ pub enum FromMetaReply { /// The toplevel built; `out_path` is its store path in the builder host's /// store and `system` the platform the builder host builds for. Built { out_path: String, system: String }, - /// `nix` ran and the build did not finish: the last lines it printed, or - /// the deadline it hit. The controller holds the deploy back. + /// `nix` ran and exited with an error: the last lines it printed. The + /// controller builds once more, and holds the deploy back if that fails too. Failed { error_tail: String }, - /// The builder did not get to run `nix`: the build slot stayed busy or the - /// forge token could not be read. Says nothing about the config, so the - /// controller tries again. + /// `nix` gave no exit: the build slot stayed busy, the forge token could + /// not be read, `nix` could not start, or it hit its deadline. Says nothing + /// about the config, so the controller tries again. Unavailable { reason: String }, }