Ensuring container services having the correct init process and other improvements related to container services #284
Conversation
For all services that are based on the project Docker image. This will ensure all orphaned child processes are reaped.
So that SIGTERM can be passed to the process correctly in events such as `podman stop`
This improves security of the image
Call to bash is not really needed if the service starts only with one command
This is a workaround for setting the `HOME` env var. See https://github.com/phusion/baseimage-docker?tab=readme-ov-file#environment-variables for details
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #284 +/- ##
=======================================
Coverage 98.75% 98.75%
=======================================
Files 48 48
Lines 2242 2242
=======================================
Hits 2214 2214
Misses 28 28 ☔ View full report in Codecov by Sentry. |
| command: [ "bash", "-c", | ||
| "git config --global --add safe.directory /app && pip3 install -U -e . && flask init-db && flask run --host=0.0.0.0 --debug" ] | ||
| command: [ | ||
| "/sbin/my_init", "--", |
There was a problem hiding this comment.
why just not to retain the same ENTRYPOINT of docker.io/phusion/baseimage:jammy-1.0.1 which is the /sbin/my_init AFAIK and then just have here the "bash", "-c", ...?
There was a problem hiding this comment.
but otherwise -- looks good if that works ;) please make it sweat (run the population script again) and see if pids are staying under control with no zombies
There was a problem hiding this comment.
why just not to retain the same ENTRYPOINT of
docker.io/phusion/baseimage:jammy-1.0.1which is the/sbin/my_initAFAIK and then just have here the"bash", "-c", ...?
The docker.io/phusion/baseimage:jammy-1.0.1 image actually doesn't have an ENTRYPOINT of "/sbin/my_init". It only have CMD ["/sbin/my_init"] as the default command to execute, which is to be overridden.
I have considered adding ENTRYPOINT ["/sbin/my_init"] to our project image, but that would require starting each command to be executed with "--". For example, the command you referenced above would have to be command: [ "--", "bash", "-c", "git config --global --add safe.directory /app && pip3 install -U -e . && flask init-db && exec flask run --host=0.0.0.0 --debug" ]. I think that makes the code less clear for it is not immediately clear what the "--" is for, and one has to remember to use "--" before specifying any command to be executed because of a hidden context (the entry point) in the project Docker image.
On the other hand, we can add ``ENTRYPOINT ["/sbin/my_init", "--"]to our project image and specify commands to be executed the way you suggested. However, doing so makes passing options to"/sbin/my_init"` much less convenient (one has to override the entry point to pass any options to `"/sbin/my_init"` itself).
Because of the above considerations, I think we should keep the current arrangement. It is a bit longer, but it is explicit and convenient to pass options to "/sbin/my_init" itself.
There was a problem hiding this comment.
please make it sweat (run the population script again) and see if pids are staying under control with no zombies
I want to sweat the server by deploying a solution for #271 instead. I think we can see zombies if this PR doesn't work when adding all the gin datasets to the registry (the worker process has already accumulated some zombies over the vacation, about 40, just by updating the registered datasets).
|
Attached is a snapshot of output of
These images can be used to as a reference for comparison in the future. |
This PR contains the following improvements in container services specification.
bashin invoking the command to be executed for a container when the calls can be avoided.execto ensure aSIGTERMsignal is received by the target process of a container whendocker/podman stopare used.