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

Add first Dockerfile implementation #1

Merged
merged 11 commits into from Jul 12, 2022
Merged

Conversation

buehner
Copy link
Member

@buehner buehner commented Oct 8, 2020

Based on https://github.com/geoserver/geoserver/wiki/GSIP-192 this is a first step towards an official GeoServer docker image.

This is work in progress, but some features are already implemented:

  • Extensions specified in the STABLE_EXTENSIONS environment variable will be downloaded and installed on container startup (before tomcat starts)
  • Marlin renderer is used
  • Recommended JVM production configuration

You can build with docker build -t geoserver:test .

You can run with docker run -it -e STABLE_EXTENSIONS='wps,csw' -p 8080:8080 geoserver:test (and pass a list of stable extensions).

More information is documented in the README.md.

TODOs:

  • CORS support
  • configuration of JNDI connections in the tomcat/custom tomcat configuration in general
  • default data for gs datadir?
  • possibility to add custom java dependencies or extensions to WEB-INF/lib
    • in this context: mount EXTENSION_DOWNLOAD_DIR and implement checks to cache extensions that have already been downloaded before?
  • log4j properties
  • possibility to add custom fonts
  • backwards compatability to which version (maybe 2.15.x ?)
  • docker-compose demo environment

Any feedback is appreciated!

@jodygarnett
Copy link
Member

No need for special treatment for oracle jdbc driver anymore, oracle changed their policy and we have been able to depend on it as a maven dependency and redistribute now.

Dockerfile Outdated
@@ -0,0 +1,57 @@
ARG TOMCAT_VERSION=9.0.38
ARG JAVA_VERSION=8
Copy link
Member

@mprins mprins Oct 13, 2020

Choose a reason for hiding this comment

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

I would stick to Java 11 which has Marlin built-in to save some hassle

I don't think anyone would want to run unsupported versions of GeoServer on Docker so sticking to Java 11 and latest stable Tomcat eg. "tomcat:jdk11-openjdk-slim" may make things easier

Copy link
Member Author

Choose a reason for hiding this comment

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

I switched to OpenJDK 11 in a03ee5a

README.md Outdated
* in this context: mount `EXTENSION_DOWNLOAD_DIR` and implement checks to cache extensions that have already been downloaded before?
* log4j properties
* add possibility to add custom fonts
* starting from which version we want to provide geoserver images (maybe 2.15.x?)/backwards compatability
Copy link
Member

Choose a reason for hiding this comment

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

supported versions are 17.x and up

Copy link
Member Author

Choose a reason for hiding this comment

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

17.x for technical reasons or because it was the latest version in Oct?

-XX:MaxGCPauseMillis=200 -XX:ParallelGCThreads=20 -XX:ConcGCThreads=5 \
-Xbootclasspath/a:${CATALINA_HOME}/lib/marlin-${MARLIN_VERSION}-Unsafe.jar \
-Dsun.java2d.renderer=org.marlin.pisces.MarlinRenderingEngine \
-Dorg.geotools.coverage.jaiext.enabled=${JAIEXT_ENABLED}"
Copy link
Member

Choose a reason for hiding this comment

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

some of these options may not be valid for Java 11 (don't know which off the top of my head)

Copy link
Member

Choose a reason for hiding this comment

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

I tend to run with Java 8 and settings very similar to what you have here.

The default for memory is 1/4 system memory, not sure what the docker image ends up?

The UseG1GC is the default for Java 11, also marlin is baked in if using Java 11.

The number of threads to devote to garbage collection is is a tradeoff based on the number of threads available.

Copy link
Member

Choose a reason for hiding this comment

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

not sure what the docker image ends up

it depends if the container is created with resource limits like --memory if the container tries to allocate more than allowed it will be killed, but users can specify those using env variables

docker/Dockerfile

Lines 20 to 21 in eee16c9

INITIAL_MEMORY="2G" \
MAXIMUM_MEMORY="4G" \

-Xms${INITIAL_MEMORY} -Xmx${MAXIMUM_MEMORY} \

Copy link
Member Author

Choose a reason for hiding this comment

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

I switched to OpenJDK 11 in a03ee5a which also adjusts the lines above

README.md Outdated
* default data for gs datadir?
* possibility to add custom java dependencies (for example the oracle jdbc driver)
* in this context: mount `EXTENSION_DOWNLOAD_DIR` and implement checks to cache extensions that have already been downloaded before?
* log4j properties
Copy link
Member

Choose a reason for hiding this comment

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

please set up a logging profile that does not spam standard out (stdout) like most of the defaults do

Copy link
Member

Choose a reason for hiding this comment

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

So you are recommending PRODUCTION.properties to start out with?

Copy link
Member

Choose a reason for hiding this comment

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

No, because PRODUCTION.properties still logs to stdout spamming the catalina logs / catalina.out /console with all it thinks we should know about:

https://github.com/geoserver/geoserver/blob/d09be5b2ebfe0593418a9ce04668be0b17ec3d61/src/main/src/main/java/PRODUCTION_LOGGING.properties#L18

I'd prefer something like PRODUCTION.properties with stdout removed, so the catalina logs will just tell me things about the servlet engine and not geoserver (because that's what the geoserver log is for)

Copy link
Member

Choose a reason for hiding this comment

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

I agree
Maybe we propose to change profile? I don't think it makes sense for production setups

Dockerfile Outdated
DEBIAN_FRONTEND=noninteractive \
EXTENSION_DOWNLOAD_DIR=/opt/geoserver_extension_downloads \
GEOSERVER_DATA_DIR=/opt/geoserver_data \
GEOWEBCACHE_CACHE_DIR=/opt/geowebcache_data
Copy link
Member

Choose a reason for hiding this comment

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

you may want to expose these as a volume so the can be backed up and migrated

Copy link
Member

Choose a reason for hiding this comment

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

The GEOSERVER_DATA_DIR is handy for outside access, easy way to stage data for publication.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. These directories should be mounted. I'll soon add a docker-compose demo for this


## install GeoServer extensions before starting the tomcat
echo "Starting installation of extensions"
/scripts/install-extensions.sh
Copy link
Member

Choose a reason for hiding this comment

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

I would not install extensions at run time. For several reasons:

  • does not work for community plugins
  • it would not work in an environment without internet access
  • extensions are downloaded every time the container is started. That's a lot of downloads
  • should be a temporary issue with sourceforge geoserver will start without the necessary plugins and not function properly (even fail to start?)

I think GeoServer war bundling should just not happen here but instead should be handled separately and documented in the README. Users still have several options available to customize the application like bind mount a locally available war or override the command / enrtypoint to actually download extensions at startup if they want to or actually change the dockerfile and ADD or COPY their war file directly into the docker image

Copy link
Member

Choose a reason for hiding this comment

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

See this example here: https://github.com/Gnafu/geoserver/blob/GSIP-176/build/cite/geoserver/Dockerfile#L39
At build time a user can pass a local file or an external URL to burn in the image or skip it entirely and rely on a bind mount to provide the war file at run time

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. I agree that downloading and installing extensions at run time should not be the default, but i'd really like to keep this as a configurable feature (disabled by default) for quick setups.

I generally agree (regarding war bundling) that it should also be possible to build offline by mounting all required resources (like customized war files).

I'll work on this sooner or later and add some changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added a flag to control whether extensions should be downloaded at runtime or not. default value is false: a4e3a83

Next i'll add a possibility to install extensions from a mounted directory.

Copy link
Member Author

@buehner buehner Dec 17, 2020

Choose a reason for hiding this comment

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

Maybe you also like to have a look at 4261b1b which introduces the possibility to add custom jars/libs for now.

It will add libs from a mounted directory on startup. I think i'll also add the possibility to install libs on build time, but not on run time...

@beginor
Copy link

beginor commented Dec 11, 2020

how about this geoserver docker image repo beginor/docker-geoserver and I have already pushed it on docker hub https://hub.docker.com/r/beginor/geoserver , you guys can just pickup it.

@buehner
Copy link
Member Author

buehner commented Dec 17, 2020

how about this geoserver docker image repo beginor/docker-geoserver and I have already pushed it on docker hub https://hub.docker.com/r/beginor/geoserver , you guys can just pickup it.

thx for your comment. if you miss any features, feel free to discuss and open pull requests

@jodygarnett
Copy link
Member

This repository is a place to collaborate, it will be easier once we have a release :)

@buehner
Copy link
Member Author

buehner commented Jan 13, 2021

@jodygarnett so you think we should merge this PR to have a better starting point for pull requests?

@buehner
Copy link
Member Author

buehner commented Jan 29, 2021

In 82e350d i enabled the usage of a custom WAR file build from local machine. This is mainly the approach provided by #2

@jodygarnett
Copy link
Member

Can we do a 2.19-RC for this? You can release a docker image as early access or something just go get the party started.

@buehner
Copy link
Member Author

buehner commented Mar 11, 2021

Of course i can care about this @jodygarnett
How do you want this to be done from a technical perspective? Just an image "somewhere" on docker hub? Or integrated in some sort of build process?

@jodygarnett
Copy link
Member

I am just learning about docker:

Please coordinate on the email list!

@buehner
Copy link
Member Author

buehner commented Mar 19, 2021

@jodygarnett @randomorder @lpasquali

I also published a version of this approach to docker hub:

docker run -d --rm -p 8888:8080 --name geoserver terrestris/geoserver:2.19-RC_GSIP-192

I'll try to find some time until the end of next week to continue the comparison on https://github.com/geoserver/docker/wiki
Next step would be the comparison of the resulting images.

@jacobwod
Copy link

jacobwod commented May 3, 2021

Great work @buehner! I think that the possibility to set pass STABLE_EXTENSIONS to docker run is particularly useful in production setups. 🚀

One thing I can't figure out, however, is this: how can I point Docker to the GS data dir which is located outside the container? I'm aware of docker build --build-arg GS_DATA_PATH={RELATIVE_PATH_TO_YOUR_GS_DATA}, but my understanding is that this affects the container at build time and "bakes in" the data dir, which is not what I'm looking for. My data dir is basically a directory /mnt/geoserver_data_dir, and I'd like to do something like:

docker run -d --rm -p 8888:8080 -e STABLE_EXTENSIONS='wps,csw' -e DATA_DIR='/mnt/geoserver_data_dir' --name geoserver terrestris/geoserver 

One super-important reason for this approach is the ability to share data dir between GS instances running in Docker. Any ideas on how this can be achieved?

@jacobwod
Copy link

jacobwod commented May 4, 2021

As a reply to my last comment, binding existing data directory into the container can be easily done. Here's how I've setup the terrestris/geoserver:2.19.0 tag in our environment:

docker run -it -p 80:8080 -e STABLE_EXTENSIONS='oracle,css' -v /mnt/gisdata/geoserver_data_dir:/opt/geos
ver_data --name geoserver terrestris/geoserver:2.19.0

I have mainly one question remaining: can we find a way to customise Tomcat's server.xml within the container? One use case would be to customise the access log format, which we currently do in server.xml:

<Valve  className="org.apache.catalina.valves.AccessLogValve" 
  directory="/mnt/gisdata/geoserver_data_dir/logs"
  prefix="access_node" suffix=".log"
  pattern="%{X-Forwarded-For}i %h %l %{X-Control-Header}i %t &quot;%r&quot; %s %b &quot;%{Referer}i&quot; &quot;%{User-Agent}i&quot;"
  renameOnRotate="true" />

@weskamm
Copy link
Contributor

weskamm commented Jun 22, 2022

We just updated this PR with latest additions and enhancements. This includes e.g. customizable tomcat version and fonts.
Maybe @buehner also wants to have a look?

Copy link
Member Author

@buehner buehner left a comment

Choose a reason for hiding this comment

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

Using docker-compose -f docker-compose-demo.yml up --build leads to

Step 33/40 : COPY $GS_DATA_PATH $GEOSERVER_DATA_DIR
ERROR: Service 'geoserver' failed to build : COPY failed: file not found in build context or excluded by .dockerignore: stat geoserver_data/: file does not exist

I think this should be fixed?

Dockerfile Outdated
# Environment variables
ENV CATALINA_HOME=/opt/apache-tomcat-${TOMCAT_VERSION}
ENV GEOSERVER_VERSION=$GS_VERSION
ENV GEOSERVER_DATA_DIR=#######/opt/geoserver_data/
Copy link
Member Author

Choose a reason for hiding this comment

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

Why #######? ;-)

Copy link
Contributor

Choose a reason for hiding this comment

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

woops, fixed!

@LukasLohoff
Copy link

We could use this github action as a starting point for automated builds: https://github.com/terrestris/docker-geoserver/blob/master/.github/workflows/build-and-push-docker.yml

@buehner
Copy link
Member Author

buehner commented Jun 23, 2022

It's working fine now for me. LGTM

@weskamm
Copy link
Contributor

weskamm commented Jun 29, 2022

@buehner Can we remove the "WIP" from the title please?
@jodygarnett Can this be merged than?

@jodygarnett
Copy link
Member

Yes you can remove the WIP, and you can merge when you are ready (this is your project to collaborate on).

When you are ready (and a build image is available) you can make a PR to https://github.com/geoserver/geoserver/blob/main/doc/en/user/source/installation/index.rst

@aaime
Copy link
Member

aaime commented Jun 29, 2022

Given the recent changes, how does it compare with #2 ?
Have Alessandro/Luca addressed most of the feedback they received?

@randomorder
Copy link
Member

Hi @aaime
I had a very quick look at this and overall look ok to me. If you don't mind I'would take a closer look next week before merging this PR and closing #2

@randomorder randomorder self-requested a review June 30, 2022 12:41
@buehner buehner changed the title WIP: Add first Dockerfile implementation Add first Dockerfile implementation Jul 4, 2022
@weskamm
Copy link
Contributor

weskamm commented Jul 12, 2022

Still any objections?

@randomorder
Copy link
Member

nope, fine by me thanks

@buehner
Copy link
Member Author

buehner commented Jul 12, 2022

OK, thanks. Then I'll merge now ;-)

@buehner buehner merged commit 0f165ec into geoserver:master Jul 12, 2022
@jodygarnett
Copy link
Member

congrats!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
9 participants