fix(firecracker): reap legacy pre-split infra VM on cutover
tracker-policy-pr / check-pr (pull_request) Successful in 12s
test / integration-docker (pull_request) Successful in 19s
lint / lint (push) Successful in 59s
test / unit (pull_request) Successful in 1m53s
test / integration-firecracker (pull_request) Successful in 3m36s
test / coverage (pull_request) Successful in 14s
test / publish-infra (pull_request) Has been skipped
tracker-policy-pr / check-pr (pull_request) Successful in 12s
test / integration-docker (pull_request) Successful in 19s
lint / lint (push) Successful in 59s
test / unit (pull_request) Successful in 1m53s
test / integration-firecracker (pull_request) Successful in 3m36s
test / coverage (pull_request) Successful in 14s
test / publish-infra (pull_request) Has been skipped
The two-VM stop() only knew the new orchestrator/gateway config paths
(<infra>/{orchestrator,gateway}/config.json), so a host still running the
OLD single combined-VM (booted from <infra>/config.json on the orchestrator
link) kept it alive across the migration — and it held the orchestrator TAP,
so the first split boot died with "Open tap device failed: Resource busy"
(seen on the CI runner's bborchci0 pool).
stop() now also reaps that legacy singleton (both its drifted pidfile and any
firecracker whose --config-file is the legacy path), so the cutover is
self-healing with no manual per-host teardown. Scoped to the legacy infra
config path, so pool agent VMs are untouched.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -263,13 +263,19 @@ def stop() -> None:
|
||||
recorded VMMs AND any orphaned firecracker still bound to either infra
|
||||
config — the PID files drift after crashes / out-of-band kills, and a
|
||||
survivor would hold a link's TAP so the next boot dies with "tap …
|
||||
Resource busy". Drops the version marker so a stopped pair is never
|
||||
treated as adoptable."""
|
||||
Resource busy". Also reaps a surviving *legacy* single combined-VM (the
|
||||
pre-split layout booted from `<infra>/config.json` on the orchestrator
|
||||
link): the two-VM `stop` otherwise wouldn't know about it, and it would
|
||||
hold the orchestrator TAP so the first split boot fails — so the cutover
|
||||
is self-healing, no manual host teardown. Drops the version marker so a
|
||||
stopped pair is never treated as adoptable."""
|
||||
_kill_pidfile(_orch_dir())
|
||||
_kill_pidfile(_gw_dir())
|
||||
_kill_pidfile(_infra_dir()) # legacy pre-split combined VM (migration)
|
||||
_kill_infra_firecrackers()
|
||||
_pid_file(_orch_dir()).unlink(missing_ok=True)
|
||||
_pid_file(_gw_dir()).unlink(missing_ok=True)
|
||||
_pid_file(_infra_dir()).unlink(missing_ok=True) # legacy
|
||||
_version_file().unlink(missing_ok=True)
|
||||
|
||||
|
||||
@@ -543,9 +549,15 @@ def _kill_infra_firecrackers(proc_root: Path = Path("/proc")) -> None:
|
||||
"""SIGKILL any firecracker VMM whose `--config-file` is one of this host's
|
||||
infra configs (orchestrator or gateway), independent of the PID files —
|
||||
reaps orphans it lost track of so both links' TAPs are free to rebind.
|
||||
Scoped to the two infra config paths, so the interactive pool's agent VMs
|
||||
(other config paths) are untouched."""
|
||||
cfgs = {str(_orch_dir() / "config.json"), str(_gw_dir() / "config.json")}
|
||||
Also matches the *legacy* pre-split combined-VM config (`<infra>/config.json`)
|
||||
so a surviving old singleton is cleared off the orchestrator link during the
|
||||
cutover. Scoped to these infra config paths, so the pool's agent VMs (other
|
||||
config paths) are untouched."""
|
||||
cfgs = {
|
||||
str(_orch_dir() / "config.json"),
|
||||
str(_gw_dir() / "config.json"),
|
||||
str(_infra_dir() / "config.json"), # legacy pre-split combined VM
|
||||
}
|
||||
for entry in proc_root.iterdir():
|
||||
if not entry.name.isdigit():
|
||||
continue
|
||||
|
||||
@@ -413,9 +413,10 @@ class TestStop(unittest.TestCase):
|
||||
patch.object(infra_vm, "_kill_pidfile") as kill, \
|
||||
patch.object(infra_vm, "_kill_infra_firecrackers"):
|
||||
infra_vm.stop()
|
||||
# Both VMs' pidfiles are reaped, and the version marker is gone so a
|
||||
# stopped pair is never adopted.
|
||||
self.assertEqual(2, kill.call_count)
|
||||
# Both VMs' pidfiles are reaped — plus the legacy pre-split combined
|
||||
# VM's — and the version marker is gone so a stopped pair is never
|
||||
# adopted.
|
||||
self.assertEqual(3, kill.call_count)
|
||||
self.assertFalse((d / "booted-version").exists())
|
||||
|
||||
|
||||
@@ -507,31 +508,35 @@ class TestKillInfraFirecrackers(unittest.TestCase):
|
||||
(p / "comm").write_text(comm + "\n")
|
||||
(p / "cmdline").write_bytes(b"\0".join(a.encode() for a in cmdline) + b"\0")
|
||||
|
||||
def test_kills_both_infra_firecrackers_only(self):
|
||||
def test_kills_both_infra_and_legacy_firecrackers_only(self):
|
||||
import tempfile
|
||||
with tempfile.TemporaryDirectory() as td, \
|
||||
tempfile.TemporaryDirectory() as proc:
|
||||
infra_dir = Path(td)
|
||||
orch_cfg = str(infra_dir / "orchestrator" / "config.json")
|
||||
gw_cfg = str(infra_dir / "gateway" / "config.json")
|
||||
legacy_cfg = str(infra_dir / "config.json") # pre-split combined VM
|
||||
root = Path(proc)
|
||||
# both infra VMs -> killed
|
||||
# both new infra VMs + the legacy combined VM -> killed
|
||||
self._fake_proc(root, 111, "firecracker",
|
||||
["firecracker", "--no-api", "--config-file", orch_cfg])
|
||||
self._fake_proc(root, 112, "firecracker",
|
||||
["firecracker", "--no-api", "--config-file", gw_cfg])
|
||||
self._fake_proc(root, 113, "firecracker",
|
||||
["firecracker", "--no-api", "--config-file", legacy_cfg])
|
||||
# a firecracker for a different (interactive) VM -> spared
|
||||
self._fake_proc(root, 222, "firecracker",
|
||||
["firecracker", "--config-file", "/home/u/other.json"])
|
||||
# a non-firecracker process on an infra config path -> spared
|
||||
self._fake_proc(root, 333, "python3", ["python3", orch_cfg])
|
||||
(root / "not-a-pid").mkdir()
|
||||
with patch.object(infra_vm, "_orch_dir", return_value=infra_dir / "orchestrator"), \
|
||||
with patch.object(infra_vm, "_infra_dir", return_value=infra_dir), \
|
||||
patch.object(infra_vm, "_orch_dir", return_value=infra_dir / "orchestrator"), \
|
||||
patch.object(infra_vm, "_gw_dir", return_value=infra_dir / "gateway"), \
|
||||
patch.object(infra_vm.os, "kill") as kill:
|
||||
infra_vm._kill_infra_firecrackers(proc_root=root)
|
||||
killed = {c.args[0] for c in kill.call_args_list}
|
||||
self.assertEqual({111, 112}, killed)
|
||||
self.assertEqual({111, 112, 113}, killed)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
||||
Reference in New Issue
Block a user