ventz / docker-bind

Docker Hub ventz/bind - Secure ISC BIND (Authoritative, Recursive, Slave, RPZ) - Docker image always latest built!
https://hub.docker.com/r/ventz/bind/
34 stars 37 forks source link

Use exec in entrypoint.sh #8

Closed tcely closed 6 years ago

ventz commented 6 years ago

@tcely Any particular reason for the exec? That is - are you trying to gain something specific from it?

tcely commented 6 years ago

My immediate goal is showing the actual named binary in my docker top output.

ventz commented 6 years ago

@tcely I don't mind adding it (I believe it adds nice stream redirection to the whole shell technically too), but just curious - why do you want the binary to show up? Trying to see if there's a scenario I have missed.

tcely commented 6 years ago

Having the /bin/sh /entrypoint.sh in my output doesn't tell me anything about what is running.

Also, there's very little reason to keep the shell around the entire time the container is running. It's essentially a dead process just eating up a PID.

ventz commented 6 years ago

@tcely I think you might be hitting a grep/parse issue on your side, because having a wrapper "entrypoint" script doesn't hide the process.

Ex:

# docker ps
6f4ec9c747d6        ventz/bind          "/entrypoint.sh"         3 months ago ...

# docker top 6f4ec9c747d6
UID                 PID                 PPID                C                   STIME               TTY                 TIME                CMD
root                1199                1135                0                   Jan05               ?                   00:00:00            /bin/sh /entrypoint.sh
systemd+            1406                1199                0                   Jan05               ?                   00:01:24            /usr/sbin/named -c /etc/bind/named.conf -g -u named

^ see last line, it's extracted for you.

tcely commented 6 years ago

You're correct it's absolutely a parsing problem. The tool I'm using is parsing the docker top output badly and only showing me the first process.

ventz commented 6 years ago

@tcely I've run into problems like that in the past - it's frustrating. Anyway, that said, I think your suggestion to add exec is still an improvement and will only benefit people. I've already added it in the 'TODO' for the next release - just waiting on the Alpine devs to release the patched version that addresses a very high severity remote exec bug.

obaarne commented 6 years ago

Another good reason for exec is that when the sigterm issued (i.e when stopping the container) will then work correctly. As it is now without exec, the signal is sent to entrypoint.sh (PID1), and bind is actually killed by docker after 10 seconds.

exec replaces the current process image with a new process image instead of forking a new process.

(https://docs.docker.com/engine/userguide/eng-image/dockerfile_best-practices/#entrypoint)

ventz commented 6 years ago

@obaarne @tcely - That's a great point. I'll definitely add it.