feat(fleet): B4.3 — per-mission rootfs selection (missions.backend)
`vm_create` takes a backend name and boots `rootfs-<backend>.ext4`; NULL or "default" boots the golden image. Makes the per-CLI images from B4.1 actually reachable (one image per CLI, per A6). A missing image is an ERROR naming the file and how to build it, never a quiet fall back to the default. That fallback is the tempting version and the wrong one: it would run a claude mission in a kimi VM, or in a rootfs with no CLI at all, and report success for whatever came out. Verified on real hardware, not just in a unit test — the selftest asks for an image that does not exist and FAILS if it boots. `create` now reports the rootfs that actually booted, not the one that was requested, so a mission artifact can show the wrong VM ran. The migration adds no CHECK constraint listing the CLIs. Which images exist is a property of the NODES, not the schema; a constraint would need migrating for every new image while still not guaranteeing the image exists anywhere. The node validates and names what is missing. Backend names are `[A-Za-z0-9_-]` and rejected rather than sanitised, since they become filenames. Verified on tank: default backend 8/8; `CLAWMATES_FC_BACKEND=agent-terminal` 9/9 including the absent-image check, create in 910ms on a rootfs built from a real Docker image. 435 tests green, no leaked processes or VM dirs. Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
fcf5d7b16c
commit
6687f8b808
@@ -127,14 +127,49 @@ async fn rpc(uds: &Path, req: &Value) -> Result<Value, String> {
|
||||
serde_json::from_slice(&buf).map_err(|e| format!("decode reply: {e}"))
|
||||
}
|
||||
|
||||
/// Resolve a backend name to the rootfs image on this node.
|
||||
///
|
||||
/// `None` (or `"default"`) means the golden `rootfs.ext4`; anything else selects
|
||||
/// `rootfs-<backend>.ext4`, built by `scripts/fc-build-rootfs.sh`.
|
||||
///
|
||||
/// A missing image is an explicit error naming the file and how to build it. The
|
||||
/// tempting fallback — quietly boot the default when the requested image is
|
||||
/// absent — would run a claude mission in a kimi VM, or in a rootfs with no CLI
|
||||
/// at all, and report success for whatever came out.
|
||||
fn rootfs_for(backend: Option<&str>) -> Result<PathBuf, String> {
|
||||
let root = work_root();
|
||||
let name = match backend {
|
||||
None | Some("") | Some("default") => return Ok(root.join("rootfs.ext4")),
|
||||
Some(b) => b,
|
||||
};
|
||||
// Becomes a filename, so the same rule as vm ids applies.
|
||||
check_id(name).map_err(|e| format!("backend {name:?}: {e}"))?;
|
||||
let path = root.join(format!("rootfs-{name}.ext4"));
|
||||
if !path.is_file() {
|
||||
return Err(format!(
|
||||
"no rootfs for backend {name:?} on this node ({} is missing) — build it: \
|
||||
scripts/fc-build-rootfs.sh <host> <docker-image> {name}",
|
||||
path.display()
|
||||
));
|
||||
}
|
||||
Ok(path)
|
||||
}
|
||||
|
||||
/// Boot a VM and wait until its agent answers.
|
||||
///
|
||||
/// "Started" is not "usable": a VM whose agent never comes up is a process that
|
||||
/// looks healthy and serves nothing, so create does not return until a `ping`
|
||||
/// has round-tripped. If it never does, the VM is destroyed rather than left
|
||||
/// registered — a half-created VM in the map is a leak with a plausible alibi.
|
||||
pub async fn create(vms: &Vms, vm_id: &str, vcpus: u32, mem_mib: u32) -> Result<Value, String> {
|
||||
pub async fn create(
|
||||
vms: &Vms,
|
||||
vm_id: &str,
|
||||
vcpus: u32,
|
||||
mem_mib: u32,
|
||||
backend: Option<&str>,
|
||||
) -> Result<Value, String> {
|
||||
check_id(vm_id)?;
|
||||
let golden = rootfs_for(backend)?;
|
||||
if vms.lock().await.contains_key(vm_id) {
|
||||
return Err(format!("vm {vm_id} already exists"));
|
||||
}
|
||||
@@ -153,7 +188,7 @@ pub async fn create(vms: &Vms, vm_id: &str, vcpus: u32, mem_mib: u32) -> Result<
|
||||
let rootfs = workdir.join("rootfs.ext4");
|
||||
let cp = tokio::process::Command::new("cp")
|
||||
.arg("--sparse=always")
|
||||
.arg(root.join("rootfs.ext4"))
|
||||
.arg(&golden)
|
||||
.arg(&rootfs)
|
||||
.output()
|
||||
.await
|
||||
@@ -241,7 +276,14 @@ pub async fn create(vms: &Vms, vm_id: &str, vcpus: u32, mem_mib: u32) -> Result<
|
||||
}
|
||||
|
||||
vms.lock().await.insert(vm_id.to_string(), vm);
|
||||
Ok(json!({ "vm_id": vm_id, "pgid": pgid, "workdir": workdir.display().to_string() }))
|
||||
Ok(json!({
|
||||
"vm_id": vm_id,
|
||||
"pgid": pgid,
|
||||
"workdir": workdir.display().to_string(),
|
||||
// Which image actually booted, not which was asked for. A mission
|
||||
// artifact that records the request cannot show that the wrong VM ran.
|
||||
"rootfs": golden.display().to_string(),
|
||||
}))
|
||||
}
|
||||
|
||||
async fn uds_of(vms: &Vms, vm_id: &str) -> Result<PathBuf, String> {
|
||||
@@ -353,7 +395,16 @@ pub async fn handle_op(op: &str, v: &Value, vms: &Vms) -> (bool, String) {
|
||||
let vm_id = s("vm_id");
|
||||
|
||||
let r: Result<Value, String> = match op {
|
||||
"vm_create" => create(vms, &vm_id, u("vcpus", 2) as u32, u("mem_mib", 2048) as u32).await,
|
||||
"vm_create" => {
|
||||
create(
|
||||
vms,
|
||||
&vm_id,
|
||||
u("vcpus", 2) as u32,
|
||||
u("mem_mib", 2048) as u32,
|
||||
v.get("backend").and_then(Value::as_str),
|
||||
)
|
||||
.await
|
||||
}
|
||||
"vm_inject" => inject(vms, &vm_id, &s("dest"), &s("tar_b64")).await,
|
||||
"vm_exec" => {
|
||||
let cwd = v.get("cwd").and_then(Value::as_str);
|
||||
@@ -400,8 +451,35 @@ pub async fn selftest() -> bool {
|
||||
// Start from a clean slate even if a previous run died mid-way.
|
||||
let _ = destroy(&vms, id).await;
|
||||
|
||||
// Which image to exercise. Defaults to the golden rootfs; set
|
||||
// CLAWMATES_FC_BACKEND=agent-terminal to prove a built per-CLI image boots.
|
||||
let backend = std::env::var("CLAWMATES_FC_BACKEND").ok();
|
||||
println!(
|
||||
" backend: {}",
|
||||
backend.as_deref().unwrap_or("(default rootfs.ext4)")
|
||||
);
|
||||
|
||||
// A backend whose image is absent must fail by name, not fall back to the
|
||||
// default — booting the wrong rootfs would report success for whatever came
|
||||
// out of it. Checked here so the guarantee is exercised on real hardware and
|
||||
// not only in a unit test with a temp dir.
|
||||
match create(&vms, "selftest-absent", 2, 512, Some("definitely-not-built")).await {
|
||||
Err(e) if e.contains("rootfs-definitely-not-built.ext4") => {
|
||||
check(true, "an absent backend image fails by name", String::new())
|
||||
}
|
||||
Err(e) => check(false, "an absent backend image fails by name", e),
|
||||
Ok(_) => {
|
||||
let _ = destroy(&vms, "selftest-absent").await;
|
||||
check(
|
||||
false,
|
||||
"an absent backend image fails by name",
|
||||
"it BOOTED — a missing image fell back to the default".into(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
let started = std::time::Instant::now();
|
||||
match create(&vms, id, 2, 1024).await {
|
||||
match create(&vms, id, 2, 1024, backend.as_deref()).await {
|
||||
Ok(v) => check(
|
||||
true,
|
||||
&format!("create ({} ms) {}", started.elapsed().as_millis(), v["vm_id"]),
|
||||
@@ -540,4 +618,39 @@ mod tests {
|
||||
"an over-long id must be rejected"
|
||||
);
|
||||
}
|
||||
|
||||
/// An absent image must be an ERROR, never a silent fall back to the golden
|
||||
/// rootfs: booting the default would run a claude mission in a kimi VM, or
|
||||
/// in a rootfs with no CLI at all, and report success for whatever came out.
|
||||
#[test]
|
||||
fn a_missing_backend_image_is_an_error_not_a_fallback() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
std::env::set_var("CLAWMATES_FC_ROOT", tmp.path());
|
||||
|
||||
let e = rootfs_for(Some("kimi")).expect_err("a missing image must fail");
|
||||
assert!(e.contains("rootfs-kimi.ext4"), "must name the file: {e}");
|
||||
assert!(e.contains("fc-build-rootfs.sh"), "must say how to fix: {e}");
|
||||
|
||||
// Present → selected.
|
||||
std::fs::write(tmp.path().join("rootfs-kimi.ext4"), b"x").unwrap();
|
||||
assert!(rootfs_for(Some("kimi"))
|
||||
.unwrap()
|
||||
.ends_with("rootfs-kimi.ext4"));
|
||||
|
||||
// The default is the golden image, and is not required to exist for the
|
||||
// name to resolve — vm_create's copy reports that.
|
||||
for none_ish in [None, Some(""), Some("default")] {
|
||||
assert!(rootfs_for(none_ish).unwrap().ends_with("rootfs.ext4"));
|
||||
}
|
||||
std::env::remove_var("CLAWMATES_FC_ROOT");
|
||||
}
|
||||
|
||||
/// A backend name becomes a filename, so it must not be able to describe a
|
||||
/// path any more than a vm id can.
|
||||
#[test]
|
||||
fn a_backend_name_cannot_escape_the_work_directory() {
|
||||
for bad in ["../etc/passwd", "a/b", ".."] {
|
||||
assert!(rootfs_for(Some(bad)).is_err(), "{bad:?} must be rejected");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user