WIP: /var/run/icinga2/icinga2.s: reload on POST /v1/reload

[Al2Klimov]

... and wait for the reload to complete before responding to let curl(?) block the reload command.

fixes #7301
resolves #8911
closes #8973

[Al2Klimov]

@dnsmichi I have something (this one) to show you and the rest of team Icinga core (one nice day).

[Al2Klimov]

@N-o-X What about this sync reload trigger as alternative to the current async one?

[Al2Klimov]

@cla-bot check

[Al2Klimov]

@julianbrost about this solution: #8973 (comment)

[Al2Klimov]

Feedback

• Julian
• Noah

[julianbrost]

I had a quick look at this to see if this could serve as basis for implementing one of the ideas I proposed in #9363, but for that, the listening socket is in the wrong process.

General impression after looking at this:

• Current state does not fix yet what it says to fix in the PR description: systemd unit file is not updated yet, the systemd unit file isn't updated so far, but well, the description also says "WIP". Also, compared to safe-reload, no validation output seems to be returned.
• In general, fork() not immediately followed by exec() is always tricky, just look at the amount of uninitialize/reinitialize workarounds, so I'm not sure about making the umbrella process even more complex that it already is. Are there any plans on extending the HTTP API with further commands? Otherwise, some simple file-based notification mechanism might be a better fit.

[Al2Klimov]

• Also, compared to safe-reload, no validation output seems to be returned.

Shall I fix this?

[julianbrost]

Not right now, please focus on the reviews and comments on your Icinga DB PRs first.

[Al2Klimov]

My question was Shall I fix this? Not Shall I fix this right now?

[julianbrost]

Before fixing anything, we should first agree on the approach. Like if we can't come up with another useful purpose of the socket, all that extra HTTP handling might be overkill and something simpler could be done without that extra threading in the umbrella process.

[Al2Klimov]

Overkill or not, it doesn’t reinvent the wheel and curl speaks it.

[julianbrost]

My complaint isn't HTTP in itself but rather that in order to implement it, you have to add threads, which you have to tear down again in order to be able to fork, and then restore again.

[Al2Klimov]

Would be not the first ones, would they?

[julianbrost]

Given that fork() from a multi-threaded applications is problematic (that's why the current state of the PR has to tear down the threads before forking) and as the umbrella process doesn't have to (and should not) do any heavy lifting: would it be possible to just run any pending coroutines in the main loop on that thread?

Would be not the first ones, would they?

I'd rather try to get rid of any existing extra threads in the umbrella process rather than adding more.

[Al2Klimov]

Well I could merge this new ASIO event loop with Application#RunEventLoop(). But this would make the still necessary teardown-fork(2)-setup parts even uglier. Shall I do this?

[julianbrost]

Well the idea behind that idea was avoid having to tear down anything. So that doesn't sound like it would move into a direction I'd like to see.

[Al2Klimov]

What about an early forked subprocess instead? Could communicate w/ umbrella via shared memory.

[Al2Klimov]

Now as I saw #9136: What about an mmap(2)ed file in /run with some state and semaphores? This way a new CLI command icinga2 reload could communicate with the daemon w/o requiring it to have an extra thread or process.

[Al2Klimov]

Are there any plans on extending the HTTP API with further commands?

• Yes, see Make use of Dockerfile HEALTHCHECK feature docker-icinga2#91 (comment)