Remove build dirs from docker image to keep the layers small #5243
Conversation
|
@michaelarnauts, thanks for your PR! By analyzing the history of the files in this pull request, we identified @balloob, @armills and @allanglen to be potential reviewers. |
|
I agree, although I would prefer one We could also remove development packages installed here: https://github.com/michaelarnauts/home-assistant/blob/6bf321e932e3441d0ccc891f0dd4757864c25296/Dockerfile#L15 cmake, swig, *-dev, etc. |
|
That's not the way Docker works. You need to do this in one step since
every step create a layer.
|
|
Good point. Nevermind! |
|
What are your thoughts on copying the build scripts into docker first, and moving all of the compilation/apt-get functions into a script that is executed in a single #!/bin/sh
# script/setup_docker_prereqs(?) something like that
set -e
echo "deb http://download.telldus.com/debian/ stable main" >> /etc/apt/sources.list.d/telldus.list
wget -qO - http://download.telldus.se/debian/telldus-public.key | apt-key add -
apt-get update
apt-get install -y --no-install-recommends nmap net-tools cython3 libudev-dev sudo libglib2.0-dev bluetooth libbluetooth-dev \
libtelldus-core2 cmake libxrandr-dev swig
script/build_python_openzwave
mkdir -p /usr/local/share/python-openzwave
mv /usr/src/app/build/python-openzwave/openzwave/config /usr/local/share/python-openzwave/config
script/build_libcec
rm -rf build/
apt-get remove cmake swig, ...
apt-get clean && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* |
|
That could be a good idea. I can set something up tomorrow. |
|
This cleans it up to 1113 GB, a saving of 236MB. |
|
Removing libbluetooth-dev and libglib2.0-dev saves us another megabyte. The build of openzwave and libcec seems to work out fine, but maybe another python library has a dependency on this? |
|
Love the idea of a script. That way we can also share it between the Dockerfiles without having to try to keep them in sync. bluetooth is needed for one of the bluetooth trackers. We should probably start documenting why each package was added. |
|
I understand for libbluetooth, but also libbluetooth-dev? I guess dev packages should only be available when we compile stuff. I should check the code of the Bluetooth tracker... |
@philhawthorne, do you know for sure if |
…first step of Dockerfile since it will be installed later on anyway. Drop libglib2.0-dev and libbluetooth-dev
| COPY script/build_libcec script/build_libcec | ||
| RUN script/build_libcec | ||
| # Copy build scripts | ||
| COPY script/setup_docker_prereqs script/build_python_openzwave script/build_libcec script/ |
There was a problem hiding this comment.
Do we want to just copy the whole script directory for simplicity? It's all going to get copied over when the source directory is copied anyway. We could even promote the COPY . . line to before running the scripts so everything is available.
Again, docker isn't my forte so I'm not sure if increasing the size of intermediate images is a concern.
There was a problem hiding this comment.
You only want to copy the file you need at that step, because when a change is made to a different file (that can be unrelated to what you need at that point), the whole docker cache gets invalidated, and it has to rebuild everything again.
Therefore, you definitely don't want to add a COPY . . in the beginning of the Dockerfile, since every tiny change in any file triggers will force docker to rebuild everything from up to the COPY . .-line.
Interesting literature here: https://docs.docker.com/engine/userguide/eng-image/dockerfile_best-practices/
|
I think this is okay now. I just would like a confirmation that bluetooth in Docker is still okay. |
|
We are at 1.09 GB now, a total saving of 259 MB :) $ docker images home-assistant
REPOSITORY TAG IMAGE ID CREATED SIZE
home-assistant latest 19e526e3aa36 10 minutes ago 1.09 GB$ docker history home-assistant
IMAGE CREATED CREATED BY SIZE COMMENT
19e526e3aa36 9 minutes ago /bin/sh -c #(nop) CMD ["python" "-m" "homeas 0 B
277032de9982 9 minutes ago /bin/sh -c #(nop) COPY dir:46ab3cda5adeedc979 21.75 MB
f1de97456ae2 9 minutes ago /bin/sh -c pip3 install --no-cache-dir -r req 271.9 MB
114ea9b14878 13 minutes ago /bin/sh -c #(nop) COPY file:dc1b865eaa2aa9bf6 16.24 kB
8819015f88c1 13 minutes ago /bin/sh -c script/setup_docker_prereqs 114.1 MB
a6fb64de5d6b 18 minutes ago /bin/sh -c #(nop) COPY multi:35bdcb2f58887554 3.818 kB
a3ef84a2e51c 6 weeks ago /bin/sh -c #(nop) WORKDIR /usr/src/app 0 B
459e745c5712 6 weeks ago /bin/sh -c mkdir -p /usr/src/app 0 B
03cbf1acb362 6 weeks ago /bin/sh -c #(nop) VOLUME [/config] 0 B
aaaa1646019d 6 weeks ago /bin/sh -c #(nop) MAINTAINER Paulus Schoutse 0 B
7045ed20ac61 7 weeks ago /bin/sh -c #(nop) CMD ["python3"] 0 B
... |
|
@michaelarnauts the Bluetooth libraries were added because the Bluetooth tracker wouldn't work in Docker without those libraries installed. I am not sure if it is still an issue, so give me a day or so and I'll build this new docker file without the Bluetooth libraries, and see if the Bluetooth device tracker still works. The libraries installed were based on what was needed for the Bluetooth Component and Bluetooth LE component. By the looks of the documentation the LE tracker requires the
|
|
Some Python packages compile things when they get installed by pip. I would not be surprised if both Bluetooth dependencies do this and thus the headers are required. |
|
Okay so I ran this branch, and edited the docker_prereqs file to remove the bluetooth libraries. After removing them, HASS can no longer use the bluetooth tracker component. So looks like the bluetooth libraries will still need to be installed in the Docker image |
|
Yes, we should not remove any packages that were there before. |
|
@philhawthorne you removed the @balloob I think it would benefit the Docker image if we know exactly what libraries are needed, and what not. It doesn't make sense to install development libraries if they are not needed. |
|
I've added them back anyway. If they should turn out to be unnecessary, we can always remove them in the future. |
|
Like I said, I'm quite certain that they compile helper C libs for the Bluetooth packages and thus need the headers |
|
Indeed. I've added them back. |
|
@balloob @michaelarnauts This change broke zwave - see #5328 for the issue and #5329 for a fix. |
|
Oops! Missed that... Thanks for the fix! |
Description:
Remove the build dir of
script/build_libcecandscript/build_python_openzwavefrom the Docker image. This saves 168 MB.Before
rm -rf build/After
rm -rf build/Related issue (if applicable): fixes #
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>
Example entry for
configuration.yaml(if applicable):Checklist:
If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
toxrun successfully. Your PR cannot be merged unless tests passREQUIREMENTSvariable (example).requirements_all.txtby runningscript/gen_requirements_all.py..coveragerc.If the code does not interact with devices:
toxrun successfully. Your PR cannot be merged unless tests pass