refactor(bottles): absorb prepare/launch fns into DockerBottlePlatform
test / run tests/run_tests.py (pull_request) Successful in 14s
test / run tests/run_tests.py (pull_request) Successful in 14s
The module-level prepare_docker_bottle and launch_docker_bottle were trivial delegations from the platform class. Move their bodies onto DockerBottlePlatform.prepare and .launch directly. The launch method keeps the @contextmanager decorator; isinstance narrows BottlePlan to DockerBottlePlan at the entry point. Internal helpers (_run_agent_container, _provision_container, runsc_available) stay at module scope — they're stateless and don't benefit from self. No behavior change. Tests pass; dry-run output unchanged.
This commit is contained in:
+160
-173
@@ -1,14 +1,14 @@
|
|||||||
"""Docker bottle factory.
|
"""Docker bottle platform.
|
||||||
|
|
||||||
Two phases:
|
DockerBottlePlatform owns the two-phase factory:
|
||||||
|
|
||||||
prepare_docker_bottle(spec, stage_dir=...) -> DockerBottlePlan
|
.prepare(spec, stage_dir=...) -> DockerBottlePlan
|
||||||
Resolve names, validate host-side prerequisites, and write
|
Resolve names, validate host-side prerequisites, and write
|
||||||
scratch files (env_file, args_file, prompt, pipelock yaml) to
|
scratch files (env_file, args_file, prompt, pipelock yaml) to
|
||||||
stage_dir. No Docker resources are created yet. Suitable to call
|
stage_dir. No Docker resources are created yet. Suitable to call
|
||||||
before the y/N preflight.
|
before the y/N preflight.
|
||||||
|
|
||||||
launch_docker_bottle(plan) -> ContextManager[Bottle]
|
.launch(plan) -> ContextManager[_DockerBottle]
|
||||||
Build the image, create networks, boot the pipelock sidecar,
|
Build the image, create networks, boot the pipelock sidecar,
|
||||||
launch the agent container (with `--runtime=runsc` iff the
|
launch the agent container (with `--runtime=runsc` iff the
|
||||||
daemon has gVisor registered), and copy prompt/skills/ssh/.git
|
daemon has gVisor registered), and copy prompt/skills/ssh/.git
|
||||||
@@ -22,7 +22,7 @@ from __future__ import annotations
|
|||||||
import os
|
import os
|
||||||
import subprocess
|
import subprocess
|
||||||
import sys
|
import sys
|
||||||
from contextlib import AbstractContextManager, contextmanager
|
from contextlib import contextmanager
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import Iterator
|
from typing import Iterator
|
||||||
@@ -37,6 +37,10 @@ from ..log import die, info
|
|||||||
from . import BottlePlan, BottlePlatform, BottleSpec
|
from . import BottlePlan, BottlePlatform, BottleSpec
|
||||||
|
|
||||||
|
|
||||||
|
# Where the repo root lives, for `docker build` context. Computed once.
|
||||||
|
_REPO_DIR = str(Path(__file__).resolve().parent.parent.parent)
|
||||||
|
|
||||||
|
|
||||||
# --- Runtime detection -----------------------------------------------------
|
# --- Runtime detection -----------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
@@ -56,8 +60,9 @@ def runsc_available() -> bool:
|
|||||||
|
|
||||||
@dataclass(frozen=True)
|
@dataclass(frozen=True)
|
||||||
class DockerBottlePlan(BottlePlan):
|
class DockerBottlePlan(BottlePlan):
|
||||||
"""Docker-specific resolved fields produced by prepare_docker_bottle.
|
"""Docker-specific resolved fields produced by
|
||||||
Inherits `spec` and `stage_dir` from BottlePlan."""
|
DockerBottlePlatform.prepare. Inherits `spec` and `stage_dir` from
|
||||||
|
BottlePlan."""
|
||||||
|
|
||||||
slug: str
|
slug: str
|
||||||
container_name: str
|
container_name: str
|
||||||
@@ -153,166 +158,7 @@ class _DockerBottle:
|
|||||||
self._teardown()
|
self._teardown()
|
||||||
|
|
||||||
|
|
||||||
# --- Prepare ---------------------------------------------------------------
|
# --- Container internals ---------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
def prepare_docker_bottle(spec: BottleSpec, *, stage_dir: Path) -> DockerBottlePlan:
|
|
||||||
"""Resolve names, validate, write scratch files. No Docker resources
|
|
||||||
are created; the only side effects are host-side files under
|
|
||||||
stage_dir and a probe of `docker info`."""
|
|
||||||
docker_mod.require_docker()
|
|
||||||
|
|
||||||
manifest = spec.manifest
|
|
||||||
manifest.require_agent(spec.agent_name)
|
|
||||||
agent = manifest.agents[spec.agent_name]
|
|
||||||
bottle = manifest.bottle_for(spec.agent_name)
|
|
||||||
bottle_name = agent.bottle
|
|
||||||
|
|
||||||
slug = docker_mod.slugify(spec.agent_name)
|
|
||||||
|
|
||||||
image = os.environ.get("CLAUDE_BOTTLE_IMAGE", "claude-bottle:latest")
|
|
||||||
derived_image = ""
|
|
||||||
runtime_image = image
|
|
||||||
if spec.copy_cwd:
|
|
||||||
derived_image = os.environ.get(
|
|
||||||
"CLAUDE_BOTTLE_DERIVED_IMAGE", f"claude-bottle:cwd-{slug}"
|
|
||||||
)
|
|
||||||
runtime_image = derived_image
|
|
||||||
|
|
||||||
default_container = f"claude-bottle-{slug}"
|
|
||||||
pinned_container = os.environ.get("CLAUDE_BOTTLE_CONTAINER", "")
|
|
||||||
container_name = pinned_container or default_container
|
|
||||||
container_name_pinned = bool(pinned_container)
|
|
||||||
suffix = 2
|
|
||||||
if container_name_pinned:
|
|
||||||
if docker_mod.container_exists(container_name):
|
|
||||||
die(
|
|
||||||
f"container '{container_name}' already exists "
|
|
||||||
f"(pinned via CLAUDE_BOTTLE_CONTAINER). "
|
|
||||||
f"Remove it with 'docker rm -f {container_name}' or unset the override."
|
|
||||||
)
|
|
||||||
else:
|
|
||||||
while docker_mod.container_exists(container_name):
|
|
||||||
container_name = f"{default_container}-{suffix}"
|
|
||||||
suffix += 1
|
|
||||||
if suffix > 100:
|
|
||||||
die(
|
|
||||||
f"could not find a free container name after "
|
|
||||||
f"{default_container}-99; clean up old containers with "
|
|
||||||
f"'docker rm -f <name>'"
|
|
||||||
)
|
|
||||||
|
|
||||||
if agent.skills:
|
|
||||||
skills_mod.skills_validate_all(list(agent.skills))
|
|
||||||
if bottle.ssh:
|
|
||||||
ssh_mod.ssh_validate_entries(bottle.ssh)
|
|
||||||
|
|
||||||
env_file = stage_dir / "agent.env"
|
|
||||||
args_file = stage_dir / "docker-args"
|
|
||||||
prompt_file = stage_dir / "prompt.txt"
|
|
||||||
pipelock_yaml_filename = "pipelock.yaml"
|
|
||||||
pipelock_yaml = stage_dir / pipelock_yaml_filename
|
|
||||||
env_file.write_text("")
|
|
||||||
env_file.chmod(0o600)
|
|
||||||
args_file.write_text("")
|
|
||||||
prompt_file.write_text("")
|
|
||||||
prompt_file.chmod(0o600)
|
|
||||||
|
|
||||||
pipelock.pipelock_write_yaml(manifest, bottle_name, pipelock_yaml)
|
|
||||||
env_resolve(manifest, spec.agent_name, env_file, args_file)
|
|
||||||
prompt_file.write_text(agent.prompt)
|
|
||||||
|
|
||||||
allowlist_summary = pipelock.pipelock_allowlist_summary(manifest, bottle_name)
|
|
||||||
use_runsc = runsc_available()
|
|
||||||
|
|
||||||
return DockerBottlePlan(
|
|
||||||
spec=spec,
|
|
||||||
slug=slug,
|
|
||||||
container_name=container_name,
|
|
||||||
container_name_pinned=container_name_pinned,
|
|
||||||
image=image,
|
|
||||||
derived_image=derived_image,
|
|
||||||
runtime_image=runtime_image,
|
|
||||||
stage_dir=stage_dir,
|
|
||||||
env_file=env_file,
|
|
||||||
args_file=args_file,
|
|
||||||
prompt_file=prompt_file,
|
|
||||||
pipelock_yaml_path=pipelock_yaml,
|
|
||||||
pipelock_yaml_filename=pipelock_yaml_filename,
|
|
||||||
allowlist_summary=allowlist_summary,
|
|
||||||
use_runsc=use_runsc,
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
# --- Launch ----------------------------------------------------------------
|
|
||||||
|
|
||||||
|
|
||||||
# Where the repo root lives, for `docker build` context. Computed once.
|
|
||||||
_REPO_DIR = str(Path(__file__).resolve().parent.parent.parent)
|
|
||||||
|
|
||||||
|
|
||||||
@contextmanager
|
|
||||||
def launch_docker_bottle(plan: DockerBottlePlan) -> Iterator[_DockerBottle]:
|
|
||||||
"""Build, launch, and provision a Docker bottle. Teardown on exit."""
|
|
||||||
state: dict[str, str] = {
|
|
||||||
"container": "",
|
|
||||||
"pipelock": "",
|
|
||||||
"internal_network": "",
|
|
||||||
"egress_network": "",
|
|
||||||
}
|
|
||||||
|
|
||||||
def teardown() -> None:
|
|
||||||
try:
|
|
||||||
if state["container"] and docker_mod.container_exists(state["container"]):
|
|
||||||
subprocess.run(
|
|
||||||
["docker", "rm", "-f", state["container"]],
|
|
||||||
stdout=subprocess.DEVNULL,
|
|
||||||
stderr=subprocess.DEVNULL,
|
|
||||||
)
|
|
||||||
state["container"] = ""
|
|
||||||
if state["pipelock"]:
|
|
||||||
pipelock.pipelock_stop(plan.slug)
|
|
||||||
state["pipelock"] = ""
|
|
||||||
if state["internal_network"]:
|
|
||||||
network_mod.network_remove(state["internal_network"])
|
|
||||||
state["internal_network"] = ""
|
|
||||||
if state["egress_network"]:
|
|
||||||
network_mod.network_remove(state["egress_network"])
|
|
||||||
state["egress_network"] = ""
|
|
||||||
except BaseException:
|
|
||||||
# Teardown must not raise; swallow so the caller's __exit__
|
|
||||||
# path can still propagate the original error.
|
|
||||||
pass
|
|
||||||
|
|
||||||
try:
|
|
||||||
docker_mod.build_image(plan.image, _REPO_DIR)
|
|
||||||
if plan.derived_image:
|
|
||||||
docker_mod.build_image_with_cwd(
|
|
||||||
plan.derived_image, plan.image, plan.spec.user_cwd
|
|
||||||
)
|
|
||||||
|
|
||||||
state["internal_network"] = network_mod.network_create_internal(plan.slug)
|
|
||||||
state["egress_network"] = network_mod.network_create_egress(plan.slug)
|
|
||||||
state["pipelock"] = pipelock.pipelock_start(
|
|
||||||
plan.slug,
|
|
||||||
state["internal_network"],
|
|
||||||
state["egress_network"],
|
|
||||||
plan.stage_dir,
|
|
||||||
plan.pipelock_yaml_filename,
|
|
||||||
)
|
|
||||||
|
|
||||||
container = _run_agent_container(plan, state["internal_network"])
|
|
||||||
state["container"] = container
|
|
||||||
|
|
||||||
prompt_path = _provision_container(plan, container)
|
|
||||||
|
|
||||||
bottle = _DockerBottle(container, teardown, prompt_path)
|
|
||||||
yield bottle
|
|
||||||
finally:
|
|
||||||
teardown()
|
|
||||||
|
|
||||||
|
|
||||||
# --- Internals -------------------------------------------------------------
|
|
||||||
|
|
||||||
|
|
||||||
def _run_agent_container(plan: DockerBottlePlan, internal_network: str) -> str:
|
def _run_agent_container(plan: DockerBottlePlan, internal_network: str) -> str:
|
||||||
@@ -441,17 +287,158 @@ def _provision_container(plan: DockerBottlePlan, container: str) -> str | None:
|
|||||||
|
|
||||||
class DockerBottlePlatform(BottlePlatform):
|
class DockerBottlePlatform(BottlePlatform):
|
||||||
"""Docker platform implementation. Selected by CLAUDE_BOTTLE_PLATFORM
|
"""Docker platform implementation. Selected by CLAUDE_BOTTLE_PLATFORM
|
||||||
(default). The methods delegate to the module-level prepare/launch
|
(default)."""
|
||||||
functions so the platform class itself stays a thin dispatch layer."""
|
|
||||||
|
|
||||||
name = "docker"
|
name = "docker"
|
||||||
|
|
||||||
def prepare(self, spec: BottleSpec, *, stage_dir: Path) -> BottlePlan:
|
def prepare(self, spec: BottleSpec, *, stage_dir: Path) -> DockerBottlePlan:
|
||||||
return prepare_docker_bottle(spec, stage_dir=stage_dir)
|
"""Resolve names, validate, write scratch files. No Docker
|
||||||
|
resources are created; the only side effects are host-side
|
||||||
|
files under stage_dir and a probe of `docker info`."""
|
||||||
|
docker_mod.require_docker()
|
||||||
|
|
||||||
def launch(self, plan: BottlePlan) -> AbstractContextManager[_DockerBottle]:
|
manifest = spec.manifest
|
||||||
|
manifest.require_agent(spec.agent_name)
|
||||||
|
agent = manifest.agents[spec.agent_name]
|
||||||
|
bottle = manifest.bottle_for(spec.agent_name)
|
||||||
|
bottle_name = agent.bottle
|
||||||
|
|
||||||
|
slug = docker_mod.slugify(spec.agent_name)
|
||||||
|
|
||||||
|
image = os.environ.get("CLAUDE_BOTTLE_IMAGE", "claude-bottle:latest")
|
||||||
|
derived_image = ""
|
||||||
|
runtime_image = image
|
||||||
|
if spec.copy_cwd:
|
||||||
|
derived_image = os.environ.get(
|
||||||
|
"CLAUDE_BOTTLE_DERIVED_IMAGE", f"claude-bottle:cwd-{slug}"
|
||||||
|
)
|
||||||
|
runtime_image = derived_image
|
||||||
|
|
||||||
|
default_container = f"claude-bottle-{slug}"
|
||||||
|
pinned_container = os.environ.get("CLAUDE_BOTTLE_CONTAINER", "")
|
||||||
|
container_name = pinned_container or default_container
|
||||||
|
container_name_pinned = bool(pinned_container)
|
||||||
|
suffix = 2
|
||||||
|
if container_name_pinned:
|
||||||
|
if docker_mod.container_exists(container_name):
|
||||||
|
die(
|
||||||
|
f"container '{container_name}' already exists "
|
||||||
|
f"(pinned via CLAUDE_BOTTLE_CONTAINER). "
|
||||||
|
f"Remove it with 'docker rm -f {container_name}' or unset the override."
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
while docker_mod.container_exists(container_name):
|
||||||
|
container_name = f"{default_container}-{suffix}"
|
||||||
|
suffix += 1
|
||||||
|
if suffix > 100:
|
||||||
|
die(
|
||||||
|
f"could not find a free container name after "
|
||||||
|
f"{default_container}-99; clean up old containers with "
|
||||||
|
f"'docker rm -f <name>'"
|
||||||
|
)
|
||||||
|
|
||||||
|
if agent.skills:
|
||||||
|
skills_mod.skills_validate_all(list(agent.skills))
|
||||||
|
if bottle.ssh:
|
||||||
|
ssh_mod.ssh_validate_entries(bottle.ssh)
|
||||||
|
|
||||||
|
env_file = stage_dir / "agent.env"
|
||||||
|
args_file = stage_dir / "docker-args"
|
||||||
|
prompt_file = stage_dir / "prompt.txt"
|
||||||
|
pipelock_yaml_filename = "pipelock.yaml"
|
||||||
|
pipelock_yaml = stage_dir / pipelock_yaml_filename
|
||||||
|
env_file.write_text("")
|
||||||
|
env_file.chmod(0o600)
|
||||||
|
args_file.write_text("")
|
||||||
|
prompt_file.write_text("")
|
||||||
|
prompt_file.chmod(0o600)
|
||||||
|
|
||||||
|
pipelock.pipelock_write_yaml(manifest, bottle_name, pipelock_yaml)
|
||||||
|
env_resolve(manifest, spec.agent_name, env_file, args_file)
|
||||||
|
prompt_file.write_text(agent.prompt)
|
||||||
|
|
||||||
|
allowlist_summary = pipelock.pipelock_allowlist_summary(manifest, bottle_name)
|
||||||
|
use_runsc = runsc_available()
|
||||||
|
|
||||||
|
return DockerBottlePlan(
|
||||||
|
spec=spec,
|
||||||
|
stage_dir=stage_dir,
|
||||||
|
slug=slug,
|
||||||
|
container_name=container_name,
|
||||||
|
container_name_pinned=container_name_pinned,
|
||||||
|
image=image,
|
||||||
|
derived_image=derived_image,
|
||||||
|
runtime_image=runtime_image,
|
||||||
|
env_file=env_file,
|
||||||
|
args_file=args_file,
|
||||||
|
prompt_file=prompt_file,
|
||||||
|
pipelock_yaml_path=pipelock_yaml,
|
||||||
|
pipelock_yaml_filename=pipelock_yaml_filename,
|
||||||
|
allowlist_summary=allowlist_summary,
|
||||||
|
use_runsc=use_runsc,
|
||||||
|
)
|
||||||
|
|
||||||
|
@contextmanager
|
||||||
|
def launch(self, plan: BottlePlan) -> Iterator[_DockerBottle]:
|
||||||
|
"""Build, launch, and provision a Docker bottle. Teardown on exit."""
|
||||||
assert isinstance(plan, DockerBottlePlan), (
|
assert isinstance(plan, DockerBottlePlan), (
|
||||||
f"DockerBottlePlatform.launch expects DockerBottlePlan, "
|
f"DockerBottlePlatform.launch expects DockerBottlePlan, "
|
||||||
f"got {type(plan).__name__}"
|
f"got {type(plan).__name__}"
|
||||||
)
|
)
|
||||||
return launch_docker_bottle(plan)
|
|
||||||
|
state: dict[str, str] = {
|
||||||
|
"container": "",
|
||||||
|
"pipelock": "",
|
||||||
|
"internal_network": "",
|
||||||
|
"egress_network": "",
|
||||||
|
}
|
||||||
|
|
||||||
|
def teardown() -> None:
|
||||||
|
try:
|
||||||
|
if state["container"] and docker_mod.container_exists(state["container"]):
|
||||||
|
subprocess.run(
|
||||||
|
["docker", "rm", "-f", state["container"]],
|
||||||
|
stdout=subprocess.DEVNULL,
|
||||||
|
stderr=subprocess.DEVNULL,
|
||||||
|
)
|
||||||
|
state["container"] = ""
|
||||||
|
if state["pipelock"]:
|
||||||
|
pipelock.pipelock_stop(plan.slug)
|
||||||
|
state["pipelock"] = ""
|
||||||
|
if state["internal_network"]:
|
||||||
|
network_mod.network_remove(state["internal_network"])
|
||||||
|
state["internal_network"] = ""
|
||||||
|
if state["egress_network"]:
|
||||||
|
network_mod.network_remove(state["egress_network"])
|
||||||
|
state["egress_network"] = ""
|
||||||
|
except BaseException:
|
||||||
|
# Teardown must not raise; swallow so the caller's
|
||||||
|
# __exit__ path can still propagate the original error.
|
||||||
|
pass
|
||||||
|
|
||||||
|
try:
|
||||||
|
docker_mod.build_image(plan.image, _REPO_DIR)
|
||||||
|
if plan.derived_image:
|
||||||
|
docker_mod.build_image_with_cwd(
|
||||||
|
plan.derived_image, plan.image, plan.spec.user_cwd
|
||||||
|
)
|
||||||
|
|
||||||
|
state["internal_network"] = network_mod.network_create_internal(plan.slug)
|
||||||
|
state["egress_network"] = network_mod.network_create_egress(plan.slug)
|
||||||
|
state["pipelock"] = pipelock.pipelock_start(
|
||||||
|
plan.slug,
|
||||||
|
state["internal_network"],
|
||||||
|
state["egress_network"],
|
||||||
|
plan.stage_dir,
|
||||||
|
plan.pipelock_yaml_filename,
|
||||||
|
)
|
||||||
|
|
||||||
|
container = _run_agent_container(plan, state["internal_network"])
|
||||||
|
state["container"] = container
|
||||||
|
|
||||||
|
prompt_path = _provision_container(plan, container)
|
||||||
|
|
||||||
|
bottle = _DockerBottle(container, teardown, prompt_path)
|
||||||
|
yield bottle
|
||||||
|
finally:
|
||||||
|
teardown()
|
||||||
|
|||||||
Reference in New Issue
Block a user