-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix multi arch build #33008
fix multi arch build #33008
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
Current dependencies on/for this PR:
This stack of pull requests is managed by Graphite. |
8abb145
to
81a0c2c
Compare
30fcab8
to
c8bf267
Compare
1839086
to
b81816b
Compare
f22c122
to
ed2441b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good deep diving! Interesting findings! Great Code!
Nothing blocking
return StepResult(self, StepStatus.FAILURE, stderr=f"Something went wrong while interacting with the local docker client: {e}") | ||
try: | ||
client = docker.from_env() | ||
image_tag = f"{self.image_tag}-{platform.replace('/', '-')}" if multi_platforms else self.image_tag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💅 Can be useful to have title transforms named
image_tag = f"{self.image_tag}-{platform.replace('/', '-')}" if multi_platforms else self.image_tag | |
image_tag = _generate_dev_tag(self.image_tag, platform, multi_platforms=multi_platforms) |
That way you can contain the why
def _generate_dev_tag(image_tag, platform, multi_platforms):
"""
Explain that multi archecture dev builds are not possible
"""
return f"{self.image_tag}-{platform.replace('/', '-')}" if multi_platforms else self.image_tag
c8bf267
to
161d3e9
Compare
c432d95
to
91139fe
Compare
91139fe
to
784475a
Compare
91bbdbf
into
bnchrch/ci/revert-revert-new-arch-flags
What
#32816 changed the way we load dagger-built container to local docker images.
We used
docker import
instead ofdocker load
.When using
docker import
we import a container filesystem as an image. Container filesystem can't be run as connector: they don't have entry points, env var, etc. set.This is why we faced errors like:
Using
docker load
works better, but did not work for "multi arch" containers.After deeply investigating I realize there's no such thing as a local multi arch docker image. Multi-arch images are only a docker registry thing: a docker manifest points to arch-specific images/layers and on
pull
the docker client pulls the correct image for the current architecture after reading the manifest.In other words: there's no way to locally build a multi-arch image, either with docker or dagger.
But it's possible to have a local image built for a different architecture.
Now that I better understand the multi arch concept I change the
--architecture
option onbuild
.If you pass multiple
--architecture
you'll get multiple local images with architecture tag suffix:airbyte/source-faker:dev-linux-arm64
airbyte/source-faker:dev-linux-amd64
If you pass a single
--architecture
you'll get:airbyte/source-faker:dev
built for the architecture option valueIf you pass no
--architecture
option you'll get:airbyte-/source-faker:dev
built for your current architecture.How
docker load
instead odocker import
spec
on the loaded docker image.