fix(missions): #54 — the worker was killing live microVM runs at 180 seconds
My hypothesis in #54 was WRONG, and it was wrong because I built it on a bad measurement: `grep -c 'microvm phase'` returned 0, so I concluded the completion log never printed and blamed the 15-minute reaper. The line was there all along, at 14:17:45. The real cause is worse. `requeue_stale` has NO TIER FILTER. A microvm run's `updated_at` is written once at insert and never again — it is driven by a `tokio::spawn` that owns it start to finish, and nothing in `microvm_executor` writes `topology_runs`. So at 180s the sweeper declared a perfectly healthy run stale and flipped it to `queued`; `claim_next_queued` (no tier filter either) handed it to the worker; `run_job` tried to parse the microvm graph placeholder, which `TopologyGraph` cannot deserialize; and it failed the run with "missing or invalid graph". Mission 019fd43e: run created 14:11:16, mission failed ~14:14:46. 210 seconds — the 180s window plus a tick. The agent went on working and finished at 14:17:45 with three modules written, by which time the phase was already dead and the VM was orphaned. A firecracker process was still alive 1h37m later. THE UNCOMFORTABLE PART: every microVM mission that appeared to work this session did so only by finishing inside three minutes. The 90-second ones dodged this. The harness scenario dodges it. Nothing about that was visible. `WORKER_DRIVEN_TIERS` (team, company, org, swarm, compare) is now the allowlist for all three sweep paths — claim, requeue, reap. An allowlist rather than a denylist so the next self-driven tier is safe by default instead of exposed until someone remembers the file. `tier='session'` had exactly the same exposure and is covered too. A unit test asserts microvm and session are NOT in it, next to the code that inserts them. Two more fixes from the same wreckage: - `destroy` reported `killed: pgid.is_some()` — true whenever there was a pgid to signal, whether or not anything died. It now sends the signal, polls /proc for the group leader, retries, and reports what it OBSERVED; `signalled` keeps the old meaning so "nothing to kill" is distinguishable from "it would not die". - the run-status update is now guarded with `AND status <> 'cancelled'`. An operator cancelling is a decision; this task reporting an outcome minutes later is an observation, and it must not overwrite one with the other. And the root cause of the collect timeout itself: `mission_fs::pack_dir` shipped `target/` in both directions. `mission_delivery` has excluded build output from the DIFF since day one; the TRANSPORT never knew. The host checkout was 9.4 MB of which 8.9 MB was `target/`, tarred and base64'd over vsock each way. `EXCLUDED_PATHS` is now one list shared by both layers, matched on directory name at any depth so a workspace's per-crate `target/` dirs are all covered. 483 tests pass, clippy clean.
This commit is contained in:
@@ -48,14 +48,73 @@ pub fn pack_dir(root: &Path, name_in_archive: &str) -> Result<Vec<u8>, String> {
|
||||
// Follow no symlinks: a checkout can contain a link pointing outside the
|
||||
// tree, and dereferencing it would pull host files into the container.
|
||||
builder.follow_symlinks(false);
|
||||
builder
|
||||
.append_dir_all(name_in_archive, root)
|
||||
append_filtered(&mut builder, root, Path::new(name_in_archive))
|
||||
.map_err(|e| format!("pack {}: {e}", root.display()))?;
|
||||
builder
|
||||
.into_inner()
|
||||
.map_err(|e| format!("finish archive for {}: {e}", root.display()))
|
||||
}
|
||||
|
||||
/// Directory names never carried across the boundary.
|
||||
///
|
||||
/// The same list the delivery diff uses, deliberately: see
|
||||
/// [`crate::mission_delivery::EXCLUDED_PATHS`]. A build directory is not work —
|
||||
/// it is regenerable output that dwarfs the source, and shipping it cost a
|
||||
/// mission its results when `vm_collect` timed out with the agent's finished work
|
||||
/// still inside the VM.
|
||||
pub fn transport_excludes() -> &'static [&'static str] {
|
||||
crate::mission_delivery::EXCLUDED_PATHS
|
||||
}
|
||||
|
||||
/// Should this directory entry be left out of the archive?
|
||||
///
|
||||
/// Matched on the entry NAME at any depth, not on a path prefix: a workspace has
|
||||
/// a `target/` per crate, and excluding only the root one would still ship the
|
||||
/// rest.
|
||||
pub fn is_excluded(name: &str) -> bool {
|
||||
transport_excludes().contains(&name)
|
||||
}
|
||||
|
||||
/// Recursive `append_dir_all` that skips [`transport_excludes`].
|
||||
///
|
||||
/// Hand-rolled because `tar::Builder::append_dir_all` takes no filter. Symlinks
|
||||
/// are added as links rather than followed, matching `follow_symlinks(false)`.
|
||||
fn append_filtered<W: std::io::Write>(
|
||||
builder: &mut tar::Builder<W>,
|
||||
dir: &Path,
|
||||
prefix: &Path,
|
||||
) -> std::io::Result<()> {
|
||||
builder.append_dir(prefix, dir)?;
|
||||
let mut entries: Vec<_> = std::fs::read_dir(dir)?.collect::<Result<Vec<_>, _>>()?;
|
||||
// Stable order so an archive of the same tree is byte-identical, which makes
|
||||
// a size or content difference between two runs mean something.
|
||||
entries.sort_by_key(|e| e.file_name());
|
||||
for entry in entries {
|
||||
let name = entry.file_name();
|
||||
let name_str = name.to_string_lossy();
|
||||
let path = entry.path();
|
||||
let dest = prefix.join(&name);
|
||||
let meta = std::fs::symlink_metadata(&path)?;
|
||||
if meta.is_dir() {
|
||||
if is_excluded(&name_str) {
|
||||
continue;
|
||||
}
|
||||
append_filtered(builder, &path, &dest)?;
|
||||
} else if meta.is_symlink() {
|
||||
let mut header = tar::Header::new_gnu();
|
||||
header.set_metadata(&meta);
|
||||
header.set_entry_type(tar::EntryType::Symlink);
|
||||
header.set_size(0);
|
||||
let target = std::fs::read_link(&path)?;
|
||||
builder.append_link(&mut header, &dest, &target)?;
|
||||
} else {
|
||||
let mut f = std::fs::File::open(&path)?;
|
||||
builder.append_file(&dest, &mut f)?;
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Unpack a tar into a host directory.
|
||||
///
|
||||
/// `tar` refuses entries whose paths escape the destination, which is the
|
||||
|
||||
Reference in New Issue
Block a user