From 567cd1230004439d525a87cb86075c0161e1ebcb Mon Sep 17 00:00:00 2001 From: atlas Date: Sun, 1 Nov 2026 14:20:00 +0100 Subject: [PATCH] closure_pullable judges an agent by its own pins, not the whole meta head --- docs/swarm/builder.md | 2 +- hive-c0re/src/attic/probe.rs | 65 +++++++++++++++++++++++---- hive-c0re/src/job_queue/exec.rs | 14 +++++- hive-c0re/src/job_queue/exec/tests.rs | 27 ++++++++++- hive-c0re/src/meta_rev.rs | 61 +++++++++++++++++++++++++ 5 files changed, 158 insertions(+), 11 deletions(-) diff --git a/docs/swarm/builder.md b/docs/swarm/builder.md index f06bdb00..cf00c877 100644 --- a/docs/swarm/builder.md +++ b/docs/swarm/builder.md @@ -462,7 +462,7 @@ decides the reason: | ---------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `no-credentials` | One of the four leaves is missing or unusable (a URL that isn't `https` is unusable). The swarm has no cache, the controller is too old to copy them, or you haven't seeded them yet. | | `no-record` | `agent-closures` holds no build for the agent. | -| `rev-mismatch` | The record's build came from another `internal/meta` commit than its head, or the hive can't read the head. | +| `rev-mismatch` | The record's build came from another `internal/meta` commit than its head that pins the agent's hyperhive or config differently, or the hive can't read the head. | | `not-cached` | The record says the builder didn't push the closure, or the cache answers 404 for the narinfo. | | `fetch-failed` | The request failed, or the cache answered with a status other than 200 or 404 (a refused token is a 401 or 403). | | `bad-signature` | The narinfo describes another path, or no signature by the public key covers it. | diff --git a/hive-c0re/src/attic/probe.rs b/hive-c0re/src/attic/probe.rs index 46f4f98d..b7d7389b 100644 --- a/hive-c0re/src/attic/probe.rs +++ b/hive-c0re/src/attic/probe.rs @@ -8,7 +8,8 @@ //! //! The verdict is the first of these that fails, in this order: //! no credentials (`no-credentials`), no record (`no-record`), a record built -//! from another `internal/meta` commit than the head (`rev-mismatch`), a record +//! from another `internal/meta` commit than the head that pins the agent +//! differently (`rev-mismatch`), a record //! the builder did not push (`not-cached`), the cache not holding the narinfo //! (`not-cached`), a request that failed or was refused (`fetch-failed`), a //! narinfo no signature of the key covers (`bad-signature`). @@ -83,15 +84,17 @@ impl Cache for Http { } } -/// `record` when it is the latest build of the `internal/meta` commit `head` -/// and the builder pushed it to the cache. +/// `record` when it was built from the `internal/meta` commit `head`, or from +/// one that pins the agent as `head` does (`same_pins`), and the builder pushed +/// it to the cache. pub(crate) fn current_record<'a>( record: Option<&'a AgentClosure>, head: Option<&str>, + same_pins: bool, ) -> Result<&'a AgentClosure, Unpullable> { let record = record.ok_or_else(|| no(Reason::NoRecord, "no build is recorded for the agent"))?; - if head != Some(record.meta_rev.as_str()) { + if head != Some(record.meta_rev.as_str()) && !same_pins { return Err(no( Reason::RevMismatch, format!( @@ -111,11 +114,13 @@ pub(crate) fn current_record<'a>( } /// Whether the closure `record` names can be pulled with `facts`. `head` is the -/// `internal/meta` commit a deploy would build from. +/// `internal/meta` commit a deploy would build from; `same_pins` is whether the +/// record's commit pins the agent as `head` does. pub(crate) async fn judge( facts: Option<&Facts>, record: Option<&AgentClosure>, head: Option<&str>, + same_pins: bool, cache: &impl Cache, ) -> Result<(), Unpullable> { let facts = facts.ok_or_else(|| { @@ -124,7 +129,7 @@ pub(crate) async fn judge( "the swarm has given this hive no cache facts", ) })?; - let record = current_record(record, head)?; + let record = current_record(record, head, same_pins)?; let text = match cache.narinfo(facts, &record.store_path).await { Ok(Some(text)) => text, Ok(None) => return Err(no(Reason::NotCached, "the cache does not hold the narinfo")), @@ -196,7 +201,18 @@ async fn pass(client: &async_nats::Client) -> Result<()> { Some(_) => read_record(client, &spec.name).await?, None => None, }; - let verdict = judge(facts.as_ref(), record.as_ref(), head.as_deref(), &http).await; + let same_pins = match (&record, head.as_deref()) { + (Some(r), Some(head)) => pins_unchanged(&spec.name, &r.meta_rev, head).await, + _ => false, + }; + let verdict = judge( + facts.as_ref(), + record.as_ref(), + head.as_deref(), + same_pins, + &http, + ) + .await; now.push((spec.name, verdict.as_ref().err().map(|u| u.reason))); report(&now[now.len() - 1], verdict.err().map(|u| u.detail)); } @@ -206,6 +222,26 @@ async fn pass(client: &async_nats::Client) -> Result<()> { Ok(()) } +/// Whether `agent` has the same pins at `built` as at `head`. A commit that +/// cannot be read counts as different, which leaves the verdict to the commit +/// ids. +async fn pins_unchanged(agent: &str, built: &str, head: &str) -> bool { + if built == head { + return true; + } + let read = async { + let archetype = crate::archetypes::of(agent)?.is_some(); + let source = crate::forge::internal_meta_source() + .await + .context("no forge token")?; + crate::meta_rev::pins_unchanged(&source, agent, archetype, built, head).await + }; + read.await.unwrap_or_else(|e| { + tracing::debug!(%agent, error = %format!("{e:#}"), "attic: cannot compare the agent's pins"); + false + }) +} + pub(crate) async fn read_record( client: &async_nats::Client, agent: &str, @@ -307,7 +343,7 @@ mod tests { head: Option<&str>, cache: Answers, ) -> Option { - judge(facts, record, head, &cache) + judge(facts, record, head, false, &cache) .await .err() .map(|u| u.reason) @@ -345,6 +381,19 @@ mod tests { assert_eq!(r, Some(Reason::RevMismatch), "no head to compare with"); } + #[tokio::test] + async fn a_record_from_another_commit_with_the_same_pins_is_pullable() { + let (facts, record) = (facts(), record()); + let judged = judge(Some(&facts), Some(&record), Some("other"), true, &good()).await; + assert!(judged.is_ok()); + let unpushed = AgentClosure { + cached: false, + ..record + }; + let judged = judge(Some(&facts), Some(&unpushed), Some("other"), true, &good()).await; + assert_eq!(judged.err().map(|u| u.reason), Some(Reason::NotCached)); + } + #[tokio::test] async fn an_unpushed_record_or_a_missing_narinfo_is_not_cached() { let unpushed = AgentClosure { diff --git a/hive-c0re/src/job_queue/exec.rs b/hive-c0re/src/job_queue/exec.rs index 39f0994b..309e3e85 100644 --- a/hive-c0re/src/job_queue/exec.rs +++ b/hive-c0re/src/job_queue/exec.rs @@ -591,7 +591,7 @@ async fn pull_closure( host.realise_closure(name, &record.store_path) .await .map_err(|e| NoClosure::Unpullable(format!("{e:#}")))?; - if record.meta_rev != rev { + if record.meta_rev != rev && !same_pins(host, name, &record.meta_rev, rev).await { tracing::warn!( %name, %rev, built_from = %record.meta_rev, "{step}: the swarm built {} from internal/meta {}, not the {rev} this hive chose; deploying the swarm's", @@ -604,6 +604,18 @@ async fn pull_closure( }) } +/// Whether the agent has the same pins at the commits `built` and `chosen`. A +/// commit whose pins cannot be read counts as different. +async fn same_pins(host: &impl Host, name: &str, built: &str, chosen: &str) -> bool { + match ( + host.pins_at(name, built).await, + host.pins_at(name, chosen).await, + ) { + (Ok(built), Ok(chosen)) => built.same_pins(&chosen), + _ => false, + } +} + /// The pins to record for a deploy of `closure` when this hive chose the commit /// `rev`: those of the commit the closure was built from, so the markers say /// what runs. `None` when they are the same commit. diff --git a/hive-c0re/src/job_queue/exec/tests.rs b/hive-c0re/src/job_queue/exec/tests.rs index a95f947e..bf694a30 100644 --- a/hive-c0re/src/job_queue/exec/tests.rs +++ b/hive-c0re/src/job_queue/exec/tests.rs @@ -41,6 +41,8 @@ struct FakeHost { failing: Vec<&'static str>, /// The sweeps and pulls never finish, so only a deadline ends them. hang: bool, + /// The agent has the same pins at every commit. + unmoved: bool, runtime: PathBuf, purge: Vec, calls: Mutex>, @@ -71,6 +73,11 @@ impl FakeHost { self } + fn unmoved(mut self) -> Self { + self.unmoved = true; + self + } + fn without_record(mut self) -> Self { self.built = None; self @@ -134,7 +141,11 @@ impl Host for FakeHost { Ok(MetaBuild { rev: rev.to_owned(), hyperhive: "dh".to_owned(), - config: Some("dc".to_owned()), + config: Some(if self.unmoved { + "dc".to_owned() + } else { + format!("dc-{rev}") + }), }) } @@ -442,6 +453,16 @@ async fn a_rebuild_without_a_closure_fails_the_prebuild_and_logs_an_error() { } } +/// Another commit that pins the agent the same is nothing to warn about. +#[tokio::test] +async fn a_closure_built_from_a_commit_with_the_same_pins_is_deployed_quietly() { + let host = FakeHost::up().built(OTHER_REV, true).unmoved(); + let (result, log) = logging(run_prebuild(&host, "alice")).await; + result.expect("drift is not a refusal"); + assert!(host.calls().contains(&format!("realise alice {CLOSURE}"))); + assert!(log.at(tracing::Level::WARN).is_empty()); +} + /// The builder's record is not for the commit this hive chose: its closure is /// deployed and the difference is a warning. #[tokio::test] @@ -546,6 +567,8 @@ async fn swap_applies_a_closure_built_from_another_commit_with_a_warning() { "closure_record alice".to_owned(), format!("realise alice {CLOSURE}"), format!("pins_at alice {OTHER_REV}"), + format!("pins_at alice {REV}"), + format!("pins_at alice {OTHER_REV}"), format!("swap_update alice {CLOSURE} node={}", id.get()), format!("record_meta alice {OTHER_REV}"), ] @@ -943,6 +966,8 @@ async fn create_uses_a_closure_built_from_another_commit_with_a_warning() { "closure_record alice".to_owned(), format!("realise alice {CLOSURE}"), format!("pins_at alice {OTHER_REV}"), + format!("pins_at alice {REV}"), + format!("pins_at alice {OTHER_REV}"), format!("create_only alice {CLOSURE}"), format!("mark_deployed alice {OTHER_REV}"), ] diff --git a/hive-c0re/src/meta_rev.rs b/hive-c0re/src/meta_rev.rs index 1573a152..700c1523 100644 --- a/hive-c0re/src/meta_rev.rs +++ b/hive-c0re/src/meta_rev.rs @@ -40,6 +40,15 @@ pub(crate) struct MetaBuild { pub config: Option, } +impl MetaBuild { + /// Whether `other` pins the same hyperhive and config commits: what decides + /// whether the agent's closure is rebuilt, whichever `internal/meta` commit + /// holds them. + pub(crate) fn same_pins(&self, other: &MetaBuild) -> bool { + self.hyperhive == other.hyperhive && self.config == other.config + } +} + /// The `internal/meta` commit to deploy `agent` from. /// /// The head is taken only when its `flake.nix` builds the agent the way it is @@ -166,6 +175,23 @@ pub(crate) async fn pins_at( }) } +/// Whether `agent` has the same pins at the commits `built` and `head`, so a +/// closure built from `built` is as good as one built from `head`. +/// +/// # Errors +/// As [`pins_at`], for either commit. +pub(crate) async fn pins_unchanged( + source: &impl MetaSource, + agent: &str, + archetype: bool, + built: &str, + head: &str, +) -> Result { + let built = pins_at(source, agent, archetype, built).await?; + let head = pins_at(source, agent, archetype, head).await?; + Ok(built.same_pins(&head)) +} + /// [`pins_at`] against the forge this hive reads. /// /// # Errors @@ -338,6 +364,41 @@ mod tests { .unwrap() } + /// `internal/meta` commits, each with its flake. + struct Commits(Vec<(&'static str, String)>); + + impl MetaSource for Commits { + async fn meta_head(&self) -> Result> { + Ok(None) + } + async fn flake_at(&self, rev: &str) -> Result { + self.0 + .iter() + .find(|(r, _)| *r == rev) + .map(|(_, flake)| flake.clone()) + .with_context(|| format!("no commit {rev}")) + } + async fn config_head(&self, _: &str) -> Result> { + Ok(None) + } + } + + #[tokio::test] + async fn pins_unchanged_follows_the_agents_pins_not_the_commit() { + let moved = flake().replace(ARGUS_HEAD, STALE); + let commits = Commits(vec![(META, flake()), (STALE, flake()), (HYPERHIVE, moved)]); + let unchanged = + |agent, archetype, built| pins_unchanged(&commits, agent, archetype, built, META); + // Another commit, the same pins. + assert!(unchanged("argus", false, STALE).await.unwrap()); + assert!(unchanged("rev", true, STALE).await.unwrap()); + // The agent's own config pin moved; an archetype agent has none. + assert!(!unchanged("argus", false, HYPERHIVE).await.unwrap()); + assert!(unchanged("rev", true, HYPERHIVE).await.unwrap()); + // A commit that cannot be read is an error, not a verdict. + assert!(unchanged("argus", false, ARGUS_HEAD).await.is_err()); + } + fn forge(config_head: Option<&'static str>) -> Forge { Forge { head: Some(META),