Compare commits
11 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 2acaa082e4 | |||
| 2c496dc3d0 | |||
| f24ae45d13 | |||
| 594d07410a | |||
| c9c62f256d | |||
| 10150ae9f5 | |||
| 86c7ac1843 | |||
| 2e0414f969 | |||
| 12b071833d | |||
| bf72282f8e | |||
| f2c3710d0d |
@@ -14,6 +14,7 @@ the private orchestrator `_launch_bottle`.
|
||||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import io
|
||||
import os
|
||||
import shutil
|
||||
import sys
|
||||
@@ -195,7 +196,11 @@ def _start_headless(
|
||||
path, so the agent still execs on the inherited stdio/PTY — an
|
||||
orchestrator allocates that PTY and relays it to its
|
||||
desktop/mobile clients."""
|
||||
if not os.isatty(sys.stdin.fileno()):
|
||||
try:
|
||||
stdin_fd = sys.stdin.fileno()
|
||||
except io.UnsupportedOperation:
|
||||
stdin_fd = -1
|
||||
if not os.isatty(stdin_fd):
|
||||
die(
|
||||
"--headless requires a PTY on stdin; run via:\n"
|
||||
" script -q /dev/null ./cli.py start ..."
|
||||
|
||||
@@ -9,8 +9,7 @@ Failure policy: when a child dies unexpectedly, the supervisor
|
||||
restarts it automatically and logs the restart. The gateway stays
|
||||
up; a temporary loss of one daemon (e.g. egress OOM-killed) is
|
||||
recovered without manual container recreation. The supervisor
|
||||
itself exits only when (a) the operator sends SIGTERM/SIGINT, or
|
||||
(b) every child has died.
|
||||
itself exits only when the operator sends SIGTERM/SIGINT.
|
||||
|
||||
Daemon subset is env-driven via `BOT_BOTTLE_GATEWAY_DAEMONS=egress`
|
||||
for callers that don't use git-gate or supervise. Default: all
|
||||
@@ -221,9 +220,10 @@ class _Supervisor:
|
||||
"""One iteration of the watch loop. Returns True when every
|
||||
child has exited and the supervisor can return.
|
||||
|
||||
A child dying unexpectedly is logged but does NOT initiate
|
||||
shutdown — see the module docstring's failure-policy
|
||||
section. Shutdown is signal-driven only."""
|
||||
A child dying unexpectedly is logged and restarted but does
|
||||
NOT initiate shutdown — see the module docstring's
|
||||
failure-policy section. Shutdown is signal-driven only."""
|
||||
restarted_children = bool(self._restart_requested)
|
||||
self._drain_restart_requests()
|
||||
|
||||
for spec, p in self.procs:
|
||||
@@ -237,6 +237,13 @@ class _Supervisor:
|
||||
else:
|
||||
_log(f"{spec.name} exited with code {rc}")
|
||||
|
||||
# Restart deaths discovered above before checking whether all
|
||||
# processes are done. Deferring this until the next tick would make a
|
||||
# single-daemon supervisor return True and exit with the restart still
|
||||
# queued.
|
||||
restarted_children |= bool(self._restart_requested)
|
||||
self._drain_restart_requests()
|
||||
|
||||
if self.shutdown_at is not None:
|
||||
elapsed = time.monotonic() - self.shutdown_at
|
||||
if elapsed > _GRACE_SECONDS:
|
||||
@@ -250,7 +257,10 @@ class _Supervisor:
|
||||
)
|
||||
self._sigkill_all()
|
||||
|
||||
done = all(p.poll() is not None for _, p in self.procs)
|
||||
done = (
|
||||
not restarted_children
|
||||
and all(p.poll() is not None for _, p in self.procs)
|
||||
)
|
||||
if done:
|
||||
for _, p in self.procs:
|
||||
if p.stdout is not None:
|
||||
|
||||
@@ -57,7 +57,7 @@ def _merge_two_bottles_runtime(base: "ManifestBottle", override: "ManifestBottle
|
||||
n for n in override_repos_by_name if n not in base_repos_by_name
|
||||
]
|
||||
merged_git = tuple(
|
||||
override_repos_by_name.get(n, base_repos_by_name[n])
|
||||
override_repos_by_name[n] if n in override_repos_by_name else base_repos_by_name[n]
|
||||
for n in merged_repos_names
|
||||
)
|
||||
|
||||
|
||||
+9
-3
@@ -1,8 +1,14 @@
|
||||
"""Foundational filesystem paths for bot-bottle.
|
||||
|
||||
`bot_bottle_root()` is the app data root — state, queue, audit logs,
|
||||
git-gate keys, and the shared DB all live under it. It defaults to
|
||||
`~/.bot-bottle` and is overridable with the **`BOT_BOTTLE_ROOT`** env var.
|
||||
`bot_bottle_root()` is the app data root — per-bottle state, git-gate
|
||||
keys, the gateway CA, and the shared SQLite DB all live under it. It
|
||||
defaults to `~/.bot-bottle` and is overridable with the
|
||||
**`BOT_BOTTLE_ROOT`** env var.
|
||||
|
||||
Note that the supervise queue and the audit log are *tables in the shared
|
||||
DB*, not directories under the root — see `queue_store.py` / `audit_store.py`.
|
||||
The root held a `queue/` directory before the SQLite migration (PRD 0067);
|
||||
nothing writes there now.
|
||||
|
||||
The env override is the single knob for redirecting the root: the test
|
||||
suite points it at a throwaway dir instead of monkey-patching the function
|
||||
|
||||
@@ -69,6 +69,9 @@ class TestCmdStartHeadless(unittest.TestCase):
|
||||
self._modal = patch.object(tui_mod, "name_color_modal").start()
|
||||
patch.dict(os.environ, {}, clear=False).start()
|
||||
os.environ.pop("BOT_BOTTLE_BACKEND", None)
|
||||
# PTY check uses os.isatty(sys.stdin.fileno()); stub both so
|
||||
# headless unit tests aren't blocked on a real TTY.
|
||||
patch("bot_bottle.cli.start.os.isatty", return_value=True).start()
|
||||
self.addCleanup(patch.stopall)
|
||||
|
||||
def _spec(self):
|
||||
|
||||
@@ -162,16 +162,17 @@ class TestSupervisor(unittest.TestCase):
|
||||
return sup.exit_code()
|
||||
|
||||
def test_all_children_succeed_returns_zero(self):
|
||||
# `sh -c :` exits 0 immediately. With the new failure
|
||||
# policy a child dying doesn't trigger shutdown, so the
|
||||
# loop only converges once BOTH have exited on their own.
|
||||
# Both exit 0 → max(0, 0) = 0.
|
||||
# `sh -c :` exits 0 immediately. Start shutdown before driving
|
||||
# the loop so the intentionally short-lived fixtures are not
|
||||
# treated as unexpected deaths and restarted.
|
||||
specs = [
|
||||
_DaemonSpec("a", ("/bin/sh", "-c", ":")),
|
||||
_DaemonSpec("b", ("/bin/sh", "-c", ":")),
|
||||
]
|
||||
sup = _Supervisor(specs)
|
||||
sup.start_all()
|
||||
time.sleep(0.1)
|
||||
sup.request_shutdown(reason="test")
|
||||
rc = self._drive(sup)
|
||||
self.assertEqual(0, rc)
|
||||
|
||||
@@ -208,6 +209,23 @@ class TestSupervisor(unittest.TestCase):
|
||||
sup.request_shutdown(reason="test-teardown")
|
||||
self._drive(sup)
|
||||
|
||||
def test_single_daemon_crash_is_restarted_before_tick_completes(self):
|
||||
specs = [_DaemonSpec("crasher", ("/bin/sh", "-c", "exit 1"))]
|
||||
sup = _Supervisor(specs)
|
||||
sup.start_all()
|
||||
original_pid = sup.procs[0][1].pid
|
||||
time.sleep(0.1)
|
||||
|
||||
done = sup.tick()
|
||||
|
||||
self.assertFalse(done)
|
||||
self.assertNotEqual(original_pid, sup.procs[0][1].pid)
|
||||
self.assertEqual(set(), sup._restart_requested)
|
||||
self.assertIsNone(sup.shutdown_at)
|
||||
|
||||
sup.request_shutdown(reason="test-teardown")
|
||||
self._drive(sup)
|
||||
|
||||
def test_crash_then_signal_surfaces_nonzero_exit_code(self):
|
||||
# The crasher's exit code is what reaches the container
|
||||
# exit even though shutdown was triggered by SIGTERM.
|
||||
@@ -224,20 +242,25 @@ class TestSupervisor(unittest.TestCase):
|
||||
rc = self._drive(sup)
|
||||
self.assertEqual(1, rc)
|
||||
|
||||
def test_all_children_die_unattended_loop_converges(self):
|
||||
# If nobody sends a signal but every child eventually
|
||||
# dies on its own, the supervisor still exits — nothing
|
||||
# left to supervise.
|
||||
def test_all_children_die_unattended_are_restarted(self):
|
||||
specs = [
|
||||
_DaemonSpec("a", ("/bin/sh", "-c", "exit 0")),
|
||||
_DaemonSpec("b", ("/bin/sh", "-c", "exit 2")),
|
||||
]
|
||||
sup = _Supervisor(specs)
|
||||
sup.start_all()
|
||||
rc = self._drive(sup)
|
||||
self.assertEqual(2, rc)
|
||||
original_pids = [p.pid for _, p in sup.procs]
|
||||
time.sleep(0.1)
|
||||
|
||||
done = sup.tick()
|
||||
|
||||
self.assertFalse(done)
|
||||
self.assertNotEqual(original_pids, [p.pid for _, p in sup.procs])
|
||||
self.assertIsNone(sup.shutdown_at)
|
||||
|
||||
sup.request_shutdown(reason="test-teardown")
|
||||
self._drive(sup)
|
||||
|
||||
def test_forward_signal_to_named_child(self):
|
||||
# SIGHUP needs to reach mitmdump inside the bundle so
|
||||
# routes.yaml reloads (egress_apply.py issues `docker kill
|
||||
|
||||
@@ -25,6 +25,10 @@ def _bottle(**kwargs: object) -> ManifestBottle:
|
||||
return ManifestBottle.from_dict("test", kwargs)
|
||||
|
||||
|
||||
def _git_repo(url: str) -> dict[str, object]:
|
||||
return {"url": url, "key": {"provider": "gitea", "forge_token_env": "TOK"}}
|
||||
|
||||
|
||||
class TestMergeBottlesRuntime(unittest.TestCase):
|
||||
def test_single_bottle_returns_as_is(self):
|
||||
b = _bottle(env={"FOO": "1"})
|
||||
@@ -115,6 +119,27 @@ class TestMergeBottlesRuntime(unittest.TestCase):
|
||||
self.assertEqual("2", result.env["B"])
|
||||
self.assertEqual("3", result.env["C"])
|
||||
|
||||
def test_git_repo_only_in_override_does_not_raise(self):
|
||||
# Regression for issue #457: override bottle declares a repo that the
|
||||
# base doesn't have → KeyError on base_repos_by_name[n].
|
||||
base = _bottle(env={"X": "base"})
|
||||
override = _bottle(**{"git-gate": {"repos": {"myrepo": _git_repo("ssh://git@example.com/repo.git")}}})
|
||||
result = merge_bottles_runtime([base, override])
|
||||
self.assertIn("myrepo", [e.Name for e in result.git])
|
||||
|
||||
def test_git_repo_only_in_base_survives_override(self):
|
||||
base = _bottle(**{"git-gate": {"repos": {"myrepo": _git_repo("ssh://git@example.com/repo.git")}}})
|
||||
override = _bottle(env={"X": "override"})
|
||||
result = merge_bottles_runtime([base, override])
|
||||
self.assertIn("myrepo", [e.Name for e in result.git])
|
||||
|
||||
def test_git_repo_override_wins_by_name(self):
|
||||
base = _bottle(**{"git-gate": {"repos": {"myrepo": _git_repo("ssh://git@base.example.com/repo.git")}}})
|
||||
override = _bottle(**{"git-gate": {"repos": {"myrepo": _git_repo("ssh://git@override.example.com/repo.git")}}})
|
||||
result = merge_bottles_runtime([base, override])
|
||||
self.assertEqual(1, len(result.git))
|
||||
self.assertEqual("ssh://git@override.example.com/repo.git", result.git[0].Upstream)
|
||||
|
||||
def test_empty_list_raises(self):
|
||||
with self.assertRaises(ValueError):
|
||||
merge_bottles_runtime([])
|
||||
|
||||
Reference in New Issue
Block a user