Switch Phusion baseimage to Debian Trixie Slim container#42
Conversation
Frenzie
left a comment
There was a problem hiding this comment.
Note that you can just run an apt upgrade. The underlying Ubuntu version only matters if it's no longer supported. (Not an objection to changing to a Debian image but the rationale seems a bit lackluster.)
| @@ -0,0 +1,34 @@ | |||
| name: Docker build | |||
There was a problem hiding this comment.
This file is identical to the other file, and insofar as it's not it doesn't make sense to duplicate it.
There was a problem hiding this comment.
You can ignore this - I’m not used to GitHub actions
| && apt-get install --no-install-recommends -y \ | ||
| libreadline-dev libncurses5-dev libpcre3-dev libssl-dev \ | ||
| ca-certificates wget curl \ | ||
| libreadline-dev libncurses5-dev libpcre2-dev libssl-dev \ |
There was a problem hiding this comment.
libpcre3-dev Was removed in Trixie
|
It’s not the underling Ubuntu versions or redis version that’s the driver for me to propose the change - more just having a base image that is ‘consistent’ and going to stay updated now and into the future. I think there is a personal element here of I trust the Debian slim image more from a security point of view than the Phusion one, especially for a service I want to host online. No issues if you want to reject this PR entirely. Same as before - happy to make the changes and run them for me and let the repo stay as is. and thanks for reviewing both PRs quickly. |
d182f6a to
aa66008
Compare
ca390d7 to
1e10112
Compare
|
Edit: I mean I was surprised it didn't require a |
|
I'm not sure if I understand what tini is bringing to the table here. With a quick check the behavior seems to be identical without it? diff --git a/Dockerfile b/Dockerfile
index 2005167..bc91295 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -8,7 +8,7 @@ RUN apt-get update \
build-essential git openssl \
luarocks unzip redis-server \
zlib1g-dev \
- runit tini \
+ runit \
&& apt-get clean && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/*
ARG OPENRESTY_VER=1.29.2.3
@@ -73,5 +73,4 @@ EXPOSE 7200
HEALTHCHECK --interval=30s --timeout=5s --retries=3 \
CMD /app/koreader-sync-server/scripts/healthcheck.sh
-ENTRYPOINT ["/usr/bin/tini", "--"]
CMD ["/usr/bin/runsvdir", "-P", "/etc/service"] |
It's just a wrapper around runsvdir which afaict can run as PID 1 for this purpose perfectly fine. Potentially needs some more research.
|
Thank you RE tini - it was suggested by Claude but still considered. My limited understanding of it though is that:
I couldn’t say confidently that runsvdir does reap processes as completely as tini so I included it. The docker init flag uses tini There are some old stackexchange questions and mailing list discussion from 2018 and older but no clear answer (to me) about using it as the PID 1 in the container, while tini as PID 1 was clearly supported. |
Apparently it used to have some minor reaping issues prior to 2007 (<1.8.0).
Sure, but that's mainly so you can avoid something bigger like runit or supervisor for simpler use cases. Afaict adding tini as well is harmless but useless. I don't see any distinction in my testing anyway. Another option is https://github.com/just-containers/s6-overlay; that might be closer actually. |
|
Thank you. I’ve had a look at s6-overlay and I’m not sure the effort to move across is worth it, plus I’m even less familiar with its use. I appreciate your time taken to look at these and as someone who doesn’t code but just wanted an app to work thanks for following through on the updates here and to docker hub. |
Phusion hasn't been updated in more than a year (the last git update was for actions) and the dockerhub build isn't reflected in the github releases.
Bump Openresty version and move to Debian 13 (Trixie) for a ?better supported container base.
Requires adding two additional deps - rinit and tini - to replace the run system from Phusion.
Tested and working from crosspoint and KOReader
This change is