Improvements to Docker file #2504

dwd · 2024-08-13T16:23:32Z

This changes the Dockerfile quite radically, so that Openfire is built within Docker rather than outside of it. This should simplify building the image, and also make the results more repeatable.

In order to maximize the use of the cache while keeping the image size under control, this uses three stages:

The first stage locates and extracts all the POM files and any JAR files. This stage will be re-run on any change, but it short.
The second stage takes the output of the first stage and gathers dependencies. These will be cached, unless the output of the first stage (ie, the dependency information) changes. Then it copies in the full source, and builds it.
Finally, the runtime container is setup much as it was before, except that the runtime files are copied from the build stage rather than the filesystem directly.

The result is that a repeat build of the docker image now takes about two minutes, but can trivially be done on any docker platform (even without Java installed locally).

Notes:

The build stage should be able to run the mvn package in offline mode, but maven (being maven) wants to download more during this stage.
The .dockerignore file has of course been changed, but someone who understands Java better than I might well improve it further.

guusdk · 2024-08-13T16:45:00Z

Thanks for this! I'm not Docker-savvy enough to review this properly. @Fishbowler @Fank can you have a look please?

Assuming that this builds Openfire from source, it may be able to leverage the Maven wrapper that's part of the repository (./mvnw) rather than download/install one.

Fishbowler

Other than the one thing on Maven, this looks really good!

First run: 534.9s
Second run: 0.9s 🎉

Dockerfile

Fishbowler · 2024-08-24T18:04:15Z

Oooh, another experiential one.
When the JDK base image updates, it invalidates the docker cache at the first line.
Wonder if we could pin stuff for the early steps, then use latest for the later stuff? Or better to pin all the way down?

dwd · 2024-08-25T11:05:21Z

Oooh, another experiential one. When the JDK base image updates, it invalidates the docker cache at the first line. Wonder if we could pin stuff for the early steps, then use latest for the later stuff? Or better to pin all the way down?

I wouldn't think you'd want to try and avoid the hit there - once you start running Java, I think you want the latest possible. You could use something else for the extraction stage, but that would end up dominated by fetching the container anyway, which is why I chose to use the same one throughout.

dwd · 2024-08-25T11:13:14Z

Other than the one thing on Maven, this looks really good!

First run: 534.9s Second run: 0.9s 🎉

That looks like full cache. The better test is an arbitrary change (comment will do) in a source file, which is more typical developer experience.

I imagine we could make things faster by examining which packages are changed most frequently, and see if we could cache the build for some fo the less frequently changed ones, but that seems quite complex.

dwd · 2024-08-25T13:39:33Z

Summary of those additional commits:

Merged main
Switch to our mvnw
Used eclipse-temurin:17 as the base/JDK image
Improved caching of dependencies
Used a skeleton stage to improve readability/image size/parallelization
Added docker build to workflow

TODO list, all can/should be done post-merge:

If we added suitable secrets to GH, then the built image can be conditionally pushed to Docker Hub (or elsewhere)
Modern method for Java docker images seems to be jlink, but this needs source support I think.

guusdk · 2024-08-26T09:25:35Z

If we added suitable secrets to GH, then the built image can be conditionally pushed to Docker Hub (or elsewhere)

We could consider publishing built images via the GitHub Packages system, which can act as a Docker registry. (That doesn't seem to require additional secrets). We've experimented recently with that in this project: https://github.com/XMPP-Interop-Testing/smack-sint-server-extensions/blob/main/.github/workflows/docker.yml

Fishbowler · 2024-09-08T16:26:02Z

The more recent changes introduced a new problem:

> docker run --rm -it openfire:latest -demoboot
Initializing /var/lib/openfire...
chown: invalid group: ‘openfire:openfire’

Fishbowler · 2024-09-08T17:31:02Z

I've popped on another commit for you to look at - it's got the groups, and moves the sudo install down to the last container, as its needed for the entrypoint. I don't think there's a way to move this any earlier, unless we want to layer it?

Fishbowler · 2024-09-08T17:35:30Z

.github/workflows/continuous-integration-workflow.yml

-            echo "is_publishable_branch=true" >> $GITHUB_OUTPUT
+          if [[ ${{ github.ref }} == 'refs/heads/main' ]]; then
+            echo "is_publishable_branch=true" >> "${GITHUB_OUTPUT}"
+            echo "branch_tag=latest" >> "${GITHUB_OUTPUT}"


Would we want latest pointing at the tip of main? We consider it unstable.
Would we be better having main be main or bleeding_edge or unstable or testing or something, and work out how to match the latest release with latest?

It's now "development".

stokito · 2024-10-18T18:11:36Z

I don't see why we need to build the Openfire in a Docker itself. It doesn't need to compile something specific for some specific platform. For the Java apps everything should be without any problems.

Ideally the whole Dockerfile should be:

FROM openjdk:11-jre
RUN apt install openfire

Fishbowler · 2024-11-01T21:13:12Z

@stokito apt install openfire assumes that you only wish to install a version of Openfire that exists in a public repository. I feel that might not work for the number of forks that exist, let alone individual developer workflows. I for one regularly test PRs in containers.

@dwd Do you have time to cast some stink eye over the mess I've made of your PR, and of my comments?

dwd · 2024-11-11T10:33:33Z

Assuming that this builds Openfire from source, it may be able to leverage the Maven wrapper that's part of the repository (./mvnw) rather than download/install one.

Done!

dwd · 2024-11-11T10:34:10Z

@dwd Do you have time to cast some stink eye over the mess I've made of your PR, and of my comments?

Also done, removed the need for sudo entirely now.

dwd · 2024-11-11T10:35:30Z

The more recent changes introduced a new problem:

> docker run --rm -it openfire:latest -demoboot
Initializing /var/lib/openfire...
chown: invalid group: ‘openfire:openfire’

I believe this now works.

dwd · 2024-11-11T10:35:56Z

I'll resolve the conflicts shortly.

mvnw

Fishbowler · 2024-11-11T22:36:30Z

Works on my machine ❤️
Time to change that maintainer label?

guusdk · 2024-11-12T08:52:46Z

Time to change that maintainer label?

Given that @Fank seems to have moved on to greener pastures, and most of the original content now has been replaced, that seems reasonable.

Fishbowler

Let's change the maintainer to dwd and merge 🎉

This changes the Dockerfile quite radically, so that Openfire is built within Docker rather than outside of it. This should simplify building the image, and also make the results more repeatable. In order to maximize the use of the cache while keeping the image size under control, this uses three stages. The first stage locates and extracts all the POM files and any JAR files. This stage will be re-run on any change, but it short. The second stage takes the output of the first stage and gathers dependencies. These will be cached, unless the output of the first stage (ie, the dependency information) changes. Then it copies in the full source, and builds it. Finally, the runtime container is setup much as it was before, except that the runtime files are copied from the build stage rather than the filesystem directly. The result is that a repeat build of the docker image now takes about two minutes, but can trivially be done on any docker platform (even without Java installed locally). Notes: * The build stage should be able to run the `mvn package` in offline mode, but maven (being maven) wants to download more during this stage. * The `.dockerignore` file has of course been changed, but someone who understands Java better than I might well improve it further.

Also build skeleton runtime as a distinct stage

My word this is broken in Maven.

Or, indeed, at all.

guusdk · 2025-01-03T13:51:20Z

I've rebased this, and switched out the maintainer label (now references @dwd and @Fishbowler ).

guusdk · 2025-01-03T14:25:10Z

This change is now merged. Thanks for all the hard work that has gone into this!

Now that it is merged, should we have a look at automatically publishing a Docker and/or OCI image? The existing GitHub workflows contain a step that conditionally publish to DockerHub, but I think that's never used (and if so, I wouldn't know what DockerHub repository is ours). Should we re-instate that? Should we instead use the GitHub Release support for this (and distribute the images through that rather than through DockerHub)?

What versioning do we use? One image for every tag, and images for every branch (Main, vX.Y) that is merged to?

Fishbowler · 2025-01-03T14:36:35Z

I used it for my own fork. It's never been used in Ignite.

guusdk · 2025-01-03T14:49:24Z

I've copy/pasted most of @Fishbowler's work in xmpp-interop-test to replace what pre-existed with an attempt to publish to the GitHub Registry in #2642

Fishbowler self-requested a review August 13, 2024 16:50

Fishbowler requested changes Aug 18, 2024

View reviewed changes

Dockerfile Outdated Show resolved Hide resolved

Fishbowler reviewed Aug 18, 2024

View reviewed changes

Dockerfile Outdated Show resolved Hide resolved

guusdk requested a review from Fishbowler August 29, 2024 08:00

Fishbowler reviewed Sep 8, 2024

View reviewed changes

guusdk reviewed Nov 11, 2024

View reviewed changes

mvnw Outdated Show resolved Hide resolved

Fishbowler approved these changes Jan 3, 2025

View reviewed changes

dwd and others added 7 commits January 3, 2025 14:45

Better caching, use mvnw

e00404d

Get mvnw working

4602199

Switch to eclipse-temurin images

bc76c2c

Also build skeleton runtime as a distinct stage

Add docker building to workflow

296f636

Add docker building to workflow (fix)

f02cf82

Add docker building to workflow (fix)

7abd494

dwd and others added 7 commits January 3, 2025 14:46

Add docker building to workflow (fix)

7ce9e2a

Add docker building to workflow (fix)

abb01d8

Fix groups and missing sudo for entrypoint

5264c42

Get offline mode to work

b73aaa0

My word this is broken in Maven.

Don't require sudo at runtime

efc4cae

Or, indeed, at all.

main:HEAD is now latest

8ed69c3

Replace Fank with with Dwd and Fishbowler as maintainer of Dockerfile

e218bbc

guusdk force-pushed the docker-improvements branch from a7b71de to e218bbc Compare January 3, 2025 13:50

guusdk merged commit 61e563b into igniterealtime:main Jan 3, 2025
18 checks passed

guusdk mentioned this pull request Jan 3, 2025

Publish Docker image to GitHub registry #2642

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improvements to Docker file #2504

Improvements to Docker file #2504

dwd commented Aug 13, 2024

guusdk commented Aug 13, 2024

Fishbowler left a comment

Fishbowler commented Aug 24, 2024

dwd commented Aug 25, 2024

dwd commented Aug 25, 2024

dwd commented Aug 25, 2024

guusdk commented Aug 26, 2024

Fishbowler commented Sep 8, 2024

Fishbowler commented Sep 8, 2024

Fishbowler Sep 8, 2024

dwd Nov 11, 2024

stokito commented Oct 18, 2024 •

edited

Loading

Fishbowler commented Nov 1, 2024

dwd commented Nov 11, 2024

dwd commented Nov 11, 2024

dwd commented Nov 11, 2024

dwd commented Nov 11, 2024

Fishbowler commented Nov 11, 2024

guusdk commented Nov 12, 2024

Fishbowler left a comment

guusdk commented Jan 3, 2025

guusdk commented Jan 3, 2025

Fishbowler commented Jan 3, 2025

guusdk commented Jan 3, 2025

Improvements to Docker file #2504

Improvements to Docker file #2504

Conversation

dwd commented Aug 13, 2024

guusdk commented Aug 13, 2024

Fishbowler left a comment

Choose a reason for hiding this comment

Fishbowler commented Aug 24, 2024

dwd commented Aug 25, 2024

dwd commented Aug 25, 2024

dwd commented Aug 25, 2024

guusdk commented Aug 26, 2024

Fishbowler commented Sep 8, 2024

Fishbowler commented Sep 8, 2024

Fishbowler Sep 8, 2024

Choose a reason for hiding this comment

dwd Nov 11, 2024

Choose a reason for hiding this comment

stokito commented Oct 18, 2024 • edited Loading

Fishbowler commented Nov 1, 2024

dwd commented Nov 11, 2024

dwd commented Nov 11, 2024

dwd commented Nov 11, 2024

dwd commented Nov 11, 2024

Fishbowler commented Nov 11, 2024

guusdk commented Nov 12, 2024

Fishbowler left a comment

Choose a reason for hiding this comment

guusdk commented Jan 3, 2025

guusdk commented Jan 3, 2025

Fishbowler commented Jan 3, 2025

guusdk commented Jan 3, 2025

stokito commented Oct 18, 2024 •

edited

Loading