Conversation
When rustguac replaces the PID 1 the shell will never invoke the trap on exit. This leaves the shell as PID 1 for the trap to be invoked on exit.
davekempe
left a comment
There was a problem hiding this comment.
Thanks for spotting this. The underlying observation is correct: with exec, the shell is replaced, so the trap ... TERM INT never fires and guacd is never signalled on shutdown.
I do not think this is quite the right fix though, and as written it regresses shutdown behaviour.
The & is missing from the rustguac line, so $! still refers to the last process that was actually backgrounded, which is guacd:
guacd ... &
GUACD_PID=$! # guacd
...
rustguac ... serve # foreground, never backgrounded
RUSTGUAC_PID=$! # still guacd
wait $RUSTGUAC_PIDSo RUSTGUAC_PID holds guacd's PID. The new trap then kills guacd twice and never signals rustguac at all. On docker stop that means rustguac receives no SIGTERM, runs until Docker's timeout and is then SIGKILLed, which is worse than the current exec behaviour where rustguac is PID 1 and gets SIGTERM directly.
For the background-and-wait pattern to work, rustguac needs a real &:
rustguac ... serve &
RUSTGUAC_PID=$!
wait "$RUSTGUAC_PID"More broadly, for a container running two processes a real init is a better answer than hand-rolled signal forwarding, since it gets reaping and propagation right without the shell having to:
ENTRYPOINT ["/usr/bin/tini", "--", "/opt/rustguac/entrypoint.sh"]Happy to take either shape. If you want to keep the shell supervisor then the & is the minimum fix; if you would rather go the init route, that would be very welcome.
Adds tini to manage processes launched from entrypoint.sh, namely guacd and rustguac.
|
Thanks for the feedback, opted to go with tini, have a look at the changes pushed in 2216317. |
When rustguac replaces the PID 1 the shell will never invoke the trap on exit. This leaves the shell as PID 1 for the trap to be invoked on exit.