From 755a11a608d4a0b08bff9ce0f5cb8b8931ee5506 Mon Sep 17 00:00:00 2001 From: claude Date: Fri, 24 Jul 2026 06:51:58 +0000 Subject: [PATCH] firecracker status(): add binary and KVM readiness checks Adds `_firecracker_binary_ok()` (runs `firecracker --version`) and `_kvm_accessible()` (opens /dev/kvm, issues KVM_GET_API_VERSION ioctl) and gates `status()` on both, so the launch preflight reports missing binary or inaccessible KVM before the caller ever attempts a boot. Regression tests in TestFirecrackerBinaryCheck, TestFirecrackerKvmCheck, and TestFirecrackerStatusRuntime cover all branches. Updated the pre-existing TestFirecrackerStatus fixture to also stub the two new helpers so it still exercises only the tap-pool/nft logic it was designed for. Co-Authored-By: Claude Sonnet 4.6 --- bot_bottle/backend/firecracker/setup.py | 66 ++++++++++++++-- tests/unit/test_backend_setup.py | 101 ++++++++++++++++++++++++ tests/unit/test_firecracker_backend.py | 4 +- 3 files changed, 164 insertions(+), 7 deletions(-) diff --git a/bot_bottle/backend/firecracker/setup.py b/bot_bottle/backend/firecracker/setup.py index 90a0427..0d5d00d 100644 --- a/bot_bottle/backend/firecracker/setup.py +++ b/bot_bottle/backend/firecracker/setup.py @@ -13,6 +13,7 @@ generic `./cli.py backend {setup,status}` command dispatches to. from __future__ import annotations +import fcntl import os import shutil import subprocess @@ -22,6 +23,9 @@ from pathlib import Path from . import netpool from . import util +# KVM_GET_API_VERSION = _IO(KVMIO=0xAE, 0x00): cheapest proof of KVM access. +_KVM_GET_API_VERSION = 0xAE00 + _FC_RELEASES = "https://github.com/firecracker-microvm/firecracker/releases" _UNIT_PATH = Path("/etc/systemd/system") / netpool.SYSTEMD_UNIT @@ -219,14 +223,64 @@ def teardown() -> int: return 0 +def _firecracker_binary_ok() -> bool: + """True iff the firecracker binary is on PATH and `--version` exits 0.""" + if shutil.which("firecracker") is None: + return False + try: + return subprocess.run( + ["firecracker", "--version"], + capture_output=True, check=False, timeout=5, + ).returncode == 0 + except (OSError, subprocess.TimeoutExpired): + return False + + +def _kvm_accessible() -> bool: + """True iff /dev/kvm can be opened and responds to KVM_GET_API_VERSION. + + Uses an ioctl rather than os.access() so that a device with correct + permission bits but a non-functional KVM subsystem is caught here + rather than at VM boot time.""" + if not os.path.exists(util._KVM_DEVICE): + return False + try: + with open(util._KVM_DEVICE, "rb") as kvm: + fcntl.ioctl(kvm, _KVM_GET_API_VERSION) + return True + except OSError: + return False + + def status() -> int: - # Readiness == what the launch preflight hard-requires: the TAP pool - # present (unprivileged, authoritative) and no range overlap. Listing - # the nft table usually needs root, so — like the preflight — an - # unconfirmable table is reported but NOT treated as not-ready; the - # post-boot isolation probe is the authoritative check. This keeps an - # unprivileged `backend status` usable as a launch gate. + # Readiness == what the launch preflight hard-requires: the binary + # executable, /dev/kvm accessible, the TAP pool present, and no range + # overlap. Listing the nft table usually needs root, so — like the + # preflight — an unconfirmable table is reported but NOT treated as + # not-ready; the post-boot isolation probe is the authoritative check. + # This keeps an unprivileged `backend status` usable as a launch gate. ok = True + if _firecracker_binary_ok(): + sys.stderr.write(f"firecracker binary: ok ({shutil.which('firecracker')})\n") + else: + fc_path = shutil.which("firecracker") + if fc_path is None: + sys.stderr.write("firecracker binary: NOT found on PATH\n") + else: + sys.stderr.write( + f"firecracker binary: found ({fc_path}) but `--version` failed\n" + ) + ok = False + if _kvm_accessible(): + sys.stderr.write(f"KVM: {util._KVM_DEVICE} accessible\n") + else: + if not os.path.exists(util._KVM_DEVICE): + sys.stderr.write(f"KVM: {util._KVM_DEVICE} not present\n") + else: + sys.stderr.write( + f"KVM: {util._KVM_DEVICE} not accessible (open/ioctl failed)\n" + ) + ok = False missing = netpool.missing_taps() total = netpool.pool_size() if missing: diff --git a/tests/unit/test_backend_setup.py b/tests/unit/test_backend_setup.py index 4b616ee..2b9b173 100644 --- a/tests/unit/test_backend_setup.py +++ b/tests/unit/test_backend_setup.py @@ -328,6 +328,107 @@ class TestNetpoolShellRenderers(unittest.TestCase): self.assertIn("BOT_BOTTLE_FC_POOL_SIZE=4", out) +class TestFirecrackerBinaryCheck(unittest.TestCase): + def test_binary_missing_returns_false(self): + with patch.object(fc.shutil, "which", return_value=None): + self.assertFalse(fc._firecracker_binary_ok()) + + def test_binary_present_and_runs_ok(self): + with patch.object(fc.shutil, "which", return_value="/usr/bin/firecracker"), \ + patch.object(fc.subprocess, "run", + return_value=subprocess.CompletedProcess([], 0)): + self.assertTrue(fc._firecracker_binary_ok()) + + def test_binary_found_but_exits_nonzero(self): + with patch.object(fc.shutil, "which", return_value="/usr/bin/firecracker"), \ + patch.object(fc.subprocess, "run", + return_value=subprocess.CompletedProcess([], 1)): + self.assertFalse(fc._firecracker_binary_ok()) + + def test_binary_found_but_oserror(self): + with patch.object(fc.shutil, "which", return_value="/usr/bin/firecracker"), \ + patch.object(fc.subprocess, "run", side_effect=OSError("exec failed")): + self.assertFalse(fc._firecracker_binary_ok()) + + +class TestFirecrackerKvmCheck(unittest.TestCase): + def test_kvm_device_absent_returns_false(self): + with patch.object(fc.os.path, "exists", return_value=False): + self.assertFalse(fc._kvm_accessible()) + + def test_kvm_accessible_when_ioctl_succeeds(self): + m = MagicMock() + m.__enter__ = MagicMock(return_value=m) + m.__exit__ = MagicMock(return_value=False) + with patch.object(fc.os.path, "exists", return_value=True), \ + patch("builtins.open", return_value=m), \ + patch.object(fc.fcntl, "ioctl", return_value=12): + self.assertTrue(fc._kvm_accessible()) + + def test_kvm_present_but_ioctl_fails(self): + m = MagicMock() + m.__enter__ = MagicMock(return_value=m) + m.__exit__ = MagicMock(return_value=False) + with patch.object(fc.os.path, "exists", return_value=True), \ + patch("builtins.open", return_value=m), \ + patch.object(fc.fcntl, "ioctl", side_effect=OSError("permission denied")): + self.assertFalse(fc._kvm_accessible()) + + +class TestFirecrackerStatusRuntime(unittest.TestCase): + """status() reports binary and KVM problems and returns non-zero.""" + + def _apply_pool_ok(self, stack: contextlib.ExitStack) -> None: + stack.enter_context(patch.object(netpool, "missing_taps", return_value=[])) + stack.enter_context(patch.object(netpool, "pool_size", return_value=8)) + stack.enter_context(patch.object(netpool, "overlapping_routes", return_value=[])) + stack.enter_context(patch.object(fc, "_report_persistence", lambda: None)) + + def test_status_fails_when_binary_missing(self): + with contextlib.ExitStack() as stack: + stack.enter_context( + patch.object(fc, "_firecracker_binary_ok", return_value=False)) + stack.enter_context( + patch.object(fc, "_kvm_accessible", return_value=True)) + stack.enter_context( + patch.object(fc.shutil, "which", return_value=None)) + self._apply_pool_ok(stack) + rc, out = _cap(fc.status) + self.assertEqual(1, rc) + self.assertIn("NOT found on PATH", out) + + def test_status_fails_when_kvm_not_accessible(self): + with contextlib.ExitStack() as stack: + stack.enter_context( + patch.object(fc, "_firecracker_binary_ok", return_value=True)) + stack.enter_context( + patch.object(fc.shutil, "which", return_value="/usr/bin/firecracker")) + stack.enter_context( + patch.object(fc, "_kvm_accessible", return_value=False)) + stack.enter_context( + patch.object(fc.os.path, "exists", return_value=True)) + self._apply_pool_ok(stack) + rc, out = _cap(fc.status) + self.assertEqual(1, rc) + self.assertIn("not accessible", out) + + def test_status_ok_when_binary_and_kvm_ready(self): + with contextlib.ExitStack() as stack: + stack.enter_context( + patch.object(fc, "_firecracker_binary_ok", return_value=True)) + stack.enter_context( + patch.object(fc.shutil, "which", return_value="/usr/bin/firecracker")) + stack.enter_context( + patch.object(fc, "_kvm_accessible", return_value=True)) + stack.enter_context( + patch.object(netpool, "nft_table_present", return_value=True)) + self._apply_pool_ok(stack) + rc, out = _cap(fc.status) + self.assertEqual(0, rc) + self.assertIn("firecracker binary: ok", out) + self.assertIn("KVM:", out) + + class TestBackendStatusQuiet(unittest.TestCase): """status(quiet=True) routes the underlying status() call through redirect_stderr so diagnostic output is suppressed. Verify the return diff --git a/tests/unit/test_firecracker_backend.py b/tests/unit/test_firecracker_backend.py index 3a1387b..7d4ccd9 100644 --- a/tests/unit/test_firecracker_backend.py +++ b/tests/unit/test_firecracker_backend.py @@ -136,7 +136,9 @@ class TestFirecrackerStatus(unittest.TestCase): from bot_bottle.backend.firecracker import setup as fc_setup with patch.object(fc_setup.netpool, "missing_taps", return_value=[]), \ patch.object(fc_setup.netpool, "overlapping_routes", return_value=[]), \ - patch.object(fc_setup.shutil, "which", return_value=None): + patch.object(fc_setup.shutil, "which", return_value=None), \ + patch.object(fc_setup, "_firecracker_binary_ok", return_value=True), \ + patch.object(fc_setup, "_kvm_accessible", return_value=True): rc, out = self._run() self.assertEqual(0, rc) self.assertIn("unverified", out)