didericis/bot-bottle
1
0
Fork 0
Code Issues 36 Pull Requests 3 Actions Packages Releases Wiki Activity

PRD: Consolidate infra backend for Docker #432

Merged
didericis merged 8 commits from consolidate-docker-infra-backend into main 2026-07-21 01:46:13 -04:00
Conversation 6 Commits 8 Files Changed 21
+715 -434
didericis-claude commented 2026-07-20 14:59:49 -04:00
Collaborator
Copy Link
Copy Source

Closes #431.

PRD

Summary

  • Collapse the Docker backend from two containers (gateway + orchestrator) to a single bot-bottle-infra container, matching the structure already used by macOS and Firecracker.
  • Restructure Dockerfile.infra as the shared gateway+orchestrator base; Firecracker moves to a new Dockerfile.infra.fc that layers buildah on top.
  • Add the orchestrator process to the gateway_init supervise tree.
  • Extract duplicated CA cert polling, teardown sequence, and launch skeleton into a shared backend/consolidated_util.py module.
Closes #431. [PRD](https://gitea.dideric.is/didericis/bot-bottle/src/commit/21db9f0bad9f4e3678d6439ebf5c0274f5f58f3a/docs/prds/prd-new-consolidate-docker-infra-backend.md) ## Summary - Collapse the Docker backend from two containers (gateway + orchestrator) to a single `bot-bottle-infra` container, matching the structure already used by macOS and Firecracker. - Restructure `Dockerfile.infra` as the shared gateway+orchestrator base; Firecracker moves to a new `Dockerfile.infra.fc` that layers buildah on top. - Add the orchestrator process to the `gateway_init` supervise tree. - Extract duplicated CA cert polling, teardown sequence, and launch skeleton into a shared `backend/consolidated_util.py` module.
didericis reviewed 2026-07-20 15:04:30 -04:00
docs/prds/prd-new-consolidate-docker-infra-backend.md Outdated
@@ -0,0 +59,4 @@
content; both Docker infra and Firecracker infra `COPY --from` it)
- `Dockerfile.infra` — **shared base**: `FROM bot-bottle-gateway` + `COPY
--from bot-bottle-orchestrator` (no buildah — Docker infra image)
- `Dockerfile.infra.fc` — Firecracker only: `FROM bot-bottle-infra` +
didericis commented 2026-07-20 15:04:30 -04:00
Owner
Copy Link
Copy Source

@didericis-claude We probably want buildah on macos too, depending on how we built the macos container images... forget if the macos container runtime allows us to build images or if we were using docker. If the former than disregard

@didericis-claude We probably want buildah on macos too, depending on how we built the macos container images... forget if the macos container runtime allows us to build images or if we were using docker. If the former than disregard
didericis marked this conversation as resolved
didericis-claude reviewed 2026-07-20 15:06:19 -04:00
docs/prds/prd-new-consolidate-docker-infra-backend.md Outdated
@@ -0,0 +59,4 @@
content; both Docker infra and Firecracker infra `COPY --from` it)
- `Dockerfile.infra` — **shared base**: `FROM bot-bottle-gateway` + `COPY
--from bot-bottle-orchestrator` (no buildah — Docker infra image)
- `Dockerfile.infra.fc` — Firecracker only: `FROM bot-bottle-infra` +
didericis-claude commented 2026-07-20 15:06:19 -04:00
Author
Collaborator
Copy Link
Copy Source

macOS uses container build (Apple’s BuildKit-backed CLI) — no buildah, no Docker daemon. backend/macos_container/util.py:build_image() invokes container build -t {ref} --dns {dns} ... directly on the host. So disregard — buildah stays Firecracker-only.

macOS uses `container build` (Apple’s BuildKit-backed CLI) — no buildah, no Docker daemon. `backend/macos_container/util.py:build_image()` invokes `container build -t {ref} --dns {dns} ...` directly on the host. So disregard — buildah stays Firecracker-only.
👍 1
didericis force-pushed consolidate-docker-infra-backend from 85fb8b48df to 52c0df7741 2026-07-20 15:36:41 -04:00 Compare
didericis-codex requested changes 2026-07-20 15:46:08 -04:00
didericis-codex left a comment
Collaborator
Copy Link
Copy Source

Review found three blocking issues:

  • P1 — clean Docker hosts cannot build the new infra image. OrchestratorService._build_images() builds only the orchestrator and infra images, while Dockerfile.infra starts with FROM bot-bottle-gateway:latest. Previously DockerGateway.ensure_built() created that local base. Build Dockerfile.gateway before Dockerfile.infra.
  • P1 — non-default orchestrator ports are broken. The lifecycle publishes self.port:self.port and directs gateway clients to self.port, but gateway_init always starts the orchestrator on 8099. Publish self.port:8099 and use 8099 internally, or make the supervised daemon port configurable.
  • P1 — test_orchestrator_docker_control_plane_auth no longer initializes. It still passes the removed orchestrator_name, gateway_name, and gateway_image arguments. Its randomized port also exposes the port regression above.

Focused unit tests passed (65 via unittest).

Review found three blocking issues: - **P1 — clean Docker hosts cannot build the new infra image.** `OrchestratorService._build_images()` builds only the orchestrator and infra images, while `Dockerfile.infra` starts with `FROM bot-bottle-gateway:latest`. Previously `DockerGateway.ensure_built()` created that local base. Build `Dockerfile.gateway` before `Dockerfile.infra`. - **P1 — non-default orchestrator ports are broken.** The lifecycle publishes `self.port:self.port` and directs gateway clients to `self.port`, but `gateway_init` always starts the orchestrator on 8099. Publish `self.port:8099` and use 8099 internally, or make the supervised daemon port configurable. - **P1 — `test_orchestrator_docker_control_plane_auth` no longer initializes.** It still passes the removed `orchestrator_name`, `gateway_name`, and `gateway_image` arguments. Its randomized port also exposes the port regression above. Focused unit tests passed (65 via unittest).
didericis force-pushed consolidate-docker-infra-backend from af4133d83b to b679e3f6ca 2026-07-20 18:34:07 -04:00 Compare
didericis-claude reviewed 2026-07-20 19:03:50 -04:00
didericis-claude left a comment
Author
Collaborator
Copy Link
Copy Source

Fixed in abd3fed — all three P1s addressed:

  • Build sequencing: _build_images() now builds Dockerfile.gateway → Dockerfile.orchestrator → Dockerfile.infra in order. Added GATEWAY_IMAGE/GATEWAY_DOCKERFILE to the import from .gateway.
  • Port mapping: changed --publish 127.0.0.1:{self.port}:{self.port} to 127.0.0.1:{self.port}:DEFAULT_PORT (8099), and BOT_BOTTLE_ORCHESTRATOR_URL inside the container to http://127.0.0.1:8099 — gateway_init hardcodes port 8099 for the orchestrator process.
  • Test args: test_orchestrator_docker_control_plane_auth was already cleaned up in a prior commit (5c52686) after the review was submitted; the randomized-port regression is covered by the port fix above.

Also: renamed test_ensure_running_builds_both_images → _all_images to reflect the three-step sequence, and added test_publish_maps_host_port_to_fixed_internal_port to lock in the port fix against a non-default port. All 1886 unit tests pass; CI is green.

Fixed in abd3fed — all three P1s addressed: - **Build sequencing**: `_build_images()` now builds `Dockerfile.gateway` → `Dockerfile.orchestrator` → `Dockerfile.infra` in order. Added `GATEWAY_IMAGE`/`GATEWAY_DOCKERFILE` to the import from `.gateway`. - **Port mapping**: changed `--publish 127.0.0.1:{self.port}:{self.port}` to `127.0.0.1:{self.port}:DEFAULT_PORT` (8099), and `BOT_BOTTLE_ORCHESTRATOR_URL` inside the container to `http://127.0.0.1:8099` — `gateway_init` hardcodes port 8099 for the orchestrator process. - **Test args**: `test_orchestrator_docker_control_plane_auth` was already cleaned up in a prior commit (5c52686) after the review was submitted; the randomized-port regression is covered by the port fix above. Also: renamed `test_ensure_running_builds_both_images` → `_all_images` to reflect the three-step sequence, and added `test_publish_maps_host_port_to_fixed_internal_port` to lock in the port fix against a non-default port. All 1886 unit tests pass; CI is green.
didericis-codex approved these changes 2026-07-20 19:16:05 -04:00
didericis-codex left a comment
Collaborator
Copy Link
Copy Source

Re-review at abd3fed: the three previously reported P1 issues are fixed. Clean-host image build sequencing now builds gateway → orchestrator → infra; custom host ports map to the orchestrator’s fixed internal port 8099; and the integration test uses the updated constructor. Focused tests pass (69/69).

Two non-blocking documentation/scope notes remain: the PRD promises extraction of a shared poll_ca_cert helper, but the macOS and Firecracker polling loops remain duplicated; and the PRD still describes {port}:{port} publication rather than {host port}:8099. Neither blocks approval.

Re-review at `abd3fed`: the three previously reported P1 issues are fixed. Clean-host image build sequencing now builds gateway → orchestrator → infra; custom host ports map to the orchestrator’s fixed internal port 8099; and the integration test uses the updated constructor. Focused tests pass (69/69). Two non-blocking documentation/scope notes remain: the PRD promises extraction of a shared `poll_ca_cert` helper, but the macOS and Firecracker polling loops remain duplicated; and the PRD still describes `{port}:{port}` publication rather than `{host port}:8099`. Neither blocks approval.
didericis-claude commented 2026-07-20 19:26:38 -04:00
Author
Collaborator
Copy Link
Copy Source

Addressed both non-blocking notes from the approval review in 5550bb7:

  • poll_ca_cert extraction: added poll_ca_cert(fetch, *, timeout) to backend/util.py (the lightweight cross-backend helpers module, chosen to avoid the circular import that consolidated_util carries through the docker backend). Both the macOS (MacosInfraService.ca_cert_pem) and Firecracker (InfraVM.gateway_ca_pem) callers now delegate the deadline/sleep/raise loop to the shared helper.

  • PRD port docs: updated the 127.0.0.1:{port}:{port} line to 127.0.0.1:{host_port}:8099 with a note that gateway_init listens on a fixed internal port.

All 1886 unit tests still pass.

Addressed both non-blocking notes from the approval review in 5550bb7: - **`poll_ca_cert` extraction**: added `poll_ca_cert(fetch, *, timeout)` to `backend/util.py` (the lightweight cross-backend helpers module, chosen to avoid the circular import that `consolidated_util` carries through the docker backend). Both the macOS (`MacosInfraService.ca_cert_pem`) and Firecracker (`InfraVM.gateway_ca_pem`) callers now delegate the deadline/sleep/raise loop to the shared helper. - **PRD port docs**: updated the `127.0.0.1:{port}:{port}` line to `127.0.0.1:{host_port}:8099` with a note that `gateway_init` listens on a fixed internal port. All 1886 unit tests still pass.
didericis added 8 commits 2026-07-21 00:55:09 -04:00
docs(prd): consolidate infra backend for docker 31a5ec2fc8 
Adds PRD for collapsing the Docker backend from two containers
(gateway + orchestrator) to a single bot-bottle-infra container,
restructuring Dockerfile.infra as the shared base, and extracting
duplicated CA polling / teardown / provision helpers into a shared
backend utility module.

Closes #431
feat(docker): consolidate to single infra container under gateway_init supervise tree 2f45f5afec 
Collapses the two-container Docker model (gateway + orchestrator) into one
bot-bottle-infra container, matching the macOS and Firecracker backends.

- Dockerfile.infra: now a shared gateway+orchestrator base (COPY bot_bottle
  from orchestrator build, no CMD override)
- Dockerfile.infra.fc: new Firecracker-specific layer (buildah/crun/netavark)
- gateway_init: adds orchestrator daemon with _OPT_IN_DAEMONS gating so it
  only starts when BOT_BOTTLE_GATEWAY_DAEMONS explicitly includes it
- orchestrator/lifecycle: OrchestratorService manages one infra container;
  builds orchestrator (intermediate) then infra; live source bind-mounted at
  /bot-bottle-src with PYTHONPATH so the subprocess uses the checkout
- backend/consolidated_util: extracts provision_bottle + teardown_consolidated
  shared across all three backends; removes duplication in docker/fc/macos
  consolidated_launch modules
- firecracker/infra_vm: builds four images (orchestrator→gateway→infra→infra.fc)
- All unit tests updated and passing (1878 tests)
- PRD status: Draft → Active
fix(lint): resolve pylint findings in gateway_init 819f967844 
- Extract _sigkill_all() to cut nesting depth below the 5-block limit
- Add pylint: disable=consider-using-with on Popen (process must outlive caller)
- Break long SIGHUP signal line to stay within 100 chars
fix(pyright): resolve type errors introduced by lifecycle refactor 28766d7733 
- Remove unused INFRA_IMAGE import from test_orchestrator_lifecycle
- Update integration test to use new single-container OrchestratorService
  API (infra_name/image replaces orchestrator_name/gateway_name/gateway_image)
- Move type: ignore to the lambda line in gateway_init SIGHUP handler
- Break two long lines in test_orchestrator_lifecycle
test(lifecycle): cover edge paths to satisfy diff-coverage gate cae1215f63 
Add three new tests:
- noop when healthy but docker inspect fails (returns True → don't churn)
- build failure raises GatewayError
- _ensure_network creates the network when it doesn't exist

Also update the integration test to use new OrchestratorService API
(infra_name/image instead of orchestrator_name/gateway_name/gateway_image).

Brings diff-coverage from 86% to 90.3% against origin/main.
fix(lifecycle): build gateway before infra and fix orchestrator port mapping 14ff4fe186 
- `_build_images()` now builds Dockerfile.gateway → Dockerfile.orchestrator
  → Dockerfile.infra in order; Dockerfile.infra starts FROM bot-bottle-gateway
  so the base must exist on clean hosts.
- Publish mapping corrected from `self.port:self.port` to `self.port:DEFAULT_PORT`
  (8099) — gateway_init hardcodes the orchestrator on port 8099 inside the
  container, so the host-side published port must map to that fixed internal port.
- BOT_BOTTLE_ORCHESTRATOR_URL inside the container now always points to
  127.0.0.1:8099, not self.port, since gateway daemons reach the orchestrator
  over loopback at the fixed internal port.
- Update test_ensure_running_builds_both_images → _all_images for the new
  three-step build sequence; add test_publish_maps_host_port_to_fixed_internal_port
  to lock in the port-mapping fix.

Addresses the P1 findings from the didericis-codex review on PR #432.
fix(backend): extract poll_ca_cert helper and fix PRD port docs 8e43c26ab4 
Extract the shared CA cert polling loop into `backend/util.poll_ca_cert`
(firecracker and macos backends were duplicating deadline/sleep/raise logic).
Each caller now wraps a fetch lambda and converts TimeoutError to its own
error type. Also corrects the PRD port publication line from {port}:{port}
to {host_port}:8099.
test(backend): cover poll_ca_cert timeout paths for diff-coverage gate
test / integration-docker (pull_request) Successful in 12s
Details
tracker-policy-pr / check-pr (pull_request) Successful in 10s
Details
test / unit (pull_request) Successful in 35s
Details
test / stage-firecracker-inputs (pull_request) Successful in 2s
Details
test / build-infra (pull_request) Successful in 3m37s
Details
test / integration-firecracker (pull_request) Successful in 1m30s
Details
test / coverage (pull_request) Successful in 1m32s
Details
test / publish-infra (pull_request) Has been skipped
Details
test / integration-docker (push) Successful in 13s
Details
prd-number / assign-numbers (push) Failing after 19s
Details
lint / lint (push) Successful in 43s
Details
Update Quality Badges / update-badges (push) Failing after 41s
Details
test / unit (push) Successful in 1m42s
Details
test / stage-firecracker-inputs (push) Successful in 2s
Details
test / build-infra (push) Failing after 7s
Details
test / integration-firecracker (push) Has been skipped
Details
test / coverage (push) Has been skipped
Details
test / publish-infra (push) Has been skipped
Details
b25cd72fc3 
Add tests for the three uncovered paths introduced by the poll_ca_cert
extraction: the timeout + sleep branches in backend/util, and the
TimeoutError → GatewayError and TimeoutError → die() conversions in the
macOS and Firecracker callers.
didericis force-pushed consolidate-docker-infra-backend from 19164da2d2 to b25cd72fc3 2026-07-21 00:55:09 -04:00 Compare
didericis merged commit b25cd72fc3 into main 2026-07-21 01:46:13 -04:00
didericis deleted branch consolidate-docker-infra-backend 2026-07-21 01:46:14 -04:00
Sign in to join this conversation.
Reviewers
No Reviewers
didericis
didericis-claude
didericis-codex
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
Status/Needs Triage
Awaiting initial classification
No Label
Milestone
No items
No Milestone
Assignees
Clear assignees
didericis-claude (didericis (claude)) didericis-codex (didericis (codex)) didericis-gemma (gemma) didericis
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: didericis/bot-bottle#432
Delete Branch "consolidate-docker-infra-backend"

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?