fix(firecracker): abort cleanup on scan errors
This commit is contained in:
@@ -29,6 +29,7 @@ import subprocess
|
|||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
from ...log import info
|
from ...log import info
|
||||||
|
from .. import EnumerationError
|
||||||
from . import util
|
from . import util
|
||||||
from .bottle_cleanup_plan import FirecrackerBottleCleanupPlan
|
from .bottle_cleanup_plan import FirecrackerBottleCleanupPlan
|
||||||
|
|
||||||
@@ -62,12 +63,23 @@ def _scan_processes(run_root: Path) -> tuple[set[str], list[int]]:
|
|||||||
* ``orphan_pids`` — firecracker pids whose run dir no longer exists
|
* ``orphan_pids`` — firecracker pids whose run dir no longer exists
|
||||||
(a lingering VMM to kill).
|
(a lingering VMM to kill).
|
||||||
"""
|
"""
|
||||||
|
try:
|
||||||
result = subprocess.run(
|
result = subprocess.run(
|
||||||
["pgrep", "-a", "firecracker"],
|
["pgrep", "-a", "firecracker"],
|
||||||
capture_output=True, text=True, check=False,
|
capture_output=True, text=True, check=False,
|
||||||
)
|
)
|
||||||
if result.returncode != 0:
|
except OSError as exc:
|
||||||
|
raise EnumerationError(
|
||||||
|
f"could not enumerate Firecracker processes: {exc}"
|
||||||
|
) from exc
|
||||||
|
if result.returncode == 1:
|
||||||
|
# pgrep's documented "no processes matched" result.
|
||||||
return set(), []
|
return set(), []
|
||||||
|
if result.returncode != 0:
|
||||||
|
detail = (result.stderr or "").strip() or f"exit {result.returncode}"
|
||||||
|
raise EnumerationError(
|
||||||
|
f"could not enumerate Firecracker processes: {detail}"
|
||||||
|
)
|
||||||
live: set[str] = set()
|
live: set[str] = set()
|
||||||
orphan_pids: list[int] = []
|
orphan_pids: list[int] = []
|
||||||
for line in result.stdout.splitlines():
|
for line in result.stdout.splitlines():
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ import unittest
|
|||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from unittest.mock import patch
|
from unittest.mock import patch
|
||||||
|
|
||||||
|
from bot_bottle.backend import EnumerationError
|
||||||
from bot_bottle.backend.firecracker import cleanup as fc_cleanup
|
from bot_bottle.backend.firecracker import cleanup as fc_cleanup
|
||||||
from bot_bottle.backend.firecracker.bottle_cleanup_plan import (
|
from bot_bottle.backend.firecracker.bottle_cleanup_plan import (
|
||||||
FirecrackerBottleCleanupPlan,
|
FirecrackerBottleCleanupPlan,
|
||||||
@@ -58,10 +59,37 @@ class TestProcessScan(unittest.TestCase):
|
|||||||
self.assertEqual({str(run_root / "live-a")}, live)
|
self.assertEqual({str(run_root / "live-a")}, live)
|
||||||
self.assertEqual([222], orphan_pids)
|
self.assertEqual([222], orphan_pids)
|
||||||
|
|
||||||
def test_scan_empty_when_pgrep_fails(self):
|
def test_scan_empty_when_pgrep_finds_no_processes(self):
|
||||||
with patch.object(fc_cleanup.subprocess, "run", return_value=_proc(returncode=1)):
|
with patch.object(fc_cleanup.subprocess, "run", return_value=_proc(returncode=1)):
|
||||||
self.assertEqual((set(), []), fc_cleanup._scan_processes(Path("/x")))
|
self.assertEqual((set(), []), fc_cleanup._scan_processes(Path("/x")))
|
||||||
|
|
||||||
|
def test_scan_raises_when_pgrep_errors(self):
|
||||||
|
proc = subprocess.CompletedProcess(
|
||||||
|
[], 2, stdout="", stderr="invalid process expression",
|
||||||
|
)
|
||||||
|
with (
|
||||||
|
patch.object(fc_cleanup.subprocess, "run", return_value=proc),
|
||||||
|
self.assertRaisesRegex(EnumerationError, "invalid process expression"),
|
||||||
|
):
|
||||||
|
fc_cleanup._scan_processes(Path("/x"))
|
||||||
|
|
||||||
|
def test_prepare_cleanup_does_not_plan_deletions_when_scan_errors(self):
|
||||||
|
with tempfile.TemporaryDirectory() as tmp:
|
||||||
|
run_root = Path(tmp)
|
||||||
|
live = run_root / "live-a"
|
||||||
|
live.mkdir()
|
||||||
|
with (
|
||||||
|
patch.object(fc_cleanup, "_run_root", return_value=run_root),
|
||||||
|
patch.object(
|
||||||
|
fc_cleanup.subprocess,
|
||||||
|
"run",
|
||||||
|
return_value=_proc(returncode=2),
|
||||||
|
),
|
||||||
|
self.assertRaises(EnumerationError),
|
||||||
|
):
|
||||||
|
fc_cleanup.prepare_cleanup()
|
||||||
|
self.assertTrue(live.is_dir())
|
||||||
|
|
||||||
def test_live_run_dirs_returns_paths_in_stable_order(self):
|
def test_live_run_dirs_returns_paths_in_stable_order(self):
|
||||||
with patch.object(fc_cleanup, "_run_root", return_value=Path("/run")), \
|
with patch.object(fc_cleanup, "_run_root", return_value=Path("/run")), \
|
||||||
patch.object(fc_cleanup, "_scan_processes",
|
patch.object(fc_cleanup, "_scan_processes",
|
||||||
|
|||||||
Reference in New Issue
Block a user