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

Dockerfile using alpine #458

Open
wants to merge 20 commits into
base: terriajs7
Choose a base branch
from

Conversation

davidedelerma
Copy link

@davidedelerma davidedelerma commented Jun 12, 2020

Using Apine to get a smaller image and runnin npm install at built time to pull the newest versions of the libraries.
Using Alpine resulted in ~50% size reduction of the image compared to data61/terria-terriamap

Copy link
Contributor

@KeyboardSounds KeyboardSounds left a comment

Choose a reason for hiding this comment

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

Significantly shrinking the size of our image sounds like a good idea, and I can't think off the top of my head why we're not already using alpine. @soyarsauce, @peterhassall and/or @steve9164 should also have a look at this.

deploy/docker/Dockerfile Show resolved Hide resolved
@soyarsauce
Copy link
Contributor

Unfortunately we can't merge this PR as is - our Dockerfile is written as a way to package up an immutable build and this undoes most of that. For some of our maps, we specify a set of dependencies that we want at time of building/deploying/releasing a given TerriaMap and commit that lockfile - and allowing dockerfile to pull in newer versions of libraries will introduce an u-tracked (bar looking at the lockfile inside a given image) and different set of dependencies.

why we're not already using alpine

One reason would be having to check puppeteer deps again, size reduction is a really good goal but this PR would better suit a directory/dockerfile targetted for running in this fashion, or to separate just the size shrink change from the dependency-update change

@KeyboardSounds
Copy link
Contributor

@soyarsauce so just to clarify, are you suggesting that @davidedelerma should set up a separate Dockerfile in deploy/docker-alternate/ or something? rather than modify the primary one?

- Original Docker file is now based on Alpine to reduce image size

- An alternative Docker file is in docker-alternate. It installs the
packages defined in package.json at build time so that it can be build
automatically from what is defined in package.json without the need of
install tha packages locally first.
@davidedelerma
Copy link
Author

davidedelerma commented Jun 15, 2020

@soyarsauce @KeyboardSounds I pushed some changes that follow up your suggestions.
Now there is a dockerfile that mimic the behaviour of the original one, but based on Alpine to reduce the size.
And then in the docker-alternate folder there is the other dockerfile still based on Alpine but that installs the packages defined in package.json at build time.

Juanezm and others added 8 commits July 1, 2020 17:12
* Adding server configuration folder

* Modifying readme for passing a whole config file instead of a single port
* changed accent colors buttons colors lateral menu colors for solomon

* better contrast on hover over menu item in catalog

* added logos for sl and vt
 Conflicts:
	deploy/docker-alternate/readme_dockerfile_alternate.md
	deploy/docker/Dockerfile
@soyarsauce soyarsauce self-requested a review July 21, 2020 15:48
@soyarsauce soyarsauce self-assigned this Jul 21, 2020
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.

5 participants