Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[refactor][ci] Build the docker image with docker-maven-plugin #17148
Changes from all commits
ea7f778
306f2e2
8855363
02898e0
85e3aaf
17e33aa
f6141c7
dc7392c
5037bb1
25b640d
8c067e4
9dd032c
833ce56
77f3fd8
a2c0e1a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
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.
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 ofpulsar:latest
.There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to change the image publish process
Please see: https://github.com/apache/pulsar/blob/master/wiki/release/release-process.md#10-publish-docker-images
There was a problem hiding this comment.
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
.There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.