job_queue: fix the boot sweep's lost declarations, drop the node wrapper
Two review findings on the resources-at-construction change. argus: `workers::auto_update`'s boot sweep constructs nodes through `templates::node` too, and it was not converted. With the kind-derived declaration gone, its sweep `MetaLock` and its per-agent `Reconcile` silently declared no resources at all — so a boot reconcile no longer held the agent lease and could race another DAG's container ops, and the sweep's meta commit could land inside another node's staged deploy window. Nothing failed to compile: removing an implicit behaviour from a helper is invisible at every call site that relied on it. The declarations now live in a pure `boot_nodes`, split out of `submit_boot_tree` so they can be exercised without a `Coordinator`. That path is the only place job nodes are built outside `job_queue/`, which is exactly why it had no coverage; `boot_sweep_nodes_declare_ their_own_resources` closes that, asserting against declared graph edges rather than against the kind. mara: `templates::node` is a redundant redirect now that it no longer derives resources — deleted, and its 43 call sites use `Job::node` directly. The reasoning it documented moved to the module docs of `templates.rs` and `resource.rs`, which is where it stays true.
This commit is contained in:
parent
10dbdb444d
commit
58a9f218f2
6 changed files with 231 additions and 180 deletions
|
|
@ -766,7 +766,7 @@ fn a_fanned_out_start_declares_the_lease_and_re_enters_its_reconciles_grant() {
|
|||
agent: "agent-a".to_owned(),
|
||||
};
|
||||
let lease = Resource::Agent(kind.agent().to_owned());
|
||||
let _ = templates::node(b, kind).needs(lease);
|
||||
let _ = b.node(kind).needs(lease);
|
||||
}),
|
||||
reconcile.node_id,
|
||||
);
|
||||
|
|
@ -798,6 +798,57 @@ fn a_fanned_out_start_declares_the_lease_and_re_enters_its_reconciles_grant() {
|
|||
assert_eq!(state_of(&q, rival), State::Done);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn boot_sweep_nodes_declare_their_own_resources() {
|
||||
// Regression, and the reason it needs its own test: `workers::auto_update`
|
||||
// is the only place job nodes are constructed *outside* `job_queue/`, so
|
||||
// nothing in this module covered it. When resource derivation moved to the
|
||||
// construction sites, this path was missed and both kinds silently declared
|
||||
// nothing — dropping the agent lease a boot `Reconcile` needs to not race
|
||||
// another DAG's container ops, and letting the sweep `MetaLock` land its
|
||||
// meta commit inside another node's staged deploy window. Nothing failed to
|
||||
// compile; only an exhaustive caller list would have caught it.
|
||||
let q = JobQueue::new(4);
|
||||
let _id = submit(
|
||||
&q,
|
||||
DagSpec {
|
||||
source: Source::AutoUpdate,
|
||||
reason: "boot".to_owned(),
|
||||
declare: Box::new(|b: &Job| {
|
||||
crate::workers::auto_update::boot_nodes(
|
||||
b,
|
||||
true,
|
||||
vec!["stale-agent".to_owned()],
|
||||
vec!["drifted-agent".to_owned()],
|
||||
);
|
||||
}),
|
||||
},
|
||||
);
|
||||
|
||||
// Both are independent roots on disjoint resources, so both start at once.
|
||||
let claims = q.claim_ready();
|
||||
let by_kind = |kind: &str| {
|
||||
claims
|
||||
.iter()
|
||||
.find(|c| c.kind.as_str() == kind)
|
||||
.unwrap_or_else(|| panic!("no {kind} claim in {claims:?}"))
|
||||
};
|
||||
|
||||
let mut lock = declared_resources(&q, by_kind("meta_lock").node_id);
|
||||
lock.sort_by_key(|r| format!("{r:?}"));
|
||||
assert_eq!(
|
||||
lock,
|
||||
vec![Resource::BuildSlot, Resource::MetaWindow],
|
||||
"the sweep MetaLock runs a nix lock bump and commits to meta"
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
declared_resources(&q, by_kind("reconcile").node_id),
|
||||
vec![Resource::Agent("drifted-agent".to_owned())],
|
||||
"a boot Reconcile touches the container, so it holds that agent's lease"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn append_subgraph_roots_on_emitter_and_rebases_local_deps() {
|
||||
// The startup-sweep mechanism: a `MetaLock` emitter grows one rebuild
|
||||
|
|
@ -808,14 +859,11 @@ fn append_subgraph_roots_on_emitter_and_rebases_local_deps() {
|
|||
source: Source::AutoUpdate,
|
||||
reason: "sweep".to_owned(),
|
||||
declare: Box::new(|b: &Job| {
|
||||
let _lock = templates::node(
|
||||
b,
|
||||
NodeKind::MetaLock {
|
||||
sweep: true,
|
||||
fanout: None,
|
||||
inputs: Vec::new(),
|
||||
},
|
||||
);
|
||||
let _lock = b.node(NodeKind::MetaLock {
|
||||
sweep: true,
|
||||
fanout: None,
|
||||
inputs: Vec::new(),
|
||||
});
|
||||
}),
|
||||
};
|
||||
let id = submit(&q, spec);
|
||||
|
|
|
|||
Loading…
Reference in a new issue