Merge pull request #6356 from sourcejedi/fix/systemd

Fix logging under systemd

doc/11-cli-commands.md

@@ -388,8 +388,10 @@ Command options:
   -z [ --no-config ]        start without a configuration file
   -C [ --validate ]         exit after validating the configuration
   -e [ --errorlog ] arg     log fatal errors to the specified log file (only
-                            works in combination with --daemonize)
+                            works in combination with --daemonize or
+                            --close-stdio)
   -d [ --daemonize ]        detach from the controlling terminal
+  --close-stdio             do not log to stdout (or stderr) after startup
 
 Report bugs at <https://github.com/Icinga/icinga2>
 Icinga home page: <https://www.icinga.com/>

etc/initsystem/icinga2.service.cmake

@@ -6,7 +6,7 @@ After=syslog.target network-online.target postgresql.service mariadb.service car
 Type=notify
 EnvironmentFile=@ICINGA2_SYSCONFIGFILE@
 ExecStartPre=@CMAKE_INSTALL_PREFIX@/lib/icinga2/prepare-dirs @ICINGA2_SYSCONFIGFILE@
-ExecStart=@CMAKE_INSTALL_FULL_SBINDIR@/icinga2 daemon -e ${ICINGA2_ERROR_LOG}
+ExecStart=@CMAKE_INSTALL_FULL_SBINDIR@/icinga2 daemon --close-stdio -e ${ICINGA2_ERROR_LOG}
 PIDFile=@ICINGA2_INITRUNDIR@/icinga2.pid
 ExecReload=@CMAKE_INSTALL_PREFIX@/lib/icinga2/safe-reload @ICINGA2_SYSCONFIGFILE@
 TimeoutStartSec=30m

lib/base/logger.cpp

@@ -245,8 +245,13 @@ Log::~Log()
 #endif /* I2_DEBUG */
 	}
 
-	if (Logger::IsConsoleLogEnabled() && entry.Severity >= Logger::GetConsoleLogSeverity())
+	if (Logger::IsConsoleLogEnabled() && entry.Severity >= Logger::GetConsoleLogSeverity()) {
 		StreamLogger::ProcessLogEntry(std::cout, entry);
+
+		/* "Console" might be a pipe/socket (systemd, daemontools, docker, ...),
+		 * then cout will not flush lines automatically. */
+		std::cout << std::flush;
+	}
 }
 
 Log& Log::operator<<(const char *val)

lib/cli/daemoncommand.cpp

@@ -52,14 +52,28 @@ static void SigHupHandler(int)
 }
 #endif /* _WIN32 */
 
-static bool Daemonize()
+/*
+ * Daemonize().  On error, this function logs by itself and exits (i.e. does not return).
+ *
+ * Implementation note: We're only supposed to call exit() in one of the forked processes.
+ * The other process calls _exit().  This prevents issues with exit handlers like atexit().
+ */
+static void Daemonize() noexcept
 {
 #ifndef _WIN32
+	try {
 		Application::UninitializeBase();
+	} catch (const std::exception& ex) {
+		Log(LogCritical, "cli")
+			<< "Failed to stop thread pool before daemonizing, unexpected error: " << DiagnosticInformation(ex);
+		exit(EXIT_FAILURE);
+	}
 
 	pid_t pid = fork();
 	if (pid == -1) {
-		return false;
+		Log(LogCritical, "cli")
+			<< "fork() failed with error code " << errno << ", \"" << Utility::FormatErrorNumber(errno) << "\"";
+		exit(EXIT_FAILURE);
 	}
 
 	if (pid) {
@@ -91,13 +105,25 @@ static bool Daemonize()
 	Log(LogDebug, "Daemonize()")
 		<< "Child process with PID " << Utility::GetPid() << " continues; re-initializing base.";
 
-	Application::InitializeBase();
+	// Detach from controlling terminal
-#endif /* _WIN32 */
+	pid_t sid = setsid();
+	if (sid == -1) {
+		Log(LogCritical, "cli")
+			<< "setsid() failed with error code " << errno << ", \"" << Utility::FormatErrorNumber(errno) << "\"";
+		exit(EXIT_FAILURE);
+	}
 
-	return true;
+	try {
+		Application::InitializeBase();
+	} catch (const std::exception& ex) {
+		Log(LogCritical, "cli")
+			<< "Failed to re-initialize thread pool after daemonizing: " << DiagnosticInformation(ex);
+		exit(EXIT_FAILURE);
+	}
+#endif /* _WIN32 */
 }
 
-static bool SetDaemonIO(const String& stderrFile)
+static void CloseStdIO(const String& stderrFile)
 {
 #ifndef _WIN32
 	int fdnull = open("/dev/null", O_RDWR);
@@ -129,14 +155,7 @@ static bool SetDaemonIO(const String& stderrFile)
 		if (fderr > 2)
 			close(fderr);
 	}
-
-	pid_t sid = setsid();
-	if (sid == -1) {
-		return false;
-	}
 #endif
-
-	return true;
 }
 
 String DaemonCommand::GetDescription() const
@@ -156,9 +175,10 @@ void DaemonCommand::InitParameters(boost::program_options::options_description&
 		("config,c", po::value<std::vector<std::string> >(), "parse a configuration file")
 		("no-config,z", "start without a configuration file")
 		("validate,C", "exit after validating the configuration")
-		("errorlog,e", po::value<std::string>(), "log fatal errors to the specified log file (only works in combination with --daemonize)")
+		("errorlog,e", po::value<std::string>(), "log fatal errors to the specified log file (only works in combination with --daemonize or --close-stdio)")
 #ifndef _WIN32
 		("daemonize,d", "detach from the controlling terminal")
+		("close-stdio", "do not log to stdout (or stderr) after startup")
 #endif /* _WIN32 */
 	;
 
@@ -245,12 +265,10 @@ int DaemonCommand::Run(const po::variables_map& vm, const std::vector<std::strin
 	if (vm.count("daemonize")) {
 		if (!vm.count("reload-internal")) {
 			// no additional fork neccessary on reload
-			try {
+
+			// this subroutine either succeeds, or logs an error
+			// and terminates the process (does not return).
 			Daemonize();
-			} catch (std::exception&) {
-				Log(LogCritical, "cli", "Daemonize failed. Exiting.");
-				return EXIT_FAILURE;
-			}
 		}
 	}
 
@@ -274,12 +292,16 @@ int DaemonCommand::Run(const po::variables_map& vm, const std::vector<std::strin
 		}
 	}
 
-	if (vm.count("daemonize")) {
+	if (vm.count("daemonize") || vm.count("close-stdio")) {
+		// After disabling the console log, any further errors will go to the configured log only.
+		// Let's try to make this clear and say good bye.
+		Log(LogInformation, "cli", "Closing console log.");
+
 		String errorLog;
 		if (vm.count("errorlog"))
 			errorLog = vm["errorlog"].as<std::string>();
 
-		SetDaemonIO(errorLog);
+		CloseStdIO(errorLog);
 		Logger::DisableConsoleLog();
 	}