Conversation
- adjust `tomcat:8.0` to `tomcat:8.0-jre7` (to be explicit about "Tomcat 8.0" and "Java 7") - adjust `tomcat:8-jre8` to `tomcat:8.0-jre8` so that if/when Tomcat 8.5 becomes stable, we don't unexpectedly switch (so that switching Tomcat release tracks becomes a concious choice instead) - add back `GN_DOWNLOAD_MD5`, and a simple `update.sh` script for updating the value (when `geonetwork.war` is updated, this value will need to be updated so that we downstream know to rebuild, and so that the cache busts naturally) - usage: `./update.sh` (to update all versions) or `./update.sh 3.2.0 3.0.5` to update one or more versions - combine `RUN` lines into a single line so that the `.war` file is completely removed, resulting in a smaller overall image - use the more reliable `export CATALINA_OPTS="$CATALINA_OPTS ..."` in the entrypoint rather than doing a fragile `sed` on a Tomcat startup file - small adjustments to `postgres/docker-entrypoint.sh` to make it more reliable at updating configuration and to allow it to run more than once in a single container safely (`docker restart my-geonetwork`, for example) - recreate the proper contents of `3.0.4/postgres/docker-entrypoint.sh` which appear to have been accidentally overwritten in a previous commit - make whitespace consistent within each file
|
|
||
| RUN apt-get update && apt-get install -y postgresql-client && \ | ||
| apt-get clean && rm -rf /var/lib/apt/lists/* | ||
| rm -rf /var/lib/apt/lists/* |
There was a problem hiding this comment.
apt-get clean is performed automatically by the base image (see https://github.com/docker/docker/blob/9c2f1669a0b556115f543a0f02bf1ea5576b7f71/contrib/mkimage/debootstrap#L82-L105)
| rm ~/.pgpass | ||
|
|
||
| #Write connection string for GN | ||
| sed -ri '/^jdbc[.](username|password|database|host|port)=/d' "$CATALINA_HOME"/webapps/geonetwork/WEB-INF/config-db/jdbc.properties |
There was a problem hiding this comment.
This removes any existing lines so that the following can simply append -- this is what makes sure we can safely run this more than once in a single container (which docker restart is the most common instigator of).
| -e 's/^(ENV GN_DOWNLOAD_MD5) .*/\1 '"$md5"'/' \ | ||
| -e 's/^(FROM geonetwork):.*/\1:'"$version"'/' \ | ||
| "$version/Dockerfile" "$version/postgres/Dockerfile" | ||
| done |
There was a problem hiding this comment.
The reason why I created the check against the actual online md5 rather than a hardcoded value, was because I realized that the war (and the checksum) had been re-generated for exactly the same (major-minor) version. The update script approach would obviously fail to address that, because we would be unaware that the file had changed and so we would not run the script...
There was a problem hiding this comment.
Sure, that's fair, but from the perspective of the official images, we definitely won't know the WAR file changed, and certainly won't auto-rebuild unless there's something more active to bust the cache and trigger a rebuild (hence the manual MD5 update). This also helps add a little bit of process to ensure that the WAR file update is intentional and not malicious, for example.
On many of the repos we maintain, we have an automated process which runs our ./update.sh scripts, and I'd be happy to discuss adding this repo to our existing tooling. 👍
| COPY ./docker-entrypoint.sh /entrypoint.sh | ||
| ENTRYPOINT ["/entrypoint.sh"] | ||
|
|
||
| CMD ["catalina.sh", "run"] |
There was a problem hiding this comment.
Do you need to explicitly add this CMD when it is already been declared on the base image (geonetwork:3.0.4)?
There was a problem hiding this comment.
This is necessary because we specify ENTRYPOINT again -- when re-specified, ENTRYPOINT will reset CMD, so we have to be explicit about it.
|
Thanks for the PR and for your explanations @tianon : I have only two comments/questions (above); otherwise, I am happy to accept it :) 👍 |
2 participants
tomcat:8.0totomcat:8.0-jre7(to be explicit about "Tomcat 8.0" and "Java 7")tomcat:8-jre8totomcat:8.0-jre8so that if/when Tomcat 8.5 becomes stable, we don't unexpectedly switch (so that switching Tomcat release tracks becomes a concious choice instead)GN_DOWNLOAD_MD5, and a simpleupdate.shscript for updating the value (whengeonetwork.waris updated, this value will need to be updated so that we downstream know to rebuild, and so that the cache busts naturally)./update.sh(to update all versions) or./update.sh 3.2.0 3.0.5to update one or more versionsRUNlines into a single line so that the.warfile is completely removed, resulting in a smaller overall imageexport CATALINA_OPTS="$CATALINA_OPTS ..."in the entrypoint rather than doing a fragilesedon a Tomcat startup filepostgres/docker-entrypoint.shto make it more reliable at updating configuration and to allow it to run more than once in a single container safely (docker restart my-geonetwork, for example)3.0.4/postgres/docker-entrypoint.shwhich appear to have been accidentally overwritten in a previous commitI'm happy to expound on any of these changes or rebase/amend/adjust/discuss as desired. 👍 ❤️
(I figured a PR would be an easier way to get my exact meaning across for all of these.)