didericis/bot-bottle
1
0
Fork 0
Code Issues 26 Pull Requests 22 Actions 1 Packages Projects Releases Wiki Activity

feat(orchestrator): slice 4 — consolidated per-host sidecar #358

Open
didericis-claude wants to merge 2 commits from orchestrator-slice4 into orchestrator-slice3
pull from: orchestrator-slice4
merge into: didericis:orchestrator-slice3
Conversation 4 Commits 2 Files Changed 11
+339 -9
didericis-claude commented 2026-07-13 15:28:43 -04:00
Collaborator
Copy Link
Copy Source

Slice 4 of PRD 0070, stacked on #357. The core consolidation win: one persistent sidecar per host, shared by every bottle, instead of a sidecar bundle per bottle — safe because the attribution invariant (source IP + identity token) lets the sidecar map each request to the right bottle.

  • sidecar.py — a backend-neutral Sidecar lifecycle contract (mirrors LaunchBroker) + a DockerSidecar. Defining behaviour is idempotent singleton: ensure_running starts the instance if absent and is a no-op if it's already up (N launches never spawn N sidecars); stop is idempotent.
  • dockerutil.py — a shared run_docker helper; DockerBroker now uses it too (DRY with slice 3).
  • service.py — the Orchestrator holds an optional Sidecar, exposes ensure_sidecar() + sidecar_status(); GET /sidecar reports it; __main__ --sidecar-image ensures the single sidecar on startup.

Tests: unit (docker mocked) — is_running, ensure idempotent (no-op when up / starts when absent), failure raises, stop idempotent; Orchestrator sidecar wiring/status; control-plane /sidecar; integration (gated) — a real idempotent singleton (one container after two ensures), stop removes. Full suite green (only pre-existing /bin/sleep errors); integration verified locally against real docker.

Merge order: #352 → #356 → #357 → this.

🤖 Generated with Claude Code

Slice 4 of PRD 0070, **stacked on #357**. The core consolidation win: **one persistent sidecar per host**, shared by every bottle, instead of a sidecar bundle per bottle — safe because the attribution invariant (source IP + identity token) lets the sidecar map each request to the right bottle. - **`sidecar.py`** — a backend-neutral `Sidecar` lifecycle contract (mirrors `LaunchBroker`) + a `DockerSidecar`. Defining behaviour is **idempotent singleton**: `ensure_running` starts the instance if absent and is a no-op if it's already up (N launches never spawn N sidecars); `stop` is idempotent. - **`dockerutil.py`** — a shared `run_docker` helper; `DockerBroker` now uses it too (DRY with slice 3). - **`service.py`** — the Orchestrator holds an optional `Sidecar`, exposes `ensure_sidecar()` + `sidecar_status()`; **`GET /sidecar`** reports it; `__main__ --sidecar-image` ensures the single sidecar on startup. **Tests:** unit (docker mocked) — is_running, ensure idempotent (no-op when up / starts when absent), failure raises, stop idempotent; Orchestrator sidecar wiring/status; control-plane `/sidecar`; integration (gated) — a real idempotent singleton (one container after two ensures), stop removes. Full suite green (only pre-existing `/bin/sleep` errors); **integration verified locally against real docker**. Merge order: #352 → #356 → #357 → this. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
didericis-claude added 1 commit 2026-07-13 15:28:44 -04:00
feat(orchestrator): slice 4 — consolidated per-host sidecar (#352)
lint / lint (push) Successful in 1m58s
Details
test / unit (pull_request) Successful in 58s
Details
test / integration (pull_request) Successful in 18s
Details
test / coverage (pull_request) Successful in 1m7s
Details
f85cbdeebf 
The core consolidation win: one persistent sidecar per host, shared by
every bottle, instead of a sidecar bundle per bottle. Safe to share
because the attribution invariant (source IP + identity token) lets the
sidecar map each request to the right bottle.

  * orchestrator/sidecar.py — a backend-neutral `Sidecar` lifecycle
    contract (mirrors LaunchBroker) + a `DockerSidecar` impl. The defining
    behaviour is idempotent singleton: `ensure_running` starts the instance
    if absent and is a no-op if it's already up, so N launches never spawn
    N sidecars; `stop` is idempotent.
  * orchestrator/dockerutil.py — a shared `run_docker` helper; DockerBroker
    now uses it too (DRY with slice 3).
  * service.py — the Orchestrator holds an optional `Sidecar`, exposes
    `ensure_sidecar()` + `sidecar_status()`.
  * control_plane.py — `GET /sidecar` reports it; __main__ gains
    `--sidecar-image` and ensures the single sidecar on startup.

Tests: unit (docker mocked) — is_running, ensure idempotent (no-op when up,
starts when absent), failure raises, stop idempotent; Orchestrator sidecar
wiring/status; control-plane /sidecar; integration (gated) — ensure is a
real idempotent singleton (one container after two ensures), stop removes.
Full suite green (only pre-existing /bin/sleep errors); integration
verified locally against real docker.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WBMWTEtQdJ4W5UrWuLHCck
didericis reviewed 2026-07-13 16:18:55 -04:00
bot_bottle/orchestrator/dockerutil.py Outdated
@@ -0,0 +6,4 @@
import subprocess
def run_docker(argv: list[str]) -> subprocess.CompletedProcess[str]:
didericis commented 2026-07-13 16:18:55 -04:00
Owner
Copy Link
Copy Source

Why not use the docker_mod util? Are there too many deps in it/does it create circular deps? If so that's a bit of a smell, this should really be placed there.

Why not use the docker_mod util? Are there too many deps in it/does it create circular deps? If so that's a bit of a smell, this should really be placed there.
didericis added 1 commit 2026-07-13 16:24:40 -04:00
refactor(orchestrator): move run_docker to a lean top-level docker_cmd (#352)
lint / lint (push) Successful in 2m2s
Details
test / unit (pull_request) Successful in 1m0s
Details
test / integration (pull_request) Successful in 18s
Details
test / coverage (pull_request) Successful in 1m7s
Details
67652899eb 
Review feedback: don't bury a parallel docker util in the orchestrator.
But reusing backend.docker.util (docker_mod) isn't right either — importing
it runs backend/__init__.py, which eagerly loads all three backends
(docker + firecracker + macos) plus the manifest/egress/git-gate/supervise
framework (~76 modules), so every orchestrator import would drag the whole
backend layer in.

Compromise: promote the helper to a top-level, framework-free
bot_bottle/docker_cmd.py (single stdlib import), a proper shared home the
orchestrator's docker components use now and backend.docker.util can adopt
later. Verified `import bot_bottle.orchestrator` stays lean (12 modules, no
firecracker/macos backends).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WBMWTEtQdJ4W5UrWuLHCck
didericis-claude commented 2026-07-13 16:24:55 -04:00
Author
Collaborator
Copy Link
Copy Source

Good instinct, and I dug in. docker_mod (backend.docker.util) is lean itself (only imports ...log), but importing it isn't cheap: it runs backend/__init__.py, which eagerly imports all three bottle backends (docker + firecracker + macos) plus the manifest/egress/git-gate/supervise/workspace framework. Measured: from bot_bottle.backend.docker import util pulls in 76 bot_bottle modules, including the entire firecracker and macos backends.

Since orchestrator/__init__ imports DockerBroker/DockerSidecar, routing them through docker_mod would make every orchestrator import (even the lean registry/control-plane tests) load all three backends — coupling the deliberately-lean orchestrator to the whole backend layer for a 3-line subprocess wrapper. No circular dep, but real weight.

So you're right it's a smell — but the smell is the eager all-backends import in backend/__init__.py, not the helper. Rather than pick between "parallel util" and "drag in the backend framework", I promoted it to a top-level, framework-free bot_bottle/docker_cmd.py (single stdlib import) — a proper shared home the orchestrator uses now and backend.docker.util can adopt later (6765289). Verified import bot_bottle.orchestrator stays lean afterward: 12 modules, no firecracker/macos backends.

Two follow-ups worth a separate cleanup if you want them: (a) make backend/__init__.py lazy-load the backends so docker_mod is cheap to import anywhere, and (b) then thread backend.docker.util's own subprocess.run(["docker", ...]) calls through docker_cmd.run_docker for full DRY.

Good instinct, and I dug in. `docker_mod` (`backend.docker.util`) is lean *itself* (only imports `...log`), but importing it isn't cheap: it runs `backend/__init__.py`, which **eagerly imports all three bottle backends** (docker + firecracker + macos) plus the manifest/egress/git-gate/supervise/workspace framework. Measured: `from bot_bottle.backend.docker import util` pulls in **76 `bot_bottle` modules**, including the entire firecracker and macos backends. Since `orchestrator/__init__` imports `DockerBroker`/`DockerSidecar`, routing them through `docker_mod` would make *every* orchestrator import (even the lean registry/control-plane tests) load all three backends — coupling the deliberately-lean orchestrator to the whole backend layer for a 3-line `subprocess` wrapper. No circular dep, but real weight. So you're right it's a smell — but the smell is the eager all-backends import in `backend/__init__.py`, not the helper. Rather than pick between "parallel util" and "drag in the backend framework", I promoted it to a **top-level, framework-free `bot_bottle/docker_cmd.py`** (single stdlib import) — a proper shared home the orchestrator uses now and `backend.docker.util` can adopt later (`6765289`). Verified `import bot_bottle.orchestrator` stays lean afterward: **12 modules, no firecracker/macos backends**. Two follow-ups worth a separate cleanup if you want them: (a) make `backend/__init__.py` lazy-load the backends so `docker_mod` is cheap to import anywhere, and (b) then thread `backend.docker.util`'s own `subprocess.run(["docker", ...])` calls through `docker_cmd.run_docker` for full DRY.
didericis commented 2026-07-13 16:32:51 -04:00
Owner
Copy Link
Copy Source

@didericis-claude file an issue for those follow ups (can put in the same issue)

@didericis-claude file an issue for those follow ups (can put in the same issue)
didericis-claude commented 2026-07-13 16:33:53 -04:00
Author
Collaborator
Copy Link
Copy Source

Filed both in #359 (one issue, two ordered parts): (1) make backend/__init__.py lazy-load the backends so importing a backend util doesn't drag in all three backends (~76 modules), then (2) DRY backend.docker.util's docker calls onto docker_cmd.run_docker. Neither blocks the orchestrator stack.

Filed both in **#359** (one issue, two ordered parts): (1) make `backend/__init__.py` lazy-load the backends so importing a backend util doesn't drag in all three backends (~76 modules), then (2) DRY `backend.docker.util`'s docker calls onto `docker_cmd.run_docker`. Neither blocks the orchestrator stack.
didericis approved these changes 2026-07-13 16:46:47 -04:00
Some checks are pending
lint / lint (push) Successful in 2m2s
Details
test / unit (pull_request) Successful in 1m0s
Details
test / integration (pull_request) Successful in 18s
Details
test / coverage (pull_request) Successful in 1m7s
Details
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin orchestrator-slice4:orchestrator-slice4
git checkout orchestrator-slice4
Sign in to join this conversation.
Reviewers
No Reviewers
didericis
Labels
Clear labels
Compat/Breaking
Breaking change that won't be backward compatible
 Kind/Bug
Something is not working
 Kind/Documentation
Documentation changes
 Kind/Enhancement
Improve existing functionality
 Kind/Feature
New functionality
 Kind/Security
This is security issue
 Kind/Testing
Issue or pull request related to testing
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Low
4
The priority is low
Priority
Medium
3
The priority is medium
Reviewed
Confirmed
1
Issue has been confirmed
Reviewed
Duplicate
2
This issue or pull request already exists
Reviewed
Invalid
3
Invalid issue
Reviewed
Won't Fix
3
This issue won't be fixed
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
didericis-claude (didericis (claude)) didericis-codex (didericis (codex)) didericis-gemma (gemma) didericis
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: didericis/bot-bottle#358
Delete Branch "orchestrator-slice4"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?