Graceful shutdown #70
Conversation
This reverts commit 31d6d84. While tini successfully forwards signals, this leads to `runuser` killing prosody now. The container does terminate in 10 seconds, so Docker is happy and you could argue that actually fixes prosody#68, but it's no graceful shutdown. The revert is done because it's easier to apply a real fix without tini.
Although cc88073 ("Fix signal handling") fixed the signal handling and signals don't end up in `entrypoint.sh` anymore, there's still no clean graceful shutdown. The reason is runuser. It runs as PID 1 and prosody only runs as child process. A SIGTERM sent to runuser lets runuser forward SIGTERM to the child process. However it does not wait, but send SIGKILL right after it. (Confirmed by looking at runuser source code in util-linux.) The output on `docker stop [prosodycontainer]` is therefore: Session terminated, killing shell...mod_posix warn Received SIGTERM portmanager info Deactivated service 'c2s' ...killed. The additional messages in between prosody log output come from runuser. This is obviously no graceful shutdown. Because prosody fordibs running as uid 0 (root) we have to run it as unpriviledged user. The docker best practices recommend to use *gosu* and gosu lists some alternatives. Instead of installing gosu to the image, we use *setpriv* from the already installed util-linux now. The version in Debian buster, on which the prosody image is based currently, is recent enough to already contain setpriv. After that, prosody itself runs with PID 1, but as unpriviledged user now, and the output of `docker stop` looks like this: mod_posix warn Received SIGTERM portmanager info Deactivated service 'c2s' general info Shutting down... general info Shutdown status: Cleaning up general info Shutdown complete Link: https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#entrypoint Signed-off-by: Alexander Dahl <[email protected]>
|
As you noted in #68 (comment) the images are not built from the latest version of this repo, but from the latest release, which is a bit old. Edit: Hm, too much text, not enough coffee. In my local tests it does seem to work as intended tho. As in, when running |
That »Session terminated, killing shell« comes from runuser from util-linux. I confirmed that by looking into the source.
That was pure luck. runuser might be faster than prosody with its »...killed.« and prosody never reaches »Shutdown complete« in that case. That's the whole point of this PR. Give prosody enough time to shutdown until it get's killed by Docker anyway. But it's certainly enough to send SIGKILL only once, and Docker does that eventually. Hence the replacement of runuser with setpriv. |
|
This looks good to me 👍 |
It doesn't FWIW, not exactly, with the exception of the What does need root however is the stuff that makes the in-container user match the owner of the data directory, without changing the ownership of the data directory. This was supposed to let you switch between a prosody install on the host to a containerized prosody without mutating the data directory. |
We have a (currently still private, that might change) image based on prosody/prosody:0.11 and graceful shutdowan did not work, yet. There are few tickets now addressing that issue: #65, #68, #69.
The requirements are complex:
Hence the entrypoint.sh script itself must ensure running prosody as unpriviledged user, which was solved with the
runusertool in commit 95a9d24.The pull request is about replacing runuser with setpriv as suggested by @maz3max in #68.
See commit messages for detailed explanations.