[improve] Allow to build and push multi-arch Docker images #19432

merlimat · 2023-02-05T17:18:12Z

Fixes #12944

Motivation

Added support to build multi-arch x86_64 & arm64 Docker images and push them to DockerHub.

The same tag will refer to the 2 different images and docker will fetch the one matching the user's machine arch.

Added Maven profile -Pdocker-push that will trigger the multi-arch build and push of the tagged image

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

Documentation

doc
doc-required
doc-not-needed
doc-complete

Matching PR in forked repository

PR in forked repository:

build/build_java_test_image.sh

build/pulsar_ci_tool.sh

tisonkun · 2023-02-10T01:52:11Z

docker/pulsar-all/pom.xml

@@ -148,7 +145,18 @@
                        <contextDir>${project.basedir}</contextDir>
                        <tags>
                          <tag>latest</tag>
+                          <tag>${project.version}</tag>


IIRC this tag is intentionally removed in previous change. cc @nodece

We only use the latest tag, see

pulsar/docker/publish.sh

Line 65 in ed9cca6

docker tag pulsar:latest ${docker_registry_org}/pulsar:latest

We must use the maven plugin to push in order to build the multi-platform image.

Buildx is enabled when there is a non-empty element inside the configuration. Only the native platform is built and saved in the local image cache by the build goal. All specified platforms are built and pushed into the remote repository by the push goal. This behavior is to prevent non-native images from tainting the local image cache.
https://dmp.fabric8.io/#build-buildx

tisonkun · 2023-02-10T01:53:17Z

docker/pulsar-all/pom.xml

@@ -161,5 +169,29 @@
        </plugins>
      </build>
    </profile>
+
+    <profile>
+      <id>docker-push</id>


@merlimat Shall this change affect existing docker push scripts and the release process?

I have the same concern, we only use https://github.com/apache/pulsar/blob/ed9cca6d5dd947c741b6d6fc579b7d526ba0274c/docker/publish.sh to publish the docker image, so I think this profile is unnecessary.

The point here is that we need a profile that does build the multi-arch when doing a release.

It's also important that the build & push are done in one step so that buildx can push a single tag with 2 separate images.

Yes, the release process would need to be adjust a little bit.

buildx can merge two images under one tag in a separate step though - https://github.com/korandoru/hawkeye/blob/cdf339c0e19a347937f310bba99b8611a7136a16/.github/actions/docker-release/action.yml#L58

But it's fine to give the current way a try :)

Yes, the release process would need to be adjust a little bit.

Can you describe what is needed to do a release ? We are considering this feature for 3.0 for which the RC1 should be released today.

nodece · 2023-02-10T02:38:53Z

docker/pulsar-all/pom.xml

@@ -139,6 +135,7 @@
                <goals>
                  <goal>build</goal>
                  <goal>tag</goal>
+                  <goal>push</goal>


We don't need to push the image by the maven, so we need to remove this goal.

I think this is the reason why we need the push goal:

Buildx is enabled when there is a non-empty element inside the configuration. Only the native platform is built and saved in the local image cache by the build goal. All specified platforms are built and pushed into the remote repository by the push goal. This behavior is to prevent non-native images from tainting the local image cache.
https://dmp.fabric8.io/#build-buildx

docker/pulsar-all/pom.xml

Co-authored-by: Lari Hotari <[email protected]>

Co-authored-by: Zixuan Liu <[email protected]>

tisonkun

One comment here - please fine a way to update the release process doc once this patch gets merged.

tisonkun · 2023-04-03T11:42:46Z

Seems we failed to build the test image in time (killed due to 1 hour timeout). I'm retriggering the workflow, but if it remains, we may find someway to speed up the execution.

tisonkun · 2023-04-05T05:12:34Z

It seems "Build Pulsar Docker image" becomes quite easily to timeout now.

codecov-commenter · 2023-04-22T20:21:31Z

Codecov Report

Merging #19432 (99b2ebc) into master (9b72302) will increase coverage by 39.76%.
The diff coverage is 82.94%.

@@              Coverage Diff              @@
##             master   #19432       +/-   ##
=============================================
+ Coverage     33.17%   72.94%   +39.76%     
- Complexity    12236    31930    +19694     
=============================================
  Files          1499     1868      +369     
  Lines        114413   138449    +24036     
  Branches      12431    15235     +2804     
=============================================
+ Hits          37962   100987    +63025     
+ Misses        71499    29414    -42085     
- Partials       4952     8048     +3096

Flag	Coverage Δ
inttests	`24.21% <17.82%> (?)`
systests	`24.85% <15.50%> (?)`
unittests	`72.22% <82.94%> (+39.04%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...e/pulsar/broker/authentication/oidc/JwksCache.java	`56.62% <0.00%> (ø)`
...thentication/oidc/OpenIDProviderMetadataCache.java	`78.48% <0.00%> (ø)`
...nt/internal/PulsarClientImplementationBinding.java	`62.50% <ø> (+12.50%)`	⬆️
...ava/org/apache/pulsar/client/api/MessageIdAdv.java	`84.21% <ø> (ø)`
...unctions/runtime/kubernetes/KubernetesRuntime.java	`38.34% <0.00%> (+38.34%)`	⬆️
...nt/impl/PulsarClientImplementationBindingImpl.java	`80.00% <28.57%> (+24.82%)`	⬆️
...broker/intercept/ManagedLedgerInterceptorImpl.java	`75.00% <75.00%> (+75.00%)`	⬆️
.../pulsar/broker/delayed/bucket/ImmutableBucket.java	`85.71% <76.19%> (+85.71%)`	⬆️
...ulsar/client/impl/transaction/TransactionImpl.java	`87.20% <80.95%> (+48.00%)`	⬆️
...elayed/bucket/BookkeeperBucketSnapshotStorage.java	`85.71% <100.00%> (+85.71%)`	⬆️
... and 11 more

... and 1519 files with indirect coverage changes

yaalsn · 2023-04-23T07:18:54Z

docker/pulsar/pom.xml

@@ -72,17 +71,27 @@
                <configuration>
                  <images>
                    <image>
-                      <name>pulsar</name>
+                      <name>${docker.organization}/pulsar</name>


This may affect the release of the image.

https://github.com/apache/pulsar/blob/master/docker/publish.sh

This issue has been fixed by #20305, could you check that?

…pache#19432)" This reverts commit 4190e40.

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)

…pache#19432)" This reverts commit 4190e40.

GerMoranBYON · 2023-05-03T21:42:39Z

I see this was merged, will arm64 images be generated now going forward for 3.0+.
I can see images in dockerhub, but didn't see this issue referenced in the release notes?.

RobertIndie · 2023-05-04T07:42:50Z

@GerMoranOverstock Hi, this feature is supported in 3.0.0.

This PR was not marked with the 3.0.0 milestone, so the release note mistakenly omitted it.
I will fix the release note later. Thanks for your report.

GerMoranBYON · 2023-05-04T07:47:45Z

@RobertIndie Thanks for the clarification.
I had seen the image up, though got a bit confused with partial reverts (appears to be on stream native repo) & cherry picks, so great to see this in and supported.

) 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]>

* [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]>

Allow to build and push multi-arch Docker images

731116a

merlimat added release/note-required ready-to-test labels Feb 5, 2023

merlimat added this to the 3.0.0 milestone Feb 5, 2023

merlimat requested review from lhotari, codelipenghui and tisonkun February 5, 2023 17:18

github-actions bot added the doc-not-needed Your PR changes do not impact docs label Feb 5, 2023

merlimat changed the title ~~Allow to build and push multi-arch Docker images~~ [improve] Allow to build and push multi-arch Docker images Feb 5, 2023

Fixed build_java_test_image.sh

99732a6

lhotari reviewed Feb 9, 2023

View reviewed changes

build/build_java_test_image.sh Outdated Show resolved Hide resolved

build/pulsar_ci_tool.sh Outdated Show resolved Hide resolved

tisonkun reviewed Feb 10, 2023

View reviewed changes

tisonkun requested a review from nodece February 10, 2023 01:53

nodece reviewed Feb 10, 2023

View reviewed changes

docker/pulsar-all/pom.xml Outdated Show resolved Hide resolved

Update build/pulsar_ci_tool.sh

ed4e357

Co-authored-by: Lari Hotari <[email protected]>

GerMoranBYON mentioned this pull request Mar 7, 2023

[improve][ci] Addresses PR comments from #19432 merlimat/pulsar#5

Closed

14 tasks

Update build/build_java_test_image.sh

9138e7f

Co-authored-by: Lari Hotari <[email protected]>

GerMoranBYON mentioned this pull request Mar 9, 2023

[improve][ci] Docker multi arch - Addresses PR comments from #19432 #19766

Closed

14 tasks

zymap approved these changes Mar 28, 2023

View reviewed changes

zymap assigned merlimat Mar 28, 2023

Update docker/pulsar-all/pom.xml

f6187b3

Co-authored-by: Zixuan Liu <[email protected]>

tisonkun approved these changes Mar 28, 2023

View reviewed changes

coderzc approved these changes Mar 28, 2023

View reviewed changes

nodece approved these changes Mar 28, 2023

View reviewed changes

Fix the image name

d32bc42

Merge branch 'master' into docker-multi-arch

2c746ef

merlimat added 12 commits April 21, 2023 11:29

Ensure test image contains latest pulsar image

4ea34d2

Fixed command

9042363

Use buildx also for test image

ba7124b

Test image needs to be tagged to be visible to "docker save"

a491c61

Removed org & tag from test image

ca935f9

Use buildx to save image directly

0e34aa7

Fixed bash function name

7ed67d9

Try newer ubuntu

73571ae

Fixed path

57098cc

docker save again

7ff21b9

Take docker.platforms as a variable

7bde89d

Removed useless changes

99b2ebc

merlimat merged commit 4190e40 into apache:master Apr 22, 2023

merlimat deleted the docker-multi-arch branch April 22, 2023 20:28

yaalsn reviewed Apr 23, 2023

View reviewed changes

coderzc added a commit to streamnative/pulsar-archived that referenced this pull request Apr 23, 2023

Revert "[improve] Allow to build and push multi-arch Docker images (a…

3f66c19

…pache#19432)" This reverts commit 4190e40.

coderzc added a commit to streamnative/pulsar-archived that referenced this pull request Apr 24, 2023

Revert "[improve] Allow to build and push multi-arch Docker images (a…

f4dfc1a

…pache#19432)" This reverts commit 4190e40.

coderzc added a commit to streamnative/pulsar-archived that referenced this pull request Apr 24, 2023

Revert "[improve] Allow to build and push multi-arch Docker images (a…

f6dbc2e

…pache#19432)" This reverts commit 4190e40.

RobertIndie modified the milestones: 3.1.0, 3.0.0 May 4, 2023

RobertIndie mentioned this pull request May 4, 2023

Add multi-arch docker images support to 3.0.0 release note apache/pulsar-site#554

Merged

4 tasks

nodece mentioned this pull request May 11, 2023

[fix][build] Fix publish image script #20305

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[improve] Allow to build and push multi-arch Docker images #19432

[improve] Allow to build and push multi-arch Docker images #19432

merlimat commented Feb 5, 2023

tisonkun Feb 10, 2023

nodece Feb 10, 2023

coderzc Mar 27, 2023 •

edited

Loading

tisonkun Feb 10, 2023

nodece Feb 10, 2023

merlimat Mar 6, 2023

tisonkun Mar 28, 2023 •

edited

Loading

cbornet Apr 11, 2023

nodece Feb 10, 2023 •

edited

Loading

coderzc Mar 27, 2023

zymap Mar 27, 2023 •

edited by coderzc

Loading

tisonkun left a comment

tisonkun commented Apr 3, 2023

tisonkun commented Apr 5, 2023

codecov-commenter commented Apr 22, 2023

yaalsn Apr 23, 2023

nodece May 12, 2023

GerMoranBYON commented May 3, 2023

RobertIndie commented May 4, 2023

GerMoranBYON commented May 4, 2023 •

edited

Loading

[improve] Allow to build and push multi-arch Docker images #19432

[improve] Allow to build and push multi-arch Docker images #19432

Conversation

merlimat commented Feb 5, 2023

Motivation

Does this pull request potentially affect one of the following parts:

Documentation

Matching PR in forked repository

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coderzc Mar 27, 2023 • edited Loading

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

tisonkun Mar 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nodece Feb 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zymap Mar 27, 2023 • edited by coderzc Loading

Choose a reason for hiding this comment

tisonkun left a comment

Choose a reason for hiding this comment

tisonkun commented Apr 3, 2023

tisonkun commented Apr 5, 2023

codecov-commenter commented Apr 22, 2023

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GerMoranBYON commented May 3, 2023

RobertIndie commented May 4, 2023

GerMoranBYON commented May 4, 2023 • edited Loading

coderzc Mar 27, 2023 •

edited

Loading

tisonkun Mar 28, 2023 •

edited

Loading

nodece Feb 10, 2023 •

edited

Loading

zymap Mar 27, 2023 •

edited by coderzc

Loading

GerMoranBYON commented May 4, 2023 •

edited

Loading