From a9c1c1d734baf47943b09a2057205e5414e33ace Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Thu, 29 Oct 2026 17:44:43 -0700 Subject: [PATCH 1/3] feat(bundle): images a bundle carries as docker save archives, loaded rather than built or pulled An env, agent or image folder can name an image saved in its folder: `image = { archive = "image.tar", dockerfile = "Dockerfile", platform = "linux/amd64" }`, or `archive =` in an image folder's artifact.toml. A run stores it as its tar.gz under `local/-:`, never builds it, and keeps the Dockerfile it names as its build context beside it. Local Docker loads it (`host_images.load`), and a VM tags what it loaded with that name when the archive saved it under another one. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 +- .../artifact/artifacts/docker_image.py | 55 ++++++- src/agent_env/bundle/authoring.py | 16 +- src/agent_env/bundle/ledger.py | 24 ++- src/agent_env/bundle/materialize.py | 22 ++- src/agent_env/bundle/plan.py | 7 +- src/agent_env/bundle/preflight.py | 8 +- src/agent_env/bundle/resolve.py | 68 ++++++++- src/agent_env/bundle/run.py | 7 +- .../sandbox_providers/host_images.py | 34 +++++ .../local_docker/provider.py | 10 +- .../providers/sandbox_providers/sandbox.py | 19 ++- tst/unit/artifact/test_docker_image_pulled.py | 6 +- tst/unit/bundle/archived_image_test.py | 144 ++++++++++++++++++ tst/unit/bundle/preflight_test.py | 26 ++++ tst/unit/bundle/resolve_test.py | 79 +++++++++- .../sandbox_providers/host_images_test.py | 78 +++++++++- .../provider_capabilities_test.py | 2 +- .../sandbox_providers/vm_sandbox_test.py | 17 ++- 19 files changed, 567 insertions(+), 57 deletions(-) create mode 100644 tst/unit/bundle/archived_image_test.py diff --git a/AGENTS.md b/AGENTS.md index 855895d24..34cd11d92 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -117,7 +117,7 @@ and deserialized through a registry. | `openenv/` | `build_app(bundle)`: the [OpenEnv](https://github.com/openenv/OpenEnv) server for a bundle's tasks, each session playing one episode through the Episode API, with the envs' MCP tools, an added `end_turn` tool and the task's verifier score as the reward. It prepares bundles as `agent-env run` does (`prepare_bundle(played=True)`, without the agent the client plays) and keeps namespace routing on for the app's life. `openenv` comes with the `openenv` extra, a floor with no cap; this package and `training/trl.py` are the only ones that import it. Each task's description in the Task API names its `toolset` (the played agent's envs, role, and whether it gets `bash`). | | `training/` | One module per outside trainer, each behind its own extra, and `_tools.py`, which turns an env's tools into typed, documented Python methods from their JSON Schema, as transformers' `get_json_schema` reads them. `trl.py` (`trl` extra): `AgentEnv(url)` over a server `openenv/` makes, with `dataset` (a row per task) and `factory` (TRL's `environment_factory`: a generated class per toolset whose methods are the tools, a background `reset` and an owned `get_reward`); `AgentEnv.local(bundle)` serves the bundle in process. | | `runner/` | The `[runner]` seam: `Runner.submit()` returns a `RunHandle` (`run_id`, `instance_id`) before the run starts; `LocalRunner` is built in. `runner/store.py` is the run index every runner writes and the explorer reads. | -| `bundle/` | A bundle is a folder of entities (`envs/`, `agents/`, `artifacts/`, `skills/`, `tasks/.json`, `evals/.toml`) that runs without a registry: `agent-env run` plans it, writes it into the `@local` namespace (reusing each version whose inputs haven't changed) and runs its evals or tasks; `agent-env bundle init` writes a stored task or eval as one, with every env, agent and artifact it uses in its own folder (each image as its build context, pruned to what its Dockerfile reads and pinned to the Python and npm versions of the stored image) so it runs with no store; `agent-env bundle register` writes a folder into the configured stores under registry ids (``), each task and eval naming the versions it reads and each image it builds pushed, through a ledger in the target store so registering again writes only what changed. | +| `bundle/` | A bundle is a folder of entities (`envs/`, `agents/`, `artifacts/`, `skills/`, `tasks/.json`, `evals/.toml`) that runs without a registry: `agent-env run` plans it, writes it into the `@local` namespace (reusing each version whose inputs haven't changed) and runs its evals or tasks; an image is built from a Dockerfile, pinned from a registry (`{ ref = "" }`), or loaded from a `docker save` archive in its folder (`{ archive = "" }`, or `archive =` in an image folder's `artifact.toml`), stored as its tar.gz under a tag its digest gives, with the Dockerfile it names kept beside it as its build context, which local Docker and VM sandboxes load and a sandbox that only pulls refuses; `agent-env bundle init` writes a stored task or eval as one, with every env, agent and artifact it uses in its own folder (each image as its build context, pruned to what its Dockerfile reads and pinned to the Python and npm versions of the stored image) so it runs with no store; `agent-env bundle register` writes a folder into the configured stores under registry ids (``), each task and eval naming the versions it reads and each image it builds pushed, through a ledger in the target store so registering again writes only what changed. | | `eval/` | `Eval` (`eval`), a versioned list of task references, and its store. | | `plugins/` | Entry-point plugins (see Extension points): the group names, `inventory()`, which `agent-env plugin list/show/check` report, `load_failures` and `settings`. The top level is public; the underscored modules are internal. | | `explorer/` | Optional local web UI: `agent-env up`, binds loopback `:8234`. | diff --git a/src/agent_env/artifact/artifacts/docker_image.py b/src/agent_env/artifact/artifacts/docker_image.py index af595cb71..86945581f 100644 --- a/src/agent_env/artifact/artifacts/docker_image.py +++ b/src/agent_env/artifact/artifacts/docker_image.py @@ -3,7 +3,9 @@ from __future__ import annotations import asyncio +import contextlib import gzip +import hashlib import logging import posixpath import re @@ -282,6 +284,35 @@ def put_context(cls, id: str, *, description: str, context_path: str, dockerfile source_digest=source_digest, )) + @classmethod + def put_archive(cls, id: str, *, description: str, archive_path: str, context_path: str | None = None, + dockerfile_path: str | None = None, platform: str | None = None) -> "DockerImageArtifact": + """Register the image ``docker save`` wrote to ``archive_path``, stored as its tar.gz, which a deploy loads, so + no Docker is needed here; with the build context at ``context_path`` kept beside it when given. Its + ``image_name``, ``local/-:``, the same for the same archive, names no registry: each load + tags the image with it, whatever name the archive gives it.""" + from agent_env.artifact.store import get_artifact_store + from agent_env.config import get_config + + context = _context_of(Path(context_path), dockerfile_path) if context_path else None + platform = normalize_platform(platform) if platform else None + store = get_artifact_store() + prefix = store.attempt_prefix("docker_image", id) + objects = get_config().get_object_store_to_write(prefix, id) + with tempfile.TemporaryDirectory(prefix="agent-env-archive-") as tmp: + gzipped, digest = _gzipped(Path(archive_path), Path(tmp) / "image.tar.gz") + tar_gz_object_url = objects.put_file_at(f"{prefix}{fs_safe(id)}.tar.gz", str(gzipped), "application/gzip") + return cls.put_tar( + id, + description=description, + image_name=f"{local_image_repository(id)}:{digest[:_SOURCES_TAG_CHARS]}", + tar_gz_object_url=tar_gz_object_url, + build_context_object_url=_upload_context(objects, prefix, context) if context else None, + dockerfile_path=context.dockerfile if context else None, + platform=platform, + source_digest=context.source_digest(platform) if context else None, + ) + @staticmethod def context_identity(id: str, context_path: str, dockerfile_path: str, platform: str | None = DEFAULT_BUILD_PLATFORM) -> tuple[str, str]: @@ -320,9 +351,11 @@ def stored_under_build_tag(self) -> bool: return bool(self.tar_gz_object_url) and not names_registry(self.image_name) def build_tag_reason(self) -> str: - """Why a sandbox elsewhere can't pull an image ``stored_under_build_tag``.""" - return (f"{self.id!r} v{self.version} is stored as a tar.gz under {self.image_name!r}, which names no registry: " - "it's the tag the machine that built it gave it, which a sandbox elsewhere can't pull") + """Why a sandbox elsewhere can't pull an image ``stored_under_build_tag``; one not yet written has no + version.""" + named = f"{self.id!r} v{self.version}" if self.version else repr(self.id) + return (f"{named} is stored as a tar.gz under {self.image_name!r}, which names no registry: it's a tag only a " + "machine that loads the tar.gz has, which a sandbox elsewhere can't pull") 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, @@ -674,6 +707,22 @@ def _context_image_name(id: str, source_digest: str) -> str: return f"{local_image_repository(id)}:{source_digest.removeprefix('sha256:')[:_SOURCES_TAG_CHARS]}" +def _gzipped(archive: Path, dest: Path) -> tuple[Path, str]: + """``archive`` as a tar.gz: itself when it is one, else gzipped to ``dest``; and the sha256 of ``archive`` as + given, hex.""" + hashed = hashlib.sha256() + with archive.open("rb") as source: + compressed = source.read(2) == b"\x1f\x8b" + source.seek(0) + out = contextlib.nullcontext() if compressed else gzip.open(dest, "wb", compresslevel=1) + with out as sink: + while chunk := source.read(1 << 20): + hashed.update(chunk) + if sink is not None: + sink.write(chunk) + return (archive if compressed else dest), hashed.hexdigest() + + def _upload_context(objects, prefix: str, context: BuildContext) -> str: """Write ``context`` under ``prefix`` as a tar.gz and return its object URL.""" with tempfile.TemporaryDirectory(prefix="agent-env-context-") as tmp: diff --git a/src/agent_env/bundle/authoring.py b/src/agent_env/bundle/authoring.py index f981e6ec9..15397d709 100644 --- a/src/agent_env/bundle/authoring.py +++ b/src/agent_env/bundle/authoring.py @@ -7,7 +7,7 @@ import stat import tempfile import unicodedata -from collections.abc import Callable, Iterator, Mapping, Sequence +from collections.abc import Callable, Collection, Iterator, Mapping, Sequence from contextlib import contextmanager from dataclasses import dataclass from pathlib import Path @@ -254,14 +254,14 @@ def entry_files(bundle: Bundle, entry: BundleEntry, *, empty_ok: bool = False) - return _Walk(bundle, entry, empty_ok).files() -def build_context_files(bundle: Bundle, entry: BundleEntry) -> dict[str, Path]: +def build_context_files(bundle: Bundle, entry: BundleEntry, exclude: Collection[str] = ()) -> dict[str, Path]: """What an image built from an entry's folder is built from: its ``entry_files`` and its own toml, which - the Dockerfile can copy too.""" + the Dockerfile can copy too, but for the paths in ``exclude``, such as an archived image's own archive.""" files = entry_files(bundle, entry) toml = entry.path / CONFIG_FILES[entry.kind] if toml.is_file(): files[toml.name] = toml - return dict(sorted(files.items())) + return {key: path for key, path in sorted(files.items()) if key not in exclude} @dataclass(frozen=True) @@ -279,11 +279,13 @@ def identity(self, image_id: str) -> tuple[str, str]: @contextmanager -def staged_build_context(bundle: Bundle, entry: BundleEntry, dockerfile: str) -> Iterator[StagedContext]: - """``entry``'s ``StagedContext`` for its ``dockerfile``, a path in its folder, removed on leaving.""" +def staged_build_context(bundle: Bundle, entry: BundleEntry, dockerfile: str, + exclude: Collection[str] = ()) -> Iterator[StagedContext]: + """``entry``'s ``StagedContext`` for its ``dockerfile``, a path in its folder, removed on leaving; without the paths + in ``exclude``.""" with tempfile.TemporaryDirectory(prefix="agent-env-build-") as staged: context = Path(staged) - for key, path in build_context_files(bundle, entry).items(): + for key, path in build_context_files(bundle, entry, exclude).items(): (context / key).parent.mkdir(parents=True, exist_ok=True) shutil.copy2(path, context / key) yield StagedContext(context, str(context / dockerfile)) diff --git a/src/agent_env/bundle/ledger.py b/src/agent_env/bundle/ledger.py index 715a3f098..f61ed895f 100644 --- a/src/agent_env/bundle/ledger.py +++ b/src/agent_env/bundle/ledger.py @@ -32,9 +32,9 @@ from agent_env.task.store import TASKS_COLLECTION from .authoring import build_context_files -from .parse import BundleKind +from .parse import Bundle, BundleKind from .plan import Plan, Write, artifact_writer, env_folder_writer, env_writer, folder_files, unpinned_store_refs -from .resolve import BuiltImage, BundleImage, ReferencedImage +from .resolve import ArchivedImage, BuiltImage, BundleImage, ReferencedImage LEDGER_COLLECTION = "bundle_ledger" # Bumped by hand when what a digest covers changes, or a writer's output changes enough that every @@ -133,7 +133,9 @@ def digest(self, write: Write, needs: Mapping[tuple[str, str], int]) -> Digest | "files": {}, "needs": {}, "store_refs": {}} from_folder = write.kind in (BundleKind.ARTIFACT, BundleKind.SKILL) or ( write.kind is BundleKind.ENV and env_folder_writer(get_env_registry().get(_type(write)))) - if from_folder and not isinstance(write.source, ReferencedImage): + if isinstance(write.source, ArchivedImage): + inputs["files"] = _archived_files(self._plan.bundle_of(write.source.entry), write.source) + elif from_folder and not isinstance(write.source, ReferencedImage): built = isinstance(write.source, BuiltImage) listing = build_context_files if built else folder_files for key, path in listing(self._plan.bundle_of(write.source.entry), write.source.entry).items(): @@ -255,7 +257,11 @@ def _config(write: Write) -> Any: source = write.source if not isinstance(source, BundleImage): return source.config - config = {"dockerfile": source.dockerfile} if isinstance(source, BuiltImage) else {"ref": source.ref} + if isinstance(source, ArchivedImage): + config = {key: value for key, value in (("archive", source.archive), ("dockerfile", source.dockerfile), + ("platform", source.platform)) if value is not None} + else: + config = {"dockerfile": source.dockerfile} if isinstance(source, BuiltImage) else {"ref": source.ref} if source.description is not None: # only an image folder's, so an image key's digest is as before config["description"] = source.description return config @@ -327,6 +333,16 @@ def _sha256(data: bytes) -> str: _USUAL_MODE = 0o644 +def _archived_files(bundle: Bundle, image: ArchivedImage) -> dict[str, str]: + """What an archived image is written from: its archive, and its build context's files, with their modes, when it + keeps one.""" + files = {image.archive: _file_sha256(image.entry.path / image.archive)} + if image.dockerfile is not None: + for key, path in build_context_files(bundle, image.entry, {image.archive}).items(): + files[key] = _file_sha256(path) + _mode(path) + return files + + def _mode(path: Path) -> str: mode = stat.S_IMODE(path.stat().st_mode) return "" if mode == _USUAL_MODE else f"+{mode:o}" diff --git a/src/agent_env/bundle/materialize.py b/src/agent_env/bundle/materialize.py index 4366fa053..04d5a8c9a 100644 --- a/src/agent_env/bundle/materialize.py +++ b/src/agent_env/bundle/materialize.py @@ -17,6 +17,7 @@ from __future__ import annotations import copy +import functools from collections.abc import Callable, Iterator, Mapping from contextlib import contextmanager, nullcontext from dataclasses import dataclass @@ -42,7 +43,7 @@ from .ledger import Ledger, locked_ids, materializing from .parse import BundleError, BundleKind from .plan import Plan, Write, artifact_writer, env_writer, unpinned_store_refs -from .resolve import BuiltImage, BundleImage, ReferencedImage, build_step +from .resolve import ArchivedImage, BuiltImage, BundleImage, ReferencedImage, build_step @dataclass(frozen=True) @@ -153,6 +154,8 @@ def through_ledger(write: Write, write_fn: Callable[[], int]) -> None: def _write_entity(plan: Plan, write: Write, here: bool, on_build: Callable[[Write], None] | None) -> int: if isinstance(write.source, ReferencedImage): return _write_referenced_image(plan, write) + if isinstance(write.source, ArchivedImage): + return _write_archived_image(plan, write) if isinstance(write.source, BuiltImage): if write.id in plan.published: return _write_published_image(plan, write) @@ -253,6 +256,21 @@ def _write_referenced_image(plan: Plan, write: Write) -> int: raise BundleError([f"{_path(plan, write)}: {e}"]) from None +def _write_archived_image(plan: Plan, write: Write) -> int: + """Register the image an entry's archive holds, stored as its tar.gz, which each deploy loads, with the build + context its Dockerfile names kept beside it when it names one.""" + image, entry = write.source, write.source.entry + put = functools.partial(DockerImageArtifact.put_archive, write.id, description=_image_description(plan, write), + archive_path=str(entry.path / image.archive), platform=image.platform) + try: + if image.dockerfile is None: + return put().version + with staged_build_context(plan.bundle_of(entry), entry, image.dockerfile, {image.archive}) as staged: + return put(context_path=str(staged.path), dockerfile_path=staged.dockerfile).version + except (ValueError, RuntimeError, OSError) as e: # a context it can't write, or the object store + raise BundleError([f"{_path(plan, write)}: {e}"]) from None + + def _latest_image_name(image_id: str) -> str | None: """The image name ``image_id``'s latest version records, when that version is an image: a folder can change type.""" try: @@ -269,6 +287,8 @@ def _image_description(plan: Plan, write: Write) -> str: return image.description if isinstance(image, BuiltImage): return f"built from {_path(plan, write)}/{image.dockerfile}" + if isinstance(image, ArchivedImage): + return f"loaded from {_path(plan, write)}/{image.archive}" return f"referenced by {_path(plan, write)}" diff --git a/src/agent_env/bundle/plan.py b/src/agent_env/bundle/plan.py index 0a8d867f3..4c3bb8c2a 100644 --- a/src/agent_env/bundle/plan.py +++ b/src/agent_env/bundle/plan.py @@ -66,8 +66,8 @@ from ._fs import fold, relative, with_article from .authoring import AuthoringContext, entry_files, staged_build_context from .parse import NAMED_BY, Bundle, BundleEntry, BundleError, BundleKind, parse_bundle -from .resolve import (BuiltImage, BundleImage, Reference, ResolvedBundle, ResolvedEntry, build_step, entity_class, - resolve_bundle) +from .resolve import (ArchivedImage, BuiltImage, BundleImage, Reference, ResolvedBundle, ResolvedEntry, build_step, + entity_class, resolve_bundle) if TYPE_CHECKING: from .installed import InstalledBundle @@ -580,7 +580,8 @@ def _check_writers(self) -> None: claims: list[tuple[_Key, str, str]] = [] for key, source in self.nodes.items(): if isinstance(source, BundleImage): - how = "built" if isinstance(source, BuiltImage) else "referenced" + how = ("built" if isinstance(source, BuiltImage) else "loaded" if isinstance(source, ArchivedImage) + else "referenced") claims.append((key, self._path(source.entry), f"the image {how} for {self._path(source.entry)}")) else: claims.append((key, self._path(source.entry), self._path(source.entry))) diff --git a/src/agent_env/bundle/preflight.py b/src/agent_env/bundle/preflight.py index 10dc4d11a..93dba1b70 100644 --- a/src/agent_env/bundle/preflight.py +++ b/src/agent_env/bundle/preflight.py @@ -70,6 +70,7 @@ get_sandbox_provider, ) from agent_env.store.base import NotFoundError +from agent_env.store.ids import local_image_repository from agent_env.task_step.task_step import TaskStep from agent_env.task_step.task_steps.deploy_agent import DeployAgentTaskStep, linked_sandbox_problem from agent_env.task_step.task_steps.install_agent import InstallAgentTaskStep @@ -80,7 +81,7 @@ from .parse import BundleError, BundleKind from .plan import AgentOverride, Plan -from .resolve import BuiltImage, Reference, ReferencedImage, ResolvedEntry, build_step, played_deploys +from .resolve import ArchivedImage, BuiltImage, Reference, ReferencedImage, ResolvedEntry, build_step, played_deploys _SHOWN = 3 _MODEL_VARS = frozenset({ENV_BASE_URL, ENV_API_KEY}) @@ -144,6 +145,11 @@ def __init__(self, plan: Plan, sandbox: str | None): self.referenced = {write.id: DockerImageArtifact(id=write.id, description="", image_name=write.source.ref) for write in plan.writes if isinstance(write.source, ReferencedImage)} self.referenced.update({id: found.unwritten(id) for id, found in plan.published.items()}) + # Each image a folder saves as an archive, as a tar.gz under a tag no registry has, which the store will hold. + self.referenced.update({write.id: DockerImageArtifact( + id=write.id, description="", image_name=f"{local_image_repository(write.id)}:archive", + tar_gz_object_url="file:///archive.tar.gz", platform=write.source.platform) + for write in plan.writes if isinstance(write.source, ArchivedImage)}) # Each deploy of an image the bundle builds: (where, what, its provider's links, the images it accepts on each) self.uses: dict[str, list[tuple[str, str, list[SandboxProvider], Callable[[SandboxProvider], Accepts]]]] = {} diff --git a/src/agent_env/bundle/resolve.py b/src/agent_env/bundle/resolve.py index 5b7e59e1b..2802f7d8a 100644 --- a/src/agent_env/bundle/resolve.py +++ b/src/agent_env/bundle/resolve.py @@ -66,6 +66,16 @@ class ReferencedImage(BundleImage): ref: str +@dataclass(frozen=True) +class ArchivedImage(BundleImage): + """An image saved in the folder by ``docker save``, which a run loads rather than builds or pulls: + ``{ archive = "" }``, with the Dockerfile it names, when it names one, kept as its build context.""" + + archive: str # relative to the entry's folder + dockerfile: str | None = None + platform: str | None = None # the platform it was built for, when given + + @dataclass(frozen=True) class Reference: """A resolved reference: to ``local``, this bundle's entity or image, or, when ``local`` is @@ -256,8 +266,23 @@ def _image_folder(self, entry: BundleEntry) -> bool: if cls is not DockerImageArtifact: return False ctx = AuthoringContext(self.bundle, entry) - fields, problems = ctx.accepted(entry.config, description=str, ref=str) - ref, built = fields.get("ref"), (entry.path / DOCKERFILE).is_file() + fields, problems = ctx.accepted(entry.config, description=str, ref=str, archive=str, platform=str) + ref, archive, built = fields.get("ref"), fields.get("archive"), (entry.path / DOCKERFILE).is_file() + if archive is not None: + if ref is not None: + problems.append(ctx.config_problem("archive names an image saved in this folder, so it takes no ref")) + elif problem := _archive_problem("archive", entry, {"archive": archive, **fields}): + problems.append(ctx.config_problem(problem)) + elif "platform" in fields: + problems.append(ctx.config_problem("platform is only for an archived image")) + if archive is not None or problems: + if problems: + for problem in problems: + self._problem(entry, problem) + return True + self.images.append(ArchivedImage(entry.id, entry, archive, DOCKERFILE if built else None, + fields.get("platform"), description=fields.get("description"))) + return True if ref is not None and built: problems.append(ctx.config_problem(f"ref names a registry image, so this folder builds none; remove ref " f"or its {DOCKERFILE}")) @@ -292,8 +317,9 @@ def _toml_class(self, entry: BundleEntry) -> type: def _image(self, entry: BundleEntry, config: dict, ref: EntityRef, references: list[Reference]) -> bool: """Record the image an image key has the bundle write: the folder's Dockerfile when a top-level ``image`` is - left out, ``{ dockerfile = "" }``, or a registry image, ``{ ref = "" }``. A key in a table, such as - a computer's ``forms.container.image``, says which. False when the key names an image artifact instead.""" + left out, ``{ dockerfile = "" }``, a registry image, ``{ ref = "" }``, or an image saved in the + folder, ``{ archive = "" }``. A key in a table, such as a computer's ``forms.container.image``, says + which. False when the key names an image artifact instead.""" if ref.artifact_type != "docker_image" or "[]" in ref.path: return False *tables, key = ref.path.split(".") @@ -314,6 +340,16 @@ def _image(self, entry: BundleEntry, config: dict, ref: EntityRef, references: l else: self._implied(ReferencedImage(derive_id(entry.id, role), entry, name), owner, key, ref, references) return True + if isinstance(value, dict) and "archive" in value: + if extra := value.keys() - {"archive", "dockerfile", "platform"}: + self._problem(entry, f"{ref.path}: an archived image takes archive, dockerfile and platform, not " + f"{', '.join(sorted(extra))}") + elif problem := _archive_problem(ref.path, entry, value): + self._problem(entry, problem) + else: + self._implied(ArchivedImage(derive_id(entry.id, role), entry, value["archive"], value.get("dockerfile"), + value.get("platform")), owner, key, ref, references) + return True if value is None: dockerfile = default_dockerfile(ref.path) elif isinstance(value, dict) and value.keys() == {"dockerfile"}: @@ -324,9 +360,7 @@ def _image(self, entry: BundleEntry, config: dict, ref: EntityRef, references: l return True else: return False - path = entry.path / dockerfile if isinstance(dockerfile, str) and dockerfile else None - inside = path is not None and not Path(dockerfile).is_absolute() and path.resolve().is_relative_to(entry.path.resolve()) - if not (inside and path.is_file()): + if not _folder_file(entry, dockerfile): self._problem(entry, f"{ref.path}: there is no {dockerfile!r} in this folder to build") return True self._implied(BuiltImage(derive_id(entry.id, role), entry, dockerfile), owner, key, ref, references) @@ -482,6 +516,26 @@ def entity_class(entry: BundleEntry) -> type | None: return get_artifact_registry().get(canonical_type(entry.type)) +def _folder_file(entry: BundleEntry, name: Any) -> bool: + """Whether ``name`` is a relative path to a file inside ``entry``'s folder.""" + if not isinstance(name, str) or not name or Path(name).is_absolute(): + return False + path = entry.path / name + return path.resolve().is_relative_to(entry.path.resolve()) and path.is_file() + + +def _archive_problem(key: str, entry: BundleEntry, value: dict) -> str | None: + """What's wrong with an archived image's ``value``, its archive, Dockerfile and platform, or None.""" + if not _folder_file(entry, value["archive"]): + return f"{key}: there is no {value['archive']!r} in this folder to load" + if value.get("dockerfile") is not None and not _folder_file(entry, value["dockerfile"]): + return f"{key}: there is no {value['dockerfile']!r} in this folder to keep as its build context" + platform = value.get("platform") + if platform is not None and not (isinstance(platform, str) and platform.startswith("linux/")): + return f"{key}: platform is the one the image was built for, such as linux/amd64, not {platform!r}" + return None + + def _registry_problem(key: str, name: Any) -> str | None: """Why ``name``, a registry image ``key`` gives, can't be pulled as written.""" if isinstance(name, str) and names_registry(name): diff --git a/src/agent_env/bundle/run.py b/src/agent_env/bundle/run.py index b3b8e11be..4f9843337 100644 --- a/src/agent_env/bundle/run.py +++ b/src/agent_env/bundle/run.py @@ -40,7 +40,8 @@ from .plan import AliasUse, Plan, Write, plan_bundle from .preflight import Preflight, preflight_run from .requires import Declaring, declare, declaring_for -from .resolve import BuiltImage, BundleImage, Reference, ReferencedImage, resolve_bundle, without_played_agents +from .resolve import (ArchivedImage, BuiltImage, BundleImage, Reference, ReferencedImage, resolve_bundle, + without_played_agents) logger = logging.getLogger(__name__) @@ -169,6 +170,8 @@ def images(self) -> tuple[str, ...]: f"{_label(plan, done.write)} v{done.version}: " + ("pulled from its registry, its tag pinned to a digest when it's written" if isinstance(done.write.source, ReferencedImage) + else "a saved image, which each deploy loads" + if isinstance(done.write.source, ArchivedImage) else f"{plan.published[done.write.id].ref}, published for its sources, which each deploy pulls by its " "digest, or builds from its build context when it can't" if done.write.id in plan.published @@ -590,4 +593,6 @@ def _label(plan: Plan, write: Write) -> str: path = plan.path(write.source.entry) if isinstance(write.source, BuiltImage): return f"{path} ({write.source.dockerfile} image)" + if isinstance(write.source, ArchivedImage): + return f"{path} ({write.source.archive} image)" return f"{path} ({write.source.ref} image)" if isinstance(write.source, ReferencedImage) else path diff --git a/src/agent_env/providers/sandbox_providers/host_images.py b/src/agent_env/providers/sandbox_providers/host_images.py index e1953d384..c86d9062b 100644 --- a/src/agent_env/providers/sandbox_providers/host_images.py +++ b/src/agent_env/providers/sandbox_providers/host_images.py @@ -16,6 +16,7 @@ import json import logging import platform as host +import shutil import subprocess import tarfile import tempfile @@ -160,6 +161,39 @@ async def pull_here(image: DockerImageArtifact) -> None: await asyncio.to_thread(pull, image) +def load(image: DockerImageArtifact) -> None: + """Have this machine's Docker hold ``image``, a tar.gz stored under a tag no registry has, under its name: loaded + from its object store, then tagged with its name, unless Docker holds that name already. Loads of one image wait + for each other. Raises RuntimeError when Docker can't load it.""" + with holding_locks([f"image-load:{image.tar_gz_object_url}"]): + if present(image.image_name): + return + with tempfile.TemporaryDirectory(prefix="agent-env-load-") as work: + saved = Path(work) / "image.tar.gz" + with get_config().get_object_store_at(image.tar_gz_object_url).open(image.tar_gz_object_url) as source, \ + saved.open("wb") as out: + shutil.copyfileobj(source, out, 1 << 20) + logger.info(f"Loading {image.image_name} from its saved image on this machine...") + loaded = subprocess.run(["docker", "load", "-i", str(saved)], capture_output=True, text=True, + errors="replace") + if loaded.returncode != 0: + why = (loaded.stderr.strip().splitlines() or [f"docker load exited {loaded.returncode}"])[-1] + raise RuntimeError(f"Couldn't load {image.id!r} v{image.version}'s saved image: {why}") + names = [line.split(":", 1)[1].strip() for line in loaded.stdout.splitlines() + if line.startswith(("Loaded image:", "Loaded image ID:"))] + if not names: + raise RuntimeError(f"{image.id!r} v{image.version}'s saved image holds no image to load") + tagged = subprocess.run(["docker", "tag", names[0], image.image_name], capture_output=True, text=True, + errors="replace") + if tagged.returncode != 0: + raise RuntimeError(f"Couldn't tag {image.id!r} v{image.version}'s loaded image: {tagged.stderr.strip()}") + + +async def load_here(image: DockerImageArtifact) -> None: + """``load`` off the event loop.""" + await asyncio.to_thread(load, image) + + async def saved_here(image: DockerImageArtifact) -> str: """The object url of ``image`` built here and saved as a tar.gz, which a VM on this machine loads; saved once per sources.""" diff --git a/src/agent_env/providers/sandbox_providers/local_docker/provider.py b/src/agent_env/providers/sandbox_providers/local_docker/provider.py index 13ca76d9b..ce4b34d91 100644 --- a/src/agent_env/providers/sandbox_providers/local_docker/provider.py +++ b/src/agent_env/providers/sandbox_providers/local_docker/provider.py @@ -38,7 +38,8 @@ from agent_env import config from agent_env.attribution import Attribution from agent_env.config.paths import state_root -from agent_env.providers.sandbox_providers.host_images import build_here, built_here, host_platform, present, pull_here +from agent_env.providers.sandbox_providers.host_images import (build_here, built_here, host_platform, load_here, + present, pull_here) from agent_env.providers.sandbox_providers.local_docker.relay import relay_image from agent_env.providers.sandbox_providers.sandbox import ( SANDBOX_LABEL, @@ -794,13 +795,16 @@ async def _start_container(self, sandbox: VmSandbox, *, image_name: str, port: i async def prepare_image(self, image: Any, *, attribution: Optional[Attribution] = None) -> None: """Have this machine's Docker hold the image the local provider starts containers from by name: one that is only - a build context built here, and one with a digest pulled by it, or built from its build context when it can't - be.""" + a build context built here, one with a digest pulled by it, or built from its build context when it can't + be, and one saved as a tar.gz under a tag no registry has, such as a bundle's archived image, loaded.""" if image.context_only: await build_here(image) elif image.image_digest: await pull_here(image) self._prepared.add(image.image_name) + elif image.stored_under_build_tag: + await load_here(image) + self._prepared.add(image.image_name) async def create_sandbox( self, diff --git a/src/agent_env/providers/sandbox_providers/sandbox.py b/src/agent_env/providers/sandbox_providers/sandbox.py index 3a37b3726..c0f111abc 100644 --- a/src/agent_env/providers/sandbox_providers/sandbox.py +++ b/src/agent_env/providers/sandbox_providers/sandbox.py @@ -236,6 +236,15 @@ async def write_file_from_text(self, content: str, destination_path: str) -> Non raise NotImplementedError(f"{self.__class__.__name__} does not support write_file_from_text") + +def _load_tarball(path: str, image_name: str) -> str: + """Shell that loads the image tar.gz at ``path`` and, when it saves the image under another name, as an archive a + bundle carries does, tags the image it loaded ``image_name``.""" + tar, out, name = shlex.quote(path), shlex.quote(f"{path}.loaded"), shlex.quote(image_name) + return (f"gunzip -c {tar} | docker load > {out} && rm -f {tar} && " + f"{{ docker image inspect {name} > /dev/null 2>&1 || " + f"docker tag \"$(awk '/^Loaded image/ {{print $NF; exit}}' {out})\" {name}; }} && rm -f {out}") + class VmSandbox(Sandbox): """VM-style sandbox with a Docker daemon and shell access inside.""" @@ -475,15 +484,11 @@ async def _load_docker_images(self, artifacts: list, signed_urls: list[str | Non if signed is not None: # Download to a file first (retry-safe with -o); a `curl | ... docker load` # pipe can't be retried without corrupting the stream (curl won't rewind). - load_commands.append( - f"(curl -fsSL {CURL_RETRY_FLAGS} {shlex.quote(signed)} -o {shlex.quote(tmp_tar)} " - f"&& gunzip -c {shlex.quote(tmp_tar)} | docker load && rm -f {shlex.quote(tmp_tar)})" - ) + fetch = f"curl -fsSL {CURL_RETRY_FLAGS} {shlex.quote(signed)} -o {shlex.quote(tmp_tar)} && " else: await self._download_object_to_vm(artifact.tar_gz_object_url, tmp_tar) - load_commands.append( - f"(gunzip -c {shlex.quote(tmp_tar)} | docker load && rm -f {shlex.quote(tmp_tar)})" - ) + fetch = "" + load_commands.append(f"({fetch}{_load_tarball(tmp_tar, artifact.image_name)})") logger.info(f" Queued: {artifact.image_name}") await self.exec_script(" & ".join(load_commands) + " & wait", max_retries=2) diff --git a/tst/unit/artifact/test_docker_image_pulled.py b/tst/unit/artifact/test_docker_image_pulled.py index 100358b26..36b3daaf5 100644 --- a/tst/unit/artifact/test_docker_image_pulled.py +++ b/tst/unit/artifact/test_docker_image_pulled.py @@ -53,9 +53,9 @@ def test_a_sandbox_elsewhere_cant_pull_from_this_machines_registry(image_name): @pytest.mark.parametrize("image_name, tar_gz, elsewhere", [ ("mcp-server-email", "s3://bucket/img.tar.gz", - "'img' v1 is stored as a tar.gz under 'mcp-server-email', which names no registry: it's the tag the machine that " - "built it gave it, which a sandbox elsewhere can't pull; run it on a provider whose VMs load its tar.gz, or put " - "the image again in an image store whose registry the sandbox can reach"), + "'img' v1 is stored as a tar.gz under 'mcp-server-email', which names no registry: it's a tag only a machine that " + "loads the tar.gz has, which a sandbox elsewhere can't pull; run it on a provider whose VMs load its tar.gz, or " + "put the image again in an image store whose registry the sandbox can reach"), ("team/img:v1", "s3://bucket/img.tar.gz", "names no registry"), (REF, "s3://bucket/img.tar.gz", None), ("python:3.12", None, None), diff --git a/tst/unit/bundle/archived_image_test.py b/tst/unit/bundle/archived_image_test.py new file mode 100644 index 000000000..c29f6f33a --- /dev/null +++ b/tst/unit/bundle/archived_image_test.py @@ -0,0 +1,144 @@ +"""Saved images: an image a bundle carries as a ``docker save`` archive is stored as its tar.gz under a name its digest +gives, never built, with the build context its Dockerfile names kept beside it, and everything naming it is written over +it.""" + +import gzip +import hashlib +import io +import json +import tarfile + +import pytest + +from agent_env.a2a_agent import A2AAgent +from agent_env.artifact.artifacts.docker_image import DockerImageArtifact +from agent_env.bundle.materialize import materialize +from agent_env.config import get_config +from agent_env.env import Env +from agent_env.store.ids import local_image_repository +from tst.unit.bundle._support import layout, local_builds, plan_of + +ROOT = "@local/~/triage" +IMAGE = f"{ROOT}/crm-image" +WEB = f"{ROOT}/web" +SAVED = b"the tar docker save wrote" +USERS = { + "envs/crm/env.toml": 'image = "crm-image"\nenvironment_name = "crm"\n', + "agents/solver/agent.toml": 'image = "crm-image"\n', + "tasks/t.json": json.dumps([ + {"id": "crm", "type": "deploy_env", "env_id": "crm"}, + {"id": "agent", "type": "deploy_agent", "env_ids": [], "a2a_agent_id": "solver"}, + ]), +} +FOLDER = {"artifacts/crm-image/artifact.toml": 'type = "docker_image"\narchive = "image.tar"\nplatform = "linux/arm64"\n', + **USERS} +WEB_FILES = { + "envs/web/Dockerfile": "FROM scratch\nCOPY server.py /app/\n", + "envs/web/server.py": "print('serving')\n", + "envs/web/env.toml": 'environment_name = "web"\nimage = { archive = "image.tar", dockerfile = "Dockerfile" }\n', + "tasks/t.json": json.dumps([{"id": "web", "type": "deploy_env", "env_id": "web"}]), +} + + +@pytest.fixture +def bundle_dir(tmp_path, monkeypatch, local_stores, cli_routing): + monkeypatch.setenv("HOME", str(tmp_path)) + return tmp_path / "triage" + + +def _run(root): + plan = plan_of(root) + return materialize(plan, build_here=local_builds(plan)) + + +def _summary(materialization): + return {done.write.id: (done.version, done.reused, done.reasons) for done in materialization.writes} + + +def _saved(root, data, name="artifacts/crm-image/image.tar"): + (root / name).parent.mkdir(parents=True, exist_ok=True) + (root / name).write_bytes(data) + + +def _stored(url): + with get_config().get_object_store_at(url).open(url) as stream: + return stream.read() + + +def _named(data): + return f"{local_image_repository(IMAGE)}:{hashlib.sha256(data).hexdigest()[:16]}" + + +def _images_named(done): + env = Env.get(f"{ROOT}/crm", done.version_of("env", f"{ROOT}/crm")).docker_image_artifact + agent = A2AAgent.get(f"{ROOT}/solver", done.version_of("agent", f"{ROOT}/solver")).docker_image_artifact + return [(image.id, image.version) for image in (env, agent)] + + +def test_a_saved_image_is_stored_gzipped_under_its_digest_unbuilt_and_everything_naming_it_is_written_over_it( + bundle_dir, builds): + layout(bundle_dir, FOLDER) + _saved(bundle_dir, SAVED) + + done = _run(bundle_dir) + + image = DockerImageArtifact.get(IMAGE) + assert (image.image_name, image.description, image.platform) == ( + _named(SAVED), "loaded from artifacts/crm-image/image.tar", "linux/arm64") + assert (image.stored_under_build_tag, image.build_context_object_url) == (True, None) + assert gzip.decompress(_stored(image.tar_gz_object_url)) == SAVED + assert _images_named(done) == [(IMAGE, 1)] * 2 + assert _summary(_run(bundle_dir))[IMAGE] == (1, True, ()) + assert builds == [] + + +def test_a_new_saved_image_is_a_new_version_under_a_new_name_and_everything_naming_it_is_rewritten(bundle_dir): + layout(bundle_dir, FOLDER) + _saved(bundle_dir, SAVED) + _run(bundle_dir) + _saved(bundle_dir, SAVED + b" again") + + done = _run(bundle_dir) + + assert _summary(done)[IMAGE][:2] == (2, False) + assert DockerImageArtifact.get(IMAGE).image_name == _named(SAVED + b" again") + assert _images_named(done) == [(IMAGE, 2)] * 2 + + +def test_a_saved_image_already_gzipped_is_stored_as_it_is(bundle_dir): + gzipped = gzip.compress(SAVED) + layout(bundle_dir, {**FOLDER, "artifacts/crm-image/artifact.toml": 'type = "docker_image"\narchive = "image.tar.gz"\n'}) + _saved(bundle_dir, gzipped, "artifacts/crm-image/image.tar.gz") + + _run(bundle_dir) + + image = DockerImageArtifact.get(IMAGE) + assert (_stored(image.tar_gz_object_url), image.image_name, image.platform) == (gzipped, _named(gzipped), None) + + +def test_a_saved_image_naming_its_dockerfile_keeps_its_folder_but_the_archive_as_its_build_context(bundle_dir, builds): + layout(bundle_dir, WEB_FILES) + _saved(bundle_dir, SAVED, "envs/web/image.tar") + + done = _run(bundle_dir) + + image = Env.get(WEB, done.version_of("env", WEB)).docker_image_artifact + with tarfile.open(fileobj=io.BytesIO(_stored(image.build_context_object_url))) as context: + files = sorted(member.name.removeprefix("./") for member in context.getmembers() if member.isfile()) + assert (image.id, image.dockerfile_path, files) == (f"{WEB}__env_image", "Dockerfile", + ["Dockerfile", "env.toml", "server.py"]) + assert image.stored_under_build_tag and image.source_digest + assert builds == [] + + +def test_an_edit_to_a_saved_images_build_context_rewrites_it_under_the_same_name(bundle_dir): + layout(bundle_dir, WEB_FILES) + _saved(bundle_dir, SAVED, "envs/web/image.tar") + first = Env.get(WEB, _run(bundle_dir).version_of("env", WEB)).docker_image_artifact + (bundle_dir / "envs/web/server.py").write_text("print('serving v2')\n") + + done = _run(bundle_dir) + + image = Env.get(WEB, done.version_of("env", WEB)).docker_image_artifact + assert (image.version, image.image_name) == (2, first.image_name) + assert image.source_digest != first.source_digest diff --git a/tst/unit/bundle/preflight_test.py b/tst/unit/bundle/preflight_test.py index 1248b24f2..719903bb3 100644 --- a/tst/unit/bundle/preflight_test.py +++ b/tst/unit/bundle/preflight_test.py @@ -271,6 +271,32 @@ def test_an_image_folder_naming_a_registry_on_this_machine_is_refused_where_a_sa assert "localhost:5000/team/solver:v1 is in a registry on this machine" in problem + +SAVED = {"artifacts/base/artifact.toml": 'type = "docker_image"\narchive = "image.tar"\n', + "artifacts/base/image.tar": "saved", "agents/solver/agent.toml": 'image = "base"\n'} + + +@pytest.mark.parametrize("sandbox", ["local", "modal_vm", "modal_vm,modal"]) +def test_an_image_folder_saving_its_image_is_loaded_wherever_its_deploys_load_a_tar_gz(bundle_dir, sandbox): + layout(bundle_dir, SAVED) + _task(bundle_dir, [AGENT]) + + dry = dry_run_bundle(bundle_dir, sandbox=sandbox) + + assert dry.built_here == set() + assert dry.images() == ("artifacts/base (image.tar image) v1: a saved image, which each deploy loads",) + + +def test_an_image_folder_saving_its_image_is_refused_where_a_sandbox_only_pulls_images(bundle_dir): + layout(bundle_dir, SAVED) + _task(bundle_dir, [AGENT]) + + [problem] = _problems(lambda: dry_run_bundle(bundle_dir, sandbox="modal")) + + assert problem.startswith("tasks/t.json: step 'agent': deploys agent ") + assert "'@local/~/triage/base' is stored as a tar.gz under " in problem + assert "a sandbox elsewhere can't pull; run it with --sandbox local, or on a provider whose VMs load" in problem + class _ContainersOnly(SandboxProvider): """A plugin's provider that runs an agent's image in a container of its own, and creates no VM.""" diff --git a/tst/unit/bundle/resolve_test.py b/tst/unit/bundle/resolve_test.py index d8440f6a0..e5779edb9 100644 --- a/tst/unit/bundle/resolve_test.py +++ b/tst/unit/bundle/resolve_test.py @@ -7,7 +7,14 @@ from agent_env.bundle import BundleError, BundleKind, parse_bundle from agent_env.bundle import resolve as resolve_module -from agent_env.bundle.resolve import BuiltImage, Output, Reference, ReferencedImage, resolve_bundle +from agent_env.bundle.resolve import ( + ArchivedImage, + BuiltImage, + Output, + Reference, + ReferencedImage, + resolve_bundle, +) from agent_env.entity.refs import EntityKind, EntityRef from agent_env.env.env import Env from agent_env.task import Task @@ -434,6 +441,66 @@ def test_an_image_folder_is_the_image_built_from_its_dockerfile_or_the_registry_ {"image": f"{ROOT}/nginx"}, [(EntityKind.ARTIFACT, f"{ROOT}/nginx", None, "nginx")]) + +def test_an_image_key_or_folder_naming_a_saved_image_has_the_bundle_load_it(make): + result = resolve_bundle(make(files={ + "envs/web/Dockerfile": "FROM scratch\n", + "envs/web/image.tar.gz": "saved", + "envs/web/env.toml": 'type = "composite_test"\nimage = { archive = "image.tar.gz", dockerfile = "Dockerfile", ' + 'platform = "linux/arm64" }\nbackend_image = { archive = "api.tar" }\n', + "envs/web/api.tar": "saved", + "artifacts/crm/image.tar": "saved", + "artifacts/crm/artifact.toml": 'type = "docker_image"\narchive = "image.tar"\nplatform = "linux/amd64"\n', + })) + web = next(e for e in result.entries if e.entry.name == "web") + crm = next(entry for entry in result.bundle.entries if entry.name == "crm") + + assert web.config == {"type": "composite_test", "image": f"{ROOT}/web__env_image", + "backend_image": f"{ROOT}/web__backend_image"} + assert [image for image in result.images if image.entry in (web.entry, crm)] == [ + ArchivedImage(f"{ROOT}/web__env_image", web.entry, "image.tar.gz", "Dockerfile", "linux/arm64"), + ArchivedImage(f"{ROOT}/web__backend_image", web.entry, "api.tar"), + ArchivedImage(f"{ROOT}/crm", crm, "image.tar", None, "linux/amd64"), + ] + + +def test_an_image_folder_saving_its_image_beside_its_dockerfile_keeps_the_dockerfile_as_its_context(make): + result = resolve_bundle(make(files={ + "artifacts/crm/Dockerfile": "FROM scratch\n", + "artifacts/crm/image.tar.gz": "saved", + "artifacts/crm/artifact.toml": 'archive = "image.tar.gz"\ndescription = "the CRM"\n', + })) + crm = next(entry for entry in result.bundle.entries if entry.name == "crm") + + assert [image for image in result.images if image.entry == crm] == [ + ArchivedImage(f"{ROOT}/crm", crm, "image.tar.gz", "Dockerfile", description="the CRM")] + + +@pytest.mark.parametrize("files, problem", [ + ({"envs/web/env.toml": 'image = { archive = "image.tar" }\n'}, + "image: there is no 'image.tar' in this folder to load"), + ({"envs/web/env.toml": 'image = { archive = "../image.tar" }\n', "envs/image.tar": "saved"}, + "image: there is no '../image.tar' in this folder to load"), + ({"envs/web/env.toml": 'image = { archive = "image.tar", dockerfile = "Dockerfile.web" }\n', + "envs/web/image.tar": "saved"}, + "image: there is no 'Dockerfile.web' in this folder to keep as its build context"), + ({"envs/web/env.toml": 'image = { archive = "image.tar", ref = "ghcr.io/acme/web:1" }\n', + "envs/web/image.tar": "saved"}, + "image: a registry image takes only ref, not archive"), + ({"envs/web/env.toml": 'image = { archive = "image.tar", platform = "arm64" }\n', "envs/web/image.tar": "saved"}, + "image: platform is the one the image was built for, such as linux/amd64, not 'arm64'"), + ({"artifacts/img/artifact.toml": 'type = "docker_image"\narchive = "image.tar"\nref = "ghcr.io/acme/img:1"\n', + "artifacts/img/image.tar": "saved"}, + "artifacts/img: artifact.toml: archive names an image saved in this folder, so it takes no ref"), + ({"envs/web/env.toml": 'image = { archive = "image.tar", size = 1 }\n', "envs/web/image.tar": "saved"}, + "image: an archived image takes archive, dockerfile and platform, not size"), + ({"artifacts/img/artifact.toml": 'type = "docker_image"\narchive = "image.tar"\n'}, + "artifacts/img: artifact.toml: archive: there is no 'image.tar' in this folder to load"), +], ids=["no archive", "outside", "no dockerfile", "with a ref", "platform", "folder with a ref", "unknown key", + "folder no archive"]) +def test_a_saved_image_the_bundle_cant_load_is_a_problem(make, files, problem): + assert any(problem in found for found in problems(make(files=files))) + @pytest.mark.parametrize("files, problem", [ ({"artifacts/img/artifact.toml": 'type = "docker_image"\n'}, 'artifacts/img: an image folder needs a Dockerfile, or ref = "" in its artifact.toml'), @@ -443,12 +510,14 @@ def test_an_image_folder_is_the_image_built_from_its_dockerfile_or_the_registry_ ({"artifacts/img/artifact.toml": 'type = "docker_image"\nref = "img:1"\n'}, "artifacts/img: artifact.toml: ref: 'img:1' doesn't name its registry; spell it out, as in " "docker.io/library/img:1"), - ({"artifacts/img/Dockerfile": "FROM scratch\n", "artifacts/img/artifact.toml": 'platform = "linux/amd64"\n'}, - "artifacts/img: artifact.toml: unknown key 'platform'; a docker_image artifact takes description, ref, type " - "and id"), + ({"artifacts/img/Dockerfile": "FROM scratch\n", "artifacts/img/artifact.toml": "size = 1\n"}, + "artifacts/img: artifact.toml: unknown key 'size'; a docker_image artifact takes description, ref, archive, " + "platform, type and id"), ({"artifacts/img/Dockerfile": "FROM scratch\n", "artifacts/img/artifact.toml": "description = 1\n"}, "artifacts/img: artifact.toml: description must be a string, not 1"), -], ids=["nothing to build", "both", "no registry", "unknown key", "description type"]) + ({"artifacts/img/Dockerfile": "FROM scratch\n", "artifacts/img/artifact.toml": 'platform = "linux/amd64"\n'}, + "artifacts/img: artifact.toml: platform is only for an archived image"), +], ids=["nothing to build", "both", "no registry", "unknown key", "description type", "platform alone"]) def test_an_image_folder_its_toml_cant_describe_is_a_problem(make, files, problem): assert problem in problems(make(files=files)) diff --git a/tst/unit/providers/sandbox_providers/host_images_test.py b/tst/unit/providers/sandbox_providers/host_images_test.py index f1b4cb356..b6630a07e 100644 --- a/tst/unit/providers/sandbox_providers/host_images_test.py +++ b/tst/unit/providers/sandbox_providers/host_images_test.py @@ -303,11 +303,74 @@ def test_a_published_image_with_no_build_context_that_cant_be_pulled_says_why(mo host_images.pull(image) + +def _archived(tmp_path): + archive = tmp_path / "image.tar" + archive.write_bytes(b"the tar docker save wrote") + return DockerImageArtifact.put_archive("crm_image", description="d", archive_path=str(archive)) + + +def _docker_load(monkeypatch, *, held=(), printed="Loaded image: acme/crm:1\n", failure=None): + """The docker commands ``load`` runs, each recorded with the bytes it was given to load; Docker holds ``held``, and + its load prints ``printed``, or exits 1 saying ``failure``.""" + ran = [] + + def run(argv, **kwargs): + if argv[1:3] == ["image", "inspect"]: + return subprocess.CompletedProcess(argv, int(argv[3] not in held), "", "") + ran.append(argv[1:] if argv[1] != "load" else ["load", gzip.decompress(open(argv[3], "rb").read())]) + if argv[1] == "load" and failure: + return subprocess.CompletedProcess(argv, 1, "", failure) + return subprocess.CompletedProcess(argv, 0, printed if argv[1] == "load" else "", "") + + monkeypatch.setattr(host_images.subprocess, "run", run) + return ran + + +def test_a_saved_image_is_loaded_from_its_store_and_tagged_with_its_name(local_stores, tmp_path, monkeypatch): + image = _archived(tmp_path) + ran = _docker_load(monkeypatch) + + host_images.load(image) + + assert ran == [["load", b"the tar docker save wrote"], ["tag", "acme/crm:1", image.image_name]] + + +def test_a_saved_image_docker_holds_isnt_loaded_again(local_stores, tmp_path, monkeypatch): + image = _archived(tmp_path) + ran = _docker_load(monkeypatch, held={image.image_name}) + + host_images.load(image) + + assert ran == [] + + +def test_a_saved_image_holding_only_an_untagged_image_is_tagged_by_its_id(local_stores, tmp_path, monkeypatch): + image = _archived(tmp_path) + ran = _docker_load(monkeypatch, printed="Loaded image ID: sha256:" + "c" * 64 + "\n") + + host_images.load(image) + + assert ran[1] == ["tag", "sha256:" + "c" * 64, image.image_name] + + +@pytest.mark.parametrize("load, problem", [ + ({"failure": "open /x: no space left on device\n"}, + "Couldn't load 'crm_image' v1's saved image: open /x: no space"), + ({"printed": ""}, "'crm_image' v1's saved image holds no image to load"), +], ids=["refused", "empty"]) +def test_a_saved_image_docker_cant_load_says_why(local_stores, tmp_path, monkeypatch, load, problem): + image = _archived(tmp_path) + _docker_load(monkeypatch, **load) + + with pytest.raises(RuntimeError, match=problem): + host_images.load(image) + @pytest.mark.asyncio -async def test_the_local_provider_pulls_a_published_image_and_builds_one_that_is_only_its_context(local_stores, context, - monkeypatch): +async def test_the_local_provider_pulls_a_published_image_builds_a_context_and_loads_a_saved_one(local_stores, context, + monkeypatch): """It starts containers from an image by name, so this machine's Docker holds the image first.""" - pulled, built = [], [] + pulled, built, loaded = [], [], [] async def pull_here(image): pulled.append(image.image_name) @@ -315,8 +378,12 @@ async def pull_here(image): async def build_here(image): built.append(image.image_name) + async def load_here(image): + loaded.append(image.image_name) + monkeypatch.setattr(local_sandbox, "pull_here", pull_here) monkeypatch.setattr(local_sandbox, "build_here", build_here) + monkeypatch.setattr(local_sandbox, "load_here", load_here) provider, context_only = LocalSandboxProvider(), _put(context) for image in (_published(context), context_only, @@ -324,7 +391,8 @@ async def build_here(image): tar_gz_object_url="s3://bucket/img.tar.gz")): await provider.prepare_image(image) - assert (pulled, built) == ([_PUBLISHED_REF], [context_only.image_name]) - assert (await provider.create_vm()).prepared == {_PUBLISHED_REF} # its containers start from it as it is + assert (pulled, built, loaded) == ([_PUBLISHED_REF], [context_only.image_name], ["img:v1"]) + # its containers start from them as they are + assert (await provider.create_vm()).prepared == {_PUBLISHED_REF, "img:v1"} assert LocalSandboxProvider()._prepared == set() diff --git a/tst/unit/providers/sandbox_providers/provider_capabilities_test.py b/tst/unit/providers/sandbox_providers/provider_capabilities_test.py index 0c81f3195..e924d815b 100644 --- a/tst/unit/providers/sandbox_providers/provider_capabilities_test.py +++ b/tst/unit/providers/sandbox_providers/provider_capabilities_test.py @@ -206,7 +206,7 @@ async def test_a_provider_whose_containers_are_vms_of_their_own_puts_none_on_a_p "dockerfile_path": "Dockerfile"}) BARE_TAG_TAR_GZ_AND_AN_OLD_CONTEXT = BARE_TAG_TAR_GZ.model_copy( update={"build_context_object_url": "s3://bucket/ctx.tar.gz"}) -BUILT_ELSEWHERE = "it's the tag the machine that built it gave it" +BUILT_ELSEWHERE = "it's a tag only a machine that loads the tar.gz has" @pytest.mark.parametrize("image, accepts, problem", [ diff --git a/tst/unit/providers/sandbox_providers/vm_sandbox_test.py b/tst/unit/providers/sandbox_providers/vm_sandbox_test.py index 7d9fd10bb..17d069a23 100644 --- a/tst/unit/providers/sandbox_providers/vm_sandbox_test.py +++ b/tst/unit/providers/sandbox_providers/vm_sandbox_test.py @@ -124,17 +124,24 @@ async def test_load_docker_images_uses_unique_tmp_per_artifact(signing_store): @pytest.mark.asyncio -async def test_a_load_of_tarballs_runs_the_script_it_always_has(signing_store): +async def test_a_load_of_tarballs_loads_each_and_tags_it_with_its_name_when_saved_under_another(signing_store): sandbox = _RecordingVmSandbox(images_stdout="a\nb\n") await sandbox.load_docker_images([_image("s3://bucket/a.tar.gz", "a:1"), _image("s3://bucket/b.tar.gz", "b:1")]) flags = "--retry 5 --retry-all-errors --retry-delay 1" + + def load(index, name): + tar = f"/tmp/_docker_image_vm-test_{index}.tar.gz" + out = f"{tar}.loaded" + return (f"gunzip -c {tar} | docker load > {out} && rm -f {tar} && " + f"{{ docker image inspect {name} > /dev/null 2>&1 || " + f"docker tag \"$(awk '/^Loaded image/ {{print $NF; exit}}' {out})\" {name}; }} && rm -f {out}") + assert sandbox.scripts == [ - f"(curl -fsSL {flags} https://signed/a.tar.gz -o /tmp/_docker_image_vm-test_0.tar.gz " - "&& gunzip -c /tmp/_docker_image_vm-test_0.tar.gz | docker load && rm -f /tmp/_docker_image_vm-test_0.tar.gz) & " - f"(curl -fsSL {flags} https://signed/b.tar.gz -o /tmp/_docker_image_vm-test_1.tar.gz " - "&& gunzip -c /tmp/_docker_image_vm-test_1.tar.gz | docker load && rm -f /tmp/_docker_image_vm-test_1.tar.gz) & wait" + f"(curl -fsSL {flags} https://signed/a.tar.gz -o /tmp/_docker_image_vm-test_0.tar.gz && {load(0, 'a:1')}) & " + f"(curl -fsSL {flags} https://signed/b.tar.gz -o /tmp/_docker_image_vm-test_1.tar.gz && {load(1, 'b:1')}) & " + "wait" ] From bea4e217d85a194e1f3f29e43e472faaabcee65b Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Thu, 29 Oct 2026 18:01:00 -0700 Subject: [PATCH 2/3] fix(bundle): store and load saved images naming no image, and keep archives out of their folder's listings docker load applies every name an archive embeds, so a bundle's archive could replace an image the loading machine holds under that name. put_archive now stores the archive naming no image (manifest.json RepoTags null, no index.json name annotations, no legacy repositories file), and host_images.load untags every tar.gz on its way to docker load, then tags the image by the immutable ID docker load reports, so concurrent loads can't race on a shared name either. entry_files leaves out every archive an entry's toml names, normalized, so no build context of the folder (a sibling built image's included) and no folder-written file holds one. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 +- .../artifact/artifacts/docker_image.py | 34 ++++----- src/agent_env/bundle/authoring.py | 44 ++++++++---- src/agent_env/bundle/ledger.py | 2 +- src/agent_env/bundle/materialize.py | 2 +- .../sandbox_providers/host_images.py | 25 ++++--- src/agent_env/utils/docker/untag.py | 70 +++++++++++++++++++ tst/unit/bundle/archived_image_test.py | 66 ++++++++++++++--- .../sandbox_providers/host_images_test.py | 69 +++++++++++++----- tst/unit/utils/docker/untag_test.py | 62 ++++++++++++++++ tst/util/docker_save.py | 17 ++++- 11 files changed, 317 insertions(+), 76 deletions(-) create mode 100644 src/agent_env/utils/docker/untag.py create mode 100644 tst/unit/utils/docker/untag_test.py diff --git a/AGENTS.md b/AGENTS.md index b793b6b8b..2918810d0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -117,7 +117,7 @@ and deserialized through a registry. | `openenv/` | `build_app(bundle)`: the [OpenEnv](https://github.com/openenv/OpenEnv) server for a bundle's tasks, each session playing one episode through the Episode API, with the envs' MCP tools, an added `end_turn` tool and the task's verifier score as the reward. It prepares bundles as `agent-env run` does (`prepare_bundle(played=True)`, without the agent the client plays) and keeps namespace routing on for the app's life. `openenv` comes with the `openenv` extra, a floor with no cap; this package and `training/trl.py` are the only ones that import it. Each task's description in the Task API names its `toolset` (the played agent's envs, role, and whether it gets `bash`). | | `training/` | One module per outside trainer, each behind its own extra, and `_tools.py`, which turns an env's tools into typed, documented Python methods from their JSON Schema, as transformers' `get_json_schema` reads them. `trl.py` (`trl` extra): `AgentEnv(url)` over a server `openenv/` makes, with `dataset` (a row per task) and `factory` (TRL's `environment_factory`: a generated class per toolset whose methods are the tools, a background `reset` and an owned `get_reward`); `AgentEnv.local(bundle)` serves the bundle in process. | | `runner/` | The `[runner]` seam: `Runner.submit()` returns a `RunHandle` (`run_id`, `instance_id`) before the run starts; `LocalRunner` is built in. `runner/store.py` is the run index every runner writes and the explorer reads. | -| `bundle/` | A bundle is a folder of entities (`envs/`, `agents/`, `artifacts/`, `skills/`, `tasks/.json`, `evals/.toml`) that runs without a registry: `agent-env run` plans it, writes it into the `@local` namespace (reusing each version whose inputs haven't changed) and runs its evals or tasks; an image is built from a Dockerfile, pinned from a registry (`{ ref = "" }`), or loaded from a `docker save` archive in its folder (`{ archive = "" }`, or `archive =` in an image folder's `artifact.toml`), stored as its tar.gz under a tag its digest gives, with the Dockerfile it names kept beside it as its build context, which local Docker and VM sandboxes load and a sandbox that only pulls refuses; `agent-env bundle init` writes a stored task or eval as one, with every env, agent and artifact it uses in its own folder (each image as its build context, pruned to what its Dockerfile reads and pinned to the Python and npm versions of the stored image) so it runs with no store; `agent-env bundle register` writes a folder into the configured stores under registry ids (``), each task and eval naming the versions it reads and each image it builds pushed, through a ledger in the target store so registering again writes only what changed. | +| `bundle/` | A bundle is a folder of entities (`envs/`, `agents/`, `artifacts/`, `skills/`, `tasks/.json`, `evals/.toml`) that runs without a registry: `agent-env run` plans it, writes it into the `@local` namespace (reusing each version whose inputs haven't changed) and runs its evals or tasks; an image is built from a Dockerfile, pinned from a registry (`{ ref = "" }`), or loaded from a `docker save` archive in its folder (`{ archive = "" }`, or `archive =` in an image folder's `artifact.toml`), stored as its tar.gz naming no image (so a load never replaces an image the machine holds under a name the archive gives) under a tag its digest gives, with the Dockerfile it names kept beside it as its build context, which local Docker and VM sandboxes load and a sandbox that only pulls refuses; `agent-env bundle init` writes a stored task or eval as one, with every env, agent and artifact it uses in its own folder (each image as its build context, pruned to what its Dockerfile reads and pinned to the Python and npm versions of the stored image) so it runs with no store; `agent-env bundle register` writes a folder into the configured stores under registry ids (``), each task and eval naming the versions it reads and each image it builds pushed, through a ledger in the target store so registering again writes only what changed. | | `eval/` | `Eval` (`eval`), a versioned list of task references, and its store. | | `plugins/` | Entry-point plugins (see Extension points): the group names, `inventory()`, which `agent-env plugin list/show/check` report, `load_failures` and `settings`. The top level is public; the underscored modules are internal. | | `explorer/` | Optional local web UI: `agent-env up`, binds loopback `:8234`. | diff --git a/src/agent_env/artifact/artifacts/docker_image.py b/src/agent_env/artifact/artifacts/docker_image.py index 86945581f..2bee2dc9c 100644 --- a/src/agent_env/artifact/artifacts/docker_image.py +++ b/src/agent_env/artifact/artifacts/docker_image.py @@ -3,7 +3,6 @@ from __future__ import annotations import asyncio -import contextlib import gzip import hashlib import logging @@ -27,6 +26,7 @@ from agent_env.store.image_store.registry_api import NoSuchImageError, pin_digest, pinned_from from agent_env.utils.docker.context import BuildContext from agent_env.utils.docker.build import DEFAULT_BUILD_PLATFORM, build_image, normalize_platform +from agent_env.utils.docker.untag import write_untagged logger = logging.getLogger(__name__) @@ -288,9 +288,10 @@ def put_context(cls, id: str, *, description: str, context_path: str, dockerfile def put_archive(cls, id: str, *, description: str, archive_path: str, context_path: str | None = None, dockerfile_path: str | None = None, platform: str | None = None) -> "DockerImageArtifact": """Register the image ``docker save`` wrote to ``archive_path``, stored as its tar.gz, which a deploy loads, so - no Docker is needed here; with the build context at ``context_path`` kept beside it when given. Its - ``image_name``, ``local/-:``, the same for the same archive, names no registry: each load - tags the image with it, whatever name the archive gives it.""" + no Docker is needed here; with the build context at ``context_path`` kept beside it when given. It's stored + naming no image, so a load tags nothing the archive names; its ``image_name``, + ``local/-:``, the same for the same archive, names no registry, and each load tags the + image with it. Raises ValueError when the archive isn't one docker save writes.""" from agent_env.artifact.store import get_artifact_store from agent_env.config import get_config @@ -299,9 +300,12 @@ def put_archive(cls, id: str, *, description: str, archive_path: str, context_pa store = get_artifact_store() prefix = store.attempt_prefix("docker_image", id) objects = get_config().get_object_store_to_write(prefix, id) + digest = _sha256(Path(archive_path)) with tempfile.TemporaryDirectory(prefix="agent-env-archive-") as tmp: - gzipped, digest = _gzipped(Path(archive_path), Path(tmp) / "image.tar.gz") - tar_gz_object_url = objects.put_file_at(f"{prefix}{fs_safe(id)}.tar.gz", str(gzipped), "application/gzip") + untagged = Path(tmp) / "image.tar.gz" + with open(archive_path, "rb") as saved, untagged.open("wb") as out: + write_untagged(saved, out, gzip_level=1) + tar_gz_object_url = objects.put_file_at(f"{prefix}{fs_safe(id)}.tar.gz", str(untagged), "application/gzip") return cls.put_tar( id, description=description, @@ -707,20 +711,12 @@ def _context_image_name(id: str, source_digest: str) -> str: return f"{local_image_repository(id)}:{source_digest.removeprefix('sha256:')[:_SOURCES_TAG_CHARS]}" -def _gzipped(archive: Path, dest: Path) -> tuple[Path, str]: - """``archive`` as a tar.gz: itself when it is one, else gzipped to ``dest``; and the sha256 of ``archive`` as - given, hex.""" +def _sha256(path: Path) -> str: hashed = hashlib.sha256() - with archive.open("rb") as source: - compressed = source.read(2) == b"\x1f\x8b" - source.seek(0) - out = contextlib.nullcontext() if compressed else gzip.open(dest, "wb", compresslevel=1) - with out as sink: - while chunk := source.read(1 << 20): - hashed.update(chunk) - if sink is not None: - sink.write(chunk) - return (archive if compressed else dest), hashed.hexdigest() + with path.open("rb") as file: + while chunk := file.read(1 << 20): + hashed.update(chunk) + return hashed.hexdigest() def _upload_context(objects, prefix: str, context: BuildContext) -> str: diff --git a/src/agent_env/bundle/authoring.py b/src/agent_env/bundle/authoring.py index 15397d709..639040c72 100644 --- a/src/agent_env/bundle/authoring.py +++ b/src/agent_env/bundle/authoring.py @@ -3,11 +3,12 @@ from __future__ import annotations import os +import posixpath import shutil import stat import tempfile import unicodedata -from collections.abc import Callable, Collection, Iterator, Mapping, Sequence +from collections.abc import Callable, Iterator, Mapping, Sequence from contextlib import contextmanager from dataclasses import dataclass from pathlib import Path @@ -25,7 +26,7 @@ from agent_env.utils.paths import os_reason from ._fs import relative, show, with_article -from .parse import CONFIG_FILES, LEAVINGS, NAMED_BY, Bundle, BundleEntry, BundleError +from .parse import CONFIG_FILES, LEAVINGS, NAMED_BY, Bundle, BundleEntry, BundleError, BundleKind A = TypeVar("A", bound=Artifact) E = TypeVar("E", bound=Env) @@ -248,20 +249,37 @@ def _fail(self, message: str) -> NoReturn: def entry_files(bundle: Bundle, entry: BundleEntry, *, empty_ok: bool = False) -> dict[str, Path]: """The files in an entry's folder, keyed by POSIX path in NFC and sorted: regular files, dot files - included, through links that stay inside the bundle, less what the OS or Python leaves behind and - the entry's own toml. Raises BundleError listing every problem found, a folder with no files among - them unless ``empty_ok``.""" + included, through links that stay inside the bundle, less what the OS or Python leaves behind, the + entry's own toml and the image archives it names. Raises BundleError listing every problem found, a + folder with no files among them unless ``empty_ok``.""" return _Walk(bundle, entry, empty_ok).files() -def build_context_files(bundle: Bundle, entry: BundleEntry, exclude: Collection[str] = ()) -> dict[str, Path]: +def entry_archives(entry: BundleEntry) -> frozenset[str]: + """The image archives ``entry``'s toml names, by an image table's ``archive``, or an image folder's, as POSIX paths + in its folder in NFC: what an image loads, which no build context or file the entry writes holds.""" + named, pending = set(), [entry.config or {}] + while pending: + value = pending.pop() + if isinstance(value, dict): + # A top-level key names an archive only in an image folder's toml, which is its image's table. + image_table = value is not entry.config or entry.kind is BundleKind.ARTIFACT + if image_table and isinstance(value.get("archive"), str): + named.add(unicodedata.normalize("NFC", posixpath.normpath(value["archive"]))) + pending.extend(value.values()) + elif isinstance(value, list): + pending.extend(value) + return frozenset(named) + + +def build_context_files(bundle: Bundle, entry: BundleEntry) -> dict[str, Path]: """What an image built from an entry's folder is built from: its ``entry_files`` and its own toml, which - the Dockerfile can copy too, but for the paths in ``exclude``, such as an archived image's own archive.""" + the Dockerfile can copy too.""" files = entry_files(bundle, entry) toml = entry.path / CONFIG_FILES[entry.kind] if toml.is_file(): files[toml.name] = toml - return {key: path for key, path in sorted(files.items()) if key not in exclude} + return dict(sorted(files.items())) @dataclass(frozen=True) @@ -279,13 +297,11 @@ def identity(self, image_id: str) -> tuple[str, str]: @contextmanager -def staged_build_context(bundle: Bundle, entry: BundleEntry, dockerfile: str, - exclude: Collection[str] = ()) -> Iterator[StagedContext]: - """``entry``'s ``StagedContext`` for its ``dockerfile``, a path in its folder, removed on leaving; without the paths - in ``exclude``.""" +def staged_build_context(bundle: Bundle, entry: BundleEntry, dockerfile: str) -> Iterator[StagedContext]: + """``entry``'s ``StagedContext`` for its ``dockerfile``, a path in its folder, removed on leaving.""" with tempfile.TemporaryDirectory(prefix="agent-env-build-") as staged: context = Path(staged) - for key, path in build_context_files(bundle, entry, exclude).items(): + for key, path in build_context_files(bundle, entry).items(): (context / key).parent.mkdir(parents=True, exist_ok=True) shutil.copy2(path, context / key) yield StagedContext(context, str(context / dockerfile)) @@ -339,6 +355,8 @@ def files(self) -> dict[str, Path]: self._problem(top, f"{how} {show(os.path.realpath(top))}, outside the bundle; {_BRING_IT_IN}") else: self._walk(top) + for archive in entry_archives(self.entry): + self.found.pop(archive, None) if not self.found and not self.problems and not self.empty_ok: self._problem(top, f"has no files to write ({', '.join(sorted(LEAVINGS))} don't count)") if self.problems: diff --git a/src/agent_env/bundle/ledger.py b/src/agent_env/bundle/ledger.py index f61ed895f..1a47d7fef 100644 --- a/src/agent_env/bundle/ledger.py +++ b/src/agent_env/bundle/ledger.py @@ -338,7 +338,7 @@ def _archived_files(bundle: Bundle, image: ArchivedImage) -> dict[str, str]: keeps one.""" files = {image.archive: _file_sha256(image.entry.path / image.archive)} if image.dockerfile is not None: - for key, path in build_context_files(bundle, image.entry, {image.archive}).items(): + for key, path in build_context_files(bundle, image.entry).items(): files[key] = _file_sha256(path) + _mode(path) return files diff --git a/src/agent_env/bundle/materialize.py b/src/agent_env/bundle/materialize.py index 04d5a8c9a..a70b28209 100644 --- a/src/agent_env/bundle/materialize.py +++ b/src/agent_env/bundle/materialize.py @@ -265,7 +265,7 @@ def _write_archived_image(plan: Plan, write: Write) -> int: try: if image.dockerfile is None: return put().version - with staged_build_context(plan.bundle_of(entry), entry, image.dockerfile, {image.archive}) as staged: + with staged_build_context(plan.bundle_of(entry), entry, image.dockerfile) as staged: return put(context_path=str(staged.path), dockerfile_path=staged.dockerfile).version except (ValueError, RuntimeError, OSError) as e: # a context it can't write, or the object store raise BundleError([f"{_path(plan, write)}: {e}"]) from None diff --git a/src/agent_env/providers/sandbox_providers/host_images.py b/src/agent_env/providers/sandbox_providers/host_images.py index c86d9062b..f7e8cce6f 100644 --- a/src/agent_env/providers/sandbox_providers/host_images.py +++ b/src/agent_env/providers/sandbox_providers/host_images.py @@ -16,7 +16,6 @@ import json import logging import platform as host -import shutil import subprocess import tarfile import tempfile @@ -27,6 +26,7 @@ from agent_env.config import get_config from agent_env.store.local_state import holding_locks from agent_env.utils.docker.build import build_image, normalize_platform +from agent_env.utils.docker.untag import write_untagged logger = logging.getLogger(__name__) @@ -163,27 +163,30 @@ async def pull_here(image: DockerImageArtifact) -> None: def load(image: DockerImageArtifact) -> None: """Have this machine's Docker hold ``image``, a tar.gz stored under a tag no registry has, under its name: loaded - from its object store, then tagged with its name, unless Docker holds that name already. Loads of one image wait - for each other. Raises RuntimeError when Docker can't load it.""" + from its object store naming no image, so no name the tar.gz embeds replaces an image Docker holds, then tagged + with its name by the ID Docker gives it, unless Docker holds that name already. Loads of one image wait for each + other. Raises RuntimeError when Docker can't load it.""" with holding_locks([f"image-load:{image.tar_gz_object_url}"]): if present(image.image_name): return with tempfile.TemporaryDirectory(prefix="agent-env-load-") as work: - saved = Path(work) / "image.tar.gz" - with get_config().get_object_store_at(image.tar_gz_object_url).open(image.tar_gz_object_url) as source, \ - saved.open("wb") as out: - shutil.copyfileobj(source, out, 1 << 20) + saved, url = Path(work) / "image.tar", image.tar_gz_object_url + try: + with get_config().get_object_store_at(url).open(url) as source, saved.open("wb") as out: + write_untagged(source, out) + except ValueError as e: + raise RuntimeError(f"Couldn't load {image.id!r} v{image.version}'s saved image: {e}") from None logger.info(f"Loading {image.image_name} from its saved image on this machine...") loaded = subprocess.run(["docker", "load", "-i", str(saved)], capture_output=True, text=True, errors="replace") if loaded.returncode != 0: why = (loaded.stderr.strip().splitlines() or [f"docker load exited {loaded.returncode}"])[-1] raise RuntimeError(f"Couldn't load {image.id!r} v{image.version}'s saved image: {why}") - names = [line.split(":", 1)[1].strip() for line in loaded.stdout.splitlines() - if line.startswith(("Loaded image:", "Loaded image ID:"))] - if not names: + ids = [line.split(":", 1)[1].strip() for line in loaded.stdout.splitlines() + if line.startswith("Loaded image ID:")] + if not ids: raise RuntimeError(f"{image.id!r} v{image.version}'s saved image holds no image to load") - tagged = subprocess.run(["docker", "tag", names[0], image.image_name], capture_output=True, text=True, + tagged = subprocess.run(["docker", "tag", ids[0], image.image_name], capture_output=True, text=True, errors="replace") if tagged.returncode != 0: raise RuntimeError(f"Couldn't tag {image.id!r} v{image.version}'s loaded image: {tagged.stderr.strip()}") diff --git a/src/agent_env/utils/docker/untag.py b/src/agent_env/utils/docker/untag.py new file mode 100644 index 000000000..748710f7d --- /dev/null +++ b/src/agent_env/utils/docker/untag.py @@ -0,0 +1,70 @@ +"""A ``docker save`` archive copied so that it names no image: loading it then tags nothing, where Docker would otherwise +apply every name it embeds, in place of whatever images the loading machine holds under them.""" + +from __future__ import annotations + +import contextlib +import gzip +import io +import json +import posixpath +import tarfile +import zlib +from typing import BinaryIO + +# The names an OCI index.json gives the image on its descriptor; manifest.json gives them as RepoTags. +_NAME_ANNOTATIONS = ("io.containerd.image.name", "org.opencontainers.image.ref.name") +_MANIFESTS = ("manifest.json", "index.json") + + +def write_untagged(saved: BinaryIO, out: BinaryIO, *, gzip_level: int | None = None) -> None: + """Copy the ``docker save`` archive ``saved``, gzipped or not, to ``out``, gzipped at ``gzip_level`` when given, so + that it names no image: its manifest.json tags none, its index.json annotates no name, and its legacy repositories + file is left out; the layers are copied as they are. Raises ValueError when ``saved`` isn't an archive docker save + writes.""" + found = False + sink_file = (gzip.GzipFile(fileobj=out, mode="wb", compresslevel=gzip_level, mtime=0) if gzip_level is not None + else contextlib.nullcontext(out)) + try: + with sink_file as sink_out, tarfile.open(fileobj=saved, mode="r|*") as source, \ + tarfile.open(fileobj=sink_out, mode="w|", format=tarfile.PAX_FORMAT) as sink: + for member in source: + name = posixpath.normpath(member.name) + if member.isreg() and name == "repositories": + continue + if member.isreg() and name in _MANIFESTS: + found = True + data = _unnamed(name, source.extractfile(member).read()) + member.size = len(data) + member.pax_headers.pop("size", None) + sink.addfile(member, io.BytesIO(data)) + else: + sink.addfile(member, source.extractfile(member) if member.isreg() else None) + except (tarfile.TarError, EOFError, zlib.error) as e: + raise ValueError(f"it isn't a tar docker save writes: {e or type(e).__name__}") from None + if not found: + raise ValueError("it holds no manifest.json or index.json, so it isn't an image docker save wrote") + + +def _unnamed(name: str, data: bytes) -> bytes: + """The manifest.json or index.json ``data``, naming no image.""" + try: + document = json.loads(data) + except ValueError as e: + raise ValueError(f"its {name} isn't JSON: {e}") from None + if name == "manifest.json": + if not isinstance(document, list) or not all(isinstance(entry, dict) for entry in document): + raise ValueError("its manifest.json isn't a list of images") + for entry in document: + entry["RepoTags"] = None + elif isinstance(document, dict): + for descriptor in document.get("manifests") or (): + annotations = descriptor.get("annotations") if isinstance(descriptor, dict) else None + if isinstance(annotations, dict): + for key in _NAME_ANNOTATIONS: + annotations.pop(key, None) + if not annotations: + del descriptor["annotations"] + else: + raise ValueError("its index.json isn't an image index") + return json.dumps(document, separators=(",", ":")).encode() diff --git a/tst/unit/bundle/archived_image_test.py b/tst/unit/bundle/archived_image_test.py index c29f6f33a..a5dba9227 100644 --- a/tst/unit/bundle/archived_image_test.py +++ b/tst/unit/bundle/archived_image_test.py @@ -1,6 +1,6 @@ -"""Saved images: an image a bundle carries as a ``docker save`` archive is stored as its tar.gz under a name its digest -gives, never built, with the build context its Dockerfile names kept beside it, and everything naming it is written over -it.""" +"""Saved images: an image a bundle carries as a ``docker save`` archive is stored as its tar.gz naming no image, under a +name its digest gives, never built, with the build context its Dockerfile names kept beside it, and everything naming it +is written over it.""" import gzip import hashlib @@ -12,16 +12,19 @@ from agent_env.a2a_agent import A2AAgent from agent_env.artifact.artifacts.docker_image import DockerImageArtifact +from agent_env.bundle import BundleError from agent_env.bundle.materialize import materialize from agent_env.config import get_config from agent_env.env import Env from agent_env.store.ids import local_image_repository from tst.unit.bundle._support import layout, local_builds, plan_of +from tst.util.docker_save import saved_image ROOT = "@local/~/triage" IMAGE = f"{ROOT}/crm-image" WEB = f"{ROOT}/web" -SAVED = b"the tar docker save wrote" +SAVED = saved_image([{"app/hello.txt": "hi\n"}], oci=True, tags=["acme/crm:1"]) +AGAIN = saved_image([{"app/hello.txt": "hi again\n"}], oci=True, tags=["acme/crm:1"]) USERS = { "envs/crm/env.toml": 'image = "crm-image"\nenvironment_name = "crm"\n', "agents/solver/agent.toml": 'image = "crm-image"\n', @@ -35,7 +38,7 @@ WEB_FILES = { "envs/web/Dockerfile": "FROM scratch\nCOPY server.py /app/\n", "envs/web/server.py": "print('serving')\n", - "envs/web/env.toml": 'environment_name = "web"\nimage = { archive = "image.tar", dockerfile = "Dockerfile" }\n', + "envs/web/env.toml": 'environment_name = "web"\nimage = { archive = "./image.tar", dockerfile = "Dockerfile" }\n', "tasks/t.json": json.dumps([{"id": "web", "type": "deploy_env", "env_id": "web"}]), } @@ -65,6 +68,13 @@ def _stored(url): return stream.read() +def _tags(stored): + """The names the stored tar.gz gives its image: in its manifest.json, and on its index.json descriptors.""" + with tarfile.open(fileobj=io.BytesIO(gzip.decompress(stored))) as tar: + manifest, index = (json.load(tar.extractfile(name)) for name in ("manifest.json", "index.json")) + return [entry["RepoTags"] for entry in manifest] + [d.get("annotations") for d in index.get("manifests", [])] + + def _named(data): return f"{local_image_repository(IMAGE)}:{hashlib.sha256(data).hexdigest()[:16]}" @@ -75,7 +85,7 @@ def _images_named(done): return [(image.id, image.version) for image in (env, agent)] -def test_a_saved_image_is_stored_gzipped_under_its_digest_unbuilt_and_everything_naming_it_is_written_over_it( +def test_a_saved_image_is_stored_naming_none_under_its_digest_unbuilt_and_everything_naming_it_is_written_over_it( bundle_dir, builds): layout(bundle_dir, FOLDER) _saved(bundle_dir, SAVED) @@ -86,7 +96,7 @@ def test_a_saved_image_is_stored_gzipped_under_its_digest_unbuilt_and_everything assert (image.image_name, image.description, image.platform) == ( _named(SAVED), "loaded from artifacts/crm-image/image.tar", "linux/arm64") assert (image.stored_under_build_tag, image.build_context_object_url) == (True, None) - assert gzip.decompress(_stored(image.tar_gz_object_url)) == SAVED + assert _tags(_stored(image.tar_gz_object_url)) == [None, {"org.example.kept": "yes"}] assert _images_named(done) == [(IMAGE, 1)] * 2 assert _summary(_run(bundle_dir))[IMAGE] == (1, True, ()) assert builds == [] @@ -96,16 +106,16 @@ def test_a_new_saved_image_is_a_new_version_under_a_new_name_and_everything_nami layout(bundle_dir, FOLDER) _saved(bundle_dir, SAVED) _run(bundle_dir) - _saved(bundle_dir, SAVED + b" again") + _saved(bundle_dir, AGAIN) done = _run(bundle_dir) assert _summary(done)[IMAGE][:2] == (2, False) - assert DockerImageArtifact.get(IMAGE).image_name == _named(SAVED + b" again") + assert DockerImageArtifact.get(IMAGE).image_name == _named(AGAIN) assert _images_named(done) == [(IMAGE, 2)] * 2 -def test_a_saved_image_already_gzipped_is_stored_as_it_is(bundle_dir): +def test_a_saved_image_already_gzipped_is_named_by_its_digest_as_given(bundle_dir): gzipped = gzip.compress(SAVED) layout(bundle_dir, {**FOLDER, "artifacts/crm-image/artifact.toml": 'type = "docker_image"\narchive = "image.tar.gz"\n'}) _saved(bundle_dir, gzipped, "artifacts/crm-image/image.tar.gz") @@ -113,7 +123,18 @@ def test_a_saved_image_already_gzipped_is_stored_as_it_is(bundle_dir): _run(bundle_dir) image = DockerImageArtifact.get(IMAGE) - assert (_stored(image.tar_gz_object_url), image.image_name, image.platform) == (gzipped, _named(gzipped), None) + assert (image.image_name, image.platform) == (_named(gzipped), None) + assert _tags(_stored(image.tar_gz_object_url))[0] is None + + +def test_a_file_docker_save_didnt_write_is_refused_naming_its_folder(bundle_dir): + layout(bundle_dir, FOLDER) + _saved(bundle_dir, b"not an image") + + with pytest.raises(BundleError) as caught: + _run(bundle_dir) + + assert caught.value.problems == ("artifacts/crm-image: it isn't a tar docker save writes: truncated header",) def test_a_saved_image_naming_its_dockerfile_keeps_its_folder_but_the_archive_as_its_build_context(bundle_dir, builds): @@ -142,3 +163,26 @@ def test_an_edit_to_a_saved_images_build_context_rewrites_it_under_the_same_name image = Env.get(WEB, done.version_of("env", WEB)).docker_image_artifact assert (image.version, image.image_name) == (2, first.image_name) assert image.source_digest != first.source_digest + + +def test_a_saved_image_is_in_no_build_context_of_its_folder_and_a_new_one_rebuilds_nothing_else(bundle_dir, builds): + layout(bundle_dir, { + "envs/shop/Dockerfile": "FROM scratch\n", + "envs/shop/env.toml": 'type = "website"\nenvironment_name = "shop"\n' + 'backend_image = { dockerfile = "Dockerfile" }\n' + 'frontend_image = { archive = "front.tar", dockerfile = "Dockerfile" }\n', + "tasks/t.json": json.dumps([{"id": "shop", "type": "deploy_env", "env_id": "shop"}]), + }) + _saved(bundle_dir, SAVED, "envs/shop/front.tar") + shop = f"{ROOT}/shop" + + first = Env.get(shop, _run(bundle_dir).version_of("env", shop)) + _saved(bundle_dir, AGAIN, "envs/shop/front.tar") + done = _run(bundle_dir) + + context = _stored(first.frontend_docker_image_artifact.build_context_object_url) + with tarfile.open(fileobj=io.BytesIO(context)) as tar: + kept = sorted(member.name.removeprefix("./") for member in tar.getmembers() if member.isfile()) + assert ([call["build"][1] for call in builds], kept) == ([["Dockerfile", "env.toml"]], ["Dockerfile", "env.toml"]) + shop_now = Env.get(shop, done.version_of("env", shop)) + assert (shop_now.backend_docker_image_artifact.version, shop_now.frontend_docker_image_artifact.version) == (1, 2) diff --git a/tst/unit/providers/sandbox_providers/host_images_test.py b/tst/unit/providers/sandbox_providers/host_images_test.py index b6630a07e..ce627a933 100644 --- a/tst/unit/providers/sandbox_providers/host_images_test.py +++ b/tst/unit/providers/sandbox_providers/host_images_test.py @@ -3,19 +3,24 @@ import gzip import io +import json import logging import subprocess +import tarfile import threading import time import pytest from agent_env.artifact.artifacts.docker_image import DockerImageArtifact +from agent_env.artifact.store import get_artifact_store +from agent_env.config import get_config from agent_env.providers.sandbox_providers import host_images from agent_env.providers.sandbox_providers.local_docker import provider as local_sandbox from agent_env.providers.sandbox_providers.local_docker.provider import LocalSandbox, LocalSandboxProvider from agent_env.providers.sandbox_providers.sandbox import VmSandbox from agent_env.utils.docker.build import DockerBuildError +from tst.util.docker_save import saved_image, tar_bytes @pytest.fixture @@ -304,21 +309,41 @@ def test_a_published_image_with_no_build_context_that_cant_be_pulled_says_why(mo +_TAGGED = saved_image([{"app/hello.txt": "hi\n"}], oci=True, tags=["acme/crm:1"]) +_LOADED_ID = "sha256:" + "c" * 64 + + def _archived(tmp_path): archive = tmp_path / "image.tar" - archive.write_bytes(b"the tar docker save wrote") + archive.write_bytes(_TAGGED) return DockerImageArtifact.put_archive("crm_image", description="d", archive_path=str(archive)) -def _docker_load(monkeypatch, *, held=(), printed="Loaded image: acme/crm:1\n", failure=None): - """The docker commands ``load`` runs, each recorded with the bytes it was given to load; Docker holds ``held``, and - its load prints ``printed``, or exits 1 saying ``failure``.""" +def _stored_tarball(tmp_path, data): + """A tar.gz another writer stored under a bare tag, as it saved it: naming whatever images it names.""" + path = tmp_path / "saved.tar.gz" + path.write_bytes(gzip.compress(data)) + prefix = get_artifact_store().attempt_prefix("docker_image", "crm_image") + url = get_config().get_object_store_to_write(prefix, "crm_image").put_file_at( + f"{prefix}crm.tar.gz", str(path), "application/gzip") + return DockerImageArtifact.put_tar("crm_image", description="d", image_name="local/crm:v1", tar_gz_object_url=url) + + +def _docker_load(monkeypatch, *, held=(), printed=f"Loaded image ID: {_LOADED_ID}\n", failure=None): + """The docker commands ``load`` runs, each load recorded with the names the archive it was given gives its image; + Docker holds ``held``, and its load prints ``printed``, or exits 1 saying ``failure``.""" ran = [] def run(argv, **kwargs): if argv[1:3] == ["image", "inspect"]: return subprocess.CompletedProcess(argv, int(argv[3] not in held), "", "") - ran.append(argv[1:] if argv[1] != "load" else ["load", gzip.decompress(open(argv[3], "rb").read())]) + if argv[1] == "load": + with tarfile.open(argv[3]) as tar: + names = [entry["RepoTags"] for entry in json.load(tar.extractfile("manifest.json"))] + names += [d.get("annotations") for d in json.load(tar.extractfile("index.json"))["manifests"]] + ran.append(["load", names]) + else: + ran.append(argv[1:]) if argv[1] == "load" and failure: return subprocess.CompletedProcess(argv, 1, "", failure) return subprocess.CompletedProcess(argv, 0, printed if argv[1] == "load" else "", "") @@ -327,38 +352,40 @@ def run(argv, **kwargs): return ran -def test_a_saved_image_is_loaded_from_its_store_and_tagged_with_its_name(local_stores, tmp_path, monkeypatch): +def test_a_saved_image_is_loaded_naming_no_image_and_tagged_with_its_name_by_its_id(local_stores, tmp_path, + monkeypatch): image = _archived(tmp_path) ran = _docker_load(monkeypatch) host_images.load(image) - assert ran == [["load", b"the tar docker save wrote"], ["tag", "acme/crm:1", image.image_name]] + assert ran == [["load", [None, {"org.example.kept": "yes"}]], ["tag", _LOADED_ID, image.image_name]] -def test_a_saved_image_docker_holds_isnt_loaded_again(local_stores, tmp_path, monkeypatch): - image = _archived(tmp_path) - ran = _docker_load(monkeypatch, held={image.image_name}) +def test_a_stored_tarball_naming_images_is_given_to_docker_naming_none(local_stores, tmp_path, monkeypatch): + """Docker would otherwise tag what it loads with every name the tarball gives, over images it holds.""" + image = _stored_tarball(tmp_path, _TAGGED) + ran = _docker_load(monkeypatch) host_images.load(image) - assert ran == [] + assert ran == [["load", [None, {"org.example.kept": "yes"}]], ["tag", _LOADED_ID, "local/crm:v1"]] -def test_a_saved_image_holding_only_an_untagged_image_is_tagged_by_its_id(local_stores, tmp_path, monkeypatch): +def test_a_saved_image_docker_holds_isnt_loaded_again(local_stores, tmp_path, monkeypatch): image = _archived(tmp_path) - ran = _docker_load(monkeypatch, printed="Loaded image ID: sha256:" + "c" * 64 + "\n") + ran = _docker_load(monkeypatch, held={image.image_name}) host_images.load(image) - assert ran[1] == ["tag", "sha256:" + "c" * 64, image.image_name] + assert ran == [] @pytest.mark.parametrize("load, problem", [ ({"failure": "open /x: no space left on device\n"}, "Couldn't load 'crm_image' v1's saved image: open /x: no space"), - ({"printed": ""}, "'crm_image' v1's saved image holds no image to load"), -], ids=["refused", "empty"]) + ({"printed": "Loaded image: acme/crm:1\n"}, "'crm_image' v1's saved image holds no image to load"), +], ids=["refused", "no id"]) def test_a_saved_image_docker_cant_load_says_why(local_stores, tmp_path, monkeypatch, load, problem): image = _archived(tmp_path) _docker_load(monkeypatch, **load) @@ -366,6 +393,16 @@ def test_a_saved_image_docker_cant_load_says_why(local_stores, tmp_path, monkeyp with pytest.raises(RuntimeError, match=problem): host_images.load(image) + +def test_a_stored_tarball_docker_save_didnt_write_isnt_given_to_docker(local_stores, tmp_path, monkeypatch): + image = _stored_tarball(tmp_path, tar_bytes({"hello.txt": "hi"})) + ran = _docker_load(monkeypatch) + + with pytest.raises(RuntimeError, match="Couldn't load 'crm_image' v1's saved image: it holds no manifest.json"): + host_images.load(image) + assert ran == [] + + @pytest.mark.asyncio async def test_the_local_provider_pulls_a_published_image_builds_a_context_and_loads_a_saved_one(local_stores, context, monkeypatch): diff --git a/tst/unit/utils/docker/untag_test.py b/tst/unit/utils/docker/untag_test.py new file mode 100644 index 000000000..dc5cbdcb7 --- /dev/null +++ b/tst/unit/utils/docker/untag_test.py @@ -0,0 +1,62 @@ +"""``write_untagged``: a ``docker save`` archive copied naming no image, its layers as they are.""" + +import gzip +import io +import json +import tarfile + +import pytest + +from agent_env.utils.docker.untag import write_untagged +from tst.util.docker_save import saved_image, tar_bytes + +LAYER = {"app/hello.txt": "hi\n"} +TAGS = ["acme/crm:1"] + + +def _untagged(data: bytes, **kwargs) -> bytes: + out = io.BytesIO() + write_untagged(io.BytesIO(data), out, **kwargs) + return out.getvalue() + + +def _members(data: bytes) -> dict[str, bytes]: + with tarfile.open(fileobj=io.BytesIO(data)) as tar: + return {member.name: tar.extractfile(member).read() for member in tar.getmembers() if member.isreg()} + + +@pytest.mark.parametrize("oci", [False, True], ids=["legacy", "oci"]) +@pytest.mark.parametrize("gz", [False, True], ids=["tar", "gzipped"]) +def test_an_archive_is_copied_naming_no_image_with_its_layers_as_they_are(oci, gz): + saved = saved_image([LAYER], oci=oci, gz=gz, tags=TAGS) + before = _members(gzip.decompress(saved) if gz else saved) + + after = _members(_untagged(saved)) + + assert [entry["RepoTags"] for entry in json.loads(after.pop("manifest.json"))] == [None] + if oci: + (descriptor,) = json.loads(after.pop("index.json"))["manifests"] + assert descriptor["annotations"] == {"org.example.kept": "yes"} + assert "repositories" not in after + assert after == {name: data for name, data in before.items() + if name not in ("manifest.json", "index.json", "repositories")} + + +def test_an_archive_naming_no_image_is_copied_naming_none_and_gzipped_when_asked(): + saved = saved_image([LAYER], oci=True) + + out = _untagged(saved, gzip_level=1) + + assert out[:2] == b"\x1f\x8b" + assert json.loads(_members(gzip.decompress(out))["manifest.json"])[0]["RepoTags"] is None + + +@pytest.mark.parametrize("data, problem", [ + (tar_bytes({"hello.txt": "hi"}), "it holds no manifest.json or index.json"), + (b"not a tar at all" * 64, "it isn't a tar docker save writes"), + (tar_bytes({"manifest.json": "{"}), "its manifest.json isn't JSON"), + (tar_bytes({"manifest.json": '{"Config": "c"}'}), "its manifest.json isn't a list of images"), +], ids=["no manifest", "not a tar", "bad json", "not a list"]) +def test_what_docker_save_didnt_write_is_refused(data, problem): + with pytest.raises(ValueError, match=problem): + _untagged(data) diff --git a/tst/util/docker_save.py b/tst/util/docker_save.py index f61fc112e..94bcc54c2 100644 --- a/tst/util/docker_save.py +++ b/tst/util/docker_save.py @@ -24,14 +24,25 @@ def tar_bytes(files: dict[str, str | bytes], links: dict[str, str] | None = None def saved_image(layers: list[dict[str, str | bytes]], oci: bool = False, gz_layers: bool = False, gz: bool = False, - manifest_first: bool = False) -> bytes: - """A ``docker save`` tarball of ``layers``, bottom first, in the legacy or the OCI layout.""" + manifest_first: bool = False, tags: list[str] | None = None) -> bytes: + """A ``docker save`` tarball of ``layers``, bottom first, in the legacy or the OCI layout; named by ``tags``, as + docker save names an image in each, when given.""" blobs = [tar_bytes(files, gz=gz_layers) for files in layers] names = ([f"blobs/sha256/{hashlib.sha256(blob).hexdigest()}" for blob in blobs] if oci else [f"{index:064x}/layer.tar" for index in range(len(blobs))]) files: dict[str, str | bytes] = dict(zip(names, blobs)) files["blobs/sha256/config" if oci else "config.json"] = json.dumps({"rootfs": {"type": "layers"}}) - manifest = {"manifest.json": json.dumps([{"Config": "config.json", "Layers": names}])} + image = {"Config": "config.json", "Layers": names, **({"RepoTags": tags} if tags is not None else {})} + manifest = {"manifest.json": json.dumps([image])} extra = {"oci-layout": '{"imageLayoutVersion": "1.0.0"}', "index.json": "{}"} if oci else {} + if oci and tags: + extra["index.json"] = json.dumps({"schemaVersion": 2, "manifests": [{ + "mediaType": "application/vnd.oci.image.index.v1+json", "digest": "sha256:" + "d" * 64, "size": 1, + "annotations": {"io.containerd.image.name": f"docker.io/{tags[0]}", + "org.opencontainers.image.ref.name": tags[0].rsplit(":", 1)[-1], "org.example.kept": "yes"}, + }]}) + elif tags: + repository, tag = tags[0].rsplit(":", 1) + extra["repositories"] = json.dumps({repository: {tag: "0" * 64}}) data = tar_bytes({**manifest, **files, **extra} if manifest_first else {**files, **extra, **manifest}) return gzip.compress(data) if gz else data From 412c69050316f9721b1e94d9f867d00c275dff54 Mon Sep 17 00:00:00 2001 From: Edgar Arakelyan Date: Thu, 29 Oct 2026 18:16:11 -0700 Subject: [PATCH 3/3] fix(bundle): an entry's archives are only the ones its image keys name entry_archives read every nested `archive` string as an image archive, so a key like an agent's `default_env_vars = { archive = "assets.zip" }` dropped that file from its build context. It now reads `archive` only at the docker_image keys the entry's type declares, as the resolver does, and at an image folder's top level. entity_class moves to authoring.py so both can use it. Co-Authored-By: Claude Opus 5.5 --- src/agent_env/bundle/authoring.py | 43 +++++++++++++++++--------- src/agent_env/bundle/resolve.py | 17 +--------- tst/unit/bundle/archived_image_test.py | 13 ++++++++ tst/unit/bundle/ledger_test.py | 6 ++-- tst/unit/bundle/plan_test.py | 6 ++-- tst/unit/bundle/resolve_test.py | 5 +-- 6 files changed, 54 insertions(+), 36 deletions(-) diff --git a/src/agent_env/bundle/authoring.py b/src/agent_env/bundle/authoring.py index 639040c72..6028c9f6f 100644 --- a/src/agent_env/bundle/authoring.py +++ b/src/agent_env/bundle/authoring.py @@ -14,11 +14,15 @@ from pathlib import Path from typing import Any, NoReturn, TypeVar, overload +from agent_env.a2a_agent import A2AAgent from agent_env.artifact.artifact import Artifact from agent_env.artifact.artifacts.docker_image import DockerImageArtifact +from agent_env.artifact.registry import canonical_type, get_artifact_registry from agent_env.entity.refs import EntityKind, parse_toml_ref from agent_env.env.env import Env from agent_env.env.envs._deployment import provider_refusal +from agent_env.env.registry import get_env_registry +from agent_env.eval.eval import Eval from agent_env.providers.env_providers.constants import GATEWAY_SERVICE_NAMES from agent_env.providers.env_providers.env_gateway_provider import EnvironmentGatewayProvider, environment_name_problem from agent_env.providers.env_providers.env_provider import _env_provider_class @@ -255,21 +259,32 @@ def entry_files(bundle: Bundle, entry: BundleEntry, *, empty_ok: bool = False) - return _Walk(bundle, entry, empty_ok).files() +def entity_class(entry: BundleEntry) -> type | None: + """The class that writes ``entry``: its env type's, its artifact type's, the agent's or the eval's; None for a type + nothing registers.""" + if entry.kind is BundleKind.ENV: + return get_env_registry().get(entry.type) + if entry.kind is BundleKind.AGENT: + return A2AAgent if entry.type == A2AAgent.type else None + if entry.kind is BundleKind.EVAL: + return Eval if entry.type == Eval.type else None + return get_artifact_registry().get(canonical_type(entry.type)) + + def entry_archives(entry: BundleEntry) -> frozenset[str]: - """The image archives ``entry``'s toml names, by an image table's ``archive``, or an image folder's, as POSIX paths - in its folder in NFC: what an image loads, which no build context or file the entry writes holds.""" - named, pending = set(), [entry.config or {}] - while pending: - value = pending.pop() - if isinstance(value, dict): - # A top-level key names an archive only in an image folder's toml, which is its image's table. - image_table = value is not entry.config or entry.kind is BundleKind.ARTIFACT - if image_table and isinstance(value.get("archive"), str): - named.add(unicodedata.normalize("NFC", posixpath.normpath(value["archive"]))) - pending.extend(value.values()) - elif isinstance(value, list): - pending.extend(value) - return frozenset(named) + """The image archives ``entry``'s toml names, as POSIX paths in its folder in NFC: an image folder's ``archive``, + and the ``archive`` of each image key its type declares, as the resolver reads them. What an image loads, which no + build context or file the entry writes holds.""" + config = entry.config if isinstance(entry.config, dict) else {} + tables = [config] if entry.kind is BundleKind.ARTIFACT and canonical_type(entry.type) == "docker_image" else [] + for ref in getattr(entity_class(entry), "toml_refs", ()): + if ref.artifact_type == "docker_image" and "[]" not in ref.path: + value: Any = config + for key in ref.path.split("."): + value = value.get(key) if isinstance(value, dict) else None + tables.append(value) + return frozenset(unicodedata.normalize("NFC", posixpath.normpath(table["archive"])) for table in tables + if isinstance(table, dict) and isinstance(table.get("archive"), str)) def build_context_files(bundle: Bundle, entry: BundleEntry) -> dict[str, Path]: diff --git a/src/agent_env/bundle/resolve.py b/src/agent_env/bundle/resolve.py index 2802f7d8a..af7aa4a9a 100644 --- a/src/agent_env/bundle/resolve.py +++ b/src/agent_env/bundle/resolve.py @@ -16,14 +16,11 @@ from pathlib import Path from typing import Any -from agent_env.a2a_agent import A2AAgent from agent_env.artifact.artifacts.docker_image import DockerImageArtifact from agent_env.artifact.registry import canonical_type, get_artifact_registry from agent_env.entity.refs import EntityKind, EntityRef, RefRole, RefSite, parse_toml_ref, ref_sites from agent_env.env.envs.gateway_server import GatewayEnv from agent_env.env.envs.service_db import ServiceDBEnv -from agent_env.env.registry import get_env_registry -from agent_env.eval.eval import Eval from agent_env.plugins import _registration from agent_env.plugins._requirements import unmet_problems from agent_env.store.ids import LOCAL_PREFIX, derive_id, validate_local_id @@ -32,7 +29,7 @@ from agent_env.task_step.task_step import TaskStep, attach_retry_config, dependencies from ._fs import fold, relative, show -from .authoring import AuthoringContext, default_dockerfile +from .authoring import AuthoringContext, default_dockerfile, entity_class from .parse import BUNDLE_TOML, DOCKERFILE, NAMED_BY, Bundle, BundleEntry, BundleError, BundleKind from .requires import UnmetRequirements @@ -504,18 +501,6 @@ def _problem(self, entry: BundleEntry, message: str) -> None: self.problems.append(f"{relative(self.bundle.root, entry.path)}: {message}") -def entity_class(entry: BundleEntry) -> type | None: - """The class that writes ``entry``: its env type's, its artifact type's, the agent's or the eval's; None for a type - nothing registers.""" - if entry.kind is BundleKind.ENV: - return get_env_registry().get(entry.type) - if entry.kind is BundleKind.AGENT: - return A2AAgent if entry.type == A2AAgent.type else None - if entry.kind is BundleKind.EVAL: - return Eval if entry.type == Eval.type else None - return get_artifact_registry().get(canonical_type(entry.type)) - - def _folder_file(entry: BundleEntry, name: Any) -> bool: """Whether ``name`` is a relative path to a file inside ``entry``'s folder.""" if not isinstance(name, str) or not name or Path(name).is_absolute(): diff --git a/tst/unit/bundle/archived_image_test.py b/tst/unit/bundle/archived_image_test.py index a5dba9227..fcc8e7927 100644 --- a/tst/unit/bundle/archived_image_test.py +++ b/tst/unit/bundle/archived_image_test.py @@ -186,3 +186,16 @@ def test_a_saved_image_is_in_no_build_context_of_its_folder_and_a_new_one_rebuil assert ([call["build"][1] for call in builds], kept) == ([["Dockerfile", "env.toml"]], ["Dockerfile", "env.toml"]) shop_now = Env.get(shop, done.version_of("env", shop)) assert (shop_now.backend_docker_image_artifact.version, shop_now.frontend_docker_image_artifact.version) == (1, 2) + + +def test_an_archive_key_that_isnt_an_images_leaves_its_file_in_the_build_context(bundle_dir, builds): + layout(bundle_dir, { + "agents/solver/Dockerfile": "FROM scratch\nCOPY assets.zip /\n", + "agents/solver/assets.zip": "zipped", + "agents/solver/agent.toml": 'default_env_vars = { archive = "assets.zip" }\n', + "tasks/t.json": json.dumps([{"id": "agent", "type": "deploy_agent", "env_ids": [], "a2a_agent_id": "solver"}]), + }) + + _run(bundle_dir) + + assert builds[0]["build"][1] == ["Dockerfile", "agent.toml", "assets.zip"] diff --git a/tst/unit/bundle/ledger_test.py b/tst/unit/bundle/ledger_test.py index 6a5a981fc..4191cb54d 100644 --- a/tst/unit/bundle/ledger_test.py +++ b/tst/unit/bundle/ledger_test.py @@ -16,6 +16,7 @@ from agent_env.artifact.artifacts.file import FileArtifact from agent_env.bundle import parse_bundle from agent_env.bundle import plan as plan_module +from agent_env.bundle import authoring as authoring_module from agent_env.bundle import resolve as resolve_module from agent_env.bundle import ledger as ledger_module from agent_env.bundle.ledger import LEDGER_COLLECTION, Ledger, materializing @@ -95,11 +96,12 @@ def from_toml(data, ctx): @pytest.fixture def bundle_dir(tmp_path, monkeypatch): - envs = {**resolve_module.get_env_registry(), **{cls.type: cls for cls in (_Shelf, _Rack, _OwnEnv, _Imaged)}} - monkeypatch.setattr(resolve_module, "get_env_registry", lambda: envs) + envs = {**authoring_module.get_env_registry(), **{cls.type: cls for cls in (_Shelf, _Rack, _OwnEnv, _Imaged)}} + monkeypatch.setattr(authoring_module, "get_env_registry", lambda: envs) monkeypatch.setattr("agent_env.bundle.ledger.get_env_registry", lambda: envs) artifacts = {**resolve_module.get_artifact_registry(), "own_file_ledger_test": _OwnFile} monkeypatch.setattr(resolve_module, "get_artifact_registry", lambda: artifacts) + monkeypatch.setattr(authoring_module, "get_artifact_registry", lambda: artifacts) monkeypatch.setattr(plan_module, "get_artifact_registry", lambda: artifacts) monkeypatch.setattr("agent_env.bundle.ledger.get_artifact_registry", lambda: artifacts) monkeypatch.setenv("HOME", str(tmp_path)) diff --git a/tst/unit/bundle/plan_test.py b/tst/unit/bundle/plan_test.py index bce8ff7bc..c963e602c 100644 --- a/tst/unit/bundle/plan_test.py +++ b/tst/unit/bundle/plan_test.py @@ -10,6 +10,7 @@ from agent_env.artifact.store import reset_artifact_store from agent_env.bundle import BundleError, BundleKind, parse_bundle from agent_env.bundle import plan as plan_module +from agent_env.bundle import authoring as authoring_module from agent_env.bundle import resolve as resolve_module from agent_env.bundle.plan import check_bundle, plan_bundle from agent_env.bundle.resolve import resolve_bundle @@ -82,8 +83,8 @@ def from_dict(cls, data): @pytest.fixture(autouse=True) def stores(monkeypatch): set_document_store(FakeDocumentStore()) - envs = resolve_module.get_env_registry() - monkeypatch.setattr(resolve_module, "get_env_registry", lambda: {**envs, _Composite.type: _Composite}) + envs = authoring_module.get_env_registry() + monkeypatch.setattr(authoring_module, "get_env_registry", lambda: {**envs, _Composite.type: _Composite}) monkeypatch.setattr("agent_env.env.registry.get_env_registry", lambda: {_NoteEnv.type: _NoteEnv}) yield reset_artifact_store() @@ -404,6 +405,7 @@ def test_a_plugin_file_type_is_checked_as_a_file_artifact_is_whichever_from_toml registry = {**artifacts, _PluginFile.model_fields["type"].default: _PluginFile, _OwnFile.model_fields["type"].default: _OwnFile} monkeypatch.setattr(resolve_module, "get_artifact_registry", lambda: registry) + monkeypatch.setattr(authoring_module, "get_artifact_registry", lambda: registry) monkeypatch.setattr(plan_module, "get_artifact_registry", lambda: registry) resolved = make(tasks={"t": [load("mine", "l1"), load("theirs", "l2")]}, files={ "artifacts/mine/artifact.toml": 'type = "plugin_file_test"\n', "artifacts/mine/a.txt": "a", diff --git a/tst/unit/bundle/resolve_test.py b/tst/unit/bundle/resolve_test.py index e5779edb9..b6a58eeb0 100644 --- a/tst/unit/bundle/resolve_test.py +++ b/tst/unit/bundle/resolve_test.py @@ -6,6 +6,7 @@ import pytest from agent_env.bundle import BundleError, BundleKind, parse_bundle +from agent_env.bundle import authoring as authoring_module from agent_env.bundle import resolve as resolve_module from agent_env.bundle.resolve import ( ArchivedImage, @@ -52,8 +53,8 @@ class _Plugin(TaskStep): @pytest.fixture(autouse=True) def registries(monkeypatch): - envs, steps = resolve_module.get_env_registry(), resolve_module.get_task_step_registry() - monkeypatch.setattr(resolve_module, "get_env_registry", lambda: {**envs, _Composite.type: _Composite}) + envs, steps = authoring_module.get_env_registry(), resolve_module.get_task_step_registry() + monkeypatch.setattr(authoring_module, "get_env_registry", lambda: {**envs, _Composite.type: _Composite}) monkeypatch.setattr(resolve_module, "get_task_step_registry", lambda: {**steps, _Plugin.type: _Plugin})