-
Notifications
You must be signed in to change notification settings - Fork 55
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
Use smaller base image #46
base: master
Are you sure you want to change the base?
Use smaller base image #46
Conversation
- multi stage build - sh instead of bash
Wow the size difference is huge! Thank you for your contribution 😊 |
config | ||
modules | ||
build.sh | ||
test-build.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about the .git directly, should it also be ignored?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see my comment #46 (comment)
Dockerfile
Outdated
RUN apk update && apk add --no-cache git \ | ||
&& git clone --depth 1 -c advice.detachedHead=false -b ${branch} https://github.com/MichMich/MagicMirror.git . \ | ||
&& apk del git |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True. I will remove apk del git
in the next commit. Nevertheless, I would prefer to stick with the same base image and definitely not switch between alpine and ubuntu or debian. Using the same base image in all the stages makes the build actually faster because docker does not have to pull different base images to start the build.
Build times on my machine
Stage 0 | Stage 1 | time |
---|---|---|
node:12 | node:12-alpine | 225,53 s |
node:12-alpine | node:12-alpine | 139,66 s |
Between the builds I did run docker system prune --all
to start from scratch. The time was recorderd with time docker build -t mm:latest .
Dockerfile
Outdated
&& cp -R \ | ||
config \ | ||
css \ | ||
fonts \ | ||
index.html \ | ||
js \ | ||
node_modules \ | ||
package.json \ | ||
package-lock.json \ | ||
serveronly \ | ||
translations \ | ||
vendor /opt/magic_mirror/dist |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to copy over all files and just delete the test
directory and some dot files, but excluding the small text files are only some KB size. The most size comes from the base image and node_modules directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what the benefit of this would be. Adding specifically the files and folders required is what docker recommends and I think it makes clearer what is used and required for MagicMirror to run and what can be left aside. Also, if the structure of MagicMirror changes or other folders are added we would have to update the Dockerfile to remove them as well, whereas now we have to add them if they are required for it to work, otherwise there's no change needed.
In short, even though it's quite the copying taking place it is much more tranparent what is added to the docker image.
Thank you for your input. I reduced the As [2020-07-08 07:04:20.600] [LOG] Ready to go! Please point your browser to: http://0.0.0.0:8080
[2020-07-08 07:05:12.323] [LOG] Create new calendar fetcher for url: http://www.calendarlabs.com/ical-calendar/ics/76/US_Holidays.ics - Interval: 300000
[2020-07-08 07:05:12.435] [LOG] Create new news fetcher for url: http://www.nytimes.com/services/xml/rss/nyt/HomePage.xml - Interval: 300000
[2020-07-08 07:05:12.474] [ERROR] Error: spawn git ENOENT
at Process.ChildProcess._handle.onexit (internal/child_process.js:267:19)
at onErrorNT (internal/child_process.js:469:16)
at processTicksAndRejections (internal/process/task_queues.js:84:21)
[2020-07-08 07:05:12.969] [INFO] Newsfeed-Fetcher: Broadcasting 57 items. The error doesn't seem to affect MagicMirror. Everything seems to work as usual. Adding [2020-07-08 07:18:18.900] [LOG] Ready to go! Please point your browser to: http://0.0.0.0:8080
[2020-07-08 07:19:25.987] [LOG] Create new calendar fetcher for url: http://www.calendarlabs.com/ical-calendar/ics/76/US_Holidays.ics - Interval: 300000
[2020-07-08 07:19:26.005] [LOG] Create new news fetcher for url: http://www.nytimes.com/services/xml/rss/nyt/HomePage.xml - Interval: 300000
[2020-07-08 07:19:26.038] [ERROR] fatal: not a git repository (or any of the parent directories): .git
[2020-07-08 07:19:26.494] [INFO] Newsfeed-Fetcher: Broadcasting 57 items. Adding the .git folder from MagicMirror to the docker image as well resolves the errors, but it feels weird to have git in a production image. The new image size with git and .git would then be 330 MB on linux/amd64 What are your thoughts on this? |
My thoughts:
|
It is clear that space is not as huge of a cost factor as it used to be. Nevertheless, I don't see the point of using up almost 2 GB of my raspberry's space when also ~330 MB will do. I still strongly believe that having a smaller image is desireable |
I like the idea of having the normal debian based image as default and the alpine image with a |
I think @bastilimbach have to decide if he wants to maintain both images. A long time ago I discussed with him the idea not only providing images for I think we should bring these things together again, so may take a look at this setup for deciding what of the stuff should also implemented here. Beside the size (the debian image is currently 544MB) there are may other things to discuss as running as non-root and copying default modules in entrypoint. |
On a side note the 544 MB you mentioned are only shown like this on Docker hub. If you pull the image the size is above 1 GB The images tagged with 2.12.0 are the ones I created during my work in progress. The one with the latest tag was pulled just now from Docker Hub Like the idea of having one repository for MagicMirror in docker. Honestly, started with my own Docker Image as well but came to my senses and thought that in the end everybody benefits if it's all maintainable in one repository. In the end, it's still your decision @bastilimbach In the meantime, maybe I will have some time to try to setup a draft of how we can build both debian and alpine images |
The 544 MB are the result building with this Dockerfile using slim base image. The build setup could be similiar to mine, building debian first and copying the mm-folder to the alpine image. |
Alright, I was able to put a little something together. Didn't have the time to have a throughout look into your repo @khassel To Do:
Looking forward to your feedback and input |
@bastilimbach can we merge #45 first? |
we can also create a GitHub action which checks for new releases of Magic Mirror and creates a pull request here with the generated dockerfiles for the new release. |
I feel not very comfortable only commenting things here ...
|
The templating was inspired by the official Docker images available for NodeJS, Go, etc. and how those repositories are set up and how their images are build. It may not be necessary to got that far here, if your proposed Git will be added, as we discussed. Sorry, if I forgot it. Nonetheless, I think using multi-stage builds is valid. This way we will not package otherwise unneccessary files such as tests from the MagicMirror repo in the Docker image. Even though it may only be small size difference, I don't see the point why we shouldn't use multi-stage builds. Instead of starting a discussion on
Instead of |
the idea was to do the stuff in one stage (see no image size diff to multi stage), this is untested:
The other questions (how many and which images) should be answered by the owner of this repo. We can think about many options, but he has to merge in the end ... |
We both just have different ways of solving the same problem, I guess. Personally, I am not so fond of big |
That is great that this repository support multiple architecture.
|
@xbgmsharp as you can tell from the discussion, the changes I proposed to make the image smaller have not (yet) been merged. |
also see #48 |
To use smaller base image, alpine is used as a new base. To keep only required files multi-stage builds are used. In addition, unnecessary files are cleaned up from node_modules folder.
This PR is intended to fix #44
The overall size reduction on linux/amd64 (Mac) can be seen here:
As you can see MagicMirror still works: