Separate builders for build tools, drop some unused library dependencies#994
Separate builders for build tools, drop some unused library dependencies#994featheredtoast wants to merge 11 commits into
Conversation
featheredtoast
force-pushed
the
superslim-base
branch
3 times, most recently
from
75ae8e3 to
7d9e6fe
Compare
featheredtoast
changed the title
featheredtoast
force-pushed
the
superslim-base
branch
from
6f30992 to
9a0ff98
Compare
| for pid in $ORPHANS; do | ||
| (timeout 5 /bin/bash -c "kill $pid && wait $pid" 2>/dev/null || kill -9 $pid 2>/dev/null) & | ||
| done | ||
| wait |
There was a problem hiding this comment.
Do you know what the history is with this? Why are we killing ORPHANS in the first place? Unclean shutdown feels out of scope for this script and ought to be handled one level up.
There was a problem hiding this comment.
I think it's related to how docker/launcher interacted with the containers previously, but I don't have history other than the note in git history:
FIX: force all sub processes to stop on TERM, wait for them to finish
This works around docker bug where containers will not stop
Change here attempted to remove parallel without rocking the boat too much on lifecycle.
|
|
||
| RUN --mount=type=tmpfs,target=/var/log \ | ||
| --mount=type=tmpfs,target=/var/cache/apt \ | ||
| --mount=type=tmpfs,target=/var/lib/apt \ |
There was a problem hiding this comment.
I don't know what the lifecycle of this container is. Are we potentially increasing the RAM usage for every Discourse here?
There was a problem hiding this comment.
No, this Dockerfile is only done by the builder for building the base dependencies image. All Discourse images /containers build or run later don't inherit a tmpfs from this.
The idea here is just to have an image that is clear of apt caches on downstream images.
We used to do this back in the day by doing a later RUN rm-all-caches and flattening image layers, but tmpfs is the way to achieve this now for directories we don't want saved in a final image.
*Cache mounting for apt is also given as an example on Docker's docs page as well.
| gawk anacron wget \ | ||
| psmisc whois brotli \ | ||
| pngcrush pngquant ripgrep poppler-utils \ | ||
| catdoc antiword \ |
There was a problem hiding this comment.
Pulling and labelling the sections below is awesome. Should we do the same with the preceding as # discourse runtime requirements?
There was a problem hiding this comment.
I've added it - Just for full transparency, I initially didn't label the first block above as it's less clear to me as to exactly what is a hard discourse requirement (I'm not so sure about a Discourse hard requirement on fping, gawk, psmisc, ripgrep, or whois for example).
Some are known for container boot (anacron, wget, gnupg, sudo, curl).
Others I know are Discourse requirements - pngcrush, pngquant, poppler-utils, catdoc, antiword.
It's possible there's crossover.
| COPY --from=thpoff_builder /usr/local/sbin/thpoff /usr/local/sbin | ||
| COPY --from=jemalloc_builder /usr/lib/libjemalloc.so /usr/lib | ||
| COPY --from=oxipng_builder /usr/local/bin/jhead /usr/local/bin | ||
| COPY --from=oxipng_builder /usr/local/bin/oxipng /usr/local/bin |
There was a problem hiding this comment.
Bikeshed: Infrastructure convention is to use hyphens instead of underscores in these names.
|
@andrewschleifer updated 👍 |
Separate out builders for thpoff, jemalloc and oxipng/jhead Copy over the results of all libraries to reduce dependencies for base image needing build tools.
Remove the dependency on parallel for two commands
drop -dev packages for nginx compilation now that this is separate
..and remove extra cmake reference, this is already in a line above
Build in arm vs amd64 acts a bit differently - arm builds to libjemalloc.so.2 and symlinks whereas amd64 does not. build and copy over library directly to /usr/lib/libjemalloc.so in both cases.
Add comment for Discourse dependencies, as best I can put.
updating name to dashes, run ruby lint
Allows for an image to potentially build from discourse-runtime-dependencies Evict all of the built-time tools from a discourse-runtime-dependencies target
Rename new target to discourse-runtime-base to better reflect.
featheredtoast
force-pushed
the
superslim-base
branch
from
59ced9e to
6f0215a
Compare
2 participants
Separate out builders for thpoff, jemalloc and oxipng/jhead
Copy over the results of all libraries to reduce dependencies
for base image needing build tools.
drop extra -dev packages from base image as we only need the nginx runtime libs.
refactor parallel stop without parallel package
mount tmpfs for apt caches
remove unreferenced libtcmalloc
reorganize gem dependencies