-
Notifications
You must be signed in to change notification settings - Fork 351
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
Add instructions to build multi-arch container image #2019
Conversation
…tools Signed-off-by: Martin Linkhorst <martin.linkhorst@zalando.de>
87d8fa8
to
762db0e
Compare
@@ -38,7 +38,7 @@ endif | |||
|
|||
build: $(SOURCES) lib skipper eskip webhook routesrv | |||
|
|||
build.linux.armv8: | |||
build.linux.arm64: |
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.
This change just aligns the names better with the rest of the internet:
When using arm
(32-bit) then you can chose whether it's version 5, 6 or 7 (GOARM
). Hence the naming is arm/v7
, arm/v6
etc. When using a 64-bit arm
there's no support for GOARM
(see goreleaser/goreleaser#36 (comment)), hence it's just arm64
(no variant).
@@ -47,7 +47,10 @@ build.linux.armv7: | |||
build.linux: | |||
GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go build -o bin/skipper -ldflags "-X main.version=$(VERSION) -X main.commit=$(COMMIT_HASH)" ./cmd/skipper | |||
|
|||
build.osx: | |||
build.darwin.arm64: |
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.
renamed osx
to darwin
to match GOOS
and docker's --platform
variable.
fi | ||
export IMAGE ARM_IMAGE ARM64_IMAGE | ||
export IMAGE ARM_IMAGE ARM64_IMAGE MULTIARCH_IMAGE |
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.
I think we can collapse this into a single image at some point. But for now I would just do it like this.
delivery.yaml
Outdated
|
||
cd packaging | ||
make docker.build.amd64 && git status && git diff && make docker.push.amd64 | ||
make docker.push.multiarch |
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.
This builds and pushes a test image.
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.
The use of &&
seems arbitrary - should we maybe &&
all of the commands or have each on the single line for clarity?
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.
I guess it means:
- do not fail on
cd packaging
because directory exists - each image push should be tested separately
Maybe it makes more sense to add && \
on the first 2 such that we have the same behavior as before and don't try to push multiarch if we got a compile error before.
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.
I think besides this here everything looks fine
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.
I separated the cd packaging
from this particular command because all subsequent commands expect to be run within that folder. That was hard to spot when it's lumped together with the first build.
I'm going with && \
all commands then because that's what you both agree on. But I wonder: putting each command on its own line should have the same affect since the pipeline will stop when one command fails.
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.
Ok, I just added && make docker.push.multiarch
. Let me know if that's not fine for you.
if [[ $CDP_TARGET_BRANCH == master && ! $CDP_PULL_REQUEST_NUMBER ]]; then | ||
echo "Created docker image registry.opensource.zalan.do/teapot/skipper:${RELEASE_VERSION}" | ||
cdp-promote-image "${MULTIARCH_IMAGE}" |
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.
This promotes the test image to the production registry. Note, this would be a no-op if we're on a branch but keeping it inside the if
makes running it locally easier (since it's not executed).
@@ -1,8 +1,14 @@ | |||
FROM registry.opensource.zalan.do/library/alpine-3.13:latest | |||
ARG BASE_IMAGE=registry.opensource.zalan.do/library/alpine-3.13:latest |
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.
Let's keep the public single-arch approved base image as default for now since this project is open source.
ADD ${BUILD_FOLDER}/${TARGETPLATFORM}/skipper \ | ||
${BUILD_FOLDER}/${TARGETPLATFORM}/eskip \ | ||
${BUILD_FOLDER}/${TARGETPLATFORM}/webhook \ | ||
${BUILD_FOLDER}/${TARGETPLATFORM}/routesrv /usr/bin/ |
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.
This is not perfectly backwards compatible because of BUILD_FOLDER=build
instead of BUILD_FOLDER=.
. However, the corresponding task in the Makefile provides that.
Changing the default to .
would require the majority of docker build
invocations to overwrite it. Having the default like this only requires one task to overwrite it.
@@ -1,10 +1,10 @@ | |||
FROM --platform=linux/amd64 alpine:3.13 AS build | |||
FROM --platform=linux/arm64 alpine:3.13 |
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.
--platform
is not strictly required but it ensures that we use the correct architecture for the added arm
binaries below.
@@ -1,10 +1,10 @@ | |||
FROM --platform=linux/amd64 alpine:3.13 AS build | |||
FROM --platform=linux/arm/v7 alpine:3.13 |
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.
--platform
is not strictly required but it ensures that we use the correct architecture for the added arm
binaries below.
@@ -52,6 +56,17 @@ docker.push.arm64: docker.build.arm64 | |||
docker.push.armv7: docker.build.armv7 | |||
docker push $(ARM_IMAGE) | |||
|
|||
docker.push.multiarch: clean build.linux docker.build.enable | |||
# Install qemu interpreter for arm64 (https://docs.docker.com/buildx/working-with-buildx/#build-multi-platform-images) | |||
docker run --rm --privileged container-registry.zalando.net/teapot/tonistiigi-binfmt:qemu-v6.2.0-main-2 --install arm64 |
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.
Please see https://github.bus.zalan.do/automata/issues/issues/4491 for why this is required and for any progress which could make this unnecessary.
docker run --rm --privileged container-registry.zalando.net/teapot/tonistiigi-binfmt:qemu-v6.2.0-main-2 --install arm64 | ||
|
||
# create a Buildkit builder with CDP specific configuration (https://cloud.docs.zalando.net/howtos/cdp-multiarch/) | ||
docker buildx create --config /etc/cdp-buildkitd.toml --driver-opt network=host --bootstrap --use |
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.
That's a buildx
builder that can build for both amd64
and arm64
and works with Zalando's internal compliance and security mechanisms on its proprietary CICD system.
|
||
# build multi-arch container image using a trusted multi-arch base image | ||
docker buildx build --rm -t $(MULTIARCH_IMAGE) --platform linux/amd64,linux/arm64 --push \ | ||
--build-arg BASE_IMAGE=container-registry.zalando.net/library/alpine-3.13:latest . |
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.
Here we use a proper multi-arch base image that's approved for production. Since it's currently not public it's not the default yet and only used from here.
Signed-off-by: Martin Linkhorst <martin.linkhorst@zalando.de>
Note, that I needed to use a third party image (mirrored to our registry) in order to initialize proper arm64 emulation (without it, it doesn't work). It's the very same approach that GitHub uses for GitHub Actions so it's not bad. However, there's active discussion on what causes the issues and what's the best approach going forward here: https://github.bus.zalan.do/automata/issues/issues/4491. For now, I hope we can also make progress with the current approach. Since it only affects the private multi-arch image we can easily track and control where it's used. |
rm -rf skipper eskip webhook routesrv build/ | ||
rm -rf $(BINARIES) build/ | ||
|
||
docker.build: clean $(BINARIES) |
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.
How does this relate to docker-build
(with a dash) target?
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.
It came from this version:
Lines 40 to 45 in 41b621e
docker.build.amd64: clean skipper eskip webhook routesrv | |
docker build -t $(IMAGE) . | |
docker.build.arm64: clean build.linux.arm64 docker.build.enable | |
docker buildx build -t $(ARM64_IMAGE) --platform linux/arm64 -f Dockerfile.arm64 . | |
docker.build.armv7: clean build.linux.armv7 docker.build.enable | |
docker buildx build -t $(ARM_IMAGE) --platform linux/arm/v7 -f Dockerfile.arm . |
With this: docker.build
is the "local" build (no architecture suffix, builds for local architecture) and uses the steps defined before.
Whereas docker.build.amd64
etc. cross-compile for the target architectures using the steps at the bottom and buildx. docker-build
and docker-push
are wrappers to to invoke all three builds for the linux OS.
We could rename docker.build
to docker.build.local
or similar to avoid confusion.
Signed-off-by: Martin Linkhorst <martin.linkhorst@zalando.de>
👍 |
1 similar comment
👍 |
This adds build steps to build a new multi-arch container image for
skipper
(and tools) forlinux/amd64
andlinux/arm64
.The heart of this change is the additional Makefile step called
docker.push.multiarch
which builds a multi-arch image usingbuildx
by providing multiple platform arguments.In order to do that I needed to change several parts of the Makefile but tried to keep everything backwards compatible. I would prefer to make the multi-arch build the single container image artifact and remove the separate Dockerfiles for
.arm
and.arm64
. However, currently we don't have a public registry that supports multi-arch images where we publish skipper to, so I leave them in place for now.The new multi-arch image is pushed to our private registry for now and since its base image only supports
amd64
andarm64
at the moment we don't add thearm/v7
variant because we don't need that at Zalando.The final multi-arch image will live at
container-registry.zalando.net/teapot/skipper
.There are some small changes though:
registry.opensource.zalan.do/teapot/skipper
) used to be build by the task that is now calleddocker.build
. This task builds binaries in the local architecture into the current working directory and usesdocker build
to stick them into the Dockerfile. This is basically the upper half of the Makefile. However, the lower half of the Makefile consists of several more tasks to build binaries for all of the supported architectures. The official docker image is now build from these Makefile steps as well as usingbuildx
. The resulting binaries and images are the same and one can think of removing the upper half of the Makefile now.build.linux.amd64
etc.). This made it very easy to build all the binaries for all architectures and then putting them all into a single multi-arch image. (Well, actually just amd64 and arm64 but you get the idea.)build/
folder has changed to be more consistent with the--platform
variable of docker and looks like this now:darwin/arm64
for the new M1/2 macbooks.If you want to check that all the binaries are build correctly, you can run
make build.package
.Test images from a previous successful run can be found here:
registry.opensource.zalan.do/teapot/skipper-test:pr-2019-7
(single-arch amd64)container-registry-test.zalando.net/teapot/skipper-test:pr-2019-7
(multi-arch)