-
Notifications
You must be signed in to change notification settings - Fork 12
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
SLING-10229: DockerHub build - using hooks to build and push #27
base: master
Are you sure you want to change the base?
Conversation
ac40404
to
b44d37b
Compare
uses the DockerHub CI and hooks to build and tag and push the docker images. tags: latest, project-version, commit-sha1, snapshot or stable Signed-off-by: Stefan Bischof <stbischof@bipolis.org>
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.
Thanks for the PR, I think the approach is good. A couple of inline comments.
Note that we don't want to publish SNAPSHOTS from DockerHub since it's a public repository, we only publish releases after the PMC has approved them.
# tags the maven-generated image with the Docker-Hub estimated image-name:latest and additional tags | ||
|
||
# name of the image, given by pom.xml | ||
BASE_IMAGE_NAME="sling/feature-launcher" |
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.
Can we use ${IMAGE_NAME}
instead?
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.
but then the build will only work on the original apache/sling-org-apache-sling-feature-launcher.
if you want to fort it an run together with your docherhub account the build will fail
docker push $DOCKER_REPO:$SOURCE_COMMIT | ||
|
||
# additional tag with project version, | ||
PROJECT_VERSION=$(mvn help:evaluate -Dexpression=project.version -q -DforceStdout) |
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.
Do we need this? I think we could use ${DOCKER_TAG}
instead.
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.
If we only build on tag thennthia should be enough
docker push $DOCKER_REPO:$PROJECT_VERSION | ||
|
||
# additional tag with snapshot or stable | ||
SNAPSHOT_REGEX="^(.*)-SNAPSHOT" |
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.
No snapshots please
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
|
||
If you are running sling-feature-launcher as an container please use env vars. | ||
``` | ||
$docker run -it --rm --env FEATURE_FILES=https://path.to/feature-model.json apache/sling-org-apache-sling-feature-launcher: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.
Please use the new name.
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.
isn't that the new name? i am a bit confused. which name do you mean?
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.
I meant sling/feature-launcher
@@ -48,4 +55,36 @@ String[] arguments = new String[] { | |||
org.apache.sling.feature.launcher.impl.Main.main(arguments); | |||
``` | |||
|
|||
# Container | |||
|
|||
The container-image `apache/sling-org-apache-sling-feature-launcher:latest` is avaiable on DockerHub. |
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.
Please use the new name.
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.
isn't that the new name? i am a bit confused. which name do you mean?
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.
I meant sling/feature-launcher
|
||
Available tags: | ||
- **latest** - latest build, could be a `snapshot` **or** `release` version. | ||
- **snapshot** - latest build of a `*-SNAPSHOT` version. |
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.
No snapshots please
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
- **latest** - latest build, could be a `snapshot` **or** `release` version. | ||
- **snapshot** - latest build of a `*-SNAPSHOT` version. | ||
- **stable** - latest build of a **release** version. | ||
- **(version)** - latest build of a fixed version, could be a `snapshot - (1.1.9-SNAPSHOT)` **or** `release - (1.1.9)` version. |
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.
No snapshots please
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.
Could you be more explicit.
Will we do this only on creating a git tag?
This should be then tagged as:
-latest
-tagname
- git hash ?
No snapshot!
No stable?
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.
For more context - the ASF release policy says that we only publish releases, not CI artifacts. So for DockerHub, which is a public repository, we should only publish for git tags. A separate discussion is to maybe generate them manually, since after a tag we have a release vote, and only them the publication should happen.
So we should have:
- version - e.g.
1.0.20
- latest
I don't think git hash is useful so I would not use that.
apt-get install default-jdk maven -y | ||
java -version | ||
mvn -version | ||
mvn clean package -P container |
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.
Maybe this would be a good place to validate that:
- we don't build a snapshot version
- the base image name is the one we expect from the pom ( compare with
${IMAGE_NAME}
)
<build> | ||
<tags> |
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.
Shouldn't we keep this for simpler local builds?
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.
Sure.
A second question that I just thought of right now; maybe it's not a good idea to tie ourselves to a java 8 runtime. Can the build phase be executed with a Docker image? I think that would be more flexible. |
Signed-off-by: Stefan Bischof <stbischof@bipolis.org>
Signed-off-by: Stefan Bischof <stbischof@bipolis.org>
I did a test to run it with docker. the jar will be build ,but when starting the docker plugin it will miss a docker inside the container. and if there would be an docker inside the image will be in the registry inside the container.
best way would be to have a newer version of Ubuntu in the dockerhub build. Especially because the End of Standard Support of Ubuntu 16.04 LTS (Xenial) is April 2021 |
Signed-off-by: Stefan Bischof <stbischof@bipolis.org>
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
@stbischof - I noticed that I had some answers that were due, sorry. Let me know if you need anything else clarified. Thanks! |
I am still working on this. Just waiting for an reaction to the Issue on docker/hub-feedback#2091 . |
Got, thanks @stbischof . |
@stbischof - while we're waiting for news on the Docker side, maybe using something like https://github.com/GoogleContainerTools/jib/tree/master/jib-maven-plugin could help us advance? IIUC that plugin does not require a Docker daemon to run and can therefore be used in a Docker buid running on Docker Hub. |
thx will have a look |
@rombert could you please explain that the reasons was to not do it via github-ci ? |
@rombert
|
@stbischof - autobuilds should still be available to the ASF account. The reason for going with Autobuilds (IIRC) is that
If this becomes too much of a hassle, we should move this to another CI system, but preferrably Jenkins, not GitHub CI, since we already know how to manage that. |
@stbischof - thanks a lot for your efforts, this has been baking for quite some time... At this point do you need input from someone else from the Sling developer team? Is there something in terms of code/prototyping that we can help with? |
@rombert At the moment I started to do docker images using buildx and github actions to build images for: os: https://github.com/geckoprojects-org/org.geckoprojects.slinglauncher.docker I need to run this images on different platforms. DocherHub does not give an option to do this. |
@stbischof - so should we then close this PR, since you have already found an alternate solution? |
We could. Maybe this is an solution for sling? |
@stbischof - sorry, I think I was a bit vague. My question was if you intend to work on this pull request with the goal to eventually build all the needed Docker images in Sling. If you do, we should keep this open. I looked at the project you referenced and it's based on GItHub actions. We don't use them right now since we use Jenkins, but I guess we can either look into GH actions or come up with a Jenkins-based way of building them. So if you think that the current way is not ideal we can start a discussion on dev@sling.apache.org and find one that works for everyone. |
I will have a look on Jenkins. We will find a way if it is okay for you to accept it in sling. |
Sounds good! In the meantime, I can confirm that we do have docker installed on all Jenkins nodes (but not sure which version) and we do have a token for DockerHub for triggering automated builds and that should (hopefully) allow us to push images to the |
uses the DockerHub CI and hooks to build and tag and push the docker images.
tags: latest, project-version, commit-sha1