You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/09/20 15:14:51 UTC

[GitHub] [pulsar] merlimat commented on a diff in pull request #17733: [feat][build] Support ARM64-based docker images

merlimat commented on code in PR #17733:
URL: https://github.com/apache/pulsar/pull/17733#discussion_r975496590


##########
docker/pulsar/Dockerfile:
##########
@@ -69,7 +69,9 @@ RUN mkdir -p /etc/apt/keyrings \
      && echo "deb [signed-by=/etc/apt/keyrings/adoptium.asc] https://packages.adoptium.net/artifactory/deb $(awk -F= '/^VERSION_CODENAME/{print$2}' /etc/os-release) main" | tee /etc/apt/sources.list.d/adoptium.list \
      && apt-get update \
      && apt-get -y dist-upgrade \
-     && apt-get -y install temurin-17-jdk
+     && apt-get -y install temurin-17-jdk \
+     && export architecture=$(uname -m | sed -r 's/aarch64/arm64/g' |  awk '!/arm64/{$0="amd64"}1') \

Review Comment:
   nit: since this is exporting a bash variable, we could use the uppercase convention here. eg: `ARCH`



##########
docker/pulsar/pom.xml:
##########
@@ -79,7 +80,7 @@
                   <executable>${project.basedir}/../../pulsar-client-cpp/docker/build-wheels.sh</executable>
                   <arguments>
                     <!-- build python 3.8 -->
-                    <argument>3.8 cp38-cp38 manylinux2014 x86_64</argument>
+                    <argument>3.8 cp38-cp38 manylinux2014 ${pythonClientBuildArch}</argument>

Review Comment:
   For now this is fine. After the Python client is removed from the main repo, we can just do `pip install pulsar-client==2.10.1` and it will automatically select the right architecture.



##########
tests/docker-images/latest-version-image/Dockerfile:
##########
@@ -29,13 +29,8 @@ RUN apt-get install -y procps curl git build-essential
 
 ENV GOLANG_VERSION 1.15.8
 
-RUN curl -sSL https://golang.org/dl/go$GOLANG_VERSION.linux-amd64.tar.gz \
-		| tar -C /usr/local -xz
-
-# RUN wget https://dl.google.com/go/go1.13.3.linux-amd64.tar.gz && tar -xvf go1.13.3.linux-amd64.tar.gz && mv go /usr/local
-# RUN export GOROOT=/usr/local/go && export GOPATH=$HOME/go && export PATH=$GOPATH/bin:$GOROOT/bin:$PATH
-# RUN echo "export GOROOT=/usr/local/go" >> ~/.profile && echo "export GOPATH=$HOME/go" >> ~/.profile && echo "export PATH=$GOPATH/bin:$GOROOT/bin:$PATH" >> ~/.profile
-
+RUN export architecture=$(uname -m | sed -r 's/aarch64/arm64/g' |  awk '!/arm64/{$0="amd64"}1') \

Review Comment:
   `architecture` -> `ARCH`



##########
docker/pulsar/scripts/install-pulsar-client.sh:
##########
@@ -23,3 +23,7 @@ set -x
 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]
+
+# 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

Review Comment:
   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)



##########
tests/docker-images/java-test-image/Dockerfile:
##########
@@ -46,10 +46,9 @@ RUN mkdir -p /etc/apt/keyrings \
      && echo "deb [signed-by=/etc/apt/keyrings/adoptium.asc] https://packages.adoptium.net/artifactory/deb $(awk -F= '/^VERSION_CODENAME/{print$2}' /etc/os-release) main" | tee /etc/apt/sources.list.d/adoptium.list \
      && apt-get update \
      && apt-get -y dist-upgrade \
-     && apt-get -y install temurin-17-jdk
-
-ENV JAVA_HOME /usr/lib/jvm/temurin-17-jdk-amd64
-RUN echo networkaddress.cache.ttl=1 >> /usr/lib/jvm/temurin-17-jdk-amd64/conf/security/java.security
+     && apt-get -y install temurin-17-jdk \
+     && export architecture=$(uname -m | sed -r 's/aarch64/arm64/g' |  awk '!/arm64/{$0="amd64"}1') \

Review Comment:
   `architecture` -> `ARCH` 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org