diff --git a/docs/agent-lifecycle/config-changes.md b/docs/agent-lifecycle/config-changes.md index b79f5401..7bf08820 100644 --- a/docs/agent-lifecycle/config-changes.md +++ b/docs/agent-lifecycle/config-changes.md @@ -149,8 +149,7 @@ every agent any hive places (the pins: every agent from it. While the repo has no commit that pins an agent's current config, that agent's build fails and names the agent. The controller queues its update of the repo before the deploy request for a merged config commit -and for a new agent, so a failure here is rare. It sends the deploy even when -that update failed, and the build then fails as described. +and for a new agent, and sends the deploy only when that update succeeded. - **Choosing the commit.** The rebuild's `MetaSync` node reads the repo's default-branch head with the hive's read token (`forge::reader_token()`) diff --git a/docs/swarm/builder.md b/docs/swarm/builder.md index 71bb26a6..3d2c3363 100644 --- a/docs/swarm/builder.md +++ b/docs/swarm/builder.md @@ -146,10 +146,10 @@ The controller commits `flake.nix` and the returned `flake.lock` to repo. It re-renders when it starts, and again every ten minutes while the latest update hasn't succeeded (a tick adds nothing while one is pending or running, or after a success), before it deploys a merged config PR, and -before the first deploy of a new agent. Those deploys run whether or not the -update succeeded: a hive builds only from `internal/meta`, so while the repo -doesn't pin the agent's current config that build fails and names the agent -(see +before the first deploy of a new agent. Those deploys run only after the +update succeeded: a hive builds only from `internal/meta`, so after a failed +update the scheduler skips the deploy node, and the graph shows the update as +failed and the deploy as skipped (see [Config changes](../agent-lifecycle/config-changes.md#the-swarms-meta-flake)). A render equal to the committed `flake.nix` sends no lock. diff --git a/swarm-controller/src/jobq.rs b/swarm-controller/src/jobq.rs index f2614181..1054a8ae 100644 --- a/swarm-controller/src/jobq.rs +++ b/swarm-controller/src/jobq.rs @@ -1139,10 +1139,10 @@ pub(crate) async fn queue_merged_config_deploy( /// The sub-DAG a merged config PR is: the meta update, then the deploy of /// `rev` on `hive`. /// -/// `after_any`: the deploy is asked for even when the update failed (no -/// builder, an unreadable config repo). A hive builds only from -/// `internal/meta`, so it fails that build, naming the agent, while the repo -/// does not pin the agent's config commit. +/// `after_ok`: a hive builds only from `internal/meta`, so a deploy asked for +/// after a failed update (no builder, an unreadable config repo) could only +/// fail its build. The failed update is `Failed` in the graph and the deploy +/// `Skipped`. fn merged_deploy_job( b: &hive_jobq::builder::JobBuilder, hive: &str, @@ -1159,7 +1159,7 @@ fn merged_deploy_job( agent: agent.to_owned(), rev: Some(rev.to_owned()), }) - .after_any(update_meta); + .after_ok(update_meta); vec![root] } @@ -1251,10 +1251,9 @@ pub(crate) fn declare_agent_job( hive: hive.to_owned(), agent: agent.to_owned(), }); - // The hive builds the agent from `internal/meta` and fails that build, - // naming the agent, until the repo pins the agent's config commit. - // `after_any` on the update, so one that failed (no builder, an unreadable - // config repo) still asks for the deploy. + // The hive builds the agent from `internal/meta` only, so the deploy needs + // the update to succeed (`after_ok`): after a failed one (no builder, an + // unreadable config repo) the hive's build could only fail. let update_meta = b .node(SwarmNodeKind::UpdateMeta) .needs(SwarmResourceKind::Meta) @@ -1289,7 +1288,7 @@ pub(crate) fn declare_agent_job( agent: agent.to_owned(), rev: None, }) - .after_any(update_meta) + .after_ok(update_meta) .after_any(mint_identity) .after_any(mint_forge_token) // `after_any` for the same reason: the container reads this token @@ -2142,39 +2141,50 @@ mod tests { .accepts(hive_jobq::TerminalState::Failed) } - /// A deploy that waits for the meta update still runs when the update - /// fails. + type TestBuilder = hive_jobq::builder::JobBuilder; + + fn create_dag(b: &TestBuilder) { + let _ = super::declare_agent_job(b, "atlas", "pr1ma", false); + } + + fn merged_dag(b: &TestBuilder) { + let _ = super::merged_deploy_job(b, "pr1ma", "atlas", "abc"); + } + + /// A deploy needs the meta update to succeed: its edge refuses a failed + /// update, in both DAGs that queue one. #[test] - fn a_failed_meta_update_does_not_hold_back_the_deploys() { - let create = |b: &hive_jobq::builder::JobBuilder| { - let _ = super::declare_agent_job(b, "atlas", "pr1ma", false); - }; - let merge = |b: &hive_jobq::builder::JobBuilder| { - let _ = super::merged_deploy_job(b, "pr1ma", "atlas", "abc"); - }; - assert!(edge_accepts_failure( - create, + fn a_deploy_needs_the_meta_update_to_succeed() { + assert!(!edge_accepts_failure( + create_dag, "trigger_deploy", "update_meta" )); - assert!(edge_accepts_failure(merge, "trigger_deploy", "update_meta")); - // The same lookup finds the edges that do hold a deploy back. assert!(!edge_accepts_failure( - create, + merged_dag, "trigger_deploy", - "set_agent_wanted" + "update_meta" + )); + // Controls: the same lookup finds the edges that do accept a failure. + assert!(edge_accepts_failure( + create_dag, + "trigger_deploy", + "mint_agent_identity" )); assert!(!edge_accepts_failure( - create, - "update_meta", + create_dag, + "trigger_deploy", "set_agent_wanted" )); } - /// The scheduler's side of the same edge: with the update failed, the - /// deploy is the next node to run. - #[tokio::test] - async fn the_merged_deploy_runs_after_a_failed_meta_update() { + /// Every node of the DAG `declare` builds, run to the end with the meta + /// update failing or succeeding and every other node succeeding: the + /// final state of each, by label. + async fn run_dag( + declare: impl FnOnce(&TestBuilder), + update_meta_fails: bool, + ) -> Vec<(String, hive_jobq::State)> { use hive_jobq::scheduler::{Outcome, Scheduler}; use hive_jobq_wire::WireNode as _; @@ -2184,39 +2194,62 @@ mod tests { ); sched .insert_job(None, |b| { - super::merged_deploy_job(b, "pr1ma", "atlas", "abc") + declare(b); + Vec::new() }) .expect("the graph inserts"); let sched = std::sync::Arc::new(std::sync::Mutex::new(sched)); - let mut ran = Vec::new(); while let Some(runner) = Scheduler::claim_next(&sched, |_id, kind, builder| async move { let outcome = match kind { - SwarmNodeKind::UpdateMeta => Outcome::Failed("no builder".to_owned()), + SwarmNodeKind::UpdateMeta if update_meta_fails => { + Outcome::Failed("no builder".to_owned()) + } _ => Outcome::Done, }; (builder, outcome) }) { runner.await.1.expect("no growth declared"); - ran.push( - sched - .lock() - .unwrap() - .graph() - .nodes() - .filter(|n| n.state.is_terminal()) - .map(|n| (n.payload.label(), n.state)) - .collect::>(), + } + let guard = sched.lock().unwrap(); + guard + .graph() + .nodes() + .map(|n| (n.payload.label(), n.state)) + .collect() + } + + fn state_of(states: &[(String, hive_jobq::State)], label: &str) -> hive_jobq::State { + states + .iter() + .find(|(l, _)| l == label) + .unwrap_or_else(|| panic!("the graph holds a {label} node")) + .1 + } + + /// The scheduler's side of the same edge, for both DAGs that queue an + /// update: with it failed, the deploy never runs. The graph shows the + /// update `Failed` and the deploy `Skipped`; the first run in each pair is + /// the control, where the deploy does run. + #[tokio::test] + async fn a_failed_meta_update_means_no_deploy_runs() { + use hive_jobq::State; + + for (name, declare) in [ + ("merged", merged_dag as fn(&TestBuilder)), + ("create", create_dag as fn(&TestBuilder)), + ] { + let ok = run_dag(declare, false).await; + assert_eq!(state_of(&ok, "update_meta"), State::Done, "{name}"); + assert_eq!(state_of(&ok, "trigger_deploy"), State::Done, "{name}"); + + let failed = run_dag(declare, true).await; + assert_eq!(state_of(&failed, "update_meta"), State::Failed, "{name}"); + assert_eq!( + state_of(&failed, "trigger_deploy"), + State::Skipped, + "{name}: the deploy must not run after a failed update" ); } - let last = ran.last().expect("both nodes ran"); - assert!( - last.contains(&("update_meta".to_owned(), hive_jobq::State::Failed)), - "{last:?}" - ); - assert!( - last.contains(&("trigger_deploy".to_owned(), hive_jobq::State::Done)), - "{last:?}" - ); } /// A fan-out record held in memory; `unreadable` stands for a bucket that