fix(firecracker): make cleanup reap orphans only, never live VMs
test / stage-firecracker-inputs (pull_request) Successful in 1s
test / integration-docker (pull_request) Successful in 24s
test / unit (pull_request) Successful in 31s
test / build-infra (pull_request) Successful in 3m53s
test / integration-firecracker (pull_request) Successful in 1m41s
test / coverage (pull_request) Successful in 1m54s
test / publish-infra (pull_request) Has been skipped
lint / lint (push) Successful in 48s
tracker-policy-pr / check-pr (pull_request) Failing after 9s

`cleanup()` enumerated *every* firecracker process under the run root and
*every* run dir, so `./cli.py cleanup` would kill running bottles and
delete their rootfs — despite being described as orphan cleanup. The
backend's `enumerate_active` registry is still a stub (#354), so there
was no live/dead signal.

Use the running firecracker processes themselves as that signal: a run
dir with a live VM is protected (never killed, never removed); only run
dirs with no live process (leaked by a hard-killed launch) are reaped,
plus firecracker pids whose run dir is already gone (lingering VMMs).

This makes cleanup safe to run alongside live bottles and lets it back a
periodic GC for the run-dir leak the launch.py teardown fix closes on the
clean-exit path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-07-21 22:28:26 -04:00
parent 06d293fbd5
commit f32342dc7d
2 changed files with 126 additions and 37 deletions
+60 -21
View File
@@ -10,7 +10,9 @@ classmethods forward to their module.
from __future__ import annotations
import subprocess
import tempfile
import unittest
from pathlib import Path
from unittest.mock import patch
from bot_bottle.backend.firecracker import cleanup as fc_cleanup
@@ -23,32 +25,69 @@ def _proc(stdout: str = "", returncode: int = 0) -> "subprocess.CompletedProcess
return subprocess.CompletedProcess([], returncode, stdout=stdout, stderr="")
class TestOrphanEnumeration(unittest.TestCase):
def test_orphan_vm_pids_filters_by_run_dir(self):
run_root = str(fc_cleanup._run_root())
out = (
f"111 firecracker --config-file {run_root}/dev-a/config.json\n"
"222 firecracker --config-file /somewhere/else/config.json\n"
"notanint firecracker --config-file " + run_root + "/x\n"
class TestProcessScan(unittest.TestCase):
def test_run_dir_of_matches_only_direct_children(self):
run_root = Path("/cache/run")
self.assertEqual(
Path("/cache/run/dev-a"),
fc_cleanup._run_dir_of(
f"firecracker --config-file {run_root}/dev-a/config.json", run_root
),
)
# infra/builder VMs elsewhere, or nested paths, are not ours.
self.assertIsNone(
fc_cleanup._run_dir_of("firecracker --config-file /elsewhere/config.json", run_root)
)
self.assertIsNone(
fc_cleanup._run_dir_of("firecracker --no-config", run_root)
)
with patch.object(fc_cleanup.subprocess, "run", return_value=_proc(out)):
self.assertEqual([111], fc_cleanup._orphan_vm_pids())
def test_orphan_vm_pids_empty_when_pgrep_fails(self):
def test_scan_splits_live_dirs_from_orphan_pids(self):
with tempfile.TemporaryDirectory() as tmp:
run_root = Path(tmp)
(run_root / "live-a").mkdir() # dir present -> live VM, protected
# "gone-b" dir intentionally absent -> lingering VMM, orphan pid
out = (
f"111 firecracker --config-file {run_root}/live-a/config.json\n"
f"222 firecracker --config-file {run_root}/gone-b/config.json\n"
"333 firecracker --config-file /elsewhere/config.json\n"
"notanint firecracker --config-file x\n"
)
with patch.object(fc_cleanup.subprocess, "run", return_value=_proc(out)):
live, orphan_pids = fc_cleanup._scan_processes(run_root)
self.assertEqual({str(run_root / "live-a")}, live)
self.assertEqual([222], orphan_pids)
def test_scan_empty_when_pgrep_fails(self):
with patch.object(fc_cleanup.subprocess, "run", return_value=_proc(returncode=1)):
self.assertEqual([], fc_cleanup._orphan_vm_pids())
self.assertEqual((set(), []), fc_cleanup._scan_processes(Path("/x")))
def test_run_dirs_empty_when_absent(self):
with patch.object(fc_cleanup.util, "cache_dir") as cache:
cache.return_value.__truediv__.return_value.is_dir.return_value = False
self.assertEqual([], fc_cleanup._run_dirs())
def test_orphan_run_dirs_excludes_live_and_missing_root(self):
with tempfile.TemporaryDirectory() as tmp:
run_root = Path(tmp)
(run_root / "live-a").mkdir()
(run_root / "dead-b").mkdir()
live = {str(run_root / "live-a")}
self.assertEqual(
[str(run_root / "dead-b")],
fc_cleanup._orphan_run_dirs(run_root, live),
)
# absent run root -> nothing to reap
self.assertEqual([], fc_cleanup._orphan_run_dirs(Path("/nope/run"), set()))
def test_prepare_cleanup_assembles_plan(self):
with patch.object(fc_cleanup, "_orphan_vm_pids", return_value=[7]), \
patch.object(fc_cleanup, "_run_dirs", return_value=["/run/x"]):
plan = fc_cleanup.prepare_cleanup()
self.assertEqual((7,), plan.vm_pids)
self.assertEqual(("/run/x",), plan.run_dirs)
def test_prepare_cleanup_reaps_orphans_only(self):
"""The live VM's dir is never in the plan; the dead one is."""
with tempfile.TemporaryDirectory() as tmp:
run_root = Path(tmp)
(run_root / "live-a").mkdir()
(run_root / "dead-b").mkdir()
out = f"111 firecracker --config-file {run_root}/live-a/config.json\n"
with patch.object(fc_cleanup, "_run_root", return_value=run_root), \
patch.object(fc_cleanup.subprocess, "run", return_value=_proc(out)):
plan = fc_cleanup.prepare_cleanup()
self.assertEqual((), plan.vm_pids)
self.assertEqual((str(run_root / "dead-b"),), plan.run_dirs)
self.assertNotIn(str(run_root / "live-a"), plan.run_dirs)
class TestCleanupRemoval(unittest.TestCase):