docker-icinga2 icon indicating copy to clipboard operation
docker-icinga2 copied to clipboard

Running with --pid=host skips data dir init

Open flyingflo opened this issue 1 year ago • 5 comments

It would be tempting to run the icinga2 container without a pid namespace, i.e. with --pid=host, to allow checks to see all processes/threads on the host.

However, the entrypoint program only initializes the data dir if its own PID is 1, which is obviously not the case with --pid=host. Is there a good reason for this behaviour? Why do we ony initialize if pid==1? Why not initialize whenever the dirs don't exist?

flyingflo avatar Jun 13 '23 11:06 flyingflo

It would be tempting to run the icinga2 container without a pid namespace

What for?

Al2Klimov avatar Jun 13 '23 15:06 Al2Klimov

It would be tempting to run the icinga2 container without a pid namespace

What for?

.. to allow checks to see all processes/threads on the host. The check_procs nagios plugin checks processes and threads which is not very meaningful inside the container.

flyingflo avatar Jun 13 '23 15:06 flyingflo

Is there a good reason for this behaviour? Why do we ony initialize if pid==1? Why not initialize whenever the dirs don't exist?

@Al2Klimov This sounds like a simple change to me. Is there any justification for pid == 1 or disadvantages for the proposed change?

lippserd avatar Jun 13 '23 15:06 lippserd

Now, looking at the code post factum, I can think only of defensive programming à la "just to be sure" as reason for that condition. (It's basically the same as comparing len() with 0 with > and not !=.)

docker exec, despite my worries, doesn’t run the entrypoint, otherwise I'd see a "Running %#v" log message. The only other command runner except already spawned processes I know is CMD, where we obviously want the entrypoint to run. In short, only CMD runs entrypoint, we're safe on that side.

On the other hand, if I collapse a few control structures to see the whole entrypoint code, I see it only runs a given command in addition to the initialisation. Nothing one urgently needs our entrypoint for, bash does it as well. In short, one basically uses /entrypoint just for initialisation.

So AFAIK nothing really speaks against removing this condition. But, as always, I'm pretty prone to overseeing 🐘🐘, so I'd like a second opinion. You're pretty familiar with both Go and containers. Please be my second opinion.

Al2Klimov avatar Jun 13 '23 16:06 Al2Klimov

One important difference is, that when running as pid 1, dumb-init is prepended to the command, such that dumb-init runs as pid 1 and forks icinga2.

This makes a lot of sense, because pid1 processes have special signal handling aspects, most importantly no default actions. Thus, a SIGTERM doesn't stop a process that isn't explicitly installing a handler for it, if it is pid 1. dumb-init takes care of this.

We could instead run the container with --init=true, to enable the init implementation that comes with docker or podman.

flyingflo avatar Jun 14 '23 06:06 flyingflo