fix(placement): a composed graph needs every backend its nodes name
The full harness found it — 12 of 13 scenarios green, `roster` red:
roster: the planner sized this mission at 2 member(s) PASS
roster: the approved roster is on the mission (2 nodes, composed) PASS
roster: this run added 1 line(s) for a 2-member roster FAIL
topology_runs.error: turn executor failed: node n1 in a microVM:
vm_create failed: no rootfs for backend "canary-claude" on this node
The roster proposed `verifier@canary-claude`. Placement asked
`online_for_backend` about the MISSION's backend — `claude` — and architect
answered, holding `claude` and `local-ornith`. The graph's first node ran and
delivered, the second could not boot, and the mission finished half-done. The
question placement asked was true and insufficient.
A composed graph runs on ONE node, so that node needs every image its nodes ask
for. `required_backends` collects the mission's plus each
`config.roster.nodes[].attrs.backend`, and `online_for_backends` passes the
whole set to the same jsonb `@>` — containment already means "contains ALL of
these", so the query shape did not have to change, only what it was asked.
This is the failure mode the roster feature creates by existing: its entire
purpose is putting a verifier on a different provider, which is exactly what
makes one node insufficient. Nothing before the full suite had a reason to
exercise it — the composed scenario uses one backend for all five nodes.
`NoCapableNode` now names the set and says why one node must hold all of them.
Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
171f901bcd
commit
529497febb
@@ -706,7 +706,26 @@ async fn launch_phase(
|
|||||||
// The node this phase will use. Starts as the mission's current pin (a
|
// The node this phase will use. Starts as the mission's current pin (a
|
||||||
// request), becomes whatever placement actually chose.
|
// request), becomes whatever placement actually chose.
|
||||||
let mut chosen_node = p.target_node_id;
|
let mut chosen_node = p.target_node_id;
|
||||||
match crate::vm_placement::choose(pool, hub, workspace_id, p.backend, chosen_node).await {
|
// A composed graph runs on ONE node, and its nodes may each name their
|
||||||
|
// own backend — an independent verifier on another provider is the
|
||||||
|
// roster's whole purpose. So the node must hold EVERY rootfs the graph
|
||||||
|
// asks for, not just the mission's. Read here rather than carried on
|
||||||
|
// `PhaseLaunch` because it is only the microVM path that cares.
|
||||||
|
let roster: Option<serde_json::Value> =
|
||||||
|
sqlx::query_scalar("SELECT config -> 'roster' FROM missions WHERE id = $1")
|
||||||
|
.bind(mission_id)
|
||||||
|
.fetch_optional(pool)
|
||||||
|
.await
|
||||||
|
.ok()
|
||||||
|
.flatten();
|
||||||
|
let backends = crate::vm_placement::required_backends(p.backend, roster.as_ref());
|
||||||
|
if backends.len() > 1 {
|
||||||
|
eprintln!(
|
||||||
|
"phase_runner: mission {mission_id} phase {phase_id} needs {backends:?} \
|
||||||
|
on a single node (composed graph with per-node backends)"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
match crate::vm_placement::choose(pool, hub, workspace_id, &backends, chosen_node).await {
|
||||||
Ok(node) => {
|
Ok(node) => {
|
||||||
if chosen_node != Some(node.as_uuid()) {
|
if chosen_node != Some(node.as_uuid()) {
|
||||||
eprintln!(
|
eprintln!(
|
||||||
|
|||||||
@@ -420,7 +420,12 @@ pub async fn capacity(
|
|||||||
) -> Result<Json<Value>, ApiError> {
|
) -> Result<Json<Value>, ApiError> {
|
||||||
let ws = user.workspace_id.as_uuid().to_owned();
|
let ws = user.workspace_id.as_uuid().to_owned();
|
||||||
let (fit, unfit) =
|
let (fit, unfit) =
|
||||||
crate::vm_placement::survey(&state.pool, &state.node_hub, ws, q.backend.as_deref())
|
crate::vm_placement::survey(
|
||||||
|
&state.pool,
|
||||||
|
&state.node_hub,
|
||||||
|
ws,
|
||||||
|
&crate::vm_placement::required_backends(q.backend.as_deref(), None),
|
||||||
|
)
|
||||||
.await
|
.await
|
||||||
.map_err(|e| {
|
.map_err(|e| {
|
||||||
eprintln!("fleet capacity survey failed: {e}");
|
eprintln!("fleet capacity survey failed: {e}");
|
||||||
|
|||||||
@@ -310,6 +310,35 @@ pub fn commitments(live_vm_ids: &[String], pinned_keys: &[String]) -> i64 {
|
|||||||
live + unbooted
|
live + unbooted
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Every backend a phase needs on ONE node: the mission's, plus each backend
|
||||||
|
/// named by a node of its composed graph.
|
||||||
|
///
|
||||||
|
/// The roster stores them as `config.roster.nodes[].attrs.backend`, and they are
|
||||||
|
/// the reason this function exists. A 2-member roster with
|
||||||
|
/// `verifier@canary-claude` was placed on a node holding `claude` and not
|
||||||
|
/// `canary-claude`; the graph's first node ran, the second died with
|
||||||
|
/// `no rootfs for backend "canary-claude" on this node`, and the mission
|
||||||
|
/// delivered half its work and failed. Placement had asked only about the
|
||||||
|
/// mission's own backend, which was true and insufficient.
|
||||||
|
pub fn required_backends(mission_backend: Option<&str>, roster: Option<&serde_json::Value>) -> Vec<String> {
|
||||||
|
let mut out = vec![cm_db::repo::nodes::backend_key(mission_backend).to_string()];
|
||||||
|
if let Some(nodes) = roster.and_then(|r| r.get("nodes")).and_then(|n| n.as_array()) {
|
||||||
|
for n in nodes {
|
||||||
|
if let Some(b) = n
|
||||||
|
.get("attrs")
|
||||||
|
.and_then(|a| a.get("backend"))
|
||||||
|
.and_then(|b| b.as_str())
|
||||||
|
.filter(|b| !b.trim().is_empty())
|
||||||
|
{
|
||||||
|
out.push(b.to_string());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
out.sort();
|
||||||
|
out.dedup();
|
||||||
|
out
|
||||||
|
}
|
||||||
|
|
||||||
/// Survey every capable node: which can take a phase VM, and why the rest cannot.
|
/// Survey every capable node: which can take a phase VM, and why the rest cannot.
|
||||||
///
|
///
|
||||||
/// `vm_list` is asked of each candidate in parallel with a short deadline. A node
|
/// `vm_list` is asked of each candidate in parallel with a short deadline. A node
|
||||||
@@ -319,11 +348,15 @@ pub async fn survey(
|
|||||||
pool: &sqlx::PgPool,
|
pool: &sqlx::PgPool,
|
||||||
hub: &crate::fleet::NodeHub,
|
hub: &crate::fleet::NodeHub,
|
||||||
workspace_id: uuid::Uuid,
|
workspace_id: uuid::Uuid,
|
||||||
backend: Option<&str>,
|
// EVERY backend the work needs, not just the mission's. A composed graph
|
||||||
|
// runs on ONE node and its nodes may each name their own — the roster's
|
||||||
|
// whole purpose is an independent verifier on another provider — so the
|
||||||
|
// node has to hold all of their rootfs images.
|
||||||
|
backends: &[String],
|
||||||
) -> Result<(Vec<NodeCapacity>, Vec<(NodeId, String, Unfit)>), String> {
|
) -> Result<(Vec<NodeCapacity>, Vec<(NodeId, String, Unfit)>), String> {
|
||||||
let candidates = cm_db::repo::nodes::online_for_backend(pool, workspace_id, backend)
|
let candidates = cm_db::repo::nodes::online_for_backends(pool, workspace_id, backends)
|
||||||
.await
|
.await
|
||||||
.map_err(|e| format!("looking up nodes for backend {backend:?}: {e}"))?;
|
.map_err(|e| format!("looking up nodes for backends {backends:?}: {e}"))?;
|
||||||
if candidates.is_empty() {
|
if candidates.is_empty() {
|
||||||
return Ok((Vec::new(), Vec::new()));
|
return Ok((Vec::new(), Vec::new()));
|
||||||
}
|
}
|
||||||
@@ -421,22 +454,23 @@ pub async fn choose(
|
|||||||
pool: &sqlx::PgPool,
|
pool: &sqlx::PgPool,
|
||||||
hub: &crate::fleet::NodeHub,
|
hub: &crate::fleet::NodeHub,
|
||||||
workspace_id: uuid::Uuid,
|
workspace_id: uuid::Uuid,
|
||||||
backend: Option<&str>,
|
backends: &[String],
|
||||||
want: Option<uuid::Uuid>,
|
want: Option<uuid::Uuid>,
|
||||||
) -> Result<NodeId, PlacementError> {
|
) -> Result<NodeId, PlacementError> {
|
||||||
|
let named = backends.join(", ");
|
||||||
let how_to_fix = format!(
|
let how_to_fix = format!(
|
||||||
"needs /dev/kvm + firecracker (scripts/fc-node-setup.sh) AND the {} rootfs \
|
"needs /dev/kvm + firecracker (scripts/fc-node-setup.sh) AND the {named} rootfs \
|
||||||
built on that node (scripts/fc-build-rootfs.sh <host> <image> {})",
|
built on ONE node (scripts/fc-build-rootfs.sh <host> <image> <name>) — a \
|
||||||
backend.unwrap_or("default"),
|
composed graph runs on a single node, so that node needs every image its \
|
||||||
backend.unwrap_or("<name>")
|
nodes ask for"
|
||||||
);
|
);
|
||||||
let (fit, unfit) = survey(pool, hub, workspace_id, backend).await.map_err(|e| {
|
let (fit, unfit) = survey(pool, hub, workspace_id, backends).await.map_err(|e| {
|
||||||
PlacementError::FleetUnreadable { report: format!(" survey failed: {e}") }
|
PlacementError::FleetUnreadable { report: format!(" survey failed: {e}") }
|
||||||
})?;
|
})?;
|
||||||
|
|
||||||
if fit.is_empty() && unfit.is_empty() {
|
if fit.is_empty() && unfit.is_empty() {
|
||||||
return Err(PlacementError::NoCapableNode {
|
return Err(PlacementError::NoCapableNode {
|
||||||
backend: backend.unwrap_or("default").to_string(),
|
backend: named,
|
||||||
how_to_fix,
|
how_to_fix,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
@@ -525,6 +559,46 @@ mod tests {
|
|||||||
/// filters on `status = 'online'`, so a draining node is not a candidate
|
/// filters on `status = 'online'`, so a draining node is not a candidate
|
||||||
/// and the pin falls through to ranking. `drain-midmission` passes on both
|
/// and the pin falls through to ranking. `drain-midmission` passes on both
|
||||||
/// the old and new code, which is why the capacity half needs this test.
|
/// the old and new code, which is why the capacity half needs this test.
|
||||||
|
/// A composed graph's per-node backends are part of what placement needs.
|
||||||
|
///
|
||||||
|
/// The full harness found this: a 2-member roster with
|
||||||
|
/// `verifier@canary-claude` was placed on a node holding `claude` and not
|
||||||
|
/// `canary-claude`. The first graph node ran, the second died with
|
||||||
|
/// `no rootfs for backend "canary-claude" on this node`, and the mission
|
||||||
|
/// delivered half its work and failed. Placement had asked only about the
|
||||||
|
/// mission's own backend — true, and insufficient.
|
||||||
|
#[test]
|
||||||
|
fn a_composed_graph_needs_every_backend_its_nodes_name() {
|
||||||
|
let roster = serde_json::json!({
|
||||||
|
"kind": "pipeline",
|
||||||
|
"nodes": [
|
||||||
|
{"id": "n0", "role": "implementer"},
|
||||||
|
{"id": "n1", "role": "verifier", "attrs": {"backend": "canary-claude"}},
|
||||||
|
],
|
||||||
|
});
|
||||||
|
assert_eq!(
|
||||||
|
required_backends(Some("claude"), Some(&roster)),
|
||||||
|
vec!["canary-claude".to_string(), "claude".to_string()],
|
||||||
|
"both images have to be on the ONE node the graph runs on"
|
||||||
|
);
|
||||||
|
|
||||||
|
// A solo mission is unchanged — this must not make ordinary placement
|
||||||
|
// stricter than it was.
|
||||||
|
assert_eq!(required_backends(Some("claude"), None), vec!["claude"]);
|
||||||
|
assert_eq!(required_backends(None, None), vec!["default"]);
|
||||||
|
|
||||||
|
// A node with no explicit backend inherits the mission's, so it adds
|
||||||
|
// nothing. Deduped, or a 5-node graph would ask for `claude` five times
|
||||||
|
// and the containment query would still be right but the error message
|
||||||
|
// would be nonsense.
|
||||||
|
let inherit = serde_json::json!({"nodes": [
|
||||||
|
{"id": "n0", "role": "a"},
|
||||||
|
{"id": "n1", "role": "b", "attrs": {}},
|
||||||
|
{"id": "n2", "role": "c", "attrs": {"backend": ""}},
|
||||||
|
]});
|
||||||
|
assert_eq!(required_backends(Some("claude"), Some(&inherit)), vec!["claude"]);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn an_unfit_previous_node_is_re_placed_not_refused() {
|
fn an_unfit_previous_node_is_re_placed_not_refused() {
|
||||||
let drained = uuid::Uuid::from_u128(1);
|
let drained = uuid::Uuid::from_u128(1);
|
||||||
|
|||||||
@@ -285,8 +285,25 @@ pub async fn online_for_backend(
|
|||||||
workspace_id: uuid::Uuid,
|
workspace_id: uuid::Uuid,
|
||||||
backend: Option<&str>,
|
backend: Option<&str>,
|
||||||
) -> Result<Vec<NodeId>, DbError> {
|
) -> Result<Vec<NodeId>, DbError> {
|
||||||
let want = backend_key(backend);
|
online_for_backends(pool, workspace_id, &[backend_key(backend).to_string()]).await
|
||||||
// `@>` on the array asks "does this node's list contain that name" — the
|
}
|
||||||
|
|
||||||
|
/// Nodes that can run EVERY one of these backends.
|
||||||
|
///
|
||||||
|
/// A composed mission runs its whole graph on one node, and the graph's nodes
|
||||||
|
/// may each name their own backend — an independent verifier on another
|
||||||
|
/// provider is the entire point of the roster. Asking only for the mission's
|
||||||
|
/// backend placed such a mission on a node with `claude` and no
|
||||||
|
/// `canary-claude`, and the run died at the second graph node with
|
||||||
|
/// `no rootfs for backend "canary-claude" on this node`. The full harness
|
||||||
|
/// caught it; nothing before it had a reason to.
|
||||||
|
pub async fn online_for_backends(
|
||||||
|
pool: &PgPool,
|
||||||
|
workspace_id: uuid::Uuid,
|
||||||
|
backends: &[String],
|
||||||
|
) -> Result<Vec<NodeId>, DbError> {
|
||||||
|
// `@>` on the array asks "does this node's list contain ALL of these" —
|
||||||
|
// containment, not intersection, which is exactly the question here and the
|
||||||
// whole reason the node reports an array rather than a count.
|
// whole reason the node reports an array rather than a count.
|
||||||
let rows: Vec<(uuid::Uuid,)> = sqlx::query_as(
|
let rows: Vec<(uuid::Uuid,)> = sqlx::query_as(
|
||||||
"SELECT id FROM nodes
|
"SELECT id FROM nodes
|
||||||
@@ -302,9 +319,12 @@ pub async fn online_for_backend(
|
|||||||
ORDER BY id",
|
ORDER BY id",
|
||||||
)
|
)
|
||||||
.bind(workspace_id)
|
.bind(workspace_id)
|
||||||
.bind(serde_json::Value::Array(vec![serde_json::Value::String(
|
.bind(serde_json::Value::Array(
|
||||||
want.to_string(),
|
backends
|
||||||
)]))
|
.iter()
|
||||||
|
.map(|b| serde_json::Value::String(b.clone()))
|
||||||
|
.collect(),
|
||||||
|
))
|
||||||
.fetch_all(pool)
|
.fetch_all(pool)
|
||||||
.await?;
|
.await?;
|
||||||
Ok(rows.into_iter().map(|(id,)| NodeId::from(id)).collect())
|
Ok(rows.into_iter().map(|(id,)| NodeId::from(id)).collect())
|
||||||
@@ -315,7 +335,7 @@ pub async fn online_for_backend(
|
|||||||
/// Must agree with `clawmates-node::microvm::rootfs_for`, which resolves the same
|
/// Must agree with `clawmates-node::microvm::rootfs_for`, which resolves the same
|
||||||
/// three spellings to the default image. If these two drift, placement promises
|
/// three spellings to the default image. If these two drift, placement promises
|
||||||
/// an image the booter cannot find — or refuses one it has.
|
/// an image the booter cannot find — or refuses one it has.
|
||||||
fn backend_key(backend: Option<&str>) -> &str {
|
pub fn backend_key(backend: Option<&str>) -> &str {
|
||||||
match backend {
|
match backend {
|
||||||
None | Some("") | Some("default") => "default",
|
None | Some("") | Some("default") => "default",
|
||||||
Some(b) => b,
|
Some(b) => b,
|
||||||
|
|||||||
Reference in New Issue
Block a user