eiffel-community / eiffel-remrem-generate

Apache License 2.0
8 stars 70 forks source link

Use built-in Tomcat in Docker deployments #142

Closed magnusbaeck closed 4 years ago

magnusbaeck commented 4 years ago

Description

The project's Dockerfile starts from a Tomcat image and drops the eiffel-remrem-generate war file into the webapps directory, thus running the app via Tomcat. In the Docker case we should use the Spring-provided Tomcat and run it standalone as described in https://eiffel-community.github.io/eiffel-remrem-generate/serviceUsage.html.

I'm not a Java expert so there might be legitimate reasons for the current design, but it strikes me as odd.

Motivation

Main points:

I'm sure there are reasons why one would want to run eiffel-remrem-generate in a "real" Tomcat instance, in particular if you want to run multiple apps in the same servlet container, but the reasonable way of doing that would be to run a service-agnostic Tomcat container and drop the war files in its webapps directory, in which case you wouldn't need a Docker image for eiffel-remrem-generate.

tobiasake commented 4 years ago

Hi, The Eiffel Operation at Ericsson uses Tomcat instances and we have been asked to use and test Eiffel applications as webapps in Tomcat instance, thats why we use Tomcat in the Dockerfile/Docker image. Just to make sure that the application works in Tomcat.

magnusbaeck commented 4 years ago

Shouldn't the contents of the eiffel-community repositories be tailored to general use rather than Ericsson's internal preferences? If you want to run tests based on the tomcat:8-jre image we can have a dockerfile for that but the dockerfile recommended for standalone use (and whose image could be subject to Docker Hub publication) should be tailored to the general use case.

I compared the memory consumption of the current image with one where we won't run a standalone Tomcat and there's no significant difference (550 vs. 650 MB as reported by cAdvisor), but I can confirm that skipping the standalone Tomcat solves https://github.com/eiffel-community/eiffel-remrem-publish/issues/177.

tobiasake commented 4 years ago

I really don't see the general usage view is impacted of which base image Dockerfile is based on, Tomcat image or based on a more pure Java Docker base image. Works more or less the same from a usage view. I have no problem with not using Tomcat base image and using a more pure Java based Docker image, e.g. fabric8/java-jboss-openjdk8-jdk As long as Docker image is configured and executed in the same way.

magnusbaeck commented 4 years ago

To be clear, the base image doesn't matter, but it does matter if the container's entrypoint is catalina.sh run or java -jar ...war. Let me summarize the most important problems with the current image:

I can prepare a PR that renames Dockerfile to Dockerfile.tomcat and adds a new Dockerfile.standalone (or .springboot?) that's based on a JRE-only image and runs java -jar .....war.

tobiasake commented 4 years ago

As I said, its ok to change to java base image, if you ask me. If you change to the docker image I suggested in my previous comment, then war file only need to be placed under "/deployments" and update the start-service.sh script to start java with command: /deployments/run-java.sh. (I have used this image earlier and it works) I prefer to only have one Dockerfile, so we don't need to maintain 2 Dockerfiles. So update the existing Dockerfile.

And, Yes, RemRem documentation is not covering everything and might be outdated. We have github issues for improving documentation.

We have Docker configuration examples in Eiffel-Intelligence documentation: https://github.com/eiffel-community/eiffel-intelligence/blob/master/wiki/markdown/docker.md RemRem docker images/container works in same way. Java/Spring properties for RemRem-Generate can be found under src/main/resources: https://github.com/eiffel-community/eiffel-remrem-generate/blob/master/service/src/main/resources/application.properties

You can create a PR and then you might get more input from more Eiffel-Community developers, hopefully :)

magnusbaeck commented 4 years ago

PR was merged, closing issue. I'll send a very similar PR for REMReM Publish.