prosody / prosody-docker

Docker image building system for the Prosody XMPP server
https://hub.docker.com/r/prosody/prosody/
147 stars 49 forks source link

Graceful shutdown #70

Closed LeSpocky closed 3 years ago

LeSpocky commented 3 years ago

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 runuser tool in commit 95a9d24b76a7596afc6e456a65febf1f53a53249.

The pull request is about replacing runuser with setpriv as suggested by @maz3max in #68.

See commit messages for detailed explanations.

Zash commented 3 years ago

As you noted in https://github.com/prosody/prosody-docker/issues/68#issuecomment-803649776 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 docker stop prosody I see:

Session terminated, killing shell...mod_posix           warn    Received SIGTERM
startup             info    Shutting down: Received SIGTERM
general             info    Shutting down...
general             info    Shutdown status: Cleaning up
general             info    Shutdown complete
 ...killed.
LeSpocky commented 3 years ago

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 docker stop prosody I see:

Session terminated, killing shell...mod_posix           warn  Received SIGTERM

That »Session terminated, killing shell« comes from runuser from util-linux. I confirmed that by looking into the source.

startup             info  Shutting down: Received SIGTERM
general             info  Shutting down...
general             info  Shutdown status: Cleaning up
general             info  Shutdown complete
 ...killed.

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.

horazont commented 3 years ago

This looks good to me :+1:

Zash commented 3 years ago

prosodyctl requires to run as root

It doesn't FWIW, not exactly, with the exception of the cert import command if the target directory is not writable by the prosody user.

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.