refactor(smolmachines): decompose launch(), add wait_exec_ready, file-lock allocate() (PRD 0032)
Decompose the 207-line launch() into six named helpers: _allocate_resources, _mint_certs, _start_bundle, _discover_urls, _launch_vm, _init_vm. Each has explicit inputs/outputs and is independently testable. Replace time.sleep(1.5) with smolvm.wait_exec_ready(), which polls `machine exec true` with exponential backoff. Exits as soon as the exec channel is ready; dies loudly with a timeout message instead of silently leaving the VM in an unknown state. File-lock loopback_alias.allocate() with fcntl.flock(LOCK_EX) so concurrent bottle launches can't race on docker state and claim the same alias.
This commit is contained in:
@@ -11,6 +11,7 @@ import json
|
||||
import sqlite3
|
||||
import subprocess
|
||||
import tempfile
|
||||
import threading
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
@@ -144,6 +145,55 @@ class TestAllocate(unittest.TestCase):
|
||||
loopback_alias.allocate("demo-overflow")
|
||||
|
||||
|
||||
class TestAllocateLock(unittest.TestCase):
|
||||
"""allocate() on macOS acquires a file lock so concurrent calls
|
||||
serialise rather than racing on docker state."""
|
||||
|
||||
def test_acquires_exclusive_lock_on_macos(self):
|
||||
import fcntl as fcntl_mod
|
||||
flock_calls: list[int] = []
|
||||
|
||||
def record_flock(fd, op):
|
||||
flock_calls.append(op)
|
||||
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
lock_path = Path(tmp) / "smolmachines.lock"
|
||||
with patch.object(loopback_alias, "_is_macos", return_value=True), \
|
||||
patch.object(loopback_alias, "_ALLOC_LOCK_PATH", lock_path), \
|
||||
patch.object(loopback_alias, "_aliases_in_use", return_value=set()), \
|
||||
patch.object(loopback_alias.fcntl, "flock",
|
||||
side_effect=record_flock):
|
||||
loopback_alias.allocate("demo")
|
||||
|
||||
self.assertIn(fcntl_mod.LOCK_EX, flock_calls)
|
||||
|
||||
def test_no_lock_on_linux(self):
|
||||
# Linux early-returns before touching the lock file.
|
||||
with patch.object(loopback_alias, "_is_macos", return_value=False), \
|
||||
patch.object(loopback_alias.fcntl, "flock") as flock:
|
||||
loopback_alias.allocate("demo")
|
||||
flock.assert_not_called()
|
||||
|
||||
def test_sequential_allocations_with_shared_lock_are_serialised(self):
|
||||
# Two sequential calls share the same lock file. The second
|
||||
# call sees {127.0.0.16} in use (as if the first caller's
|
||||
# docker run completed between the two lock acquisitions) and
|
||||
# returns the next alias.
|
||||
in_use_seq = [set(), {"127.0.0.16"}]
|
||||
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
lock_path = Path(tmp) / "smolmachines.lock"
|
||||
results: list[str] = []
|
||||
for _ in range(2):
|
||||
with patch.object(loopback_alias, "_is_macos", return_value=True), \
|
||||
patch.object(loopback_alias, "_ALLOC_LOCK_PATH", lock_path), \
|
||||
patch.object(loopback_alias, "_aliases_in_use",
|
||||
return_value=in_use_seq.pop(0)):
|
||||
results.append(loopback_alias.allocate("demo"))
|
||||
|
||||
self.assertEqual(["127.0.0.16", "127.0.0.17"], results)
|
||||
|
||||
|
||||
class TestAliasInUseDetection(unittest.TestCase):
|
||||
"""`_aliases_in_use` inspects every running bundle and pulls
|
||||
each container's port-binding `HostIp` out. The detection has
|
||||
|
||||
@@ -12,6 +12,7 @@ import unittest
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
from bot_bottle.backend.smolmachines import smolvm as smolvm_mod
|
||||
from bot_bottle.backend.smolmachines.smolvm import (
|
||||
SmolvmError,
|
||||
SmolvmRunResult,
|
||||
@@ -23,6 +24,7 @@ from bot_bottle.backend.smolmachines.smolvm import (
|
||||
machine_start,
|
||||
machine_stop,
|
||||
pack_create,
|
||||
wait_exec_ready,
|
||||
)
|
||||
|
||||
|
||||
@@ -204,6 +206,45 @@ class TestErrorPath(unittest.TestCase):
|
||||
self.assertEqual(SmolvmRunResult(42, "", "nope"), r)
|
||||
|
||||
|
||||
class TestWaitExecReady(unittest.TestCase):
|
||||
"""wait_exec_ready polls machine_exec(name, ["true"]) until it
|
||||
returns 0, then exits. On timeout it calls die()."""
|
||||
|
||||
def test_returns_immediately_when_exec_succeeds_first_try(self):
|
||||
with patch.object(smolvm_mod, "machine_exec",
|
||||
return_value=SmolvmRunResult(0, "", "")) as m:
|
||||
wait_exec_ready("vm-x")
|
||||
m.assert_called_once_with("vm-x", ["true"])
|
||||
|
||||
def test_retries_on_nonzero_and_returns_on_success(self):
|
||||
results = [
|
||||
SmolvmRunResult(1, "", "not ready"),
|
||||
SmolvmRunResult(1, "", "not ready"),
|
||||
SmolvmRunResult(0, "", ""),
|
||||
]
|
||||
with patch.object(smolvm_mod, "machine_exec",
|
||||
side_effect=results) as m, \
|
||||
patch.object(smolvm_mod.time, "sleep"):
|
||||
wait_exec_ready("vm-x")
|
||||
self.assertEqual(3, m.call_count)
|
||||
|
||||
def test_dies_on_timeout(self):
|
||||
# machine_exec always returns non-zero; monotonic advances past
|
||||
# the deadline after the first sleep so the loop exits.
|
||||
ticks = [0.0, 0.0, 10.0] # third call puts us past deadline
|
||||
with patch.object(smolvm_mod, "machine_exec",
|
||||
return_value=SmolvmRunResult(1, "", "")), \
|
||||
patch.object(smolvm_mod.time, "monotonic",
|
||||
side_effect=ticks), \
|
||||
patch.object(smolvm_mod.time, "sleep"), \
|
||||
patch.object(smolvm_mod, "die",
|
||||
side_effect=SystemExit("die")) as die_mock:
|
||||
with self.assertRaises(SystemExit):
|
||||
wait_exec_ready("vm-x", timeout=5.0)
|
||||
die_mock.assert_called_once()
|
||||
self.assertIn("vm-x", die_mock.call_args.args[0])
|
||||
|
||||
|
||||
class TestIsAvailable(unittest.TestCase):
|
||||
def test_true_when_on_path(self):
|
||||
with patch(
|
||||
|
||||
Reference in New Issue
Block a user