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

Docker: Multi-arch & cross-compile build with docker buildx #14208

Merged
merged 2 commits into from
Jan 25, 2021

Conversation

jrajahalme
Copy link
Member

@jrajahalme jrajahalme commented Nov 30, 2020

Note that this PR adds functionality that currently already exists in images/
to the main Dockerfiles.

Add support for Docker BuildX multi-arch builds for arm64 and amd64 to
Cilium runtime, builder, and release images. Add Go cross-compilation
support to Cilium release images to make the arm64 builds faster on
amd64 hosts.

If both DOCKER_BUILDKIT=1 and DOCKER_BUILDX=1 are defined, the
makefiles create a cross-compilation builder instance ('cross') and
switch buildx to use it.

Images depend on each other, so the images normally should be built in
a specific order.

  1. Export variables

$ export DOCKER_BUILDKIT=1
$ export DOCKER_BUILDX=1
$ export DOCKER_REGISTRY=docker.io
$ export DOCKER_DEV_ACCOUNT=your-account

  1. Runtime

$ make docker-image-runtime
$ docker buildx imagetools inspect ${DOCKER_REGISTRY}/${DOCKER_DEV_ACCOUNT}/cilium-runtime:2020-11-30

Then update the runtime image references in other Dockerfiles

  1. Builder

$ make docker-image-builder
$ docker buildx imagetools inspect ${DOCKER_REGISTRY}/${DOCKER_DEV_ACCOUNT}/cilium-builder:2020-12-01

Update the main Cilium Dockerfile with the new builder reference

  1. Hubble

Hubble builds via buildx QEMU integration, unless you have an ARM machine added to your buildx builder.

  • Check out hubble branch pr/jrajahalme/docker-buildx-support

$ export IMAGE_REPOSITORY=${DOCKER_REGISTRY}/${DOCKER_DEV_ACCOUNT}
$ IMAGE_TAG=v0.7.1x CONTAINER_ENGINE="docker buildx" DOCKER_FLAGS="--push --platform=linux/arm64,linux/amd64" make image

$ docker buildx imagetools inspect ${IMAGE_REPOSITORY}/hubble:v0.7.1x

Update the main Cilium Dockerfile with the new Hubble reference

  1. Cilium release images

Cilium Dockerfiles are modified for cross-compilation based on the multi-arch support in the Cilium builder image:

$ make docker-images-all

@jrajahalme jrajahalme requested review from a team as code owners November 30, 2020 09:06
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 30, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.10.0 Nov 30, 2020
@jrajahalme jrajahalme added the release-note/misc This PR makes changes that have no direct user impact. label Nov 30, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 30, 2020
@jrajahalme
Copy link
Member Author

test-me-please

@jrajahalme jrajahalme marked this pull request as draft November 30, 2020 09:07
@jrajahalme jrajahalme force-pushed the pr/jrajahalme/cilium-multi-arch branch 2 times, most recently from 9ce16d4 to e358e9c Compare December 1, 2020 05:39
@jrajahalme
Copy link
Member Author

test-me-please

@jrajahalme jrajahalme marked this pull request as ready for review December 1, 2020 05:52
@jrajahalme jrajahalme requested a review from a team December 1, 2020 05:52
@jrajahalme jrajahalme requested a review from a team as a code owner December 1, 2020 05:52
@jrajahalme jrajahalme marked this pull request as draft December 1, 2020 06:28
@jrajahalme jrajahalme marked this pull request as ready for review December 1, 2020 06:28
@jrajahalme jrajahalme force-pushed the pr/jrajahalme/cilium-multi-arch branch from e358e9c to 83cd1de Compare December 1, 2020 19:32
@jrajahalme
Copy link
Member Author

Tests passed with test builds of runtime and builder, now removed the last commit to not impose un-official builds.

@jrajahalme
Copy link
Member Author

test-me-please

Copy link
Contributor

@kkourt kkourt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a tricky one to review...

I wish there was a cleaner way to implement all of this, but I'm not aware of any.
It might be a good idea to have some additional eyes on this, so I'll request additional reviews for my groups.

@kkourt kkourt requested a review from a team December 2, 2020 16:02
@jrajahalme jrajahalme force-pushed the pr/jrajahalme/cilium-multi-arch branch from a1e865c to 9c803c1 Compare December 11, 2020 16:11
@jrajahalme
Copy link
Member Author

test-me-please

@jrajahalme
Copy link
Member Author

retest-gke

@jrajahalme
Copy link
Member Author

retest-4.9

Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW I've been running DOCKER_BUILDKIT=1 for a while now and it seems to work well enough for me (though it can hide legitimate useful build output and I haven't figured out how to ensure it doesn't hide that on demand yet..)

I also tested this PR in its current state with buildkit enabled (but not buildx) for both of the make dev-docker-image and make microk8s targets, and there doesn't seem to be any kind of regression.

But then I also noticed that this PR changes the Dockerfile.builder but those changes are not being consumed by the main docker image build yet because there's no change to the main dockerfile to use a newer version of the builder image that would be generated by these code changes. So I think we need to generate images based on these latest changes and then add another change on top to integrate the use of those new builder images into the main dockerfiles.

Couple of other minor nits below.

Makefile.docker Outdated Show resolved Hide resolved
contrib/packaging/docker/Dockerfile.runtime Show resolved Hide resolved
@jrajahalme jrajahalme force-pushed the pr/jrajahalme/cilium-multi-arch branch from 9c803c1 to 6fb3b1f Compare December 30, 2020 22:19
@jrajahalme
Copy link
Member Author

Rebased and addressed review comments.

@jrajahalme
Copy link
Member Author

test-me-please

@jrajahalme
Copy link
Member Author

But then I also noticed that this PR changes the Dockerfile.builder but those changes are not being consumed by the main docker image build yet because there's no change to the main dockerfile to use a newer version of the builder image that would be generated by these code changes. So I think we need to generate images based on these latest changes and then add another change on top to integrate the use of those new builder images into the main dockerfiles.

These changes allow multi-arch builds for the builder and runtime. After this PR is merged we should make a decision to build the builder and runtime for both arm64 and amd64. I can't say if that can be easily automated yet, though, so it needs to be a separate discussion.

@jrajahalme jrajahalme force-pushed the pr/jrajahalme/cilium-multi-arch branch from 6fb3b1f to e08f9f7 Compare December 30, 2020 22:36
@jrajahalme
Copy link
Member Author

test-me-please

@jrajahalme
Copy link
Member Author

retest-gke

Apply go caching to RUN..go in addition to RUN..make Docker commands.

Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Add support for Docker BuildX multi-arch builds for arm64 and amd64 to
Cilium runtime, builder, and release images. Add Go cross-compilation
support to release images to make the arm64 builds faster on
amd64 hosts.

If both DOCKER_BUILDKIT=1 and DOCKER_BUILDX=1 are defined, the
makefiles create a cross-compilation builder instance ('cross') and
switch buildx to use it.

Images depend on each other, so the images normally should be built in
a specific order:

0. Export variables

$ export DOCKER_BUILDKIT=1
$ export DOCKER_BUILDX=1
$ export DOCKER_REGISTRY=docker.io
$ export DOCKER_DEV_ACCOUNT=your-account

1. Runtime

$ make docker-image-runtime
$ docker buildx imagetools inspect ${DOCKER_REGISTRY}/${DOCKER_DEV_ACCOUNT}/cilium-runtime:2021-01-25

Update the runtime image reference in Dockerfile and Dockerfile.builder.

2. Builder

$ make docker-image-builder
$ docker buildx imagetools inspect ${DOCKER_REGISTRY}/${DOCKER_DEV_ACCOUNT}/cilium-builder:2021-01-25

Update the main Cilium Dockerfile with the new builder reference.

3. Hubble

Hubble builds via buildx QEMU integration, unless you have an ARM machine added to your buildx builder.

- Check out hubble master branch (github.com/cilium/hubble)

$ export IMAGE_REPOSITORY=${DOCKER_REGISTRY}/${DOCKER_DEV_ACCOUNT}/hubble
$ CONTAINER_ENGINE="docker buildx" DOCKER_FLAGS="--push --platform=linux/arm64,linux/amd64" make image

$ docker buildx imagetools inspect ${IMAGE_REPOSITORY}:latest

Update the main Cilium Dockerfile with the new Hubble reference.

4. Cilium release images

Cilium Dockerfiles are modified for cross-compilation based on the multi-arch support in the Cilium builder image:

$ make docker-images-all

Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
@jrajahalme jrajahalme force-pushed the pr/jrajahalme/cilium-multi-arch branch from e08f9f7 to 48440cb Compare January 25, 2021 05:50
@jrajahalme
Copy link
Member Author

test-me-please

@jrajahalme
Copy link
Member Author

retest-gke

2 similar comments
@jrajahalme
Copy link
Member Author

retest-gke

@jrajahalme
Copy link
Member Author

retest-gke

@jrajahalme jrajahalme merged commit 7d67a82 into master Jan 25, 2021
@jrajahalme jrajahalme deleted the pr/jrajahalme/cilium-multi-arch branch January 25, 2021 08:58
jrajahalme added a commit to jrajahalme/cilium that referenced this pull request Jan 27, 2021
Docker buildkit sometimes fails to actually pull images referenced on
FROM lines. We mitigate for this by pre-pulling all images before
docker build. A recent change to allow for multi-arch image builds
caused cilium-builder image to not be pre-pulled due to additional
filtering of Dockerfiles. Fix this by scanning the FROM lines from the
original dockerfiles instead of the generated ones.

Fixes: cilium#14208
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
aanm pushed a commit that referenced this pull request Jan 28, 2021
Docker buildkit sometimes fails to actually pull images referenced on
FROM lines. We mitigate for this by pre-pulling all images before
docker build. A recent change to allow for multi-arch image builds
caused cilium-builder image to not be pre-pulled due to additional
filtering of Dockerfiles. Fix this by scanning the FROM lines from the
original dockerfiles instead of the generated ones.

Fixes: #14208
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
lyveng pushed a commit to lyveng/cilium that referenced this pull request Mar 4, 2021
Docker buildkit sometimes fails to actually pull images referenced on
FROM lines. We mitigate for this by pre-pulling all images before
docker build. A recent change to allow for multi-arch image builds
caused cilium-builder image to not be pre-pulled due to additional
filtering of Dockerfiles. Fix this by scanning the FROM lines from the
original dockerfiles instead of the generated ones.

Fixes: cilium#14208
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

7 participants