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

[refactor][ci] Build the docker image with docker-maven-plugin #17148

Merged
merged 15 commits into from
Aug 22, 2022

Conversation

Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun tisonkun marked this pull request as ready for review August 18, 2022 02:19
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Aug 18, 2022
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun
Copy link
Member Author

tisonkun commented Aug 18, 2022

Verify build successfully on macOS with M1 chip. Although a weird issue doesn't exist in the CI environment: https://github.com/apache/pulsar/pull/17129/files#r948602287. It should not be included in this patch, though.

@tisonkun
Copy link
Member Author

cc @skyleaworlder you're welcome to review this patch. It's based on your work.

cc @merlimat @nicoloboschi @eolivelli @Shoothzj @michaeljmarshall @mattisonchao

Build docker image succeed. I may retrigger other flaky tests when all steps finish.

@tisonkun
Copy link
Member Author

/pulsarbot run-failure-checks

@tisonkun
Copy link
Member Author

/pulsarbot run-failure-checks

@tisonkun
Copy link
Member Author

All required tests pass now.

@skyleaworlder
Copy link

I think packaging part is pretty good now! Thank you very much for fixing my previous pending errors!

tests/docker-images/java-test-image/pom.xml Show resolved Hide resolved
tests/docker-images/java-test-image/pom.xml Show resolved Hide resolved
docker/pulsar/pom.xml Show resolved Hide resolved
docker/pulsar-all/pom.xml Show resolved Hide resolved
</configuration>
</execution>
<execution>
<id>add-no-repo-and-latest</id>
<id>push-latest</id>
Copy link
Member

Choose a reason for hiding this comment

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

By the way, I notice that there are redundant configurations here, and I think we can simplify them.

<executions>
      <execution>
        <id>default</id>
        <phase>package</phase>
        <goals>
          <goal>build</goal>
          <goal>push</goal>
        </goals>
        <configuration>
          <images>
            <image>
              <name>${docker.organization}/pulsar</name>
              <build>
                <contextDir>${project.basedir}</contextDir>
                <tags>
                  <tag>latest</tag>
                  <tag>${project.version}</tag>
                </tags>
              </build>
            </image>
          </images>
        </configuration>
      </execution>
</executions>

Then maybe we also need to update the https://github.com/apache/pulsar/blob/master/docker/publish.sh#L65

So like using ${docker.organization}/pulsar:latest instead of pulsar:latest.

Copy link
Member

@nodece nodece Aug 18, 2022

Choose a reason for hiding this comment

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

There needs the PMC confirmation, I don't know how to publish this image when they release the Pulsar.

Copy link
Member Author

Choose a reason for hiding this comment

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

This seems reasonable. I'm glad to integrate with this change if @sijie @merlimat @codelipenghui can confirm.

I found the original logic was introduced at #4705

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

@codelipenghui I checked this process. Using the above config, we need to change the docker tag in publish.sh.

Copy link
Member Author

Choose a reason for hiding this comment

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

@nodece I think the point here is that it's possible that the release manager doesn't have the permission to push images to apachepulasr org. Thus we leave the ability to push to another org and ask the maintainer to sync. In this case, we generate the image with no repository for adding org when publish.sh.

Based on this information, I tend to keep the logic as is and if you have further improvement idea, we can start a new thread to discuss it.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, we can make a new PR to improve that.

docker/pulsar-all/pom.xml Show resolved Hide resolved
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun
Copy link
Member Author

Thanks for your detailed review @nodece! Comments addressed or replied.

@tisonkun tisonkun requested review from nodece and removed request for nicoloboschi and michaeljmarshall August 19, 2022 01:17
@nodece
Copy link
Member

nodece commented Aug 19, 2022

Thanks for your detailed review @nodece! Comments addressed or replied.

LGTM, I will approve this PR after add-no-repo is solved.

@tisonkun
Copy link
Member Author

/pulsarbot run-failure-checks

@tisonkun
Copy link
Member Author

/pulsarbot run-failure-checks

Copy link
Member

@nodece nodece left a comment

Choose a reason for hiding this comment

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

LGTM

@codelipenghui codelipenghui added this to the 2.12.0 milestone Aug 22, 2022
@codelipenghui codelipenghui merged commit a68b58d into apache:master Aug 22, 2022
@tisonkun tisonkun deleted the docker-maven-plugin branch August 22, 2022 08:15
lhotari added a commit that referenced this pull request Sep 21, 2022
- #17148 changes aren't in branch-2.11
- sync pulsar_ci_tool.sh with #17770
nodece pushed a commit to nodece/pulsar that referenced this pull request Apr 28, 2024
…e#17148)

(cherry picked from commit a68b58d)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>
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 <mmarshall@apache.org>

(cherry picked from commit 2b2e0c5)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>

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

Signed-off-by: Zixuan Liu <nodeces@gmail.com>

(cherry picked from commit 4378856)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>

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

(cherry picked from commit a68b58d)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>

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

(cherry picked from commit 9a2aeb2)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>

* 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 <nodeces@gmail.com>

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

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
(cherry picked from commit 79a97a9)

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

Co-authored-by: Lari Hotari <lhotari@users.noreply.github.com>
Co-authored-by: Yong Zhang <zhangyong1025.zy@gmail.com>
Co-authored-by: Zixuan Liu <nodeces@gmail.com>
Co-authored-by: tison <wander4096@gmail.com>

(cherry picked from commit 4190e40)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>

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

Signed-off-by: Zixuan Liu <nodeces@gmail.com>

(cherry picked from commit 94c7bf3)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>

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

Signed-off-by: Zike Yang <zike@apache.org>
Co-authored-by: Lari Hotari <lhotari@apache.org>

(cherry picked from commit d7f3558)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>

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

Signed-off-by: Zixuan Liu <nodeces@gmail.com>

* [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 <nodeces@gmail.com>

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

From apache#22415

Signed-off-by: Zixuan Liu <nodeces@gmail.com>

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

Signed-off-by: Zixuan Liu <nodeces@gmail.com>

* [fix][build] Fix ubuntu mirror

Signed-off-by: Zixuan Liu <nodeces@gmail.com>

* [fix][build] Fix license

Signed-off-by: Zixuan Liu <nodeces@gmail.com>

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

Signed-off-by: Zixuan Liu <nodeces@gmail.com>

---------

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
Signed-off-by: Zike Yang <zike@apache.org>
Co-authored-by: Lari Hotari <lhotari@users.noreply.github.com>
Co-authored-by: Matteo Merli <mmerli@apache.org>
Co-authored-by: Michael Marshall <mmarshall@apache.org>
Co-authored-by: tison <wander4096@gmail.com>
Co-authored-by: Yong Zhang <zhangyong1025.zy@gmail.com>
Co-authored-by: Zike Yang <zike@apache.org>
Co-authored-by: Lari Hotari <lhotari@apache.org>
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 <mmarshall@apache.org>

(cherry picked from commit 2b2e0c5)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>

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

Signed-off-by: Zixuan Liu <nodeces@gmail.com>

(cherry picked from commit 4378856)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>

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

(cherry picked from commit a68b58d)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>

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

(cherry picked from commit 9a2aeb2)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>

* 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 <nodeces@gmail.com>

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

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
(cherry picked from commit 79a97a9)

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

Co-authored-by: Lari Hotari <lhotari@users.noreply.github.com>
Co-authored-by: Yong Zhang <zhangyong1025.zy@gmail.com>
Co-authored-by: Zixuan Liu <nodeces@gmail.com>
Co-authored-by: tison <wander4096@gmail.com>

(cherry picked from commit 4190e40)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>

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

Signed-off-by: Zixuan Liu <nodeces@gmail.com>

(cherry picked from commit 94c7bf3)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>

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

Signed-off-by: Zike Yang <zike@apache.org>
Co-authored-by: Lari Hotari <lhotari@apache.org>

(cherry picked from commit d7f3558)
Signed-off-by: Zixuan Liu <nodeces@gmail.com>

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

Signed-off-by: Zixuan Liu <nodeces@gmail.com>

* [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 <nodeces@gmail.com>

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

From apache#22415

Signed-off-by: Zixuan Liu <nodeces@gmail.com>

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

Signed-off-by: Zixuan Liu <nodeces@gmail.com>

* [fix][build] Fix ubuntu mirror

Signed-off-by: Zixuan Liu <nodeces@gmail.com>

* [fix][build] Fix license

Signed-off-by: Zixuan Liu <nodeces@gmail.com>

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

Signed-off-by: Zixuan Liu <nodeces@gmail.com>

---------

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
Signed-off-by: Zike Yang <zike@apache.org>
Co-authored-by: Lari Hotari <lhotari@users.noreply.github.com>
Co-authored-by: Matteo Merli <mmerli@apache.org>
Co-authored-by: Michael Marshall <mmarshall@apache.org>
Co-authored-by: tison <wander4096@gmail.com>
Co-authored-by: Yong Zhang <zhangyong1025.zy@gmail.com>
Co-authored-by: Zike Yang <zike@apache.org>
Co-authored-by: Lari Hotari <lhotari@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-not-needed Your PR changes do not impact docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build the docker image without dockerfile-maven-plugin
5 participants