freedomofpress / securethenews

An automated scanner and web dashboard for tracking TLS deployment across news organizations
https://securethe.news
GNU Affero General Public License v3.0
100 stars 25 forks source link

Uses exec in container entrypoint #233

Closed conorsch closed 4 years ago

conorsch commented 4 years ago

The container image should report its process as gunicorn in prod contexts, so that the application gracefully terminates when it receives a SIGTERM from the managing process [0]. Without using exec, the container reports that sh is the active process (sh -> bash -> gunicorn), so a SIGTERM to sh results in SIGKILL on the children.

[0] https://docs.gunicorn.org/en/stable/signals.html

echoesactiii commented 4 years ago

(Moving to web board and off infra board since review is waiting on web folks! :smiley:)

conorsch commented 4 years ago

@harrislapiroff I'd initially implemented the change by using ENTRYPOINT, but all the ad-hoc calls for testing and CI rely on overriding CMD, not ENTRYPOINT. Rather than update all those calls to reset entrypoint to /bin/sh, I updated CMD to use the multi-arg format, which is a nice balance between easily overridden and explicit about which command is long-running in the container.

The table on this page was educational, when deciding between the various formats for CMD/ENTRYPOINT: https://docs.docker.com/engine/reference/builder/#understand-how-cmd-and-entrypoint-interact

harrislapiroff commented 4 years ago

@conorsch You're right, that makes sense!