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

RobertIndie · 2023-05-30T02:42:23Z

Fixes #20420

Motivation

When building the release candidate, the release mangers use their own account name to build and push the pulsar image. But currently the pulsar-all image always use apachepulsar/pulsar as the upstream image. And it may won't use the locally built image, which causes the issue.

Modifications

Specify the correct image name for pulsar image to build pulsar-all
Remove the latest version from the build. The latest tag should only point to the latest major version. It's better to let the release manager specify the latest tag manually.
Add github commit id to the image tag.

The image would like: apachepulsar/pulsar:3.1.0-SNAPSHOT-97a1a87

Verifying this change

Prepare the pulsar image: docker tag apachepulsar/pulsar:3.0.0 snzkyang/pulsar:3.1.0-SNAPSHOT-97a1a87
Run the build command of pulsar-all image:

mvn install \
        -DskipTests \
        -Pdocker \
        -Ddocker.platforms=linux/amd64 \
        -Ddocker.organization=snzkyang \
         -pl docker/pulsar-all

It will build successfully:

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  09:29 min
[INFO] Finished at: 2023-05-30T11:29:28+08:00
[INFO] ------------------------------------------------------------------------
[INFO] 12 goals, 12 executed

Verify

pulsar git:(iss-20420) ✗ docker run snzkyang/pulsar-all:3.1.0-SNAPSHOT-97a1a87 ls connectors
LICENSE
README
pulsar-io-aerospike-3.1.0-SNAPSHOT.nar
pulsar-io-alluxio-3.1.0-SNAPSHOT.nar
pulsar-io-batch-data-generator-3.1.0-SNAPSHOT.nar
pulsar-io-canal-3.1.0-SNAPSHOT.nar
pulsar-io-cassandra-3.1.0-SNAPSHOT.nar
pulsar-io-data-generator-3.1.0-SNAPSHOT.nar
pulsar-io-debezium-mongodb-3.1.0-SNAPSHOT.nar
...

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:

lhotari · 2023-05-30T05:32:23Z

@RobertIndie there's already #20433 which contains a better solution and on the way to preventing the issue.

RobertIndie · 2023-05-30T06:53:04Z

@RobertIndie there's already #20433 which contains a better solution and on the way to preventing the issue.

This PR is focusing on the fix. And it has been tested and verified. From my understanding, #20433 may have the problem when building the image and still need some changes. Please see #20433 (review) for more detail.

lhotari · 2023-05-30T07:30:01Z

This PR is focusing on the fix. And it has been tested and verified.

Well, there are remaining issues which I have commented on #20433 .

Git SHA should be part of the tag (it could be an extra tag) so that it is unique and ensures that the correct parent image is used, also in the case that the build ordering wouldn't be effective for some reason.
- what if "pulsar-all" gets built before "pulsar" in the build?
Pushing the latest tag should be prevented in maintenance branch publishing process and scripts used to do the publishing

The PR #20433 was started before this. Could we get to an agreement which PR to proceed with?

lhotari

Git SHA should be part of the tag (it could be an extra tag) so that it is unique and ensures that the correct parent image is used, also in the case that the build ordering wouldn't be effective for some reason.
- what if "pulsar-all" gets built before "pulsar" in the build?
Pushing the latest tag should be prevented in maintenance branch publishing process and scripts used to do the publishing

nodece · 2023-05-30T11:24:29Z

If you want to remove the latest image, please check the latest image usage in the codebase.

RobertIndie · 2023-06-02T07:03:39Z

If you want to remove the latest image, please check the latest image usage in the codebase.

This PR will not affect the image usage in the codebase. It just affects how the RC image is built.

nodece

LGTM

docker/pulsar/pom.xml

docker/pulsar-all/pom.xml

docker/pulsar/pom.xml

lhotari · 2023-06-02T10:01:11Z

One aspect to check is what happens when the docker image is built from the sources .tar.gz file without a git repository. I'm not sure if that has worked in the past so I'm not sure if it needs to work. It would be useful to have it working.

docker/pulsar-all/pom.xml

nodece · 2023-06-02T11:33:34Z

If we consider the git issue, I suggest that we only fix the pulsar image issue.

RobertIndie · 2023-06-05T10:10:28Z

One aspect to check is what happens when the docker image is built from the sources .tar.gz file without a git repository. I'm not sure if that has worked in the past so I'm not sure if it needs to work. It would be useful to have it working.

@lhotari I have pushed a commit to enable building the image without git repo. And I have verified it works. The image name will be like snzkyang/pulsar:3.1.0-SNAPSHOT-no-git if there is no git directory.

pom.xml

docker/pulsar/pom.xml

Signed-off-by: Zike Yang <[email protected]>

…ty to top level

lhotari

I fixed the issues and pushed. It should be ready for merging now.

codecov-commenter · 2023-06-06T10:50:13Z

Codecov Report

Merging #20435 (f46db28) into master (3b862ae) will increase coverage by 36.19%.
The diff coverage is 63.15%.

@@              Coverage Diff              @@
##             master   #20435       +/-   ##
=============================================
+ Coverage     36.78%   72.98%   +36.19%     
- Complexity    12059    31934    +19875     
=============================================
  Files          1690     1867      +177     
  Lines        129001   138537     +9536     
  Branches      14041    15207     +1166     
=============================================
+ Hits          47453   101109    +53656     
+ Misses        75294    29416    -45878     
- Partials       6254     8012     +1758

Flag	Coverage Δ
inttests	`24.13% <17.10%> (-0.01%)`	⬇️
systests	`25.07% <1.31%> (+0.11%)`	⬆️
unittests	`72.26% <63.15%> (+40.31%)`	⬆️

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

Impacted Files	Coverage Δ
.../apache/pulsar/broker/admin/impl/PackagesBase.java	`76.19% <ø> (+23.89%)`	⬆️
...rg/apache/pulsar/broker/service/BrokerService.java	`80.55% <ø> (+24.22%)`	⬆️
.../apache/pulsar/functions/instance/ContextImpl.java	`61.00% <0.00%> (+18.41%)`	⬆️
...s/runtime/kubernetes/KubernetesRuntimeFactory.java	`80.21% <50.00%> (+80.21%)`	⬆️
...pache/pulsar/broker/admin/impl/NamespacesBase.java	`72.05% <56.86%> (+41.36%)`	⬆️
...che/bookkeeper/mledger/impl/ManagedCursorImpl.java	`78.58% <85.71%> (+32.43%)`	⬆️
...ker/authorization/PulsarAuthorizationProvider.java	`71.42% <100.00%> (+39.74%)`	⬆️
.../org/apache/pulsar/broker/admin/AdminResource.java	`79.57% <100.00%> (+35.76%)`	⬆️
...pulsar/broker/admin/impl/PersistentTopicsBase.java	`64.21% <100.00%> (+54.04%)`	⬆️

... and 1424 files with indirect coverage changes

#20435) Signed-off-by: Zike Yang <[email protected]> Co-authored-by: Lari Hotari <[email protected]> (cherry picked from commit d7f3558)

tisonkun · 2023-06-06T11:48:08Z

Cool!

nodece · 2023-06-08T15:08:01Z

When I build the Pulsar 3.0.1, I got the following error:

Error:  Failed to execute goal pl.project13.maven:git-commit-id-plugin:4.9.10:revision (git-info) on project pulsar-docker-image: Could not complete Mojo execution... pl.project13.core.NativeGitProvider$NativeCommandException: Git command exited with invalid status [128]: directory: `/home/ubuntu/actions-runner/_work/xxx/pulsar/docker/pulsar`, command: `git describe --dirty=-dirty --match=* --abbrev=7`, stdout: ``, stderr: `fatal: No tags can describe '6302ccf8f61e2539e5c52335c3d68b3055ef2cff'.Try --always, or create some tags.` -> [Help 1]
Error:  
Error:  To see the full stack trace of the errors, re-run Maven with the -e switch.
Error:  Re-run Maven using the -X switch to enable full debug logging.
Error:  
Error:  For more information about the errors and possible solutions, please read the following articles:
Error:  [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
Error: Process completed with exit code 1.

Do you have this error?

Follow up on apache#20435 changes to fix build that could fail with git describe error "fatal: No names found, cannot describe anything."

nodece · 2023-06-30T11:13:25Z

Do we consider keep the <tag>${project.version}</tag>?

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

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

RobertIndie added type/bug The PR fixed a bug or issue reported a bug area/build labels May 30, 2023

RobertIndie added this to the 3.1.0 milestone May 30, 2023

RobertIndie self-assigned this May 30, 2023

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

RobertIndie mentioned this pull request May 30, 2023

[improve][build] Improve the publishing of docker image #20433

Closed

4 tasks

lhotari requested changes May 30, 2023

View reviewed changes

lhotari requested a review from nicoloboschi May 30, 2023 07:32

RobertIndie requested a review from lhotari June 2, 2023 06:53

RobertIndie added the ready-to-test label Jun 2, 2023

nodece mentioned this pull request Jun 2, 2023

[fix][build] Improve the build and publish of pulsar images #20462

Closed

5 tasks

nodece approved these changes Jun 2, 2023

View reviewed changes

tisonkun approved these changes Jun 2, 2023

View reviewed changes

tisonkun reviewed Jun 2, 2023

View reviewed changes

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

lhotari requested changes Jun 2, 2023

View reviewed changes

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

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

lhotari reviewed Jun 2, 2023

View reviewed changes

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

RobertIndie requested a review from lhotari June 5, 2023 10:10

lhotari reviewed Jun 5, 2023

View reviewed changes

pom.xml Show resolved Hide resolved

lhotari requested changes Jun 5, 2023

View reviewed changes

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

RobertIndie added 3 commits June 6, 2023 10:06

[fix][build] Fix the pulsar-all image may use the wrong upstream image

25a5662

Signed-off-by: Zike Yang <[email protected]>

Fix license header issue

bbb48be

Signed-off-by: Zike Yang <[email protected]>

Inject github commit id to the image tag

1e101e2

Signed-off-by: Zike Yang <[email protected]>

RobertIndie and others added 6 commits June 6, 2023 10:06

Move plugin git-commit-id-plugin to root pom.xml

46bbfff

Add latest tag back and skip git-commit-id-plugin on other modules

ed822f2

Enable building the image wihout git repo

dbea642

Enable git-commit-id-plugin in docker/pom.xml and move default proper…

645fd8f

…ty to top level

Fix ROOT_DIR to work without git directory

a713f79

Revisit publish script

d24058e

lhotari force-pushed the iss-20420 branch 2 times, most recently from 7f68159 to d24058e Compare June 6, 2023 08:39

lhotari approved these changes Jun 6, 2023

View reviewed changes

Fix license check issue

f46db28

RobertIndie added the release/3.0.1 label Jun 6, 2023

lhotari merged commit d7f3558 into apache:master Jun 6, 2023

lhotari pushed a commit that referenced this pull request Jun 6, 2023

[fix][build] Fix the pulsar-all image may use the wrong upstream image (

fa63b68

#20435) Signed-off-by: Zike Yang <[email protected]> Co-authored-by: Lari Hotari <[email protected]> (cherry picked from commit d7f3558)

lhotari added the cherry-picked/branch-3.0 label Jun 6, 2023

lhotari mentioned this pull request Jun 9, 2023

[fix][build] Configure git-commit-id-plugin to skip git describe #20550

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

RobertIndie commented May 30, 2023 •

edited

Loading

lhotari commented May 30, 2023

RobertIndie commented May 30, 2023

lhotari commented May 30, 2023

lhotari left a comment

nodece commented May 30, 2023

RobertIndie commented Jun 2, 2023

nodece left a comment

lhotari commented Jun 2, 2023

nodece commented Jun 2, 2023

RobertIndie commented Jun 5, 2023

lhotari left a comment

codecov-commenter commented Jun 6, 2023

tisonkun commented Jun 6, 2023

nodece commented Jun 8, 2023

nodece commented Jun 30, 2023

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

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

Conversation

RobertIndie commented May 30, 2023 • edited Loading

Motivation

Modifications

Verifying this change

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

Documentation

Matching PR in forked repository

lhotari commented May 30, 2023

RobertIndie commented May 30, 2023

lhotari commented May 30, 2023

lhotari left a comment

Choose a reason for hiding this comment

nodece commented May 30, 2023

RobertIndie commented Jun 2, 2023

nodece left a comment

Choose a reason for hiding this comment

lhotari commented Jun 2, 2023

nodece commented Jun 2, 2023

RobertIndie commented Jun 5, 2023

lhotari left a comment

Choose a reason for hiding this comment

codecov-commenter commented Jun 6, 2023

Codecov Report

tisonkun commented Jun 6, 2023

nodece commented Jun 8, 2023

nodece commented Jun 30, 2023

RobertIndie commented May 30, 2023 •

edited

Loading