-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Feature: Transition Docker to use s6 overlay #8886
base: dev
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #8886 +/- ##
=======================================
Coverage 97.69% 97.69%
=======================================
Files 494 494
Lines 21059 21059
Branches 1716 1716
=======================================
Hits 20574 20574
+ Misses 485 482 -3
- Partials 0 3 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Cool stumpy, I've been looking at this branch of yours for a while, reading a little about it. I dont pretend to understand all of the benefits but certainly seems it could be helpful given the complexity of the project. Thanks for taking it on, dont know that I'll have much to add (huge diff!) when its ready but will of course test etc. |
Besides the technical stuff, it does feel like things start faster. Perhaps due to some parallel actions. Ideally, everything in the original 2 scripts is mostly replicated into the smaller units. I think I captured every recent update to the startup, but that's good to double check me on. |
5a16463
to
6b76141
Compare
Some things to help guide testing I can think of:
There's probably more, but that's the main bulk of the startup sequence. |
…ervision instead of supervisord
c466b1e
to
52bd92f
Compare
This comment was marked as duplicate.
This comment was marked as duplicate.
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.
Managed to test all of these (and in general) and certainly seems all good to me!
- ✅ Waiting for Redis/broker still working
*2025-02-01 23:09:41 Connected to Redis broker.
- ✅ Waiting for database working
- ✅ Postgres
-
2025-02-01 22:51:32 Waiting for PostgreSQL to start... 2025-02-01 22:51:45 Connected to PostgreSQL
-
- ✅ MariaDB
-
[init-db-wait] Waiting for MariaDB to report ready Connected to MariaDB
-
- ✅ Postgres
- ✅ Language installation working
*2025-02-01 23:09:43 [init-tesseract-langs] Installed tesseract-ocr-tur
- ✅ Custom initialization
*2025-02-01 23:37:35 [custom-init] custom-cont-init.d: executing... 2025-02-01 23:37:35 hello world
- ✅ Setting environment via files
*[env-init] PAPERLESS_APP_TITLE set from PAPERLESS_APP_TITLE_FILE
Proposed change
This PR transitions to Docker image away from supervisord to use s6, as provided by s6-overlay.
What
s6 is a process supervision suite of tools, and s6-overlay is a nice implementation wrapping them up into useful settings for Docker use. It's the same set of tools used by the LinuxServer.io team for their base images.
The previous 2 bash scripts have been broken apart into distinct oneshot services handling a single thing, with dependencies as required. For the most part, start up ordering is identical, with some gains for doing a few init actions in parallel. I've included a flowchart in the folder of how the startup dependencies are setup.
Why
In the second paragraph of the
supervisord
documentation, it is noted to not be a proper replacement for PID 1. s6 is a proper PID 1 process supervision suite with some nice utilities for process control as well. It's good to have a proper PID 1 for signal handling, clean shutdowns and reaping zombies properly.All the s6 services, oneshot or longrun can have dependencies defined between them, ensuring and enforcing a valid ordering of startup. This includes more correct handling of optional services like
flower
, which previously was kind of hacky and just allowed to exit, even if it should have started.I'm sure this will break someone's workflow, but hopefully not too badly.
Type of change
Checklist:
pre-commit
hooks, see documentation.