Docker support #6 #17

sergeytrasko · 2017-09-26T05:59:20Z

sergeytrasko · 2017-09-26T06:56:55Z

Open questions:

I'm attaching volume that references local gradle caches. Main reason - speed-up container initialization and start-up. However it is not a clean environment - leads to dependencies to local file system that might have some side-effects (like locally installed dependency).
Tests container still relies on .env file. Should it be placed inside container?

Production image

sergeytrasko · 2017-09-26T12:19:44Z

For production image I had to use 'chown'. There is a related PR merged (moby/moby#34263), however it's not released yet (https://github.com/docker/docker-ce/releases/tag/v17.09.0-ce-rc1)
Also see moby/moby#6119 for details

trioletas · 2017-09-26T12:51:03Z

Dockerfile

@@ -0,0 +1,21 @@
+FROM gradle:4.1-jdk8


Can we use gradle:4.1-jdk8-alpine instead ?

Yes, we can. Adjusted.

trioletas · 2017-09-26T12:52:09Z

Dockerfile

+ENV HOME /home/gradle
+
+RUN mkdir -p $HOME/app \
+    && cd $HOME/app


The same is achieved with WORKDIR $HOME/app

Thanks, fixed

trioletas · 2017-09-26T12:53:08Z

Dockerfile

+
+WORKDIR $HOME/app
+
+USER root


docker engine runs containers as root by default, is this required?

'gradle' image creates own 'gradle' user and sets it as current user. Therefore switch back to 'root' to do all pre-cooking is required.
Added comment to Dockerfile.dev

trioletas · 2017-09-26T12:55:19Z

Dockerfile

@@ -0,0 +1,21 @@
+FROM gradle:4.1-jdk8
+
+ENV HOME /home/gradle


If this is supposed to be a root directory for our application I would use something like /opt/app instead, /home/gradle is a bit misleading because it's not just gradle which is copied there

trioletas · 2017-09-26T12:59:17Z

Dockerfile

+
+EXPOSE 8080
+
+CMD ["gradle", "bootRun"]


Is gradle really required to run our application? To minimize the image size and reduce the amount of processes running in a container I would suggest that we compile that app first, create an executable jar, then start over from a blank base JDK image and copy only the jar there. This is quite easily achieved with https://docs.docker.com/engine/userguide/eng-image/multistage-build/

trioletas · 2017-09-26T13:00:41Z

Dockerfile.dev

+COPY .env .
+COPY settings.gradle .
+
+CMD ["gradle", "bootRun"]


Consider using ENTRYPOINT instead of CMD to use the same Dockerfile for dev and test, see https://github.com/ctco-dev/nodejs-graphql-template/blob/issue-11-docker/Dockerfile.dev

Thanks, adjusted

trioletas · 2017-09-26T13:06:17Z

api/build.gradle

@@ -26,6 +26,7 @@ apply from: 'dotenv.gradle'

 bootRun() {
  environment << dotenv
+  jvmArgs = ['-agentlib:jdwp=transport=dt_socket,server=y,suspend=n,address=5005']


Can we extract this to Dockerfile or docker-compose ?

As discussed, there is a option to run the application using gradle from CLI and attach remote debugger from IDE.
Agree, that it is not very realistic case, but let's keep door open unless we have better solution

Part of PR feedback addressed

Part of PR feedback addressed - production build

trioletas · 2017-09-27T06:47:36Z

Dockerfile

+# Create new user to run application on behalf of
+RUN addgroup -S -g 1001 app \
+	&& adduser -D -S -G app -u 1001 -s /bin/ash app \
+    && chown -R app:app /opt/app


Please align arguments vertically for better readability

Formatting

trioletas · 2017-09-27T14:31:03Z

Dockerfile

+RUN mkdir -p /opt/app
+WORKDIR /opt/app
+
+COPY .env .


I don't think that we should copy .env for prod build

Right... Copy-paste issue

trioletas · 2017-09-27T14:31:28Z

Dockerfile

+RUN gradle clean build
+
+
+# Run the application in a smil container


Typo smil -> small

Fixed. Was not sure if to use 'small' or 'slim'...

slim is better, my bad for suggesting small, I though you meant to write small and made a typo, slim conveys the required property of a final image better!

trioletas · 2017-09-27T14:32:48Z

Dockerfile.dev

+COPY settings.gradle .
+
+# Run the build on behalf of non-root 'boot' user
+USER boot


Do we really care which user runs the app in the dev container?

Generally, no. Removed additional user creation

trioletas · 2017-09-27T14:33:23Z

Dockerfile.dev

+USER boot
+
+# Release gradle caches if any
+RUN find /opt/.gradle -type f -name "*.lock" | while read f; do rm $f; done


Is that generally necessary ?

Depends... Our docker-compose.yml mounts volumes from local file system, including gradle caches to speed-up startup of the contenerized application. In case another gradle daemon is already running then application inside container will fail to start

trioletas · 2017-09-27T14:34:33Z

Dockerfile.dev

@@ -0,0 +1,25 @@
+FROM gradle:4.1-jdk8-alpine


Let's think how we could sync build image specification with production Dockerfile

Any ideas on this?

No idea yet, a head scratcher for me as well!

Ideal case would be to have some kind of includes (see moby/moby#735), however it is not supported by Docker (and not clear if it will be supported).
Other solutions were using e.g. C pre-processors or similar tools to manipulate text files that also seems to be more a workaround.

For sure another alternative is KISS - the less moving parts we have in Dockerfile the less things to keep in sync.

I agree, it’s not that big of a deal, code reviews should help keeping things in sync between the two Dockerfiles

PR feedback - don't use .env from prod - don't create a separet user for dev

trioletas · 2017-09-28T16:41:32Z

Dockerfile.dev

+COPY settings.gradle .
+
+# Release gradle caches if any
+RUN find /opt/.gradle -type f -name "*.lock" | while read f; do rm $f; done


I suggest that we keep Dockerfile as simple as humanly possible and add this one in to the documentation instead. In a troubleshooting section or something.

@sergeytrasko please follow up on this and we can merge this one to trunk I believe

Will do today evening

This is not required any more (as we are not mounting local FS gradle caches any more)

- Don't mount gradle caches from local FS - Pre-fetch gradle dependencies before copying sources to cache them

Docker support #6

e8012f4

Docker support #6

e1cc251

Production image

trioletas requested a review from vlev September 26, 2017 12:49

trioletas reviewed Sep 26, 2017

View reviewed changes

sergeytrasko added 2 commits September 26, 2017 21:16

Docker support #6

b328787

Part of PR feedback addressed

Docker support #6

ac28988

Part of PR feedback addressed - production build

trioletas reviewed Sep 27, 2017

View reviewed changes

sergeytrasko added 2 commits September 27, 2017 09:49

Docker support #6

54397a7

Formatting

Docker support #6

595cab2

Formatting

trioletas reviewed Sep 27, 2017

View reviewed changes

Docker support #6

3565fd5

PR feedback - don't use .env from prod - don't create a separet user for dev

trioletas reviewed Sep 28, 2017

View reviewed changes

Docker support #6

1c70d74

- Don't mount gradle caches from local FS - Pre-fetch gradle dependencies before copying sources to cache them

trioletas merged commit afc04ea into master Oct 5, 2017

trioletas deleted the issue-6-docker branch October 5, 2017 17:46

		RUN gradle clean build


		# Run the application in a smil container


		WORKDIR $HOME/app

		USER root

Docker support #6 #17

Docker support #6 #17

Conversation

sergeytrasko commented Sep 26, 2017 • edited

sergeytrasko commented Sep 26, 2017

sergeytrasko commented Sep 26, 2017

trioletas Sep 26, 2017 • edited by sergeytrasko

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trioletas Sep 26, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trioletas Sep 28, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trioletas Oct 4, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sergeytrasko commented Sep 26, 2017 •

edited

trioletas Sep 26, 2017 •

edited by sergeytrasko

trioletas Sep 26, 2017 •

edited

trioletas Sep 28, 2017 •

edited

trioletas Oct 4, 2017 •

edited