Let callers choose the image platform in ContainerManager.create - #842
Open
rcfa wants to merge 1 commit into
Open
Let callers choose the image platform in ContainerManager.create#842rcfa wants to merge 1 commit into
rcfa wants to merge 1 commit into
Conversation
`create` resolves an image against `.current` in two places — the image config
and the rootfs unpack:
let imageConfig = try await image.config(for: .current).config // :295
return try await unpacker.unpack(image, for: .current, at: destination) // :346
So a container can only ever be created from an image variant matching the host
architecture. On an arm64 host, creating a container from an amd64 image fails
with `platform linux/arm64` — naming the platform that was requested rather than
the one the image actually has, which reads as a corrupt or mistagged image.
This is reachable even with Rosetta enabled on the `ContainerManager`: the
failure happens at config resolution, before the runtime would apply any
translation, so `rosetta: true` does not help and gives no hint as to why.
Both underlying APIs already take a platform (`Image.config(for:)` and
`EXT4Unpacker.unpack(_:for:at:progress:)`); only the `create` path pins it. This
adds `platform: Platform = .current` to the three `create` overloads and the
private `unpack`, passing it through. The default preserves existing behaviour,
so the change is additive and no caller needs updating.
Motivation: running SWE-bench's task images on Apple silicon. That corpus is
mixed — of the 500 SWE-bench Verified instances, 281 publish arm64 images and
219 publish amd64 only — so a host-pinned platform makes 44% of the benchmark
unrunnable, and evaluating only the remainder would report a score over a
non-random subset.
Verified on an arm64 host with both variants, same command shape:
* arm64 image — conda env activates, `import astropy` succeeds
* amd64 image — `uname -m` reports x86_64 under Rosetta, `import _pytest`
succeeds
The amd64 case is the one that could not be expressed before.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ContainerManager.createresolves images against.current, so a container can only be createdfrom an image variant matching the host architecture. This adds an optional
platformparameter(defaulting to
.current) so callers can select a different variant.The problem
Two sites on the
createpath pin the platform:Creating a container from an amd64 image on an arm64 host fails with:
Two things make this hard to diagnose from the outside:
like the image is mistagged or corrupt.
rosetta: trueon theContainerManageras well — the failure is at configresolution, before the runtime reaches the point where translation applies. So the flag that
exists to make this case work appears to do nothing, with no indication why.
The change
Image.config(for:)andEXT4Unpacker.unpack(_:for:at:progress:)already accept a platform; onlycreatepins it. This threadsplatform: Platform = .currentthrough the threecreateoverloadsand the private
unpack.The default preserves current behaviour exactly — the change is additive, and no existing caller
needs to change.
Why it matters in practice
We run SWE-bench's per-instance task images on Apple silicon. That corpus is mixed: of the 500
SWE-bench Verified instances, 281 publish arm64 images and 219 publish amd64 only. A host-pinned
platform therefore makes 44% of the benchmark unrunnable — and evaluating only the arm64 remainder
would produce a score over a non-random subset (architecture correlates with repository and era),
which would not be a valid benchmark result.
Verification
Tested on an arm64 host against both variants, running the same command shape in each:
astropyinstance (arm64)import astropy→5.1.dev623+gd16bfe05a7pytestinstance (amd64, Rosetta)uname -m→x86_64;import _pytest→3.9.20The second row is the case that could not be expressed before this change.