Skip to content
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 multiarch building support for arm64 #5039

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
10 participants
@lubinsz
Copy link

commented Dec 10, 2018

Signed-off-by: Bin Lu bin.lu@arm.com

@helm-bot helm-bot added the size/M label Dec 10, 2018

@lubinsz

This comment has been minimized.

Copy link
Author

commented Dec 10, 2018

Currently, there is no multiarch building step for Arm64 platform.
So I added this step in the Makefile.
With this patch, we can use 'make docker-build-all' on x86 platform to cross-build docker images for different platforms(amd64, arm64).
In order to build container images for mult-arch platforms we rely on cross-compilers and QEMU.
We can use binfmt_misc to support this feature.
For more reference, please see following as reference:
https://lobradov.github.io/Building-docker-multiarch-images/
https://hub.docker.com/r/multiarch/qemu-user-static/

@lubinsz lubinsz force-pushed the lubinsz:master branch from 4f1b682 to 879842c Dec 10, 2018

@helm-bot helm-bot added size/M and removed size/M labels Dec 10, 2018

@lubinsz

This comment has been minimized.

Copy link
Author

commented Dec 10, 2018

@alicefr
Any comments on this?
Thanks.

@lubinsz lubinsz force-pushed the lubinsz:master branch from 879842c to 1418969 Dec 10, 2018

@helm-bot helm-bot added size/M and removed size/M labels Dec 10, 2018

@alicefr

This comment has been minimized.

Copy link
Contributor

commented Dec 10, 2018

@lubinsz thanks to open the PR. I have a couple of suggestions. Instead of using a Dockerfile per architecture, you can use --platform arch with docker build. However, it is still experimental and you need to set it true in the /etc/docker/daemon.json. If you don't want to copy the qemu-arch-static inside the container, you need a 4.8 or greater kernel and the flag F. See https://www.kernel.org/doc/html/latest/admin-guide/binfmt-misc.html.
If you register the binfmt_misc correctly, you can build a similar dockerfile:

FROM alpine

RUN apk add --no-cache curl

you can now build the arm image on x86 in this way:

docker build --platform=linux/arm64 -t image-example:arm64 .

In this way, you can generalize and use the same dockerfile for multiple architectures. It is enough to register the binfmt_misc and set --platform
Please, be aware that multiarch/qemu-user-static:register doesn't have the F flag. See https://github.com/multiarch/qemu-user-static/blob/22b0013668d2aed4a2cfd21650e85c664b1f21c6/register/qemu-binfmt-conf.sh#L351

@lubinsz lubinsz force-pushed the lubinsz:master branch from 1418969 to 446405a Dec 11, 2018

@helm-bot helm-bot added size/S and removed size/M labels Dec 11, 2018

@lubinsz lubinsz force-pushed the lubinsz:master branch from 446405a to 16bef9b Dec 11, 2018

@helm-bot helm-bot added size/S and removed size/S labels Dec 11, 2018

@lubinsz

This comment has been minimized.

Copy link
Author

commented Dec 11, 2018

@alicefr
Hi,
I have updated it according to your suggestions.
Please check it.

@alicefr

This comment has been minimized.

Copy link
Contributor

commented Dec 11, 2018

hi, i guess you're missing the qemu-static inside the container. As you're registering with multiarch/qemu-user-static:register, you're not using the F flag and you need qemu-static inside the container. In any case, you need to install the qemu-arch-static

@alicefr

This comment has been minimized.

Copy link
Contributor

commented Dec 11, 2018

Instead of using the docker image you could directly use the script qemu-binfmt-conf.sh from qemu. It has the persistent flag that adds the F flag.
For example:

 ./qemu-binfmt-conf.sh --qemu-path /usr/bin --qemu-suffix "-static" --persistent yes

@lubinsz lubinsz force-pushed the lubinsz:master branch from 16bef9b to 5bbd42c Dec 12, 2018

@helm-bot helm-bot added size/S and removed size/S labels Dec 12, 2018

@lubinszARM

This comment has been minimized.

Copy link

commented Dec 12, 2018

@alicefr
Done. Any comments?
Thanks.

@alicefr

This comment has been minimized.

Copy link
Contributor

commented Dec 12, 2018

@lubinszARM it's ok for me. Then you decided to not use the F flag. I find that it's the most elegant solution because you don't have the qemu-static binary inside the docker image. Anyway, that solution has of course some restrictions. Then, if you decide to copy the qemu-static you could remove it at the end of the build time. In this way, you don't have the qemu-static inside the final image. I'd also suggest that you use the --squash flag (also experimental). Hence, you can squash all the generated layers into a single one and you shouldn't be able to see the qemu-*-static in the final image anymore. You generate a layer when you copy the qemu binary inside and when you remove it. Does it make sense? See the calico example https://github.com/projectcalico/node/blob/master/Dockerfile.arm64

@lubinsz lubinsz force-pushed the lubinsz:master branch from 5bbd42c to ca7a6d1 Dec 12, 2018

@helm-bot helm-bot added size/S and removed size/S labels Dec 12, 2018

@lubinszARM

This comment has been minimized.

Copy link

commented Dec 12, 2018

@alicefr
Thank you so much for your suggestion. :)
I have updated it according to your suggestions.

@alicefr

This comment has been minimized.

Copy link
Contributor

commented Dec 12, 2018

Thanks to address the PR. It solves the problem also for other archs

@lubinszARM

This comment has been minimized.

Copy link

commented Dec 12, 2018

@alicefr
Can you merge it?
Thanks.

@bacongobbler
Copy link
Member

left a comment

If I understand the PR, In order to build tiller images for amd64 as well as arm64, our CI infrastructure has to enable Docker's experimental features. I don't believe that's enabled by default on CircleCI. Can you check and see how we can enable experimental Docker features for Circle?

- setup_remote_docker:
version: 17.09.0-ce

Additionally, I'm not too sure how comfortable I feel about relying on unstable features for releasing production software. In the past, we've avoided relying on unstable features for our release pipeline. Do you know how long it may take until this becomes a stable feature of Docker? We may want to hold out and wait until it does to ensure that existing users of Helm aren't relying on experimental features.

Makefile Outdated
.PHONY: check-docker-experimental
check-docker-experimental:
@if [ $$(docker version -f '{{json .Server.Experimental}}') != true ]; then \
echo "Docker experimental needs to be enable"; \

This comment has been minimized.

Copy link
@bacongobbler

bacongobbler Jan 4, 2019

Member

typo: "enable" should be "enabled".

This comment has been minimized.

Copy link
@lubinsz

lubinsz Jan 28, 2019

Author

I have removed the target: check-docker-experimental.
Now, cross-building does not depend on any demo version of docker.

@alicefr

This comment has been minimized.

Copy link
Contributor

commented Jan 7, 2019

you could also set the parent image explicitly to the target architecture as you're already using a dockerfile per architecture. You need just an extra-line and remove the experimental check and --platform.
sed -i "s/alpine/$arch\/alpine/g" ./_dist/linux-$$arch/Dockerfile

FROM arm64/alpine:3.7

You also need to remove the --squash flag. In this case, there are additional layers for adding and removing the qemu-static binary.

@kfox1111

This comment has been minimized.

Copy link

commented Jan 7, 2019

rook just had that problem when they broke out rook-ceph. I think their solution was to parametrize the FROM.

@stephenmoloney

This comment has been minimized.

Copy link

commented Jan 7, 2019

Just wondering, will this PR add an image for armv7hf ?

@lubinsz

This comment has been minimized.

Copy link
Author

commented Jan 8, 2019

Just wondering, will this PR add an image for armv7hf ?

Hi @stephenmoloney ,
No. It seems Alpine only available for arm32v6.
Maybe I can enable it in other PR.

@lubinsz

This comment has been minimized.

Copy link
Author

commented Jan 8, 2019

parametrize

Hi @kfox1111 ,

If you don't want to execute any binary files in Dockerfile, you can use the method to parametrize the FROM.
If not, parameterization is not enough.
In fact, the method of parameterization is not enough for rook-nfs.
So I deliver this PR for rook-nfs:
rook/rook#2277

@lubinsz lubinsz force-pushed the lubinsz:master branch from 4c3769c to f8aeec7 Jan 8, 2019

@helm-bot helm-bot added size/M and removed size/M labels Jan 8, 2019

@lubinsz lubinsz force-pushed the lubinsz:master branch from f8aeec7 to 6f2159c Jan 24, 2019

@helm-bot helm-bot added size/L and removed size/M labels Jan 24, 2019

@lubinsz

This comment has been minimized.

Copy link
Author

commented Jan 24, 2019

@bacongobbler @alicefr
I have reorganized the code according to the compilation method in Kubernetes.
Any comments?
Thanks.

FROM alpine:3.7
FROM BASEIMAGE

CROSS_BUILD_COPY qemu-QEMUARCH-static /usr/bin/

This comment has been minimized.

Copy link
@kfox1111

kfox1111 Jan 24, 2019

I'm confused here. This is the docker file for the released image isn't it? It should not contain an emulator, as the image should run natively on the target arch.

In general, I'm also a bit confused by including an emulator at all. I thought golang could cross compile all by itself. Greping through the kubernetes code for qemu, they do have qemu in there, but from a superficial skimming of the code, only looks to be used to test the binaries, not build/ship them? Maybe I'm totally wrong.

This comment has been minimized.

Copy link
@lubinsz

lubinsz Jan 25, 2019

Author

Hi @kfox1111
Thank you for your comments. :)

I have added a command to delete the emulator in the end of each Dockerfiles.

Regarding to your second question, you should notice the following line in each Dockerfile:
"RUN apk update && apk add ... "
It means, it needs to execute non-x86 instruction in docker multi-arch building step.
So the emulator in Dockerfile is mandatory.
Regarding to rook/ceph, we don't need to copy emulator in Dockerfile. Because we don't need to run any non-x86 instruction in docker multi-arch building step.
But for rook/nfs, the emulator in Dockerfile is mandatory. The reason for this problem is same as the problem in helm.
You can see that there are many commands in the Dockerfile of ‘rook/nfs' that need to use non-x86 instruction. Such as: 'dnf install', 'make' ...

You can test my patch in your x86 ubuntu environment.
The building will be failed if without the following line in Dockerfile:
"CROSS_BUILD_COPY qemu-QEMUARCH-static /usr/bin/"
The error should be like following:

Step 2/9 : RUN apk update && apk add ca-certificates socat && rm -rf /var/cache/apk/*
---> Running in 02f412954455
standard_init_linux.go:190: exec user process caused "no such file or directory"
The command '/bin/sh -c apk update && apk add ca-certificates socat && rm -rf /var/cache/apk/*' returned a non-zero code: 1
Makefile:120: recipe for target 'docker-build' failed

Introduction article: https://lobradov.github.io/Building-docker-multiarch-images/
Please see the section titled "Different Dockerfile per architecture" as reference.

In addition, I need to explain that these problems have nothing to do with golang cross compilation.
For helm, the problem is that it uses the following command in Dockerfile:
"RUN apk update && apk add ... "

For rook/nfs, the problem is that it uses the following command in Dockerfile:
"RUN dnf install ... make...."

If we only need to simply COPY the cross-compiled golang binary files in Dockerfile, we don't need to COPY the emulator in Dockerfile. Just like in rook/ceph.

This comment has been minimized.

Copy link
@kfox1111

kfox1111 Jan 25, 2019

Hmm... I see. thanks for the explanation.

removing the emulator in a subsequent layer will only hide it, not remove it. so it will still be shipped to the end devices anyway.

I was thinking the emulator was fairly sizeable, but it looks like its pretty small compared to tiller so it may not be too bad to ship to machines that never will use it.

This comment has been minimized.

Copy link
@alexellis

alexellis Feb 6, 2019

Contributor

This is a tricky problem @justincormack might have an idea other than the experimental version of Docker? In OpenFaaS we build for amd64, armhf, arm64 but with the two arm platforms built on a device and not via cloud CI. It's a huge pain so we may move to qemu like this.

This comment has been minimized.

Copy link
@justincormack

justincormack Feb 6, 2019

You can remove the emulator using a multi-stage build. Cross builds are kind of horrible compared to native, but better than no build...

@bacongobbler

This comment has been minimized.

Copy link
Member

commented Feb 2, 2019

removing from the 2.13 milestone given the current discussions/concerns

@alexellis

This comment has been minimized.

Copy link
Contributor

commented Feb 6, 2019

Hi @bacongobbler are there any open concerns with the PR at the moment?

@vielmetti

This comment has been minimized.

Copy link

commented Mar 5, 2019

@lubinsz it looks like the Makefile has a conflict at the moment, perhaps needs a rebase?

@bacongobbler

This comment has been minimized.

Copy link
Member

commented Mar 5, 2019

Hi @bacongobbler are there any open concerns with the PR at the moment?

Yes. Please see #5039 (review).

@lubinsz

This comment has been minimized.

Copy link
Author

commented Mar 5, 2019

Hi @bacongobbler are there any open concerns with the PR at the moment?

Yes. Please see #5039 (review).

@bacongobbler
I have reorganized the code according to the compilation method in Kubernetes.
Now, cross-building does not depends on any experimental features of docker.
Please check it.
The issue of #5039 has gone .

Add multiarch building support for arm64
Signed-off-by: Bin Lu <bin.lu@arm.com>

@lubinsz lubinsz force-pushed the lubinsz:master branch from 0b247dc to a184c1c Mar 11, 2019

@helm-bot helm-bot added size/L and removed size/L labels Mar 11, 2019

@lubinsz

This comment has been minimized.

Copy link
Author

commented Mar 11, 2019

@lubinsz it looks like the Makefile has a conflict at the moment, perhaps needs a rebase?

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.