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.

Sign up for GitHub

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

Jump to bottom

Docker images with arm64 support #86

Merged
merged 10 commits into from
Jul 22, 2022
Merged

Docker images with arm64 support #86

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a42eac1
support building arm64 images
mcbex May 2, 2022
c56a12d
pass in docker build args
mcbex May 2, 2022
90cf858
add just target for cleaning up docker images
mcbex May 2, 2022
b924c5d
add documentation
mcbex May 2, 2022
a6b4203
fix typo
mcbex May 2, 2022
344dd4d
Merge branch 'main' into docker-platform-images
mcbex May 3, 2022
445c7ab
update dev container and support ARM
mcbex May 3, 2022
b4982d7
use TARGETARCH env var
mcbex May 11, 2022
ac6f856
address PR feedback
mcbex Jul 21, 2022
a397ce7
remove debug comment
mcbex Jul 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/actions/test/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ RUN groupadd -g $JENKINS_GID jenkins && \
echo "Defaults secure_path='/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin:/usr/local/pyenv/bin'" >> /etc/sudoers

# AWS CLI
RUN curl "https://awscli.amazonaws.com/awscli-exe-linux-x86_64.zip" -o "awscliv2.zip" && \
RUN curl "https://awscli.amazonaws.com/awscli-exe-linux-$(uname -m).zip" -o "awscliv2.zip" && \
unzip -q awscliv2.zip && \
./aws/install && \
rm awscliv2.zip
Expand Down
52 changes: 50 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ We use `just` to run project specific commands. See the
examples.

You will also need [Docker](https://docs.docker.com/get-docker/) if you wish to
run the integration tests.
run the integration and end-to-end tests.

## Testing

Expand Down Expand Up @@ -92,15 +92,63 @@ MODULE=pkg/rsnotify just test -v github.com/rstudio/platform-lib/pkg/rsnotify/lo

# Run the PgxNotifySuite suite tests with docker-compose
MODULE=pkg/rsnotify just test-integration -v github.com/rstudio/platform-lib/pkg/rsnotify/pgxlistener -check.f=PgxNotifySuite
```

# Run the end-to-end tests
### Testing with Docker

End-to-end and integration tests are run with Docker.

By default, Docker will use your host machine to infer which [platform to use
when building images](https://docs.docker.com/engine/reference/builder/#from).
You can specify another architecture with the `--platform` flag. Supported
options are `linux/amd64` and `linux/arm64`.

If your machine is running on the same platform as the image you plan to use for
testing, you can simply build the code natively. If not you will need to build a
separate Docker image to cross-compile your code.

>Note: On Apple silicon, Docker will build linux/arm64 images by default.
> However, you will need to cross-compile your code for the end-to-end tests
> because binaries built on Darwin are not compatible with the Linux environment
> in the Docker images and vice-versa.

Examples:

```bash
# Build the docker image used for end-to-end testing
just build-e2e-env

# Build the end-to-end testing image for ARM
just build-e2e-env --platform=linux/arm64

# Build the build-env image to cross-compile for ARM
just build-build-env --platform=linux/arm64

# Build the code natively
just build

# Build the code in Docker
just build-docker

# Run the end-to-end tests
just test-e2e

# Open an interactive container for end-to-end testing
just start-e2e-env
just test
exit

# Run the integration tests (uses the build-env image)
just test-integration
```

When re-building images for a different platform, Docker may ignore the platform
flag and use cached layers from the previous build. To resolve this issue,
remove the images and clear the build cache. You can do this manually for
a single image or use the `just` target to clean up both images at once:

```bash
just clean-docker
```

## Licenses
Expand Down
9 changes: 6 additions & 3 deletions docker/bionic/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ ENV DEBIAN_FRONTEND=noninteractive
RUN pip3 install --upgrade pip

# Install AWS CLI v2
RUN curl "https://awscli.amazonaws.com/awscli-exe-linux-x86_64.zip" -o "awscliv2.zip" && \
RUN curl "https://awscli.amazonaws.com/awscli-exe-linux-$(uname -m).zip" -o "awscliv2.zip" && \
unzip -q awscliv2.zip && \
./aws/install && \
rm -rf awscliv2.zip aws
Expand All @@ -51,8 +51,11 @@ RUN chmod +x /usr/local/bin/docker-compose

# Install Go. Keep in sync with other Dockerfiles.
ENV GOLANG_VERSION 1.18
ENV GOLANG_DOWNLOAD_SHA256 e85278e98f57cdb150fe8409e6e5df5343ecb13cebf03a5d5ff12bd55a80264f
RUN curl -fsSL "https://dl.google.com/go/go${GOLANG_VERSION}.linux-amd64.tar.gz" -o golang.tar.gz \
RUN ARCH=$(if [ $(uname -m) = "aarch64" ]; then echo "arm64"; else echo "amd64"; fi) \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here, and above, I wonder if we should consider simply using an ARG ARCH=amd64 and optionally passing the architecture to the build. For example, see the Launcher dockerfile: https://github.com/rstudio/launcher/blob/main/jenkins/docker/Dockerfile.bionic#L1.

Using an ARG would keep the Dockerfile a bit cleaner and let us handle the shell logic elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the issue would be that when we install awscli and Go the URLs contain the architecture but it's not consistent: aarch64 and arm64 vs x86_64 and amd64. So passing in a single variable would not remove the need to resolve those URLs dynamically.... unless you're suggesting the part where we build the URLs should move out of the Dockerfile?

A default architecture hard-coded in the image would also mean that anyone not on an amd64 machine has to remember to pass the parameter every time. That really only affects me at the moment but it feels kind of clunky to require from users.

Maybe another solution is to have two Dockerfiles and separate build targets for each architecture?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I was thinking more along the lines of delegating the responsibility of determining architecture to the caller. In other words, instead of determining architecture in the docker file, you'd determine it in the justfile or in a script called by the justfile, and then you'd pass the ARCH arg to the docker build command.

Copy link
Contributor

Choose a reason for hiding this comment

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

I comment this above before I saw this comment. Docker BuildKit will set TARGETARCH to the architecture you're building for, so you won't have to pass any extra parameters. The info is pretty far down the page:

https://www.docker.com/blog/faster-multi-platform-builds-dockerfile-cross-compilation-guide/

BUILDPLATFORM — matches the current machine. (e.g. linux/amd64)

BUILDOS — os component of BUILDPLATFORM, e.g. linux

BUILDARCH — e.g. amd64, arm64, riscv64

BUILDVARIANT — used to set ARM variant, e.g. v7

TARGETPLATFORM — The value set with --platform flag on build

TARGETOS - OS component from --platform, e.g. linux

TARGETARCH - Architecture from --platform, e.g. arm64

TARGETVARIANT

Whoever is building this Dockerfile would then use the BuildKit --platform argument which would take care of all of the architecture-related stuff, e.g. setting the environment variable and choosing the right multi-platform base image.

You would want to remove the --platform option in your Dockerfile and instead pass that in the command line, e.g. docker build --platform linux/amd64. When a platform argument isn't specified Docker will use the host architecture.

This is a really nice approach because consumers can use the same Docker image without worrying about architecture, and you only have to worry about one Dockerfile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. Thanks for the feedback @jonyoder @shepherdjerred

I briefly read some of the documentation on BuildKit features but I wasn't sure if using BuildKit was considered portable or not (since it looked pretty new). So good to know others are also looking at this and thinking about it!

Copy link
Contributor

Choose a reason for hiding this comment

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

BuildKit is now about 3 years old: https://docs.docker.com/engine/release-notes/18.09/

As far as I know, it's the best way to make cross-platform images. You are right that it's not completely mature, this issue shows the gaps: moby/moby#40379

I believe that BuildKit is the default builder now, so that at least shows that Docker considers it to be the way forward: docker/cli#3314

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TIL!

&& GOLANG_DOWNLOAD_SHA256=$(if [ $(uname -m) = "aarch64" ]; \
then echo "7ac7b396a691e588c5fb57687759e6c4db84a2a3bbebb0765f4b38e5b1c5b00e"; \
else echo "e85278e98f57cdb150fe8409e6e5df5343ecb13cebf03a5d5ff12bd55a80264f"; fi) \
&& curl -fsSL "https://dl.google.com/go/go${GOLANG_VERSION}.linux-${ARCH}.tar.gz" -o golang.tar.gz \
&& echo "$GOLANG_DOWNLOAD_SHA256 golang.tar.gz" | sha256sum -c - \
&& tar -C /usr/local -xzf golang.tar.gz \
&& rm golang.tar.gz
Expand Down
15 changes: 11 additions & 4 deletions justfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,20 @@ clean:
rm -rf data/
rm -rf .chart/

# Remove docker images and clear build cache (useful to run before building cross platform images)
clean-docker:
docker image rm rstudio/platform-lib:lib-build rstudio/platform-lib:lib-e2e
docker builder prune

# Builds the docker image used for building Go code
build-build-env:
docker build -t rstudio/platform-lib:lib-build -f docker/bionic/Dockerfile docker/bionic
# * args - Optional additional docker build args
build-build-env *args:
docker build {{args}} -t rstudio/platform-lib:lib-build -f docker/bionic/Dockerfile docker/bionic

# Builds the docker image for e2e testing
build-e2e-env:
docker build --network host -t rstudio/platform-lib:lib-e2e -f .github/actions/test/Dockerfile .github/actions/test
# * args - Optional additional docker build args
build-e2e-env *args:
docker build {{args}} --network host -t rstudio/platform-lib:lib-e2e -f .github/actions/test/Dockerfile .github/actions/test

# Creates a container for e2e testing
# * name - The container name
Expand Down