entrypoint: Monitor config dir for changes

[oliv3r]

We see a lot of crudges and hacks to notify nginx or the nginx container informing it it needs to restart. While there certainly cases that require manual control, for the most, this could be easily automated.

With inotify, we can recursively monitor /etc/nginx (or any directory per config) for changes (currently, not monitoring for for access time changes, e.g. reads or touch events). On an event, we sleep first for (configurable) seconds, the default is 10, so that multiple updates don't cause multiple restarts. E.g. copying 10 certificates into /etc/nginx/certs, won't trigger 10 reloads.

The monitor will run indefinably, and can't be easily killed. This isn't a problem however, as this is specifically a docker entry point and it is fair to assume this will only ever be run under docker.

The current configuration won't change existing behavior, it needs to be explicitly enabled.

[thresheek]

Hi @oliv3r !

I think it only makes sense with an init-like system (see e.g. https://github.com/docker-library/official-images/#init) and that is too big of a change.

[oliv3r]

@thresheek it would be nicer, if nginx would monitor this via inotify library internally, for sure.

The problem I'm trying to solve, is that, nginx is running in a (compose) container, doing happily its thing.

Through external means (the only way anyway?) the configuration files OR the certificate files get updated (e.g. certbot ran and renewed) so now nginx needs to be sigusr-ed. The problem is of course, if certbot also runs in a container, it can't really do that (easily).

Instead, it's much easier and nicer, to have inotifywait (or nginx itself) watch for differences in these files, and (after nginx -t) restart itself, which is what the script is for (not activated by default).

So nginx works exactly the same as before, but if you set the correct environment variables, nginx will restart itself if a configuration file is detected through inotifywait.

So not sure why you refer to an init system, the nginx container doesn't have it right now, and doesn't need it imo after this change either, or rather, because of this change (using tini would still probably be wise, and would be a trivial change, as only the shebang of the entrypoint script would need to change, and tini is like 34k).

[thresheek]

I'm referring to an init system because we now have two types of processes running inside a container, with no proper management for those. Imagine if inotifywait exits during the lifetime of a container - we have no way to restart it, or to know if it's stil running as it should. And we still explicitely rely on the services it provides since we enabled it in the first place.

[oliv3r]

I'm referring to an init system because we now have two types of processes running inside a container, with no proper management for those. Imagine if inotifywait exits during the lifetime of a container - we have no way to restart it, or to know if it's stil running as it should. And we still explicitely rely on the services it provides since we enabled it in the first place.

You are very correct, and nginx is currently doing it wrong to begin with (hence my last sentence in my previous comment).

Unless nginx becomes a process monitor (which as you said it doesn't) it 'violates' all the init reasoning mentioned by these init systems. Signal handling, process reaping etc, is all left to nginx. And as there's commands being executed (grep, sed just to name something) these also can cause problems. So in that sense, adding dumb-init is a good idea generally anyway.

As for why dumb-init and not the built-in tini-init. For one, docker has added, removed, re-added it so it becomes unpredictable. Secondly, the initial entrypoint depends on it, yet it requires the user to run the container with the --init flag (or the compose init: true field). It makes things way to fragile.

[thresheek]

I don't see how nginx is currently doing it wrong, to be honest. It will manage the processes it spawns (workers, cache processes, etc.). It does not execute grep or sed. Are you referring to entrypoints? If those fail on a startup, there is no need to invoke nginx or init, since they are not managed by those in any case.

[oliv3r]

So nginx fails, in that it is trying to 'be' an init system, handling signals etc (the dumb init describes it quite well). What happens to zombie processes? Will nginx reap those?

As for grep/sed, nginx container does very much so, doesn't it? I mean The dockerfile defines an entrypoint https://github.com/nginxinc/docker-nginx/blob/master/Dockerfile-debian.template#L103 which means the container always runs https://github.com/nginxinc/docker-nginx/blob/master/entrypoint/docker-entrypoint.sh which is a) a shell script, so now your shell script is responsible for initially handling signals and reaping processes.

Anyway, this change shouldn't affect the container on nginx in a negative way, but will improve nginx container and make it adhere to the 'official docker guidelines' which would be a pro :)

[thresheek]

See also #509

[yosifkit]

What happens to zombie processes? Will nginx reap those?

Yes, nginx is well-behaved and cleans up its own children (and is basically a "init" system to its worker processes/threads).

now your shell script is responsible for initially handling signals and reaping processes.

For the small amount of time during startup, the shell (and script) will be responsible for signals and reaping (which sh does fine). It doesn't start any async background processes that would need monitoring. As nginx is started, it replaces the shell process (via exec), so it is then PID 1 and responsible for its children. It would inherit (and mostly ignore) any background processes started in the script (but there aren't any since that is a bad practice).

Any real (systemd, SysVinit, etc) or pretend (like a bash script that stays resident as PID 1) init system that isn't the main software of the image (i.e., nginx for these images) would be unlikely to be accepted into official images (https://github.com/docker-library/official-images/tree/5e0978be78c717ebe8d274db0065348df3f7d1e4#init). We only recommend something like tini when the main process itself doesn't reap zombies or doesn't handle SIGTERM. We highly discourage running multiple untracked processes since that can lead to the untracked process unexpectedly crashing and then never being restarted.