-
Notifications
You must be signed in to change notification settings - Fork 38
feat: Pushing example server docker container to Dockerhub #44
feat: Pushing example server docker container to Dockerhub #44
Conversation
17606ac
to
7b753ad
Compare
server/push-script.sh
Outdated
#!/bin/sh | ||
|
||
#use TAG env. variable to create the container with the given tag | ||
TAG="${TAG:-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.
Could it be both latest and actual tag?
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.
It works this way - when something is published to master
it makes it without tag, so it's published as latest
, on release it provides the TAG
.
It can happen that the latest
and latest released version are not the same. I'm not sure, how to fix it.
Maybe build it only on release and make it publish both version with the tag and 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.
I meant that on release should also update latest. This will fix it.
EDIT: Yes. Exactly your suggestion. That was the whole point. Always update latest, on release push additionally to tag.
.circleci/config.yml
Outdated
cd server | ||
[ -z "$DOCKERHUB_USERNAME" ] && echo "Undefined DOCKERHUB_USERNAME, skipping publish" && exit 0; | ||
docker login -u $DOCKERHUB_USERNAME -p $DOCKERHUB_PASSWORD | ||
TAG=$CIRCLE_TAG ./push-script.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.
Maybe we can have everything above inside this script?
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.
moved
Small improvements suggested. It's more important if that works. |
Please don't merge. It's not working yet. |
CONTAINER_LATEST="$NAMESPACE/voyager-server-example-task:latest" | ||
|
||
echo "Building docker container $CONTAINER" | ||
docker build -f Dockerfile -t "$CONTAINER" "$WORKDIR" && docker push "$CONTAINER" && docker push "$CONTAINER_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.
IMO, it's much better to put these statements on their own lines. There is no need to group them together because failure in any of them will fail the build. It makes the line harder to read and during failures in CI it makes the error output more difficult to debug because you're wondering which statement failed.
Additionally, the docker push $CONTAINER_LATEST
statement will likely fail here because you need to tag the image. You need to do something like docker build -f Dockerfile -t $CONTAINER -t CONTAINER_LATEST
that will ensure the image is tagged with both the version tag and with 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 is docker tag
command. It will add new tag there. No need to build it again.
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.
Sorry I added it after you added your comments.
5707642
to
90d400f
Compare
90d400f
to
0b7c463
Compare
* feat: pushing container for server example * Addressing PR comments
* feat: pushing container for server example * Addressing PR comments
Description
https://issues.jboss.org/browse/AEROGEAR-8437
Automated publishing of
aerogear/voyager-server-example-task
container to Dockerhub.Tested in my fork https://circleci.com/gh/wojta/voyager-ionic-example/9