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

[feat][build] Support ARM64-based docker images #17733

Merged
merged 8 commits into from
Sep 21, 2022

Conversation

tisonkun
Copy link
Member

@tisonkun tisonkun commented Sep 20, 2022

This refers to #12944.

Verifying this change

mvn clean install -DskipTests
./build/build_java_test_image.sh                                                
mvn package -Pdocker,-main -am -pl docker/pulsar-all -DskipTests -DpythonClientBuildArch=aarch64

Documentation

  • doc-required
    (Your PR needs to update docs and you will update later)

  • doc-not-needed
    (Please explain why)

  • doc
    (Your PR contains doc changes)

  • doc-complete
    (Docs have been already added)

Matching PR in forked repository

PR in forked repository: tisonkun#2

Signed-off-by: tison <[email protected]>
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Sep 20, 2022
@tisonkun
Copy link
Member Author

tisonkun commented Sep 20, 2022

cc @nodece @BewareMyPower @merlimat @lhotari @michaeljmarshall @heesung-sn

@tisonkun tisonkun changed the title Docker images build on arm64 [feat][build] Support ARM64-based docker images Sep 20, 2022
Comment on lines 75 to 82
&& apt-get -y --purge autoremove \

# Cleanup apt
RUN apt-get -y --purge autoremove \
&& apt-get autoclean \
&& apt-get clean \
&& rm -rf /var/lib/apt/lists/* \
&& pip3 install pyyaml==5.4.1
&& rm -rf /var/lib/apt/lists/*

RUN pip3 install pyyaml==5.4.1
Copy link
Member Author

@tisonkun tisonkun Sep 20, 2022

Choose a reason for hiding this comment

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

I don't know why. But this change causes local build fails with returned a non-zero code: 100. So keep it as is and revert intermediate changes.

docker/pulsar/Dockerfile Outdated Show resolved Hide resolved
docker/pulsar/pom.xml Show resolved Hide resolved

# TODO: remove these lines once grpcio doesn't need to compile from source on ARM64 platform
apt update
apt -y install build-essential python3-dev
Copy link
Contributor

Choose a reason for hiding this comment

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

This would be increasing the size of the pulsar image.

I could see 2 different options to avoid it:

  1. Only install compilers on ARM64 build (at least we don't make the x86_64 image any worse)
  2. Do the Grpc pip install in a separate Docker stage and then only copy the built wheel file. (this should work for both arm & x86)

Copy link
Member Author

Choose a reason for hiding this comment

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

Resolved by option 1 at 2793885.

Option 2 seems also to require installing build-essential python3-dev for pip install grpcio on ARM64 platform? Conditional installation is easier to perform inside a script than inside the Dockerfile AFAIK...

tests/docker-images/java-test-image/Dockerfile Outdated Show resolved Hide resolved
tests/docker-images/latest-version-image/Dockerfile Outdated Show resolved Hide resolved
@merlimat merlimat added this to the 2.12.0 milestone Sep 20, 2022
@Technoboy- Technoboy- merged commit 9a2aeb2 into apache:master Sep 21, 2022
@tisonkun tisonkun deleted the build-on-arm64 branch September 21, 2022 03:28
apt update
apt -y install build-essential python3-dev
fi

PYTHON_MAJOR_MINOR=$(python3 -V | sed -E 's/.* ([[:digit:]]+)\.([[:digit:]]+).*/\1\2/')
WHEEL_FILE=$(ls /pulsar/pulsar-client | grep "cp${PYTHON_MAJOR_MINOR}")
pip3 install /pulsar/pulsar-client/${WHEEL_FILE}[all]
Copy link
Contributor

Choose a reason for hiding this comment

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

[INFO] DOCKER> [91m++ python3 -V
++ sed -E 's/.* ([[:digit:]]+)\.([[:digit:]]+).*/\1\2/'

[INFO] DOCKER> [91m+ PYTHON_MAJOR_MINOR=38

[INFO] DOCKER> [91m++ ls /pulsar/pulsar-client
++ grep cp38

[INFO] DOCKER> [91m+ WHEEL_FILE='pulsar_client-2.11.0-cp38-cp38-manylinux1_x86_64.whl
pulsar_client-2.11.0-cp38-cp38-manylinux_2_17_aarch64.manylinux2014_aarch64.whl'

[INFO] DOCKER> [91m+ pip3 install /pulsar/pulsar-client/pulsar_client-2.11.0-cp38-cp38-manylinux1_x86_64.whl 'pulsar_client-2.11.0-cp38-cp38-manylinux_2_17_aarch64.manylinux2014_aarch64.whl[all]'

[INFO] DOCKER> [91mERROR: pulsar_client-2.11.0-cp38-cp38-manylinux1_x86_64.whl is not a supported wheel on this platform.

[INFO] DOCKER> Removing intermediate container a88eb585f4d1
[ERROR] DOCKER> Unable to build image [apachepulsar/pulsar] : "The command '/bin/sh -c /pulsar/bin/install-pulsar-client.sh' returned a non-zero code: 1"  ["The command '/bin/sh -c /pulsar/bin/install-pulsar-client.sh' returned a non-zero code: 1" ]

observed this error.

Copy link
Contributor

Choose a reason for hiding this comment

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

this problem will anyway go away once we make the changes for PIP-209 (moving C++ client out of repo)

Copy link
Member Author

Choose a reason for hiding this comment

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

@heesung-sn

mvn package -Pdocker,-main -am -pl docker/pulsar-all -DskipTests -DpythonClientBuildArch=aarch64

Be aware that you should pass -DpythonClientBuildArch=aarch64.

Copy link
Member Author

@tisonkun tisonkun Sep 26, 2022

Choose a reason for hiding this comment

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

I made this patch to support ARM64-based docker images while keeping the current workflow as is. That is, you should manually specify that you want to build an ARM64-based image.

About automatically detecting platforms, see also #17733 (comment) that we have some name matching problems.

And a clean solution would be, as @merlimat described, we publish C++/Python client alone and download them from the package manager so that leveraging the package manager's platform detect functionality.

Copy link
Contributor

@heesung-sn heesung-sn Sep 26, 2022

Choose a reason for hiding this comment

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

I provided the pythonClientBuildArch option when I encountered the error.

mvn package -Pdocker,-main -am -pl docker/pulsar-all -DskipTests -DpythonClientBuildArch=aarch64

I tried to hard-code like the below, then it passes. I wonder if the client somehow output wheel files for both archs.

pip3 install /pulsar/pulsar-client/pulsar_client-2.11.0-cp38-cp38-manylinux_2_17_aarch64.manylinux2014_aarch64.whl[all]

nodece pushed a commit to nodece/pulsar that referenced this pull request Apr 28, 2024
nodece added a commit to ascentstream/pulsar that referenced this pull request May 10, 2024
* [Dockerfile] Enable retries for apt-get when building Pulsar docker image (apache#14513)

- also reduce default timeout to 30 seconds
- prevents issues where apt repository doesn't respond

(cherry picked from commit d3f6fe3)

* PIP-155: Removed Python 2 support (apache#15376)

* Remove Pulsar Client Build for Python 2.7

* Remove outdated homebrew files (source of truth is upstream homebrew)

* Remove Python 2.7 build references; print error in some cases

* Update python client tests to run with python client for python 3.5m

* PIP-155: Removed Python 2 support

* Fixed invocation in pulsar-build image

* Fixed clang-format-10 indent differences

* Fixed script invocation with wrong python

* We don't need to rebuild the manylinux image each time

* Fixed image name

* Reverted back to use newer protobuf

* Fixed image name

* Fixed missing python3 in centos:7 image

* Use python3 for gtest-parallel

* Show bash commands in docker-tests.sh

* Fixed gh action issue with git directory permissions

* Fixed python to 3

* Fixed custom_logger_test.py

* Fixed path in run_python_instance_tests.sh

* Function runtime should use python3

* Fixed function runtime test python expectation

* Fixed presto worker launcher

* Fixed notes on how to format C++ code

Co-authored-by: Michael Marshall <[email protected]>

(cherry picked from commit 2b2e0c5)
Signed-off-by: Zixuan Liu <[email protected]>

* [improve][docker] Switch to Temurin JDK (apache#17129)

Signed-off-by: Zixuan Liu <[email protected]>

(cherry picked from commit 4378856)
Signed-off-by: Zixuan Liu <[email protected]>

* [refactor][ci] Build the docker image with docker-maven-plugin (apache#17148)

(cherry picked from commit a68b58d)
Signed-off-by: Zixuan Liu <[email protected]>

* [feat][build] Support ARM64-based docker images (apache#17733)

(cherry picked from commit 9a2aeb2)
Signed-off-by: Zixuan Liu <[email protected]>

* PIP-209: Removed C++/Python clients from main repo (apache#17881)

* PIP-209: Removed C++/Python clients from main repo

* Removed python directory from Docekrfile

* Fixed python client version argument scoping

* Fixed handling of pulsar.functions.serde

(cherry picked from commit f3c547b)
Signed-off-by: Zixuan Liu <[email protected]>

* [improve][build] Avoid building image multiple times (apache#17208)

Signed-off-by: Zixuan Liu <[email protected]>
(cherry picked from commit 79a97a9)

* [improve] Allow to build and push multi-arch Docker images (apache#19432)

Co-authored-by: Lari Hotari <[email protected]>
Co-authored-by: Yong Zhang <[email protected]>
Co-authored-by: Zixuan Liu <[email protected]>
Co-authored-by: tison <[email protected]>

(cherry picked from commit 4190e40)
Signed-off-by: Zixuan Liu <[email protected]>

* [fix][build] Fix publish image script (apache#20305)

Signed-off-by: Zixuan Liu <[email protected]>

(cherry picked from commit 94c7bf3)
Signed-off-by: Zixuan Liu <[email protected]>

* [fix][build] Fix the pulsar-all image may use the wrong upstream image (apache#20435)

Signed-off-by: Zike Yang <[email protected]>
Co-authored-by: Lari Hotari <[email protected]>

(cherry picked from commit d7f3558)
Signed-off-by: Zixuan Liu <[email protected]>

* [feat][build] Adapt to Python client to be compatible with ARM arch

Signed-off-by: Zixuan Liu <[email protected]>

* [fix][build] Configure git-commit-id-plugin to skip git describe (apache#20550)

(cherry picked from commit 05f7e62)

* [improve][misc] Include native epoll library for Netty for arm64

From apache#22319

Signed-off-by: Zixuan Liu <[email protected]>

* [fix][misc] Rename all shaded Netty native libraries

From apache#22415

Signed-off-by: Zixuan Liu <[email protected]>

* [cleanup][build] Cleanup -Ddocker.nocache=true

Signed-off-by: Zixuan Liu <[email protected]>

* [fix][build] Fix ubuntu mirror

Signed-off-by: Zixuan Liu <[email protected]>

* [fix][build] Fix license

Signed-off-by: Zixuan Liu <[email protected]>

* [fix][build] Downgrade docker-maven to 0.43.3

Signed-off-by: Zixuan Liu <[email protected]>

---------

Signed-off-by: Zixuan Liu <[email protected]>
Signed-off-by: Zike Yang <[email protected]>
Co-authored-by: Lari Hotari <[email protected]>
Co-authored-by: Matteo Merli <[email protected]>
Co-authored-by: Michael Marshall <[email protected]>
Co-authored-by: tison <[email protected]>
Co-authored-by: Yong Zhang <[email protected]>
Co-authored-by: Zike Yang <[email protected]>
Co-authored-by: Lari Hotari <[email protected]>
nodece added a commit to nodece/pulsar that referenced this pull request May 11, 2024
* [Dockerfile] Enable retries for apt-get when building Pulsar docker image (apache#14513)

- also reduce default timeout to 30 seconds
- prevents issues where apt repository doesn't respond

(cherry picked from commit d3f6fe3)

* PIP-155: Removed Python 2 support (apache#15376)

* Remove Pulsar Client Build for Python 2.7

* Remove outdated homebrew files (source of truth is upstream homebrew)

* Remove Python 2.7 build references; print error in some cases

* Update python client tests to run with python client for python 3.5m

* PIP-155: Removed Python 2 support

* Fixed invocation in pulsar-build image

* Fixed clang-format-10 indent differences

* Fixed script invocation with wrong python

* We don't need to rebuild the manylinux image each time

* Fixed image name

* Reverted back to use newer protobuf

* Fixed image name

* Fixed missing python3 in centos:7 image

* Use python3 for gtest-parallel

* Show bash commands in docker-tests.sh

* Fixed gh action issue with git directory permissions

* Fixed python to 3

* Fixed custom_logger_test.py

* Fixed path in run_python_instance_tests.sh

* Function runtime should use python3

* Fixed function runtime test python expectation

* Fixed presto worker launcher

* Fixed notes on how to format C++ code

Co-authored-by: Michael Marshall <[email protected]>

(cherry picked from commit 2b2e0c5)
Signed-off-by: Zixuan Liu <[email protected]>

* [improve][docker] Switch to Temurin JDK (apache#17129)

Signed-off-by: Zixuan Liu <[email protected]>

(cherry picked from commit 4378856)
Signed-off-by: Zixuan Liu <[email protected]>

* [refactor][ci] Build the docker image with docker-maven-plugin (apache#17148)

(cherry picked from commit a68b58d)
Signed-off-by: Zixuan Liu <[email protected]>

* [feat][build] Support ARM64-based docker images (apache#17733)

(cherry picked from commit 9a2aeb2)
Signed-off-by: Zixuan Liu <[email protected]>

* PIP-209: Removed C++/Python clients from main repo (apache#17881)

* PIP-209: Removed C++/Python clients from main repo

* Removed python directory from Docekrfile

* Fixed python client version argument scoping

* Fixed handling of pulsar.functions.serde

(cherry picked from commit f3c547b)
Signed-off-by: Zixuan Liu <[email protected]>

* [improve][build] Avoid building image multiple times (apache#17208)

Signed-off-by: Zixuan Liu <[email protected]>
(cherry picked from commit 79a97a9)

* [improve] Allow to build and push multi-arch Docker images (apache#19432)

Co-authored-by: Lari Hotari <[email protected]>
Co-authored-by: Yong Zhang <[email protected]>
Co-authored-by: Zixuan Liu <[email protected]>
Co-authored-by: tison <[email protected]>

(cherry picked from commit 4190e40)
Signed-off-by: Zixuan Liu <[email protected]>

* [fix][build] Fix publish image script (apache#20305)

Signed-off-by: Zixuan Liu <[email protected]>

(cherry picked from commit 94c7bf3)
Signed-off-by: Zixuan Liu <[email protected]>

* [fix][build] Fix the pulsar-all image may use the wrong upstream image (apache#20435)

Signed-off-by: Zike Yang <[email protected]>
Co-authored-by: Lari Hotari <[email protected]>

(cherry picked from commit d7f3558)
Signed-off-by: Zixuan Liu <[email protected]>

* [feat][build] Adapt to Python client to be compatible with ARM arch

Signed-off-by: Zixuan Liu <[email protected]>

* [fix][build] Configure git-commit-id-plugin to skip git describe (apache#20550)

(cherry picked from commit 05f7e62)

* [improve][misc] Include native epoll library for Netty for arm64

From apache#22319

Signed-off-by: Zixuan Liu <[email protected]>

* [fix][misc] Rename all shaded Netty native libraries

From apache#22415

Signed-off-by: Zixuan Liu <[email protected]>

* [cleanup][build] Cleanup -Ddocker.nocache=true

Signed-off-by: Zixuan Liu <[email protected]>

* [fix][build] Fix ubuntu mirror

Signed-off-by: Zixuan Liu <[email protected]>

* [fix][build] Fix license

Signed-off-by: Zixuan Liu <[email protected]>

* [fix][build] Downgrade docker-maven to 0.43.3

Signed-off-by: Zixuan Liu <[email protected]>

---------

Signed-off-by: Zixuan Liu <[email protected]>
Signed-off-by: Zike Yang <[email protected]>
Co-authored-by: Lari Hotari <[email protected]>
Co-authored-by: Matteo Merli <[email protected]>
Co-authored-by: Michael Marshall <[email protected]>
Co-authored-by: tison <[email protected]>
Co-authored-by: Yong Zhang <[email protected]>
Co-authored-by: Zike Yang <[email protected]>
Co-authored-by: Lari Hotari <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build doc-not-needed Your PR changes do not impact docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants