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

[docker] Reduce the context to load at build time #441

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jeanpommier
Copy link
Member

Using .dockerignore file allows to reduce drastically the context loading on the docker build (1,5G -> 200Mb)

@jeanpommier jeanpommier changed the title Reduce the context to load at build time [docker] Reduce the context to load at build time Dec 8, 2021
@pierrejego
Copy link
Member

@jeanpommier can we merge ? How to test ?

@jeanpommier
Copy link
Member Author

I believe we can even make it more strict and exclude the web/target folder. Looking at the Dockerfile, the only source it is copying from right now is the docker/ folder. This is thus the only one that needs to be included in the context.

Testing ? I'd say that if it builds, it should be fine. This is just about the files that are loaded in memory by docker when it build the image. By default, it copies the whole bunch of files lygin around rhe Dockerfile. Here, we reduce to what is useful only (the built war file, actually)

@pierrejego
Copy link
Member

Seems clear to me.
I'd like this PR to be merge but I don't use Mapstore with Docker for the moment.
No risk ? I merge ?

@jeanpommier
Copy link
Member Author

Nope, don't. This PR is more than 1Y-old, I need to dedicate it a few minutes at least to check the docker image will still build. I really don't see what could go wrong, but you're never to careful

@jeanpommier
Copy link
Member Author

What is the docker build process, BTW ? I can see one in the GH actions, but I can't see any reference/documented build process for the docker image, apart from that. Did I overlook something ?

@jeanpommier
Copy link
Member Author

Wasn't there a maven target to build the docker image ?

@jeanpommier
Copy link
Member Author

OK @pierrejego , you can merge, I builds, it runs. And it prevents you from loading more than 2Gb of context during the build process

@fvanderbiest
Copy link
Member

Just passing by.
Willing to merge, but should it be ported to stable branches as well ?

@jeanpommier
Copy link
Member Author

Hi,
Reviving this PR. OK to merge guys ? @pierrejego, @fvanderbiest

Load only folders that are used in the Dockerfile
Copy link
Member

@edevosc2c edevosc2c left a comment

Choose a reason for hiding this comment

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

Ok.

But ideally you shouldn't be doing that if the Dockerfile was able to compile the application from the code directly. (not requiring to already compile it from outside of docker)

But that's another take...

@jeanpommier
Copy link
Member Author

ideally you shouldn't be doing that if the Dockerfile was able to compile the application from the code directly. (not requiring to already compile it from outside of docker)

agreed

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

Successfully merging this pull request may close these issues.

4 participants