Build Arm supporting docker image #4703

Lewuathe · 2020-08-05T21:49:26Z

Since Presto now supports Arm architecture officially, it would be great if we could also try the Arm platform with docker out-of-the-box. This change introduces a new docker image suffixed with -arm64v8 supporting Arm platform. I confirmed the image works well with the Graviton2 processor in AWS.

It's related to #2262.

Although I assumed build-remote.sh would distribute the image in the DockerHub, it might not be correct because I could not find the usage of the script in the repository. Does just building an image with built-remote.sh correctly push it to DockerHub?

docker/Dockerfile-arm64v8

docker/README.md

docker/build-local.sh

electrum · 2020-08-06T02:02:01Z

Thanks for contributing this. It's certainly useful for ARM servers, and will be needed with the upcoming ARM-based Macs. In my very brief research, it looks like we can build a single image with support for multiple architectures, rather than separately tagged images: https://medium.com/nttlabs/buildx-multiarch-2c6c2df00ca2

Do you have any experience with that? If it works, that seems like a better approach.

build-local.sh is for building from a server tarball that you built in your local source tree. build-remote.sh is for building from a version in a Maven repository (typically this is a release version, but could be a snapshot). The Presto maintainers publish the image to Docker hub after a Presto release with this: https://github.com/prestosql/release-scripts/blob/master/presto-docker.sh

Lewuathe · 2020-08-06T21:47:59Z

@electrum

it looks like we can build a single image with support for multiple architectures, rather than separately tagged images: https://medium.com/nttlabs/buildx-multiarch-2c6c2df00ca2

All I tried was to use the OpenJDK image supporting Armv8 as the base image. To use the different base images, we need to use the different Dockerfile which prevents us from building a single image supporting multiple architectures with a single command.

But I found the CentOS image supports multiple architectures. Instead of using OpenJDK image, we may be able to build a single image with CentOS base by manually installing Zulu supporting Arm. I'll try.

I think I can work on that after merging #4642.

electrum · 2020-08-13T22:40:38Z

@Lewuathe the switch to Zulu JDK is merged now

Lewuathe · 2020-08-17T07:48:32Z

Unfortunately, we could not build multi-arch images with one line command due to several reasons.

`--load` option is not supported

We could not use BuildKit to build multiple architectures with one command with --load option.

docker/buildx#59

According to the above issue, --load option does not allow us to build multi-arch images. Without --load option, the build image will not be visible in the list of images shown by docker images.

Zulu OpenJDK for ARM64 is not available

Additionally, Zulu OpenJDK for ARM64 is not distributed in RPM format. That indicates we cannot use the same Dockerfile for amd64 and arm64/v8. The PR has another Dockerfile for ARM64.

CentOS 7 image does not support arm64/v8

Looking at the list of available images, CentOS 7 for arm64/v8 is not distributed.

Therefore, we need to prepare the separated Dockerfile and build it with an additional command. I confirmed the image is working with Graviton 2 processor (m6g.medium)

electrum

Great progress!

It seems like we're still not building a multi architecture image, since the ARM image has a separate tag. Based on the buildx docs, it seems like we need to invoke buildx with multiple platforms at once:

docker buildx build --platform=linux/amd64,linux/arm64/v8

For this to work, I think we need to use a single Dockerfile -- I couldn't find a way to use a separate one per platform. It seems you can use TARGETPLATFORM to do conditional logic based on the target platform.

It's fine to use CentOS 8 for both architectures. We simply haven't had a need to upgrade yet, but it would be easier and better to upgrade so that both architectures are consistent.

.github/workflows/ci.yml

docker/Dockerfile-arm64v8

Lewuathe · 2020-09-10T07:25:36Z

docker/build-remote.sh


 rm -r ${WORK_DIR}

 # Source common testing functions
 . container-test.sh

 test_container "presto:${PRESTO_VERSION}"
+
+# Push multi-arch images supporting amd64 and arm64 architecture.
+DOCKER_CLI_EXPERIMENTAL=enabled docker buildx build ${WORK_DIR} --platform linux/amd64,linux/arm64/v8 -f Dockerfile --build-arg "PRESTO_VERSION=${PRESTO_VERSION}" -t "presto:${PRESTO_VERSION}" --push


Although --load is not supported for the multi-arch image, we can use --push to publish the images at once. Is it okay to push images here?

The published images will look like this.

Hi, @Lewuathe What about using aarch64 instead of arm64/v8? I found presto use aarch64 in source code here and looks like aarch64 is more popular.

As far as I know, there is no aarch64 parameter in the list supported by Docker builder. We need to specify arm64/v8 as a TARGETPLATFORM parameter. But arm64/v8 and aarch64 are basically the same thing, IIUC.

$ docker buildx ls NAME/NODE DRIVER/ENDPOINT STATUS PLATFORMS default docker default default running linux/amd64, linux/arm64, linux/ppc64le, linux/s390x, linux/386, linux/arm/v7, linux/arm/v6

We should make this a separate script like push-image.sh, so that we can build images separately from pushing them.

I'm confused what --push does. It says it pushes it to a repository, but which one? We don't specify prestosql anywhere as the Docker Hub repository. Is there a way to build the ARM64 image without pushing?

I think we should prepare the separate script push-image.sh so that we can push both images by --push option to prestosql orgniazation. But I do not have a way to check pushing to prestosql organization properly. How do you usually push Docker images to prestosql?

electrum · 2020-09-11T01:51:00Z

Why do we need --load? I don't understand what that does (reading the manual didn't help).

Lewuathe · 2020-09-11T05:02:59Z

@electrum --load and --push specifies the location where the resulting image appears. Without --load, the image won't be visible with docker images command, which means we cannot test the image with test_container script.

Lewuathe · 2020-11-04T04:42:00Z

@electrum Could you take a look into this when you get a chance?

electrum

Sorry for the delay in following up. It might help for us to chat on Slack about this.

docker/README.md

electrum · 2020-11-24T22:40:38Z

docker/build-remote.sh


 rm -r ${WORK_DIR}

 # Source common testing functions
 . container-test.sh

 test_container "presto:${PRESTO_VERSION}"
+
+# Push multi-arch images supporting amd64 and arm64 architecture.
+DOCKER_CLI_EXPERIMENTAL=enabled docker buildx build ${WORK_DIR} --platform linux/amd64,linux/arm64/v8 -f Dockerfile --build-arg "PRESTO_VERSION=${PRESTO_VERSION}" -t "presto:${PRESTO_VERSION}" --push


We should make this a separate script like push-image.sh, so that we can build images separately from pushing them.

electrum · 2020-11-24T22:48:55Z

docker/build-remote.sh


 rm -r ${WORK_DIR}

 # Source common testing functions
 . container-test.sh

 test_container "presto:${PRESTO_VERSION}"
+
+# Push multi-arch images supporting amd64 and arm64 architecture.
+DOCKER_CLI_EXPERIMENTAL=enabled docker buildx build ${WORK_DIR} --platform linux/amd64,linux/arm64/v8 -f Dockerfile --build-arg "PRESTO_VERSION=${PRESTO_VERSION}" -t "presto:${PRESTO_VERSION}" --push


I'm confused what --push does. It says it pushes it to a repository, but which one? We don't specify prestosql anywhere as the Docker Hub repository. Is there a way to build the ARM64 image without pushing?

Since Presto now supports Arm architecture officially, it would be great if we could also try Arm platform with docker out-of-the-box. This change introduces new docker image suffixed with `-arm64v8` supporting Arm platform.

Lewuathe · 2020-11-27T02:21:06Z

I updated the scripts accordingly and confirmed the image works properly in t4g instance type with Ubuntu Bionic.

GaruGaru · 2021-03-11T08:32:05Z

@electrum what are the needed steps to see this merged ?
This would be a nice feature since most cloud providers are providing arm64 vms with nice performance/price ratio, I guess that it would also enable apple M1 users to run trino locally using the official docker image

martin-g · 2021-03-11T08:35:16Z

There are some conflicting files. The first step is to resolve the conflicts.

svarosky90 · 2021-03-11T09:25:27Z

@Lewuathe Are you still working on this ?
Is there any possibility to work on this pull request? Maybe open a new one related to #4703 and try to speed up the process?

Lewuathe · 2021-03-12T12:32:56Z

@svarosky90 It may be better to open another pull request after I resolve the conflict. But I'm not sure what steps will be necessary to be merged because I do not get any feedback from the last update.

Anyway, I'll reopen the pull request.

Lewuathe · 2021-03-16T03:35:54Z

@electrum The new image seems to use the image zulu-openjdk-centos.
https://hub.docker.com/r/azul/zulu-openjdk-centos/tags?page=1&ordering=last_updated

But it does not support ARM architecture. Do you think it's okay to create another Dockerfile for ARM or replacing the zulu image with manual Zulu installation as we did before?

svarosky90 · 2021-06-05T20:16:06Z

Any update on this? It would be very useful running trino on arm based machines.

Lewuathe · 2021-06-06T22:52:43Z

@svarosky90 If we can create a separate image for Arm, it must be easy. But if we need to support multi-arch image, there are several things to be clarified (e.g. #4703 (comment)).

@electrum Is it okay to build another image from a separate Dockerfile?

electrum · 2021-06-24T17:23:48Z

@Lewuathe Apologies for not following up. I contacted Azul about supporting arm64 in their Docker images. Until that happens, I think we could go with the simple approach of using the CentOS image as the base and installing Zulu using the RPM. Then we should be able to use the normal Docker multi-arch build stuff. This prevents sharing the same base image across multiple Trino releases, but I think that's ok. Thoughts?

electrum · 2021-06-24T17:27:29Z

It looks like we need to use the tarball to install Zulu, since the RPM is only for amd64 :(

Lewuathe · 2021-06-24T23:50:31Z

@electrum Thank you for the feedback!

I contacted Azul about supporting arm64 in their Docker images.

Sounds nice. I'm looking forward to that.

I think we could go with the simple approach of using the CentOS image as the base and installing Zulu using the RPM. Then we should be able to use the normal Docker multi-arch build stuff.

It also looks good. That's pretty similar to the original approach we've tried previously. Thanks. I'll try to update accordingly.

electrum · 2021-06-25T00:30:43Z

I've been playing around with this, mostly to learn more about how this works. If you don't mind, I'll put up a new PR with some modifications to your original changes.

Lewuathe · 2021-06-25T02:36:58Z

If you don't mind, I'll put up a new PR with some modifications to your original changes.

That sounds nice. It's totally okay, I want to take a look at the PR then.

electrum · 2021-06-26T22:59:32Z

@Lewuathe I created a new PR with the changes: #8397

I now finally understand how this actually works and the problems you ran into. Apologies that it took so long. My conclusion is that Docker multi-architecture is a mess.

I ended up going back to your original approach of separate Dockerfiles, since that is simple, supports separate base images, and allows us to test the individual images before pushing them.

I'm using use OpenJDK from CentOS because Zulu crashes in Docker for Mac -- which I also reported to Azul.

It took me a long time to figure out that multi-arch manifests are a feature of the registry, at a higher level than images, and don't exist for local images. This is why --push is needed for docker buildx build, and also why docker manifest create requires that you first push the constituent images to the registry. None of the documentation or tutorials that I found explained this. I was also surprised that while you can push tags with docker push, there is no command to delete them.

electrum · 2021-06-27T19:54:59Z

Replaced by #8397

cla-bot bot added the cla-signed label Aug 5, 2020

electrum reviewed Aug 6, 2020

View reviewed changes

docker/Dockerfile-arm64v8 Outdated Show resolved Hide resolved

electrum reviewed Aug 6, 2020

View reviewed changes

docker/README.md Outdated Show resolved Hide resolved

electrum reviewed Aug 6, 2020

View reviewed changes

docker/build-local.sh Outdated Show resolved Hide resolved

Lewuathe force-pushed the distribute-arm-image branch 2 times, most recently from f2150fd to 0f34881 Compare August 17, 2020 07:32

Lewuathe requested review from electrum and ebyhr August 17, 2020 22:46

electrum reviewed Sep 8, 2020

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

docker/Dockerfile-arm64v8 Outdated Show resolved Hide resolved

Lewuathe force-pushed the distribute-arm-image branch from 8781444 to e46e69a Compare September 10, 2020 07:06

Lewuathe commented Sep 10, 2020

View reviewed changes

Lewuathe requested a review from electrum September 10, 2020 21:33

electrum reviewed Nov 24, 2020

View reviewed changes

Lewuathe force-pushed the distribute-arm-image branch 2 times, most recently from 371591e to de5f902 Compare November 27, 2020 02:17

Build Arm supporting docker image

647f295

Since Presto now supports Arm architecture officially, it would be great if we could also try Arm platform with docker out-of-the-box. This change introduces new docker image suffixed with `-arm64v8` supporting Arm platform.

Lewuathe force-pushed the distribute-arm-image branch from de5f902 to 647f295 Compare November 27, 2020 02:19

Lewuathe requested a review from electrum November 27, 2020 02:20

electrum closed this Jun 27, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build Arm supporting docker image #4703

Build Arm supporting docker image #4703

Lewuathe commented Aug 5, 2020

electrum commented Aug 6, 2020

Lewuathe commented Aug 6, 2020

electrum commented Aug 13, 2020

Lewuathe commented Aug 17, 2020 •

edited

electrum left a comment

Lewuathe Sep 10, 2020

Lewuathe Sep 10, 2020

ssquan Sep 15, 2020

Lewuathe Sep 15, 2020 •

edited

electrum Nov 24, 2020

electrum Nov 24, 2020

Lewuathe Nov 26, 2020

electrum commented Sep 11, 2020

Lewuathe commented Sep 11, 2020

Lewuathe commented Nov 4, 2020

electrum left a comment

electrum Nov 24, 2020

electrum Nov 24, 2020

Lewuathe commented Nov 27, 2020

GaruGaru commented Mar 11, 2021

martin-g commented Mar 11, 2021

svarosky90 commented Mar 11, 2021 •

edited

Lewuathe commented Mar 12, 2021

Lewuathe commented Mar 16, 2021

svarosky90 commented Jun 5, 2021

Lewuathe commented Jun 6, 2021

electrum commented Jun 24, 2021

electrum commented Jun 24, 2021

Lewuathe commented Jun 24, 2021

electrum commented Jun 25, 2021

Lewuathe commented Jun 25, 2021

electrum commented Jun 26, 2021

electrum commented Jun 27, 2021

Build Arm supporting docker image #4703

Build Arm supporting docker image #4703

Conversation

Lewuathe commented Aug 5, 2020

electrum commented Aug 6, 2020

Lewuathe commented Aug 6, 2020

electrum commented Aug 13, 2020

Lewuathe commented Aug 17, 2020 • edited

--load option is not supported

Zulu OpenJDK for ARM64 is not available

CentOS 7 image does not support arm64/v8

electrum left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lewuathe Sep 15, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

electrum commented Sep 11, 2020

Lewuathe commented Sep 11, 2020

Lewuathe commented Nov 4, 2020

electrum left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lewuathe commented Nov 27, 2020

GaruGaru commented Mar 11, 2021

martin-g commented Mar 11, 2021

svarosky90 commented Mar 11, 2021 • edited

Lewuathe commented Mar 12, 2021

Lewuathe commented Mar 16, 2021

svarosky90 commented Jun 5, 2021

Lewuathe commented Jun 6, 2021

electrum commented Jun 24, 2021

electrum commented Jun 24, 2021

Lewuathe commented Jun 24, 2021

electrum commented Jun 25, 2021

Lewuathe commented Jun 25, 2021

electrum commented Jun 26, 2021

electrum commented Jun 27, 2021

Lewuathe commented Aug 17, 2020 •

edited

`--load` option is not supported

Lewuathe Sep 15, 2020 •

edited

svarosky90 commented Mar 11, 2021 •

edited