fix(fc): ship agent-image build context files to the infra VM
test / integration-docker (pull_request) Has been cancelled
prd-number-check / require-numbered-prds (pull_request) Successful in 11s
lint / lint (push) Successful in 59s
test / unit (pull_request) Successful in 1m0s
test / coverage (pull_request) Has been skipped
test / image-input-builds (pull_request) Successful in 1m1s
tracker-policy-pr / check-pr (pull_request) Successful in 17s
test / integration-docker (pull_request) Has been cancelled
prd-number-check / require-numbered-prds (pull_request) Successful in 11s
lint / lint (push) Successful in 59s
test / unit (pull_request) Successful in 1m0s
test / coverage (pull_request) Has been skipped
test / image-input-builds (pull_request) Successful in 1m1s
tracker-policy-pr / check-pr (pull_request) Successful in 17s
Agent images build with buildah inside the infra VM, and
`_build_in_infra` shipped only the Dockerfile into an empty `{ctx}/ctx`
context — on the documented assumption that agent Dockerfiles COPY
nothing from the context. The "pin & verify image inputs" work broke
that: the codex Dockerfile COPYs `codex-package_SHA256SUMS` and the
claude/pi Dockerfiles COPY their npm `package.json`/`package-lock.json`,
so any fresh agent image build fails with `COPY ...: no such file or
directory` (existing agents only survive on a rootfs cached from before
the change).
Parse each Dockerfile's context COPY sources, tar those committed files
from the build root into the VM-side `{ctx}/ctx`, and fold their content
into the rootfs cache digest so a repinned input rebuilds instead of
reusing stale bytes. `COPY --from=<stage>` and absolute/traversing
sources are excluded.
Closes #538.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -7,6 +7,7 @@ the smoke-test no-op — the logic that must hold without a VM.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import subprocess
|
||||
import tempfile
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
@@ -91,6 +92,91 @@ class TestBuildAgentRootfsDir(unittest.TestCase):
|
||||
self.assertNotEqual(first, second)
|
||||
|
||||
|
||||
class TestBuildContext(unittest.TestCase):
|
||||
"""The COPY-source parsing + context shipping that lets a Dockerfile pin an
|
||||
input by COPYing a committed file (codex checksum list, claude/pi npm
|
||||
lockfiles) through the otherwise-empty VM-side build context."""
|
||||
|
||||
def setUp(self):
|
||||
self._tmp = tempfile.TemporaryDirectory()
|
||||
self.root = Path(self._tmp.name)
|
||||
self.addCleanup(self._tmp.cleanup)
|
||||
codex = self.root / "bot_bottle" / "contrib" / "codex"
|
||||
codex.mkdir(parents=True)
|
||||
self.sums = codex / "codex-package_SHA256SUMS"
|
||||
self.sums.write_text("aaaa codex-package-x86_64-unknown-linux-musl.tar.gz\n")
|
||||
self.dockerfile = self.root / "Dockerfile"
|
||||
self.dockerfile.write_text(
|
||||
"FROM node:22-slim\n"
|
||||
"COPY --chown=node:node "
|
||||
"bot_bottle/contrib/codex/codex-package_SHA256SUMS /tmp/x\n"
|
||||
)
|
||||
|
||||
def _patch_root(self):
|
||||
return patch.object(
|
||||
image_builder.resources, "build_root", return_value=self.root)
|
||||
|
||||
def test_copy_sources_parses_flags_and_continuations(self):
|
||||
df = self.root / "Multi"
|
||||
df.write_text(
|
||||
"FROM x\n"
|
||||
"COPY a/one.json \\\n a/two.json /dest/\n"
|
||||
"COPY --from=builder /built /built\n" # excluded: build stage
|
||||
"COPY --chown=n:n b/three /dest\n"
|
||||
)
|
||||
self.assertEqual(
|
||||
image_builder._context_copy_sources(df),
|
||||
["a/one.json", "a/two.json", "b/three"],
|
||||
)
|
||||
|
||||
def test_context_files_resolves_existing_under_root(self):
|
||||
with self._patch_root():
|
||||
files = image_builder._context_files(self.dockerfile)
|
||||
self.assertEqual(
|
||||
[rel for rel, _ in files],
|
||||
["bot_bottle/contrib/codex/codex-package_SHA256SUMS"],
|
||||
)
|
||||
|
||||
def test_context_files_drops_missing_and_traversal(self):
|
||||
df = self.root / "Bad"
|
||||
df.write_text("FROM x\nCOPY ../escape /d\nCOPY does/not/exist /d\n")
|
||||
with self._patch_root():
|
||||
self.assertEqual(image_builder._context_files(df), [])
|
||||
|
||||
def test_rootfs_digest_tracks_context_file_content(self):
|
||||
with self._patch_root(), \
|
||||
patch.object(image_builder.util, "cache_dir", return_value=self.root):
|
||||
first = image_builder._rootfs_digest(self.dockerfile)
|
||||
self.sums.write_text("bbbb codex-package-x86_64-unknown-linux-musl.tar.gz\n")
|
||||
second = image_builder._rootfs_digest(self.dockerfile)
|
||||
self.assertNotEqual(first, second)
|
||||
|
||||
def test_send_build_context_noop_without_copy(self):
|
||||
df = self.root / "None"
|
||||
df.write_text("FROM x\n")
|
||||
with self._patch_root(), \
|
||||
patch.object(image_builder.subprocess, "Popen") as popen:
|
||||
image_builder._send_build_context(Path("/k"), "10.0.0.1", df, "/tmp/c")
|
||||
popen.assert_not_called()
|
||||
|
||||
def test_send_build_context_streams_copied_files_to_vm(self):
|
||||
completed = subprocess.CompletedProcess([], 0, stdout=b"", stderr=b"")
|
||||
with self._patch_root(), \
|
||||
patch.object(image_builder.subprocess, "Popen") as popen, \
|
||||
patch.object(image_builder.subprocess, "run",
|
||||
return_value=completed) as run:
|
||||
popen.return_value.stdout = None
|
||||
popen.return_value.returncode = 0
|
||||
popen.return_value.wait.return_value = 0
|
||||
image_builder._send_build_context(
|
||||
Path("/k"), "10.0.0.1", self.dockerfile, "/tmp/c")
|
||||
tar_argv = popen.call_args.args[0]
|
||||
self.assertEqual(tar_argv[:3], ["tar", "-C", str(self.root)])
|
||||
self.assertIn(
|
||||
"bot_bottle/contrib/codex/codex-package_SHA256SUMS", tar_argv)
|
||||
self.assertIn("tar -C /tmp/c/ctx -xf -", run.call_args.args[0][-1])
|
||||
|
||||
|
||||
class TestSmokeTest(unittest.TestCase):
|
||||
def test_buildah_receives_centralized_image_build_args(self):
|
||||
with patch.object(
|
||||
|
||||
Reference in New Issue
Block a user