Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Running with --pid=host skips data dir init #101

Open
flyingflo opened this issue Jun 13, 2023 · 5 comments
Open

Running with --pid=host skips data dir init #101

flyingflo opened this issue Jun 13, 2023 · 5 comments
Assignees

Comments

@flyingflo
Copy link

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?

@Al2Klimov
Copy link
Member

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

What for?

@flyingflo
Copy link
Author

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.

@Icinga Icinga deleted a comment from Al2Klimov Jun 13, 2023
@lippserd
Copy link
Member

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?

@Al2Klimov Al2Klimov self-assigned this Jun 13, 2023
@Al2Klimov
Copy link
Member

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 Al2Klimov assigned lippserd and unassigned Al2Klimov Jun 13, 2023
@flyingflo
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants