CUSTCOM-130 Better compatibility of docker images #4594

dmatej · 2020-03-27T09:01:14Z

Description

Fixes incompatibility with debian/ubuntu based JDK11 images
Fixes warning about unsupported JVM option
Adds features - JVM memory settings for Micro and Server-Full and JVM_ARGS for Micro
All Dockerfiles use same commands for same things
URL for gpg can be changed with -D parameter

Important Info

Testing

JDK8

mvn clean install -PBuildExtras,BuildDockerImages -pl :docker-images -amd

JDK11

mvn clean install -PBuildExtras,BuildDockerImages -pl :docker-images -amd -Ddocker.payara.tag=5.202-SNAPSHOT-JDK11 -Ddocker.java.repository=adoptopenjdk/openjdk11 -Ddocker.java.tag=jdk-11.0.6_10-debian-slim

Payara4

Build some Payara4 versions including all required distribution packages
This will generate docker images
PayaraServerNodeTest will fail, because there is a difference in syntax, but other images should pass their respective tests

mvn clean install -PBuildExtras,BuildDockerImages -pl :docker-images -amd -Dpayara.version=4.1.2.191.14-SNAPSHOT -Ddocker.payara.tag=4.1.2.191.14-SNAPSHOT -Ddocker.payara.rootDirectoryName=payara41 -Ddocker.payara.domainName=payaradomain

Changing env variables, extending

docker create --name payaramicroxxx --env "MEM_XSS=2m" --env MEM_MAX_RAM_PERCENTAGE=25.0 payara/micro:latest
# replace dcb7e7328f90 with a result of docker create
docker start dcb7e7328f90
docker logs dcb7e7328f90

Notes

I need a tester with Mac and Windows, all tests were done on Linux, but I know that other systems have limitations on Docker and TestContainers.
EDIT: passed on Kubuntu, Windows 10 and Mac

appserver/extras/docker-images/micro/src/main/docker/Dockerfile

dmatej · 2020-03-27T09:08:27Z

appserver/extras/docker-images/micro/src/main/docker/Dockerfile

    DEPLOY_DIR=/opt/payara/deployments

 # Create and set the Payara user and working directory owned by the new user
 RUN true \
-    && addgroup payara \
-    && adduser -D -h ${PAYARA_HOME} -H -s /bin/bash payara -G payara \
+    && addgroup --gid 1000 payara \


Evil block - this works in alpine, debian and ubuntu

appserver/extras/docker-images/server-full/src/main/docker/Dockerfile

rdebusscher

The memory changes is part of QACI-40

appserver/extras/docker-images/micro/src/main/docker/Dockerfile

dmatej · 2020-03-27T11:41:11Z

The memory changes is part of QACI-40

There is no PR, memory changes are not even mentioned in description and finally - Patrik already did it in micro here. So I only updated the full server dockerfile with same settings.
Also I think original fraction value 1 was a bad idea, because there is also off-heap memory ... I doubt even about the 90% value ... but we need performance tests for this.

rdebusscher · 2020-04-06T15:27:43Z

Use an environment variable to define the MaxRamPercentage as suggested here https://github.com/payara/docker-payaraserver-full/pull/118/files

dmatej · 2020-04-06T16:15:55Z

Use an environment variable to define the MaxRamPercentage as suggested here https://github.com/payara/docker-payaraserver-full/pull/118/files

This should go to standalone issue and PR, here I only "synchronized" it between containers. At this moment this PR is useful for both Patrik's and my work, other changes should have own issue and also testing. Also I don't think this option is sufficient, there are at least other three extremely important (ie -XX:MaxDirectMemorySize - probably 90% fraction for heap is too much).
Again, this is for another discussion and testing.

poikilotherm · 2020-04-06T16:21:30Z

As the author of the mentioned PR: good to know development moved here. What do you want me to do? This stuff is crucial IMHO

dmatej · 2020-04-06T16:35:57Z

As the author of the mentioned PR: good to know development moved here. What do you want me to do? This stuff is crucial IMHO

Probably only wait, we did more changes and at this moment we have some internal discussion about it ;)

I expect we will push and merge the final version soon, very soon ;)

poikilotherm · 2020-04-06T17:52:49Z

Great. I'm really eager to release Dataverse images based on Payara 5 :-)

appserver/extras/docker-images/micro/src/main/docker/Dockerfile

appserver/extras/docker-images/server-full/src/main/docker/Dockerfile

dmatej · 2020-04-07T14:35:23Z

Jenkins test please

rdebusscher

Probably due to he change from java to sh in the entrypoint, this doesn't work anymore.

FROM payara/micro:5.202-SNAPSHOT
ADD testapp.war /opt/payara/deployments

No app is deployed anymore

…ing the MaxRAMPercentage currently. Can be removed later, when upstream merged payara/Payara#4594

rdebusscher · 2020-04-17T10:29:52Z

The Image for Payara Micro looks good and Application deployment is ok again.

But the PayaraMicroTestfails

ERROR [main] docker[payara/micro:5.202-SNAPSHOT]:472 - Could not start container
com.github.dockerjava.api.exception.InternalServerErrorException: Mounts denied:
path /var/folders/2p/g987c05s2f1d8tw5pzhr4tt00000gn/T/TestServlet7784738673461879995.war

dmatej · 2020-04-17T10:52:41Z

Nevermind, I used temporary file, and it seems on your computer it cannot be mapped into the container. I will change it to use the target folder instead, I hope it will be enough.

- adduser has different syntax in each distribution

- it often fails, so you can use only -Ddocker.verbose to watch what it does and to see why it failed. - the url is not filtered from pom.xml file, so you can use another url ie. -Ddocker.keyserver.url=hkp://ipv4.pool.sks-keyservers.net or -Ddocker.keyserver.url=hkp://p80.pool.sks-keyservers.net:80

- all dockerfiles use same mechanisms and conventions - useradd and groupadd are not in alpine by default - adduser and addgroup abbreviated parameters are ambiguous on debian/ubuntu - => removed coreutils dependency, using adduser and addgroup with long params - it is also a bit faster - tested with - mvn clean install -PBuildExtras,BuildDockerImages -pl :docker-images -amd - mvn clean install -PBuildExtras,BuildDockerImages -pl :docker-images -amd -Ddocker.java.repository=adoptopenjdk/openjdk11 -Ddocker.java.tag=x86_64-debian-jdk-11.0.5_10

…entage - it was already used in micro - JDK8 supports it since u191 too

- server-node - no changes, configuration is obtained from DAS - server-full and micro - basic configuration for DAS - MEM_MAX_RAM_PERCENTAGE aka -XX:MaxRAMPercentage - 70% of host memory by default - the rest for OS+NIO+stacks - MEM_XSS aka Xss - default 512k; enough for most applications - micro - JVM_ARGS - possibility to add more JVM options - hint for extending these docker images: read docker create --help

- deployment did not work - extended test to deploy a trivial servlet application

- verification of tini and user management is done only once, then the image is shared - this image has caching enabled by default - only micro has it's own, because it uses leaner jdk image based on alpine - renamed some properties to use common scheme - PayaraServerNodeTest with lower timeouts as it waits for the DAS; if something went wrong, it hanged for too long

dmatej · 2020-04-19T09:14:17Z

I need someone to verify that everything works as expected as I described in the description in other than Linux systems: Mac and Windows.
Currently I believe that @rdebusscher's problem is now fixed, but to believe is not enough.
In last PR I also made build much faster - now three of four images use the same precreated image, only micro is alpine-based to be lean as possible.

dmatej · 2020-04-19T09:20:54Z

Jenkins test please

- sometimes it was not enough

rdebusscher

Change order of COPY statements to allow better re-usage of layers.

appserver/extras/docker-images/micro/src/main/docker/Dockerfile

appserver/extras/docker-images/server-full/src/main/docker/Dockerfile

appserver/extras/docker-images/server-node/src/main/docker/Dockerfile

pdudits

Tested on Windows, passes after small changes:

Had add .gitattributes for Windows, and brought back CMD support for Payara Micro.

pdudits · 2020-04-20T10:36:34Z

The necessary changes are in https://github.com/dmatej/Payara/pull/3

New shell folder, new .gitattributes

- deploymentDir is now CMD concern, overrideable by user - shell executes exec, so that java becomes the main process of container - shell expansion is not available in CMD, therefore DEPLOY_DIR used expanded, but this is usual Docker behavior

dmatej · 2020-04-20T13:09:16Z

Jenkins test please

- removed unuseful comments - sync with other docker files - tested - note: even if environment variables are not used here, they may be used by users

dmatej · 2020-04-20T15:24:07Z

Jenkins test please

poikilotherm · 2020-04-21T08:57:29Z

appserver/extras/docker-images/basic/src/main/docker/Dockerfile

+    MEM_XSS="512k"
+ENV PATH="${PATH}:${PAYARA_DIR}/bin"
+
+ARG TINI_VERSION=v0.18.0


Heads up that tini v0.19 has been released two days ago.
Releases now offer SHA256 sums, thus easing installation A LOT!
https://github.com/krallin/tini/releases/tag/v0.19.0

See also krallin/tini#153

I think we will merge this first and then upgrade this in standalone PR, but thanks for the good news!

poikilotherm · 2020-04-29T13:15:31Z

Congrats getting this huge PR done and merged 🎉 🥳
Looking forward to build awesome stuff with it!

dmatej requested a review from pdudits March 27, 2020 09:01

dmatej self-assigned this Mar 27, 2020

dmatej commented Mar 27, 2020

View reviewed changes

appserver/extras/docker-images/micro/src/main/docker/Dockerfile Outdated Show resolved Hide resolved

dmatej commented Mar 27, 2020

View reviewed changes

pdudits approved these changes Mar 27, 2020

View reviewed changes

appserver/extras/docker-images/server-full/src/main/docker/Dockerfile Outdated Show resolved Hide resolved

appserver/extras/docker-images/server-full/src/main/docker/Dockerfile Outdated Show resolved Hide resolved

rdebusscher suggested changes Mar 27, 2020

View reviewed changes

appserver/extras/docker-images/micro/src/main/docker/Dockerfile Outdated Show resolved Hide resolved

dmatej added the PR: DO NOT MERGE Don't merge PR until further notice label Mar 27, 2020

rdebusscher self-requested a review March 27, 2020 12:54

dmatej removed the PR: DO NOT MERGE Don't merge PR until further notice label Apr 6, 2020

poikilotherm reviewed Apr 7, 2020

View reviewed changes

appserver/extras/docker-images/micro/src/main/docker/Dockerfile Outdated Show resolved Hide resolved

poikilotherm reviewed Apr 7, 2020

View reviewed changes

appserver/extras/docker-images/server-full/src/main/docker/Dockerfile Outdated Show resolved Hide resolved

poikilotherm reviewed Apr 7, 2020

View reviewed changes

appserver/extras/docker-images/server-full/src/main/docker/Dockerfile Outdated Show resolved Hide resolved

rdebusscher suggested changes Apr 8, 2020

View reviewed changes

dmatej added the PR: DO NOT MERGE Don't merge PR until further notice label Apr 8, 2020

poikilotherm added a commit to gdcc/dataverse-kubernetes that referenced this pull request Apr 15, 2020

Add workaround for memory limitations in Payara upstream image not us…

26ca2ce

…ing the MaxRAMPercentage currently. Can be removed later, when upstream merged payara/Payara#4594

dmatej mentioned this pull request Apr 16, 2020

CUSTCOM-130 Add payara-web Docker Image #4628

Merged

David Matejcek added 5 commits April 18, 2020 12:09

CUSTCOM-130 Fixed incompatibility with debian-based docker images

e4869d1

- adduser has different syntax in each distribution

CUSTCOM-130 Upgraded TestContainers and removed unused imports from test

75a5939

CUSTCOM-130 MaxRAMFraction replaced by UseContainerSupport+MaxRAMPerc…

752c21e

…entage - it was already used in micro - JDK8 supports it since u191 too

David Matejcek added 7 commits April 18, 2020 12:09

CUSTCOM-130 Reverting ADD based on Patrik's review

010c3d0

CUSTCOM-130 Removed obsoleted JVM options from server-full dockerfile

27000d9

CUSTCOM-130 Fixed Payara Micro docker image

540fc57

- deployment did not work - extended test to deploy a trivial servlet application

CUSTCOM-130 Using target directory in tests instead of temporary file

646f5d6

CUSTCOM-130 Same rules for payara-web docker image

1468ce1

dmatej requested review from Pandrex247, rdebusscher and pdudits April 19, 2020 09:09

CUSTCOM-130 Longer timeouts for tests

39dc236

- sometimes it was not enough

rdebusscher suggested changes Apr 20, 2020

View reviewed changes

pdudits approved these changes Apr 20, 2020

View reviewed changes

pdudits and others added 4 commits April 20, 2020 13:57

CUSTOM-130: Fix line endings for windows

f807bef

New shell folder, new .gitattributes

CUSTCOM-130 Reorder copy commands for better layering

a2017f7

CUSTCOM-130 Updated TestContainers and JUnit5

207c527

dmatej removed the PR: DO NOT MERGE Don't merge PR until further notice label Apr 20, 2020

dmatej requested a review from rdebusscher April 20, 2020 13:08

CUSTCOM-130 Cleanup in Micro Dockerfile

a617417

- removed unuseful comments - sync with other docker files - tested - note: even if environment variables are not used here, they may be used by users

poikilotherm reviewed Apr 21, 2020

View reviewed changes

rdebusscher approved these changes Apr 29, 2020

View reviewed changes

dmatej merged commit f540098 into payara:master Apr 29, 2020

dmatej deleted the CUSTCOM-130-docker-micro-jdk11 branch April 29, 2020 13:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CUSTCOM-130 Better compatibility of docker images #4594

CUSTCOM-130 Better compatibility of docker images #4594

dmatej commented Mar 27, 2020 •

edited

Loading

dmatej Mar 27, 2020

rdebusscher left a comment

dmatej commented Mar 27, 2020 •

edited

Loading

rdebusscher commented Apr 6, 2020

dmatej commented Apr 6, 2020 •

edited

Loading

poikilotherm commented Apr 6, 2020

dmatej commented Apr 6, 2020

poikilotherm commented Apr 6, 2020

dmatej commented Apr 7, 2020

rdebusscher left a comment

rdebusscher commented Apr 17, 2020

dmatej commented Apr 17, 2020

dmatej commented Apr 19, 2020

dmatej commented Apr 19, 2020

rdebusscher left a comment

pdudits left a comment

pdudits commented Apr 20, 2020

dmatej commented Apr 20, 2020

dmatej commented Apr 20, 2020

poikilotherm Apr 21, 2020 •

edited

Loading

dmatej Apr 21, 2020

poikilotherm commented Apr 29, 2020 •

edited

Loading

CUSTCOM-130 Better compatibility of docker images #4594

CUSTCOM-130 Better compatibility of docker images #4594

Conversation

dmatej commented Mar 27, 2020 • edited Loading

Description

Important Info

Testing

JDK8

JDK11

Payara4

Changing env variables, extending

Notes

dmatej Mar 27, 2020

Choose a reason for hiding this comment

rdebusscher left a comment

Choose a reason for hiding this comment

dmatej commented Mar 27, 2020 • edited Loading

rdebusscher commented Apr 6, 2020

dmatej commented Apr 6, 2020 • edited Loading

poikilotherm commented Apr 6, 2020

dmatej commented Apr 6, 2020

poikilotherm commented Apr 6, 2020

dmatej commented Apr 7, 2020

rdebusscher left a comment

Choose a reason for hiding this comment

rdebusscher commented Apr 17, 2020

dmatej commented Apr 17, 2020

dmatej commented Apr 19, 2020

dmatej commented Apr 19, 2020

rdebusscher left a comment

Choose a reason for hiding this comment

pdudits left a comment

Choose a reason for hiding this comment

pdudits commented Apr 20, 2020

dmatej commented Apr 20, 2020

dmatej commented Apr 20, 2020

poikilotherm Apr 21, 2020 • edited Loading

Choose a reason for hiding this comment

dmatej Apr 21, 2020

Choose a reason for hiding this comment

poikilotherm commented Apr 29, 2020 • edited Loading

dmatej commented Mar 27, 2020 •

edited

Loading

dmatej commented Mar 27, 2020 •

edited

Loading

dmatej commented Apr 6, 2020 •

edited

Loading

poikilotherm Apr 21, 2020 •

edited

Loading

poikilotherm commented Apr 29, 2020 •

edited

Loading