[dev.icinga.com #5788] Improve daemon startup logic/order

[icinga-migration]

This issue has been migrated from Redmine: https://dev.icinga.com/issues/5788

Created by gvegidy on 2014-03-17 23:15:13 +00:00

Assignee: gvegidy
Status: Resolved (closed on 2014-04-29 11:19:47 +00:00)
Target Version: 0.0.10
Last Update: 2014-04-29 15:10:10 +00:00 (in Redmine)

---

Currently the configuration is verified in the init script before the actual icinga daemon is started. Due to subtle differences between these calls (e.g. we had the permission problem before #5761 was fixed, similar bugs might still be there, or the files might change in between the calls), the config could verify ok, but the daemon not start correctly.

When reloading is initiated through the init script, it verifies the config again and only sends the SIGHUP when it is ok. This does not handle cases where SIGHUP is not sent by the init script.

In all cases where we have errors during config load, we currently have problems sending the error message to the user because

1. logging is not set up correctly yet because the config says how to log
2. we are already daemonized so we can't use stdout/stderr
   See Bugs #5787, #5773, #5666

When we have daemonized, the initscript has no chance of finding out if icinga was really started or not.

To solve all this, I propose the following order during daemon start:

1. main()
2. all output to stdout/stderr
3. setuid
4. config check: bail out on error: error messages written to stderr, exit(errorcode)
5. config activated, loggers switched on as defined in config
6. output to stdout/stderr off
7. daemonize

When the daemon receives SIGHUP it checks the current config.
when not ok: log the error, stay alive with the current config
when ok: log restart, shutdown, fork new process as it is currently done

Does this make sense?

Changesets

2014-04-13 19:17:42 +00:00 by gvegidy 4a5181f

split ConfigItem::ActivateItems() into ConfigItem::ValidateItems() and ConfigItem::ActivateItems()

also removes the -Z commandline parameter: won't be needed when this feature is done

Refs #5788

2014-04-13 22:16:48 +00:00 by gvegidy b7ba9e5

add --reload commandline parameter

Refs #5788

2014-04-27 19:47:25 +00:00 by gvegidy 9eb06ed

Fork new process from previous daemon on reloading

The previously planned logic of forking a new daemon from the reload-process didn't work with
systemd: systemd does not allow long-running processes started from within the reload command.

Replaces parameter --reload with --reload-internal which is used when starting the new daemon.

Refs #5788

2014-04-27 21:40:37 +00:00 by gvegidy b24c2b9

Fix logging during shutdown procedure

Refs #5788

2014-04-28 11:17:05 +00:00 by gvegidy a0cd174

Split ConfigItem::ActivateItems() into ConfigItem::ValidateItems() and ConfigItem::ActivateItems().

Also removes the -Z commandline parameter: won't be needed when this feature is done.

Refs #5788

2014-04-28 11:17:05 +00:00 by gvegidy 2c6abdb

Add --reload command-line parameter.

Refs #5788

2014-04-28 11:17:05 +00:00 by gvegidy 21bc529

Fork new process from previous daemon on reload.

The previously planned logic of forking a new daemon from the reload-process didn't work with
systemd: systemd does not allow long-running processes started from within the reload command.

Replaces parameter --reload with --reload-internal which is used when starting the new daemon.

Refs #5788

2014-04-28 11:17:05 +00:00 by gvegidy f7dd5fe

Fix logging during shutdown procedure.

Refs #5788

2014-04-28 12:59:20 +00:00 by gbeutner 509a904

Use fork() instead of the Process class.

Refs #5788

2014-04-28 19:34:35 +00:00 by gvegidy d5fec60

Split ConfigItem::ActivateItems() into ConfigItem::ValidateItems() and ConfigItem::ActivateItems().

Also removes the -Z commandline parameter: won't be needed when this feature is done.

Refs #5788

2014-04-28 19:34:35 +00:00 by gvegidy 36f5377

Add --reload command-line parameter.

Refs #5788

2014-04-28 19:36:54 +00:00 by gvegidy 244d761

Fork new process from previous daemon on reload.

The previously planned logic of forking a new daemon from the reload-process didn't work with
systemd: systemd does not allow long-running processes started from within the reload command.

Replaces parameter --reload with --reload-internal which is used when starting the new daemon.

Refs #5788

2014-04-28 19:36:58 +00:00 by gvegidy e400e03

Fix logging during shutdown procedure.

Refs #5788

2014-04-28 21:30:56 +00:00 by gvegidy 8aa7e7e

Fix handling of m_RequestRestart in RunEventLoop, improve reload timeout

Refs #5788

2014-04-28 22:05:39 +00:00 by gvegidy ebdd28d

Fix possible race when the reload-process determines it's parent pid and the true parent has ended

Now transfers the true parent pid as parameter to --reload-internal.

Refs #5788

2014-04-29 08:34:01 +00:00 by gvegidy 19afcd8

Split ConfigItem::ActivateItems() into ConfigItem::ValidateItems() and ConfigItem::ActivateItems().

Also removes the -Z commandline parameter: won't be needed when this feature is done.

Refs #5788

2014-04-29 08:34:01 +00:00 by gvegidy 33bd909

Add --reload command-line parameter.

Refs #5788

2014-04-29 08:34:01 +00:00 by gvegidy 3a294bb

Fork new process from previous daemon on reload.

The previously planned logic of forking a new daemon from the reload-process didn't work with
systemd: systemd does not allow long-running processes started from within the reload command.

Replaces parameter --reload with --reload-internal which is used when starting the new daemon.

Refs #5788

2014-04-29 08:34:02 +00:00 by gvegidy 3ece2ba

Fix logging during shutdown procedure.

Refs #5788

2014-04-29 08:34:02 +00:00 by gvegidy 9f56b6e

Fix handling of m_RequestRestart in RunEventLoop, improve reload timeout

Refs #5788

2014-04-29 08:34:02 +00:00 by gvegidy 1e321f0

Fix possible race when the reload-process determines it's parent pid and the true parent has ended

Now transfers the true parent pid as parameter to --reload-internal.

Refs #5788

2014-04-29 08:34:08 +00:00 by gbeutner 50b878c

Merge branch 'feature/improve-daemon-start-5788' into next

Fixes #5788

2014-05-01 09:27:43 +00:00 by (unknown) e9fddcc

Build fix for Windows.

Refs #5788

2014-05-01 18:09:38 +00:00 by (unknown) 630a1a2

Make Application::ReadPidFile work on Windows.

Refs #5788

[icinga-migration]

Updated by gvegidy on 2014-03-23 19:53:20 +00:00

on it now, see https://github.com/gvegidy/icinga2/tree/feature/daemon-startup-5788 for my progress

[icinga-migration]

Updated by gvegidy on 2014-04-06 22:54:25 +00:00

after working on this & some irc discussion:

• ConfigItem::ActivateItems is split in two parts, one for checking, the other for activating
• checking is done before daemonization, activating only afterwards
• this is because activation spawns a lot of threads and that is incompatible with forking

reload handling:

• SIGHUP will be ignored
• a new cmdline parameter "--reload" is introduced
• when called, we check if icinga is currently running (pid file) and the current config is valid
• errors during these checks are directly written to the console -> direct user feedback
• when the config is valid, a SIGTERM is sent to the running process
• after the old process terminated, the new process (the --reload one) daemonizes and activates its new config

[icinga-migration]

Updated by gvegidy on 2014-04-15 21:43:36 +00:00

implemented and tested the logic as described: it works flawlessly with a classic sysv-init or upstart.

But newer systemd versions (I tried 208 from Fedora 20) seem to observe the processes started from a reload task trough cgroups. When the reload command is done, the new daemon forked from the reload is killed by systemd.

I've observed that through strace, the systemd debug log says nothing about that. Will take a look at the systemd code next, maybe I don't understand this fully yet or there is a workaround available.

[icinga-migration]

Updated by gbeutner on 2014-04-29 11:19:47 +00:00

• Status changed from New to Resolved
• Done % changed from 0 to 100

Applied in changeset 50b878c.

[icinga-migration]

Updated by mfriedrich on 2014-04-29 15:10:11 +00:00

• Category set to libbase
• Target Version set to 0.0.10

Running it in foreground on terminal and sending SIGHUP is a bit weird but to be honest - you won't do that when debugging icinga 2 anyways.

The rest works like a charm - thanks for the hard work & joining the team!