diff --git a/src/agent_env/artifact/artifacts/docker_image.py b/src/agent_env/artifact/artifacts/docker_image.py index 14af42d7..04356b8f 100644 --- a/src/agent_env/artifact/artifacts/docker_image.py +++ b/src/agent_env/artifact/artifacts/docker_image.py @@ -275,12 +275,18 @@ def context_only(self) -> bool: tag the image's build gives it.""" return bool(self.build_context_object_url) and not self.tar_gz_object_url and not names_registry(self.image_name) + def rebuilt_for(self, platform: str | None) -> bool: + """Whether a VM sandbox of ``platform`` builds this image from its build context rather than load its tar.gz, + which is built for another platform: the image runs, though not byte for byte the one stored.""" + return bool(self.tar_gz_object_url and self.build_context_object_url) and self.platform_problem(platform) is not None + def load_problem(self, *, on_this_machine: bool, platform: str | None = None) -> str | None: """Why a VM sandbox can't get this image, or None when it can: a tar.gz is loaded, an image with none is pulled when its name names a registry, one that a VM ``on_this_machine`` or not reaches, and built from its build - context when it has only that; and a VM of ``platform`` runs only an image built for it.""" + context when it has only that; and a VM of ``platform`` runs only an image built for it, or one it rebuilds + from the build context beside a tar.gz of another platform.""" if self.tar_gz_object_url or self.context_only: - return self.platform_problem(platform) + return None if self.rebuilt_for(platform) else self.platform_problem(platform) if names_registry(self.image_name): return pull_problem(self.image_name, on_this_machine=on_this_machine) or self.platform_problem(platform) return (f"{self.id!r} v{self.version} has no tar.gz, and its image name {self.image_name!r} doesn't name a " diff --git a/src/agent_env/bundle/preflight.py b/src/agent_env/bundle/preflight.py index d99b8a6a..dc282c0c 100644 --- a/src/agent_env/bundle/preflight.py +++ b/src/agent_env/bundle/preflight.py @@ -16,9 +16,10 @@ themselves. One version is one of the two, so a run deploying it both ways is refused, as is one deploying it on a provider that runs an image by name. -A gateway deploy on the local provider runs on infra envs the store may not hold yet: the gateway, the service-db its -local Postgres state runs from, and the website browser it adds for websites. The run builds those once its writes are -done (``agent_env.env.bootstrap``), so they're named here, and refused here when they can't be built. +A gateway deploy runs on infra envs the store may not hold yet: the gateway, the service-db its local Postgres state +runs from, and the website browser it adds for websites. In local stores the run builds those once its writes are done +(``agent_env.env.bootstrap``), for this machine's platform and with their build contexts, which a VM or Modal running +another platform builds them from; so they're named here, and refused here when they can't be built. """ from __future__ import annotations @@ -39,7 +40,9 @@ InfraError, default_env_id, infra_to_build, + needs_docker, put_command, + stores_are_local, ) from agent_env.env.env import Env from agent_env.env.envs._deployment import provider_or_class @@ -154,11 +157,14 @@ def finish(self) -> Preflight: self.run_problems.extend(e.problems) contexts = self._contexts() self._default_agent() - if self.docker_users and (reason := docker_unreachable()): + reason = docker_unreachable() if self.docker_users or infra else None + if reason and self.docker_users: shown = ", ".join(self.docker_users[:_SHOWN_DOCKER_USERS]) more = len(self.docker_users) - _SHOWN_DOCKER_USERS self.run_problems.append(f"the local sandbox provider runs containers for {shown}" f"{f' and {more} more' if more > 0 else ''}, and {reason}") + elif reason: # infra the run builds for deploys elsewhere + self.run_problems.append(needs_docker(infra, reason)) problems = list(self.run_problems) for problem, wheres in self.problems.items(): first, *others = wheres @@ -191,10 +197,12 @@ def _env(self, where: str, step: DeployEnvTaskStep) -> None: if any(link.ON_THIS_MACHINE for link in taking): self.infra |= kinds self.docker_users.append(where) + elif taking and stores_are_local(): + self.infra |= kinds def _gateway_elsewhere(self, env_id: str, env_provider: type, kinds: set[str], link: SandboxProvider) -> list[str]: """Why ``link``, a provider not on this machine, can't run env ``env_id``'s gateway deploy: its sandboxes, or - the infra envs of ``kinds`` it runs, which agent-env builds only for the local provider.""" + the infra envs of ``kinds`` it runs.""" found = [] if problem := env_provider.sandbox_problem(link, websites=WEBSITE_BROWSER in kinds): found.append(f"deploys env {env_id!r} on the {_shown(link)} sandbox provider, which {problem}; run it on a VM " @@ -386,20 +394,22 @@ def _refusal(self, link: SandboxProvider, form: Accepts, image: _Image, *, vms_l f"on a provider {builds}") def _remote_infra(self, link: SandboxProvider, kinds: set[str]) -> list[str]: - """Why ``link``, a provider not on this machine, can't run the infra envs of ``kinds``.""" + """Why ``link``, a provider not on this machine, can't run the infra envs of ``kinds``. One the run builds, in + local stores missing or built from other inputs, isn't checked: it's built anew, with its build context.""" + built = {build.kind for build in infra_to_build(kinds)} if stores_are_local() else set() found = [] - for kind in sorted(kinds): + for kind in sorted(kinds - built): env_id = default_env_id(kind) try: env = Env.get(env_id) except NotFoundError: - found.append(f"deploys on the {_shown(link)} sandbox provider, which needs the {kind} env {env_id!r}, and " - "the store doesn't hold it; agent-env builds it only for the local sandbox provider, so put " - f"it in a store that provider can reach (`{put_command(kind)}`)") + found.append(f"deploys on the {_shown(link)} sandbox provider, which needs the {kind} env {env_id!r}, " + f"and the store doesn't hold it; agent-env builds infra envs only into local stores, so put " + f"it with `{put_command(kind)}`") continue artifacts = ([env.db_docker_image_artifact, env.db_web_docker_image_artifact, env.db_mcp_docker_image_artifact] if isinstance(env, ServiceDBEnv) else [env.docker_image_artifact]) - accepts = EnvironmentGatewayProvider.accepts(link, prepared=kind == GATEWAY) + accepts = EnvironmentGatewayProvider.accepts(link) found += [refusal for image in artifacts if (refusal := self._refusal(link, accepts, _Image(f"the {kind} env {env_id!r}'s image {image.id!r}", image), vms_load=True))] diff --git a/src/agent_env/env/bootstrap.py b/src/agent_env/env/bootstrap.py index fa54ddfb..02263c5a 100644 --- a/src/agent_env/env/bootstrap.py +++ b/src/agent_env/env/bootstrap.py @@ -3,6 +3,8 @@ (``default_gateway_env_id``, ``default_service_db_env_id``, ``default_website_browser_env_id``). The put commands build them one at a time; ``ensure_default_envs`` builds the ones a run needs, into local stores only. +Each image keeps its build context beside its tar.gz, so a VM or Modal running another platform than the one it was +built on builds it again from that. """ from __future__ import annotations @@ -17,11 +19,7 @@ from agent_env.env.env import Env from agent_env.env.envs import GatewayEnv, MCPServerEnv from agent_env.env.envs.service_db import ServiceDBEnv -from agent_env.env.envs.website_browser import ( - PLAYWRIGHT_MCP_VERSION, - WEBSITE_BROWSER_ENVIRONMENT_NAME, - WEBSITE_BROWSER_IMAGE_TAG, -) +from agent_env.env.envs.website_browser import WEBSITE_BROWSER_ENVIRONMENT_NAME, WEBSITE_BROWSER_IMAGE_TAG from agent_env.store.base import NotFoundError from agent_env.store.document_store import LocalSqliteDocumentStore from agent_env.store.ids import derive_id @@ -29,12 +27,13 @@ from agent_env.store.local_state import holding_locks from agent_env.store.object_store import LocalFilesystemObjectStore from agent_env.store.routing import configured_store +from agent_env.utils.build_context import BuildContext from agent_env.utils.build_metadata import detect_env_metadata from agent_env.utils.docker_build import build_image, docker_unreachable _ENV_PACKAGE = Path(__file__).parent GATEWAY_DOCKERFILE = _ENV_PACKAGE / "gateway" / "Dockerfile" -GATEWAY_CONTEXT = _ENV_PACKAGE +GATEWAY_CONTEXT = GATEWAY_DOCKERFILE.parent GATEWAY_IMAGE_TAG = "env-gateway" SERVICE_DB_DOCKERFILE = _ENV_PACKAGE / "envs" / "service_db" / "Dockerfile" SERVICE_DB_IMAGE_NAME = "agent-env-service-db" @@ -55,15 +54,10 @@ def _quiet(line: str) -> None: def put_gateway_env(env_id: str, *, platform: str | None, metadata: Mapping[str, str] | None = None, say: Callable[[str], None] = _quiet) -> GatewayEnv: """Build the gateway image for ``platform`` (this host's when None) and write it as the gateway env ``env_id``.""" - say("Building gateway Docker image...") - build_image(GATEWAY_DOCKERFILE, GATEWAY_CONTEXT, GATEWAY_IMAGE_TAG, platform=platform) - say("Creating DockerImageArtifact...") - artifact = DockerImageArtifact.put(id=derive_id(env_id, "env_image"), description="Created from agent-env CLI", - image_name=GATEWAY_IMAGE_TAG) - say(f"Created artifact: id={artifact.id} version={artifact.version}") + artifact = _build_and_put("gateway", derive_id(env_id, "env_image"), "Created from agent-env CLI", + GATEWAY_DOCKERFILE, GATEWAY_IMAGE_TAG, platform=platform, say=say) say("Creating GatewayEnv...") - env = GatewayEnv.put(id=env_id, docker_image_artifact=artifact, - metadata=_metadata(GATEWAY, GATEWAY_DOCKERFILE, GATEWAY_CONTEXT, metadata)) + env = GatewayEnv.put(id=env_id, docker_image_artifact=artifact, metadata=_metadata(GATEWAY, metadata)) say(f"Created GatewayEnv: id={env.id} version={env.version}") return env @@ -72,28 +66,17 @@ def put_service_db_env(env_id: str, *, platform: str | None, metadata: Mapping[s say: Callable[[str], None] = _quiet) -> ServiceDBEnv: """Build the service-db, db-web and db-mcp images for ``platform`` (this host's when None) and write them as the service-db env ``env_id``.""" - artifacts = [] - for label, dockerfile, image, artifact_id, description in ( - ("ServiceDB", SERVICE_DB_DOCKERFILE, SERVICE_DB_IMAGE_NAME, derive_id(env_id, "db_image"), - f"ServiceDB PostgreSQL image for {env_id}"), - ("db-web", DB_WEB_DOCKERFILE, DB_WEB_IMAGE_NAME, derive_id(env_id, "db_web_image"), - "db-web lightweight web UI for database inspection"), - ("db-mcp", DB_MCP_DOCKERFILE, DB_MCP_IMAGE_NAME, derive_id(env_id, "db_mcp_image"), - "db-mcp PostgreSQL MCP server for direct DB access"), - ): - say(f"Building {label} image from {dockerfile}...") - build_image(dockerfile, dockerfile.parent, image, platform=platform) - say(f"Creating {label} DockerImageArtifact...") - artifacts.append(DockerImageArtifact.put(id=artifact_id, description=description, image_name=image)) - say(f"Created {label} DockerImageArtifact: id={artifacts[-1].id} version={artifacts[-1].version}") + db = _build_and_put("ServiceDB", derive_id(env_id, "db_image"), f"ServiceDB PostgreSQL image for {env_id}", + SERVICE_DB_DOCKERFILE, SERVICE_DB_IMAGE_NAME, platform=platform, say=say) + db_web = _build_and_put("db-web", derive_id(env_id, "db_web_image"), + "db-web lightweight web UI for database inspection", DB_WEB_DOCKERFILE, DB_WEB_IMAGE_NAME, + platform=platform, say=say) + db_mcp = _build_and_put("db-mcp", derive_id(env_id, "db_mcp_image"), + "db-mcp PostgreSQL MCP server for direct DB access", DB_MCP_DOCKERFILE, DB_MCP_IMAGE_NAME, + platform=platform, say=say) say("Creating ServiceDBEnv...") - env = ServiceDBEnv.put( - id=env_id, - db_docker_image_artifact=artifacts[0], - db_web_docker_image_artifact=artifacts[1], - db_mcp_docker_image_artifact=artifacts[2], - metadata=_metadata(SERVICE_DB, SERVICE_DB_DOCKERFILE, SERVICE_DB_DOCKERFILE.parent, metadata), - ) + env = ServiceDBEnv.put(id=env_id, db_docker_image_artifact=db, db_web_docker_image_artifact=db_web, + db_mcp_docker_image_artifact=db_mcp, metadata=_metadata(SERVICE_DB, metadata)) say(f"Created ServiceDBEnv: id={env.id} version={env.version}") return env @@ -102,50 +85,53 @@ def put_website_browser_env(env_id: str, *, platform: str | None, metadata: Mapp say: Callable[[str], None] = _quiet) -> MCPServerEnv: """Build the website browser image for ``platform`` (this host's when None) and write it as the MCP server env ``env_id``.""" - say("Building website browser Docker image...") - build_image(WEBSITE_BROWSER_DOCKERFILE, WEBSITE_BROWSER_CONTEXT, WEBSITE_BROWSER_IMAGE_TAG, platform=platform, - build_args={"PLAYWRIGHT_MCP_VERSION": PLAYWRIGHT_MCP_VERSION}) - say("Creating DockerImageArtifact...") - artifact = DockerImageArtifact.put(id=derive_id(env_id, "env_image"), description="Website browser MCP server", - image_name=WEBSITE_BROWSER_IMAGE_TAG) - say(f"Created artifact: id={artifact.id} version={artifact.version}") + artifact = _build_and_put("website browser", derive_id(env_id, "env_image"), "Website browser MCP server", + WEBSITE_BROWSER_DOCKERFILE, WEBSITE_BROWSER_IMAGE_TAG, platform=platform, say=say) say("Creating MCPServerEnv...") env = MCPServerEnv.put(id=env_id, docker_image_artifact=artifact, environment_name=WEBSITE_BROWSER_ENVIRONMENT_NAME, - metadata=_metadata(WEBSITE_BROWSER, WEBSITE_BROWSER_DOCKERFILE, WEBSITE_BROWSER_CONTEXT, - metadata)) + metadata=_metadata(WEBSITE_BROWSER, metadata)) say(f"Created MCPServerEnv: id={env.id} version={env.version} environment_name={env.environment_name}") return env -def _metadata(kind: str, dockerfile: Path, context: Path, extra: Mapping[str, str] | None) -> dict[str, str]: - return {**detect_env_metadata(dockerfile, context), **(extra or {}), BUILD_INPUTS_KEY: build_inputs_digest(kind)} +def _build_and_put(label: str, artifact_id: str, description: str, dockerfile: Path, image_name: str, *, + platform: str | None, say: Callable[[str], None]) -> DockerImageArtifact: + """Build ``dockerfile``'s image for ``platform`` from the folder it's in, and write it with that folder as its build + context, which a sandbox that can't run its tar.gz builds it from.""" + say(f"Building {label} image from {dockerfile}...") + build_image(dockerfile, dockerfile.parent, image_name, platform=platform) + say(f"Creating {label} DockerImageArtifact...") + artifact = DockerImageArtifact.put(id=artifact_id, description=description, image_name=image_name, + build_context_path=str(dockerfile.parent), dockerfile_path=dockerfile.name) + say(f"Created {label} DockerImageArtifact: id={artifact.id} version={artifact.version}") + return artifact + + +def _metadata(kind: str, extra: Mapping[str, str] | None) -> dict[str, str]: + dockerfile = _DOCKERFILES[kind][0] + return {**detect_env_metadata(dockerfile, dockerfile.parent), **(extra or {}), + BUILD_INPUTS_KEY: build_inputs_digest(kind)} BUILD_INPUTS_KEY = "build_inputs_sha256" # the metadata key a put records build_inputs_digest under -_BUILD_INPUTS = { - GATEWAY: lambda: _files_under(GATEWAY_DOCKERFILE.parent), # the gateway's Dockerfile copies only its own folder - SERVICE_DB: lambda: [SERVICE_DB_DOCKERFILE, DB_WEB_DOCKERFILE, DB_MCP_DOCKERFILE], # they copy nothing - WEBSITE_BROWSER: lambda: [WEBSITE_BROWSER_DOCKERFILE, WEBSITE_BROWSER_CONTEXT / "entrypoint.sh"], +# Each kind's Dockerfiles, each built from the folder it's in; the first is the one its env records. +_DOCKERFILES = { + GATEWAY: [GATEWAY_DOCKERFILE], + SERVICE_DB: [SERVICE_DB_DOCKERFILE, DB_WEB_DOCKERFILE, DB_MCP_DOCKERFILE], + WEBSITE_BROWSER: [WEBSITE_BROWSER_DOCKERFILE], } -_BUILD_ARGS = {WEBSITE_BROWSER: {"PLAYWRIGHT_MCP_VERSION": PLAYWRIGHT_MCP_VERSION}} def build_inputs_digest(kind: str) -> str: - """A digest of what agent-env builds ``kind``'s images from: the Dockerfiles it ships, the files they copy and the - build args. What a build fetches (base images, packages) isn't an input.""" + """A digest of the build contexts agent-env builds ``kind``'s images from, as each put stores them: the Dockerfiles + it ships and the files beside them. They hold every build arg's value too, so a build from a build context alone + builds the same thing. What a build fetches (base images, packages) isn't an input.""" digest = hashlib.sha256() - for path in sorted(_BUILD_INPUTS[kind]()): - digest.update(path.relative_to(_ENV_PACKAGE).as_posix().encode() + b"\0" + path.read_bytes() + b"\0") - for name, value in sorted(_BUILD_ARGS.get(kind, {}).items()): - digest.update(f"{name}={value}\0".encode()) + for dockerfile in _DOCKERFILES[kind]: + digest.update(BuildContext.of(dockerfile.parent, dockerfile).source_digest(None).encode() + b"\0") return digest.hexdigest() -def _files_under(folder: Path) -> list[Path]: - return [path for path in folder.rglob("*") - if path.is_file() and "__pycache__" not in path.parts and path.suffix != ".pyc"] - - _PUTS = {GATEWAY: put_gateway_env, SERVICE_DB: put_service_db_env, WEBSITE_BROWSER: put_website_browser_env} @@ -208,6 +194,11 @@ def infra_to_build(kinds: Iterable[str]) -> list[InfraBuild]: return builds +def needs_docker(builds: Iterable[InfraBuild], reason: str) -> str: + """Why ``builds`` can't be built here, given why docker can't be used (``docker_unreachable``).""" + return f"building the {', '.join(build.kind for build in builds)} env needs docker, and {reason}" + + def ensure_default_envs(kinds: Iterable[str], *, say: Callable[[str], None] = _quiet) -> list[InfraBuild]: """Build the infra envs of ``kinds`` that ``infra_to_build`` names, for this host's platform, and return them. Each id is locked while it's checked and built, so concurrent runs build it once. Raises InfraError before building @@ -219,7 +210,7 @@ def ensure_default_envs(kinds: Iterable[str], *, say: Callable[[str], None] = _q with holding_locks(ids, on_wait=lambda: say("waiting for another agent-env run to finish building the infra envs")): builds = infra_to_build(kinds) if builds and (reason := docker_unreachable()): - raise InfraError([f"building the {', '.join(build.kind for build in builds)} env needs docker, and {reason}"]) + raise InfraError([needs_docker(builds, reason)]) for build in builds: say(f"{build}: building, which can take minutes") _PUTS[build.kind](build.id, platform=None) @@ -234,14 +225,11 @@ def stores_are_local() -> bool: and isinstance(configured_store(config.get_image_store()), LocalRegistryImageStore)) -_STOCK_DOCKERFILES = {GATEWAY: GATEWAY_DOCKERFILE, SERVICE_DB: SERVICE_DB_DOCKERFILE, WEBSITE_BROWSER: WEBSITE_BROWSER_DOCKERFILE} - - def _built_from_stock(env: Env, kind: str) -> bool: """Whether ``env`` was built from the Dockerfile agent-env ships for ``kind``, in this install or another one, as the put commands record it; an env someone built from their own Dockerfile is theirs to rebuild.""" recorded = (env.metadata or {}).get("dockerfile_path") - shipped = _STOCK_DOCKERFILES[kind].relative_to(_ENV_PACKAGE.parent.parent).parts # ("agent_env", "env", ...) + shipped = _DOCKERFILES[kind][0].relative_to(_ENV_PACKAGE.parent.parent).parts # ("agent_env", "env", ...) return recorded is not None and Path(recorded).parts[-len(shipped):] == shipped diff --git a/src/agent_env/env/envs/service_db/Dockerfile.db-mcp b/src/agent_env/env/envs/service_db/Dockerfile.db-mcp index 869f9c04..5b71e411 100644 --- a/src/agent_env/env/envs/service_db/Dockerfile.db-mcp +++ b/src/agent_env/env/envs/service_db/Dockerfile.db-mcp @@ -10,17 +10,19 @@ WORKDIR /app RUN apt-get update \ && apt-get install -y libpq-dev gcc git \ && rm -rf /var/lib/apt/lists/* -RUN git clone https://github.com/crystaldba/postgres-mcp.git /app +# Pinned, so every build, here or in a VM, builds the same source; move the pin on purpose. +RUN git clone https://github.com/crystaldba/postgres-mcp.git /app \ + && git -C /app checkout --quiet 15c8e33353546148acc2d8bd784551cf3905d1e2 # Patch: disable DNS rebinding protection in FastMCP constructor. # FastMCP auto-enables it when host="127.0.0.1" (the default), which rejects # requests arriving through reverse proxies with non-localhost Host headers. # postgres-mcp creates FastMCP("postgres-mcp") with defaults, then sets # mcp.settings.host later — but transport_security is already locked in. -RUN sed -i 's/mcp = FastMCP("postgres-mcp")/from mcp.server.transport_security import TransportSecuritySettings\nmcp = FastMCP("postgres-mcp", transport_security=TransportSecuritySettings(enable_dns_rebinding_protection=False))/' /app/src/postgres_mcp/server.py +RUN sed -i 's/mcp = FastMCP("postgres-mcp")/from mcp.server.transport_security import TransportSecuritySettings\nmcp = FastMCP("postgres-mcp", transport_security=TransportSecuritySettings(enable_dns_rebinding_protection=False))/' /app/src/postgres_mcp/server.py \ + && grep -q "enable_dns_rebinding_protection=False" /app/src/postgres_mcp/server.py -RUN --mount=type=cache,target=/root/.cache/uv \ - uv sync --frozen --no-dev +RUN uv sync --frozen --no-dev FROM public.ecr.aws/docker/library/python:3.12-slim-bookworm RUN groupadd -r app && useradd -r -g app app diff --git a/src/agent_env/env/envs/website_browser/Dockerfile b/src/agent_env/env/envs/website_browser/Dockerfile index 43b5b40c..83b24f35 100644 --- a/src/agent_env/env/envs/website_browser/Dockerfile +++ b/src/agent_env/env/envs/website_browser/Dockerfile @@ -28,7 +28,8 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ WORKDIR /app -ARG PLAYWRIGHT_MCP_VERSION +# The pinned @playwright/mcp version; update it here when upgrading the Playwright MCP server. +ARG PLAYWRIGHT_MCP_VERSION=0.0.41 ENV PLAYWRIGHT_MCP_VERSION=${PLAYWRIGHT_MCP_VERSION} # Pre-install the pinned version and Playwright's Chromium, which is built for linux/amd64 and linux/arm64 alike diff --git a/src/agent_env/env/envs/website_browser/__init__.py b/src/agent_env/env/envs/website_browser/__init__.py index b0c40000..ef99d7b1 100644 --- a/src/agent_env/env/envs/website_browser/__init__.py +++ b/src/agent_env/env/envs/website_browser/__init__.py @@ -1,9 +1,5 @@ """Website browser (Playwright) MCP server constants.""" -# Pinned version of @playwright/mcp to install in the Docker image. -# Update this when upgrading the Playwright MCP server. -PLAYWRIGHT_MCP_VERSION = "0.0.41" - # Docker image tag for the website browser server image. WEBSITE_BROWSER_IMAGE_TAG = "mcp-website-browser" @@ -11,7 +7,6 @@ WEBSITE_BROWSER_ENVIRONMENT_NAME = "website-browser" __all__ = [ - "PLAYWRIGHT_MCP_VERSION", "WEBSITE_BROWSER_IMAGE_TAG", "WEBSITE_BROWSER_ENVIRONMENT_NAME", ] diff --git a/src/agent_env/env/gateway/Dockerfile b/src/agent_env/env/gateway/Dockerfile index dd79e8fc..73000f01 100644 --- a/src/agent_env/env/gateway/Dockerfile +++ b/src/agent_env/env/gateway/Dockerfile @@ -4,10 +4,10 @@ RUN apt-get update && apt-get install -y curl && rm -rf /var/lib/apt/lists/* WORKDIR /app -COPY gateway/requirements.txt . +COPY requirements.txt . RUN pip install --no-cache-dir -r requirements.txt -COPY gateway/ ./gateway/ +COPY . ./gateway/ ENV PYTHONPATH=/app diff --git a/src/agent_env/providers/env_providers/env_gateway_provider.py b/src/agent_env/providers/env_providers/env_gateway_provider.py index 4adc83c3..206fea94 100644 --- a/src/agent_env/providers/env_providers/env_gateway_provider.py +++ b/src/agent_env/providers/env_providers/env_gateway_provider.py @@ -137,13 +137,12 @@ def in_vm(sandbox_provider: SandboxProvider) -> bool: return SANDBOX_MODE_VM in sandbox_provider.SUPPORTED_SANDBOX_MODES @classmethod - def accepts(cls, sandbox_provider: SandboxProvider, *, prepared: bool = True) -> Accepts: + def accepts(cls, sandbox_provider: SandboxProvider) -> Accepts: """The images a deploy on ``sandbox_provider`` runs: in a VM, any it loads; in containers, those the provider's - containers run, of the images the deploy prepares (its servers' and the gateway's), and the rest (its - service-db's) by name.""" + containers run, as it prepares each (its servers', the gateway's and its service-db's).""" if cls.in_vm(sandbox_provider): return Accepts.LOADABLE - return sandbox_provider.CONTAINER_ACCEPTS if prepared else Accepts.NAME + return sandbox_provider.CONTAINER_ACCEPTS @classmethod def sandbox_problem(cls, sandbox_provider: SandboxProvider, *, websites: bool) -> str | None: @@ -894,15 +893,20 @@ async def _deploy_via_containers( config = get_config() gateway_env = Env.get(config.default_gateway_env_id) - # Each image is checked, and prepared (one that is only a build context, built), before anything is created, so - # an image the provider can't run, or a failed build, leaves nothing. + # Each image is checked, and prepared (one the provider can't pull, built from its build context), before + # anything is created, so an image the provider can't run, or a failed build, leaves nothing. images = [gateway_env.docker_image_artifact, *mcp_server_images] - accepts = self.accepts(sandbox_provider) - problems = [problem for image in images - if (problem := accepts.problem(image, on_this_machine=sandbox_provider.ON_THIS_MACHINE))] + problems = [] if self._needs_local_postgres: # the servicedb and its sidecars, by the names the state provider picks - problems += [problem for name in self._state_provider.container_images() - if (problem := pull_problem(name, on_this_machine=sandbox_provider.ON_THIS_MACHINE))] + stored = {image.image_name: image for image in self._state_provider.store_images_to_load()} + for name in self._state_provider.container_images(): + if name in stored: + images.append(stored[name]) + elif problem := pull_problem(name, on_this_machine=sandbox_provider.ON_THIS_MACHINE): + problems.append(problem) + accepts = self.accepts(sandbox_provider) + problems += [problem for image in images + if (problem := accepts.problem(image, on_this_machine=sandbox_provider.ON_THIS_MACHINE))] if problems: raise ValueError(f"Can't deploy a gateway on {type(sandbox_provider).__name__}, which runs its images by " f"name: {'; '.join(problems)}") diff --git a/src/agent_env/providers/sandbox_providers/modal_sandbox.py b/src/agent_env/providers/sandbox_providers/modal_sandbox.py index af1ad617..9c5bee77 100644 --- a/src/agent_env/providers/sandbox_providers/modal_sandbox.py +++ b/src/agent_env/providers/sandbox_providers/modal_sandbox.py @@ -216,7 +216,8 @@ def __init__( # Looked up on demand and cached by name. self._apps: dict[str, modal.App] = {} self._token_id = "" - # The context-only images prepare_image was given, by name, so create_container builds them rather than pull. + # The images prepare_image builds from their build contexts, by name, so create_container builds them rather + # than pull. self._context_images: dict[str, DockerImageArtifact] = {} async def _get_client(self) -> modal.Client: @@ -243,10 +244,15 @@ async def _get_app(self, app_name: str, app_tags: Optional[dict[str, str]] = Non return app async def prepare_image(self, image: DockerImageArtifact, *, attribution: Optional[Attribution] = None) -> None: - """Build ``image`` when it's only a build context, unless this process has already built its sources.""" - if image.context_only: + """Build ``image`` from its build context when Modal can't run it by name: it's only a build context, its name + is in a registry Modal can't reach, or it's built for another platform than Modal's; unless this process has + already built its sources. One it pulls drops an image prepared earlier under its name.""" + if image.build_context_object_url and (image.by_name_problem(on_this_machine=self.ON_THIS_MACHINE) + or image.platform_problem(self._sandbox_cls.platform)): self._context_images[image.image_name] = image await self._context_image(image) + else: + self._context_images.pop(image.image_name, None) async def _context_image(self, image: DockerImageArtifact, *, stale: str | None = None) -> tuple[modal.Image, str]: @@ -298,8 +304,8 @@ async def create_container( app = await self._get_app(app_name, app_tags) built_id = None - if (context_only := self._context_images.get(image_name)) is not None: - image, built_id = await self._context_image(context_only) + if (prepared := self._context_images.get(image_name)) is not None: + image, built_id = await self._context_image(prepared) else: image = await self._registry_image(image_name) @@ -349,7 +355,7 @@ async def create(image: modal.Image) -> modal.Sandbox: if built_id is None: raise # Modal no longer holds the image this process built from the context, so it's built again, once. - image, built_id = await self._context_image(context_only, stale=built_id) + image, built_id = await self._context_image(prepared, stale=built_id) sb = await create(image) except Exception as e: raise RuntimeError( diff --git a/src/agent_env/providers/sandbox_providers/sandbox.py b/src/agent_env/providers/sandbox_providers/sandbox.py index 2cb83a35..3016b919 100644 --- a/src/agent_env/providers/sandbox_providers/sandbox.py +++ b/src/agent_env/providers/sandbox_providers/sandbox.py @@ -294,32 +294,35 @@ async def wait_for_vm(self) -> None: async def load_docker_images(self, artifacts: list) -> None: """Put each DockerImageArtifact's image on the VM: those with a tar.gz are loaded from it, in parallel, those - with only a build context are built from it, and the rest are pulled by image name. An image this VM can't get, - such as one to pull from a registry on this machine when the VM isn't on it, or can't run, being built for - another platform than its own, is refused before anything is loaded.""" + with only a build context are built from it, and the rest are pulled by image name; one whose tar.gz is built + for another platform than the VM's is built from the build context beside it, for the VM's. An image this VM + can't get, such as one to pull from a registry on this machine when the VM isn't on it, or can't run, being + built for another platform with no build context, is refused before anything is loaded.""" if not artifacts: return if problems := self.load_problems(artifacts): raise RuntimeError(f"Can't load images: {'; '.join(problems)}") - if tarballs := [artifact for artifact in artifacts if artifact.tar_gz_object_url]: + built = [artifact for artifact in artifacts if artifact.context_only or artifact.rebuilt_for(self.platform)] + if tarballs := [artifact for artifact in artifacts + if artifact.tar_gz_object_url and not artifact.rebuilt_for(self.platform)]: await self._load_tarballs(tarballs) if pulled := [artifact.image_name for artifact in artifacts if not artifact.tar_gz_object_url and not artifact.context_only]: await self.pull_images(pulled) - if built := [artifact for artifact in artifacts if artifact.context_only]: + if built: await self.build_images(built) def load_problems(self, artifacts: list) -> list[str]: """Why this VM can't get or run each DockerImageArtifact it can't, as ``load_docker_images`` refuses them: one to pull from a registry on this machine when the VM isn't on it, one with no tar.gz whose name names no - registry, or one built for another platform than its own.""" + registry, or one built for another platform than its own with no build context beside it.""" return [problem for artifact in artifacts if (problem := artifact.load_problem(on_this_machine=self.ON_THIS_MACHINE, platform=self.platform))] async def build_images(self, artifacts: list) -> None: - """Build each context-only image from its build context, one build at a time, so heavy builds don't contend - for one VM; images with the same ``source_digest`` share a build. The build pulls its base images and packages - itself, so a network policy restricting egress must allow them; only the context's download is let through.""" + """Build each image from its build context, one build at a time, so heavy builds don't contend for one VM; + images with the same ``source_digest`` share a build. The build pulls its base images and packages itself, so + a network policy restricting egress must allow them; only the context's download is let through.""" builds: dict[str, list] = {} for artifact in artifacts: builds.setdefault(artifact.source_digest or artifact.build_context_object_url, []).append(artifact) @@ -327,13 +330,14 @@ async def build_images(self, artifacts: list) -> None: await self._build_image(images) async def _build_image(self, images: list) -> None: - """Download the context ``images`` share, extract it, and ``docker build`` it, tagged with each image's name. - The work folder is this build's own, since a host can run builds of one context at once: two of the local - provider's sandboxes, or two deploys onto one VM.""" + """Download the context ``images`` share, extract it, and ``docker build`` it, tagged with each image's name, + for the platform it records, or this VM's when its tar.gz is for another. The work folder is this build's own, + since a host can run builds of one context at once: two of the local provider's sandboxes, or two deploys onto + one VM.""" first = images[0] names = list(dict.fromkeys(image.image_name for image in images)) work = f"/tmp/agent-env-build-{uuid.uuid4().hex}" - platform = first.platform or DEFAULT_BUILD_PLATFORM + platform = self.platform if first.rebuilt_for(self.platform) else first.platform or DEFAULT_BUILD_PLATFORM tags = " ".join(f"-t {shlex.quote(name)}" for name in names) logger.info(f"Building {', '.join(names)} from its build context, for {platform}...") await self.exec_script(f"rm -rf {work} && mkdir -p {work}/context") diff --git a/src/agent_env/providers/sandbox_providers/sandbox_provider.py b/src/agent_env/providers/sandbox_providers/sandbox_provider.py index 32a71655..bb6dc46f 100644 --- a/src/agent_env/providers/sandbox_providers/sandbox_provider.py +++ b/src/agent_env/providers/sandbox_providers/sandbox_provider.py @@ -42,15 +42,15 @@ class Accepts(Enum): LOADABLE = "loadable" # any a VM can get: a tar.gz it loads, a registry name it pulls, a build context it builds NAME = "name" # one it starts by its name, which an image that is only a build context gets only from a build - NAME_OR_CONTEXT = "name_or_context" # one it starts by its name, or a build context it builds first (prepare_image) + NAME_OR_CONTEXT = "name_or_context" # one it starts by its name, or builds first from its build context (prepare_image) def problem(self, image: DockerImageArtifact, *, on_this_machine: bool) -> str | None: """Why a provider that accepts these, whose sandboxes run ``on_this_machine`` or elsewhere, can't run ``image``, or None when it can: one elsewhere can't pull from a registry on this machine.""" if self is Accepts.LOADABLE: return image.load_problem(on_this_machine=on_this_machine) - if self is Accepts.NAME_OR_CONTEXT and image.context_only: - return None # the provider builds it + if self is Accepts.NAME_OR_CONTEXT and image.build_context_object_url: + return None # the provider builds what it can't pull return image.by_name_problem(on_this_machine=on_this_machine) diff --git a/tst/integration/env/envs/build_context_local_test.py b/tst/integration/env/envs/build_context_local_test.py new file mode 100644 index 00000000..8b6cffde --- /dev/null +++ b/tst/integration/env/envs/build_context_local_test.py @@ -0,0 +1,98 @@ +"""Images a VM sandbox builds from their build contexts, on the local backend's, which runs this machine's Docker: one +that is only a build context, and one whose tar.gz is built for another platform than the VM's, which the VM builds from +the context beside it instead of loading. Each deploys an MCP server env through the gateway and reads its server's +card. Requires a docker daemon; spins up a throwaway ``registry:2`` and skips if it can't start. +""" + +import logging +import shutil +import uuid +from pathlib import Path + +import pytest +from agentenv_protocol import client as protocol_v1 + +from agent_env.artifact import DockerImageArtifact +from agent_env.artifact.store import get_artifact_store +from agent_env.env import legacy_protocol +from agent_env.env.envs.mcp_server import MCPServerEnv +from agent_env.providers.sandbox_providers import sandbox +from agent_env.providers.sandbox_providers.local_sandbox import LocalSandbox +from agent_env.utils.docker_build import normalize_platform +from tst.integration.env.envs.server_env_local_test import local_stack # noqa: F401 (fixture) +from tst.integration.store.local_parallel_deploy_test import _docker, _seed_bootstrap_envs + +pytestmark = [pytest.mark.integration, pytest.mark.int_test_slow] + +_REPO = Path(__file__).resolve().parents[4] +_OTHER_PLATFORM = {"linux/amd64": "linux/arm64", "linux/arm64": "linux/amd64"} + + +@pytest.fixture +def vm_platform(monkeypatch, caplog): + """The local sandbox records no platform, its Docker being this machine's; given this machine's, as a remote VM + records its own, it rebuilds an image recorded for another one as a remote VM does.""" + platform = normalize_platform(f"linux/{_docker('info', '--format', '{{.Architecture}}').stdout.strip()}") + if platform not in _OTHER_PLATFORM: + pytest.skip(f"no other platform to record for a {platform} host") + monkeypatch.setattr(LocalSandbox, "platform", platform) + caplog.set_level(logging.INFO, logger=sandbox.__name__) + return platform + + +@pytest.mark.asyncio +async def test_an_image_that_is_only_a_build_context_is_built_on_the_vm(local_stack, vm_platform, tmp_path, caplog): + _seed_bootstrap_envs() + uid = uuid.uuid4().hex[:8] + image = DockerImageArtifact.put_context(f"ctx-items-{uid}", description="build context e2e", + context_path=str(_items_context(tmp_path / "items")), + dockerfile_path="Dockerfile", platform=vm_platform) + + await _deploys_and_answers(image, uid) + + assert f"Building {image.image_name} from its build context, for {vm_platform}..." in caplog.messages + + +@pytest.mark.asyncio +async def test_an_image_whose_tar_gz_is_for_another_platform_is_built_from_the_context_beside_it( + local_stack, vm_platform, tmp_path, caplog): + _seed_bootstrap_envs() + uid = uuid.uuid4().hex[:8] + context = _items_context(tmp_path / "items") + tag = f"ctx-items-{uid}" + build = _docker("build", "-t", tag, str(context)) + assert build.returncode == 0, f"items build failed: {build.stderr[-2000:]}" + stored = DockerImageArtifact.put(id=tag, description="build context e2e", image_name=tag, + build_context_path=str(context), dockerfile_path="Dockerfile") + # As a machine of the other platform would have written it, with a tar.gz a VM that loaded it would fail on. + image = get_artifact_store().put_document(stored.model_copy(update={ + "id": f"{tag}-other", "version": 0, "platform": _OTHER_PLATFORM[vm_platform], + "tar_gz_object_url": "file:///nonexistent/items.tar.gz"})) + assert image.rebuilt_for(vm_platform) + + await _deploys_and_answers(image, uid) + + assert f"Building {image.image_name} from its build context, for {vm_platform}..." in caplog.messages + + +async def _deploys_and_answers(image: DockerImageArtifact, uid: str) -> None: + """Deploy an items server env of ``image`` through the gateway on the local sandbox, read its card, and close it.""" + env = MCPServerEnv.put(id=f"ctx-items-env-{uid}", docker_image_artifact=image, environment_name="items") + deployed = await env.deploy(sandbox_type="local", ttl_seconds=900) + work_dir = env._sandbox.work_dir + try: + base_url = await legacy_protocol.v1_base_url(deployed, deployed.gateway_url, "items") + assert (await protocol_v1.get_card(base_url))["name"] == "items" + await env.close() + finally: + _docker("compose", "--project-directory", str(work_dir), "down", "-v", "--remove-orphans") + + +def _items_context(root: Path) -> Path: + """The in-memory items server's build context (card 'items'), with the vendored agentenv_protocol.""" + data = _REPO / "tst" / "data" / "agentenv_mcp" + shutil.copytree(_REPO / "packages" / "agentenv-protocol" / "src" / "agentenv_protocol", root / "agentenv_protocol", + ignore=shutil.ignore_patterns("__pycache__")) + for name in ("server.py", "Dockerfile", "seed.json"): + shutil.copy(data / name, root / name) + return root diff --git a/tst/integration/env/gateway/gateway_test.py b/tst/integration/env/gateway/gateway_test.py index 3b77d160..d1a79517 100644 --- a/tst/integration/env/gateway/gateway_test.py +++ b/tst/integration/env/gateway/gateway_test.py @@ -31,7 +31,6 @@ ) from agent_env.env.envs.service_db import ServiceDBEnv from agent_env.env.envs.website_browser import ( - PLAYWRIGHT_MCP_VERSION, WEBSITE_BROWSER_IMAGE_TAG, WEBSITE_BROWSER_ENVIRONMENT_NAME, ) @@ -64,7 +63,7 @@ class DockerImageConfig: name="gateway", dockerfile=ENV_DIR / "gateway" / "Dockerfile", tag="env-gateway", - context=ENV_DIR, + context=ENV_DIR / "gateway", service_name="gateway", ) @@ -229,7 +228,6 @@ def website_browser_env() -> MCPServerEnv: logger.info(f"Building {img.name} Docker image...") result = subprocess.run( ["docker", "build", "--platform", "linux/amd64", - "--build-arg", f"PLAYWRIGHT_MCP_VERSION={PLAYWRIGHT_MCP_VERSION}", "-f", str(img.dockerfile), "-t", img.tag, str(img.context)], capture_output=True, text=True, diff --git a/tst/integration/env/task_step/task_steps_test.py b/tst/integration/env/task_step/task_steps_test.py index 9a2ac0c9..25a63d9a 100644 --- a/tst/integration/env/task_step/task_steps_test.py +++ b/tst/integration/env/task_step/task_steps_test.py @@ -64,7 +64,7 @@ class DockerImageConfig: name="gateway", dockerfile=ENV_DIR / "gateway" / "Dockerfile", tag="env-gateway", - context=ENV_DIR, + context=ENV_DIR / "gateway", service_name="gateway", ) diff --git a/tst/unit/artifact/test_docker_image_build_context.py b/tst/unit/artifact/test_docker_image_build_context.py index 5662cc9b..cdf93344 100644 --- a/tst/unit/artifact/test_docker_image_build_context.py +++ b/tst/unit/artifact/test_docker_image_build_context.py @@ -175,3 +175,20 @@ def test_a_put_records_the_platform_as_docker_build_names_it(local_stores, conte platform="linux/arm64") assert image.platform == "linux/arm64" and image.source_digest == named.source_digest + + +@pytest.mark.parametrize("context_url, platform, rebuilt, problem", [ + ("s3://bucket/ctx.tar.gz", "linux/amd64", True, None), + ("s3://bucket/ctx.tar.gz", "linux/arm64", False, None), + ("s3://bucket/ctx.tar.gz", None, False, None), + (None, "linux/amd64", False, "'img' v1 is a linux/arm64 image, and the sandbox runs linux/amd64"), +], ids=["context-another-platform", "context-same-platform", "vm-runs-any", "no-context"]) +def test_a_vm_of_another_platform_builds_an_image_from_the_context_beside_its_tar_gz(context_url, platform, rebuilt, + problem): + image = DockerImageArtifact(id="img", version=1, description="d", image_name="localhost:5000/img:v1", + tar_gz_object_url="file:///state/img.tar.gz", build_context_object_url=context_url, + platform="linux/arm64") + + assert image.rebuilt_for(platform) is rebuilt + assert (image.load_problem(on_this_machine=False, platform=platform) or "").startswith(problem or "") + assert (image.load_problem(on_this_machine=False, platform=platform) is None) is (problem is None) diff --git a/tst/unit/bundle/preflight_test.py b/tst/unit/bundle/preflight_test.py index cae607f2..8076c19f 100644 --- a/tst/unit/bundle/preflight_test.py +++ b/tst/unit/bundle/preflight_test.py @@ -367,23 +367,27 @@ def test_a_lone_server_whose_image_is_only_a_build_context_is_refused_on_any_pro assert [entry.name for entry in dry_run_bundle(bundle_dir, sandbox="modal").runs] == ["t"] -def test_modal_builds_a_gateway_image_that_is_only_a_build_context_but_not_a_service_db_one(bundle_dir): +def _built_here(id): + """An image as the infra bootstrap builds it: in this machine's registry, for its platform, with its tar.gz and + its build context.""" + with namespace_routing(): + return get_artifact_store().put_document(DockerImageArtifact( + id=id, description=id, image_name=f"localhost:5000/{id}:v1", tar_gz_s3_url="file:///tmp/img-v1.tar.gz", + build_context_object_url="file:///tmp/ctx.tar.gz", dockerfile_path="Dockerfile", platform="linux/arm64", + source_digest="sha256:" + "2" * 64)) + + +@pytest.mark.parametrize("image", [_context_image, _built_here], ids=["only-a-build-context", "built-here"]) +def test_modal_builds_the_gateway_and_service_db_images_it_cant_pull_from_their_build_contexts(bundle_dir, image): with namespace_routing(): MCPServerEnv.put(id="crm", docker_image_artifact=_image("crm-image", REMOTE), environment_name="crm") + GatewayEnv.put(id="default", docker_image_artifact=image("gateway-default")) + ServiceDBEnv.put(id="default-db", db_docker_image_artifact=image("service-db-default-db"), + db_web_docker_image_artifact=image("db-web-default-db"), + db_mcp_docker_image_artifact=image("db-mcp-default-db")) _task(bundle_dir, [{"id": "env", "type": "deploy_env", "env_id": "crm"}]) - _infra(REMOTE) - with namespace_routing(): - GatewayEnv.put(id="default", docker_image_artifact=_context_image("gateway-default")) - assert [entry.name for entry in dry_run_bundle(bundle_dir, sandbox="modal").runs] == ["t"] - with namespace_routing(): - ServiceDBEnv.put(id="default-db", db_docker_image_artifact=_context_image("service-db-default-db"), - db_web_docker_image_artifact=_image("db-web-default-db", REMOTE), - db_mcp_docker_image_artifact=_image("db-mcp-default-db", REMOTE)) - assert _problems(lambda: dry_run_bundle(bundle_dir, sandbox="modal")) == [ - "tasks/t.json: step 'env': deploys the service-db env 'default-db''s image 'service-db-default-db' on the 'modal' " - "sandbox provider, which runs it by name: 'service-db-default-db' v2 is only a build context, which has to be " - "built, not pulled; run it on a provider whose VMs build it, such as --sandbox modal_vm"] + assert [entry.name for entry in dry_run_bundle(bundle_dir, sandbox="modal").runs] == ["t"] # Images built for another platform than the sandboxes run, which a VM refuses as it loads them @@ -684,13 +688,43 @@ def test_missing_infra_in_stores_that_arent_local_is_refused_naming_its_put_comm ] -def test_a_gateway_deploy_on_another_provider_needs_infra_it_can_reach(bundle_dir): +def test_a_gateway_deploy_on_another_provider_has_its_missing_infra_built_in_local_stores(bundle_dir, monkeypatch): + """Built for this machine, with the build context a VM of another platform builds it from.""" _env("crm", REMOTE) _task(bundle_dir, [{"id": "env", "type": "deploy_env", "env_id": "crm"}]) + assert [str(build) for build in dry_run_bundle(bundle_dir, sandbox="modal_vm").infra] == [ + "service-db env 'default-db' (missing)", "gateway env 'default' (missing)"] + monkeypatch.setattr("agent_env.bundle.preflight.stores_are_local", lambda: False) (problem, *_) = _problems(lambda: dry_run_bundle(bundle_dir, sandbox="modal_vm")) - assert problem.startswith("tasks/t.json: step 'env': deploys on the 'modal_vm' sandbox provider, which needs the " - "gateway env 'default', and the store doesn't hold it") + assert problem == ("tasks/t.json: step 'env': deploys on the 'modal_vm' sandbox provider, which needs the gateway " + "env 'default', and the store doesn't hold it; agent-env builds infra envs only into local " + "stores, so put it with `agent-env env gateway put --id default`") + + +def test_stock_infra_built_from_other_inputs_is_rebuilt_for_modal_rather_than_refused(bundle_dir): + """Infra built before its images kept their build contexts is in this machine's registry, which Modal can't pull + from; built anew, it carries the contexts Modal builds it from.""" + _env("crm", REMOTE) + _infra(PULLED_HERE, built_from="an earlier release's") + _task(bundle_dir, [{"id": "env", "type": "deploy_env", "env_id": "crm"}]) + + assert [str(build) for build in dry_run_bundle(bundle_dir, sandbox="modal").infra] == [ + "service-db env 'default-db' (its build inputs changed)", "gateway env 'default' (its build inputs changed)"] + + +def test_infra_built_for_deploys_elsewhere_needs_docker_here(bundle_dir, monkeypatch): + _env("crm", REMOTE) + _task(bundle_dir, [{"id": "env", "type": "deploy_env", "env_id": "crm"}]) + monkeypatch.setattr(preflight_module, "docker_unreachable", lambda: "docker isn't on PATH") + + assert _problems(lambda: dry_run_bundle(bundle_dir, sandbox="modal_vm")) == [ + "building the service-db, gateway env needs docker, and docker isn't on PATH"] + + +def test_a_gateway_deploy_on_another_provider_needs_infra_it_can_reach(bundle_dir): + _env("crm", REMOTE) + _task(bundle_dir, [{"id": "env", "type": "deploy_env", "env_id": "crm"}]) _infra(PULLED_HERE) problems = _problems(lambda: dry_run_bundle(bundle_dir, sandbox="modal_vm")) assert problems[0] == ("tasks/t.json: step 'env': deploys the gateway env 'default''s image 'gateway-default' on the " diff --git a/tst/unit/cli/build_platform_test.py b/tst/unit/cli/build_platform_test.py index ae15625a..8b365d18 100644 --- a/tst/unit/cli/build_platform_test.py +++ b/tst/unit/cli/build_platform_test.py @@ -8,7 +8,7 @@ from click.testing import CliRunner from agent_env.cli import cli -from agent_env.env.envs.website_browser import PLAYWRIGHT_MCP_VERSION, WEBSITE_BROWSER_IMAGE_TAG +from agent_env.env.envs.website_browser import WEBSITE_BROWSER_IMAGE_TAG from agent_env.utils.docker_build import DockerBuildError gateway = importlib.import_module("agent_env.cli.env.gateway") # the packages export the click commands @@ -50,7 +50,7 @@ def _put_commands(tmp_path): service_db.SERVICE_DB_IMAGE_NAME), None), "website-browser": ("agent_env.env.bootstrap", ["env", "website-browser", "put", "--id", "x"], (website_browser.WEBSITE_BROWSER_DOCKERFILE, website_browser.WEBSITE_BROWSER_CONTEXT, - WEBSITE_BROWSER_IMAGE_TAG), {"PLAYWRIGHT_MCP_VERSION": PLAYWRIGHT_MCP_VERSION}), + WEBSITE_BROWSER_IMAGE_TAG), None), } diff --git a/tst/unit/env/bootstrap_test.py b/tst/unit/env/bootstrap_test.py index 60d49fa4..4f557f91 100644 --- a/tst/unit/env/bootstrap_test.py +++ b/tst/unit/env/bootstrap_test.py @@ -2,6 +2,8 @@ stores only, for this host's platform, once each however many runs ask.""" import importlib +import re +from pathlib import Path import pytest from click.testing import CliRunner @@ -24,7 +26,7 @@ def write(env_id, *, platform): calls.append((kind, env_id, platform)) artifact = get_artifact_store().put_document(DockerImageArtifact( id=f"{kind}-{env_id}", description=kind, image_name=f"{kind}:v1", tar_gz_s3_url="file:///x.tar.gz")) - shipped = bootstrap._STOCK_DOCKERFILES[kind].relative_to(bootstrap._ENV_PACKAGE.parent.parent) + shipped = bootstrap._DOCKERFILES[kind][0].relative_to(bootstrap._ENV_PACKAGE.parent.parent) GatewayEnv.put(id=env_id, docker_image_artifact=artifact, metadata={ "dockerfile_path": f"/another/install/site-packages/{shipped}", bootstrap.BUILD_INPUTS_KEY: bootstrap.build_inputs_digest(kind)}) @@ -35,6 +37,31 @@ def write(env_id, *, platform): return calls +@pytest.mark.parametrize("kind, put, contexts", [ + (GATEWAY, bootstrap.put_gateway_env, [(bootstrap.GATEWAY_CONTEXT, "Dockerfile")]), + (SERVICE_DB, bootstrap.put_service_db_env, [(bootstrap.SERVICE_DB_DOCKERFILE.parent, name) + for name in ("Dockerfile", "Dockerfile.db-web", "Dockerfile.db-mcp")]), + (WEBSITE_BROWSER, bootstrap.put_website_browser_env, [(bootstrap.WEBSITE_BROWSER_CONTEXT, "Dockerfile")]), +], ids=[GATEWAY, SERVICE_DB, WEBSITE_BROWSER]) +def test_each_infra_image_keeps_the_build_context_it_is_built_from(local_stores, monkeypatch, kind, put, contexts): + """So a VM or Modal running another platform than this machine builds it again from that; and those contexts are + what its recorded build inputs cover.""" + kept = [] + + def keep(id, *, description, image_name, build_context_path=None, dockerfile_path=None, platform=None): + kept.append((Path(build_context_path), dockerfile_path)) + return get_artifact_store().put_document(DockerImageArtifact( + id=id, description=description, image_name=image_name, tar_gz_s3_url="file:///x.tar.gz")) + + monkeypatch.setattr(bootstrap, "build_image", lambda *args, **kwargs: None) + monkeypatch.setattr(bootstrap.DockerImageArtifact, "put", keep) + + put("x", platform=None) + + assert kept == contexts + assert [folder / name for folder, name in kept] == bootstrap._DOCKERFILES[kind] + + def test_missing_infra_is_built_for_this_host_in_order_and_once(puts): said = [] @@ -168,12 +195,53 @@ def test_a_put_records_what_the_env_was_built_from_whatever_metadata_it_was_give assert env.metadata["owner"] == "me" -def test_the_build_inputs_are_the_shipped_files_and_build_args_less_python_caches(tmp_path, monkeypatch): - (tmp_path / "__pycache__").mkdir() - (tmp_path / "__pycache__/gateway.cpython-312.pyc").write_text("compiled") +def test_the_build_inputs_are_every_file_in_the_build_context_less_python_caches(tmp_path, monkeypatch): + (tmp_path / "Dockerfile").write_text("FROM python:3.12\nCOPY . /app\n") (tmp_path / "gateway.py").write_text("served") - before = bootstrap.build_inputs_digest(WEBSITE_BROWSER) - monkeypatch.setitem(bootstrap._BUILD_ARGS, WEBSITE_BROWSER, {"PLAYWRIGHT_MCP_VERSION": "9.9.9"}) + monkeypatch.setitem(bootstrap._DOCKERFILES, GATEWAY, [tmp_path / "Dockerfile"]) + before = bootstrap.build_inputs_digest(GATEWAY) - assert bootstrap._files_under(tmp_path) == [tmp_path / "gateway.py"] - assert bootstrap.build_inputs_digest(WEBSITE_BROWSER) != before + (tmp_path / "__pycache__").mkdir() + (tmp_path / "__pycache__/gateway.cpython-312.pyc").write_text("compiled") + assert bootstrap.build_inputs_digest(GATEWAY) == before + (tmp_path / "gateway.py").write_text("served, changed") + assert bootstrap.build_inputs_digest(GATEWAY) != before + + +# What Docker's legacy builder refuses, by what it is. +_BUILDKIT_ONLY = { + "a syntax directive": r"^\s*#\s*syntax\s*=", + "RUN --mount, --network or --security": r"^\s*RUN\s+(--\S+\s+)*--(mount|network|security)\b", + "COPY or ADD --chmod, --link, --parents, --exclude, --checksum or --keep-git-dir": + r"^\s*(COPY|ADD)\s+(--\S+\s+)*--(chmod|link|parents|exclude|checksum|keep-git-dir)\b", + "a heredoc": r"<<-?\s*[\"']?\w+", + "a build arg with no default": r"^\s*ARG\s+\w+\s*$", +} + + +def _legacy_build_problems(dockerfile: str) -> list[str]: + return [what for what, pattern in _BUILDKIT_ONLY.items() + if re.search(pattern, dockerfile, re.IGNORECASE | re.MULTILINE)] + + +@pytest.mark.parametrize("dockerfile", [dockerfile for dockerfiles in bootstrap._DOCKERFILES.values() + for dockerfile in dockerfiles], + ids=lambda dockerfile: dockerfile.relative_to(bootstrap._ENV_PACKAGE).as_posix()) +def test_each_infra_image_builds_from_its_context_alone_on_dockers_legacy_builder(dockerfile): + """A sandbox rebuilds an infra image from its build context, which carries no build args, and a VM with no buildx, + as modal_vm's has none, builds with Docker's legacy builder.""" + assert _legacy_build_problems(dockerfile.read_text()) == [] + + +@pytest.mark.parametrize("line, problem", [ + ("# syntax=docker/dockerfile:1", "a syntax directive"), + ("RUN --mount=type=cache,target=/root/.npm npm ci", "RUN --mount, --network or --security"), + ("copy --chown=1:1 --chmod=755 entrypoint.sh /app/", + "COPY or ADD --chmod, --link, --parents, --exclude, --checksum or --keep-git-dir"), + ("RUN < Non self.write_calls.append((destination_path, content)) -@pytest.mark.parametrize("facts, in_vm, accepts, by_name_otherwise", [ - ({"SUPPORTED_SANDBOX_MODES": VMS_AND_CONTAINERS, "CONTAINER_ACCEPTS": Accepts.NAME}, True, Accepts.LOADABLE, Accepts.LOADABLE), - ({"SUPPORTED_SANDBOX_MODES": CONTAINERS, "CONTAINER_ACCEPTS": Accepts.NAME_OR_CONTEXT}, False, Accepts.NAME_OR_CONTEXT, Accepts.NAME), - ({"SUPPORTED_SANDBOX_MODES": CONTAINERS, "CONTAINER_ACCEPTS": Accepts.NAME}, False, Accepts.NAME, Accepts.NAME), +@pytest.mark.parametrize("facts, in_vm, accepts", [ + ({"SUPPORTED_SANDBOX_MODES": VMS_AND_CONTAINERS, "CONTAINER_ACCEPTS": Accepts.NAME}, True, Accepts.LOADABLE), + ({"SUPPORTED_SANDBOX_MODES": CONTAINERS, "CONTAINER_ACCEPTS": Accepts.NAME_OR_CONTEXT}, False, Accepts.NAME_OR_CONTEXT), + ({"SUPPORTED_SANDBOX_MODES": CONTAINERS, "CONTAINER_ACCEPTS": Accepts.NAME}, False, Accepts.NAME), ], ids=["vm", "containers-that-build", "containers-by-name"]) -def test_a_gateway_runs_in_a_vm_where_its_provider_creates_them_and_else_in_containers(facts, in_vm, accepts, - by_name_otherwise): - """In containers, the images the deploy prepares (its servers' and the gateway's) run as the provider's containers - run them, and the rest (its service-db's) by name.""" +def test_a_gateway_runs_in_a_vm_where_its_provider_creates_them_and_else_in_containers(facts, in_vm, accepts): + """In containers, every image the deploy prepares (its servers', the gateway's and its service-db's) runs as the + provider's containers run them.""" provider = MagicMock(**facts) assert EnvironmentGatewayProvider.in_vm(provider) is in_vm assert EnvironmentGatewayProvider.accepts(provider) is accepts - assert EnvironmentGatewayProvider.accepts(provider, prepared=False) is by_name_otherwise @pytest.mark.parametrize("facts, websites, problem", [ @@ -242,6 +240,38 @@ async def test_container_mode_refuses_a_servicedb_image_in_this_machines_registr ) +_SERVICEDB_BUILT_HERE = DockerImageArtifact(id="default-db__db_image", version=1, description="d", + image_name="localhost:5000/local/servicedb:v1", + tar_gz_object_url="file:///state/servicedb.tar.gz", + build_context_object_url="file:///state/ctx.tar.gz", platform="linux/arm64") + + +@pytest.mark.asyncio +async def test_container_mode_prepares_the_servicedb_images_it_starts_as_it_does_the_gateways(): + """As the infra bootstrap writes them: in this machine's registry, which Modal can't pull from, with the build + contexts Modal builds them from.""" + provider = ModalSandboxProvider() + gp = EnvironmentGatewayProvider() + gp._state_provider = MagicMock(spec=LocalPostgresStateProvider) + gp._state_provider.container_images.return_value = [_SERVICEDB_BUILT_HERE.image_name] + gp._state_provider.store_images_to_load.return_value = [_SERVICEDB_BUILT_HERE] + prepared = [] + + async def prepare(image, *, attribution=None): + prepared.append(image) + + with patch.object(provider, "prepare_image", side_effect=prepare), \ + patch.object(provider, "create_container", side_effect=RuntimeError("stop")), \ + patch("agent_env.env.env.Env.get", return_value=MagicMock(docker_image_artifact=_image("agent-gateway"))): + with pytest.raises(RuntimeError, match="stop"): + await gp._deploy_via_containers( + sandbox_provider=provider, mcp_servers=[MCPServerConfig(image="mcp-slack", environment_name="slack")], + mcp_server_images=[_image("mcp-slack")], gateway_port=18765, website_configs=None, + gateway_mode=MagicMock(value="performance"), ttl_seconds=60, disk_size_gb=10, + ) + assert [image.id for image in prepared] == ["agent-gateway", "mcp-slack", "default-db__db_image"] + + @pytest.mark.asyncio @pytest.mark.parametrize("unloadable", ["gateway", "server", "sidecar"]) async def test_a_vm_gateway_refuses_an_image_its_vm_cant_get_before_creating_the_vm(unloadable): diff --git a/tst/unit/providers/sandbox_providers/modal_context_build_test.py b/tst/unit/providers/sandbox_providers/modal_context_build_test.py index 7a0f38a6..af052a45 100644 --- a/tst/unit/providers/sandbox_providers/modal_context_build_test.py +++ b/tst/unit/providers/sandbox_providers/modal_context_build_test.py @@ -166,13 +166,43 @@ def test_everything_else_is_left_as_written(tmp_path, line): @pytest.mark.asyncio -async def test_an_image_that_is_not_only_a_build_context_is_not_built(local_stores, builds): +async def test_an_image_with_no_build_context_is_not_built(local_stores, builds): await _provider().prepare_image(DockerImageArtifact(id="solver-image", description="d", image_name="registry.example/solver:v2")) assert builds.calls == [] +@pytest.mark.asyncio +@pytest.mark.parametrize("image_name, platform, built", [ + ("localhost:5000/local/gateway:v1", "linux/amd64", True), + ("registry.example/gateway:v1", "linux/arm64", True), + ("registry.example/gateway:v1", "linux/amd64", False), +], ids=["in-this-machines-registry", "for-another-platform", "pulled"]) +async def test_an_image_with_a_build_context_beside_its_tar_gz_is_built_when_modal_cant_run_it_by_name( + local_stores, context, builds, image_name, platform, built): + """As the infra bootstrap writes its images: a tar.gz, which Modal can't load, and the context it was built from.""" + image = _context_image("gateway-image", context).model_copy(update={ + "image_name": image_name, "tar_gz_object_url": "file:///state/gateway.tar.gz", "platform": platform}) + + await _provider().prepare_image(image) + + assert len(builds.calls) == int(built) + + +@pytest.mark.asyncio +async def test_an_image_modal_pulls_drops_one_prepared_earlier_under_its_name(local_stores, context, builds): + provider = _provider() + built = _context_image("gateway-image", context).model_copy(update={ + "image_name": "registry.example/gateway:v1", "tar_gz_object_url": "file:///state/gateway.tar.gz", + "platform": "linux/arm64"}) + await provider.prepare_image(built) + + await provider.prepare_image(built.model_copy(update={"platform": "linux/amd64"})) + + assert provider._context_images == {} + + def _fake_create(): """A patched ``modal.Sandbox._experimental_create`` that stops right after recording its call.""" create = MagicMock() diff --git a/tst/unit/providers/sandbox_providers/provider_capabilities_test.py b/tst/unit/providers/sandbox_providers/provider_capabilities_test.py index 3870748e..73326044 100644 --- a/tst/unit/providers/sandbox_providers/provider_capabilities_test.py +++ b/tst/unit/providers/sandbox_providers/provider_capabilities_test.py @@ -164,6 +164,7 @@ async def test_a_provider_whose_containers_are_vms_of_their_own_puts_none_on_a_p image_name="localhost:5000/local/img:v2") WITH_TAR_GZ = DockerImageArtifact(id="img", version=2, description="d", image_name="localhost:5000/local/img:v2", tar_gz_object_url="file:///state/img.tar.gz") +WITH_TAR_GZ_AND_CONTEXT = WITH_TAR_GZ.model_copy(update={"build_context_object_url": "file:///state/ctx.tar.gz"}) HERE = "is in a registry on this machine, which a sandbox elsewhere can't pull from" @@ -181,9 +182,12 @@ def test_an_image_is_refused_by_the_form_its_provider_accepts(image, accepts, pr (IN_THIS_MACHINES_REGISTRY, LOADABLE, HERE), (IN_THIS_MACHINES_REGISTRY, NAME, HERE), (IN_THIS_MACHINES_REGISTRY, NAME_OR_CONTEXT, HERE), (WITH_TAR_GZ, LOADABLE, None), (WITH_TAR_GZ, NAME, HERE), (WITH_TAR_GZ, NAME_OR_CONTEXT, HERE), + (WITH_TAR_GZ_AND_CONTEXT, LOADABLE, None), (WITH_TAR_GZ_AND_CONTEXT, NAME, HERE), + (WITH_TAR_GZ_AND_CONTEXT, NAME_OR_CONTEXT, None), ]) def test_a_sandbox_elsewhere_cant_pull_from_this_machines_registry_and_one_on_it_can(image, accepts, problem): - """A VM loads an image's tar.gz, which agent-env pushes to it, and pulls one only when it has none.""" + """A VM loads an image's tar.gz, which agent-env pushes to it, and pulls one only when it has none; a provider that + builds images builds one it can't pull from its build context.""" found = accepts.problem(image, on_this_machine=False) assert (found is None) if problem is None else (problem in found) assert accepts.problem(image, on_this_machine=True) is None diff --git a/tst/unit/providers/sandbox_providers/vm_sandbox_test.py b/tst/unit/providers/sandbox_providers/vm_sandbox_test.py index edc937be..367829e3 100644 --- a/tst/unit/providers/sandbox_providers/vm_sandbox_test.py +++ b/tst/unit/providers/sandbox_providers/vm_sandbox_test.py @@ -244,6 +244,27 @@ async def test_a_vm_refuses_a_tar_gz_for_a_platform_it_doesnt_run_before_anythin assert "docker load" in anything.scripts[0] +@pytest.mark.asyncio +async def test_a_vm_builds_an_image_from_the_build_context_beside_a_tar_gz_for_another_platform(signing_store): + """As the infra bootstrap writes its images on Apple silicon: built for the VM's platform, the image runs, though + not byte for byte the one stored; a VM of the tar.gz's platform loads the tar.gz.""" + arm64 = DockerImageArtifact(id="gw", version=1, description="d", image_name="localhost:5000/gw:v1", + platform="linux/arm64", tar_gz_object_url="s3://bucket/gw.tar.gz", + build_context_object_url="s3://bucket/gw/build-context.tar.gz", + dockerfile_path="Dockerfile", source_digest="sha256:" + "3" * 64) + + amd = _RecordingVmSandbox() + await amd.load_docker_images([arm64]) + assert not any("docker load" in script for script in amd.scripts) + assert any("docker build --platform linux/amd64 -f Dockerfile -t localhost:5000/gw:v1 ." in script + for script in amd.scripts) + + arm = _RecordingVmSandbox(images_stdout="localhost:5000/gw:v1\n") + arm.platform = "linux/arm64" + await arm.load_docker_images([arm64]) + assert "docker load" in arm.scripts[0] and not any("docker build" in script for script in arm.scripts) + + @pytest.mark.asyncio async def test_a_login_is_minted_once_per_registry(): minted = []