]> granicus.if.org Git - icinga2/commitdiff
Fix logging under systemd 6356/head
authorAlan Jenkins <alan.christopher.jenkins@gmail.com>
Tue, 29 May 2018 16:24:05 +0000 (17:24 +0100)
committerAlan Jenkins <alan.christopher.jenkins@gmail.com>
Sat, 25 Aug 2018 09:21:06 +0000 (10:21 +0100)
icinga2.service used `-e ${ICINGA2_ERROR_LOG}`, but this is documented
as having no effect without `-d`.  Furthermore, icinga2 under systemd
unconditionally logged everything to the system log (but without setting
the log level etc), which contradicted the documentation.  (Issue #6339)

Stop icinga2 on systemd from logging to stdout - and hence the system log -
once it has finished starting up.  Just like when you start icinga2 from a
terminal using `-d`.  And just like -d, we stop logging fatal errors to
stderr, and instead write to the log file passed with `-e`.

As per docs, mainlog (icinga2.log) is already enabled by default.  And
pre-startup messages including config errors will still appear in the
system log.

This uses a new option --close-stdio, which has the same effect on logging as
--daemonize, but does not fork or call setsid().

For this purpose, I moved setsid() up and into Daemonize().

Consequence of that last point: if anyone is weird enough to specify a TTY
device file as the fatal error log (-e option), that will become icinga's
controlling terminal, which you generally don't want as a daemon.  This
makes it consistent with the existing behaviour for icinga mainlog.  For
this reason you're supposed to use O_NOCTTY in Linux daemons.  But I wasn't
sure where icinga would want to put the ugly `#ifdef _WIN32 ... #else ...`.

doc/11-cli-commands.md
etc/initsystem/icinga2.service.cmake
lib/cli/daemoncommand.cpp

index 012fe72f3182523edb713f58bcf1fe707d3adf72..a6a625e5fe53e45ff871dd949a90c5929c3af8f4 100644 (file)
@@ -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/>
index 94b7dcd2ff951c5e28a3c2d4c0a44af634cc0f72..9c7d817484aa9642fc1c764ede9bfda4d73a8f0f 100644 (file)
@@ -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
index 02d8b1ea18dee45b98245eae48794be2a9f7d6dc..d744d0dc141a46023faa7c0c2d028e1df7ab3de1 100644 (file)
@@ -105,6 +105,14 @@ static void Daemonize() noexcept
        Log(LogDebug, "Daemonize()")
                << "Child process with PID " << Utility::GetPid() << " continues; re-initializing base.";
 
+       // Detach from controlling terminal
+       pid_t sid = setsid();
+       if (sid == -1) {
+               Log(LogCritical, "cli")
+                       << "setsid() failed with error code " << errno << ", \"" << Utility::FormatErrorNumber(errno) << "\"";
+               exit(EXIT_FAILURE);
+       }
+
        try {
                Application::InitializeBase();
        } catch (const std::exception& ex) {
@@ -115,7 +123,7 @@ static void Daemonize() noexcept
 #endif /* _WIN32 */
 }
 
-static bool SetDaemonIO(const String& stderrFile)
+static void CloseStdIO(const String& stderrFile)
 {
 #ifndef _WIN32
        int fdnull = open("/dev/null", O_RDWR);
@@ -147,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
@@ -174,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 */
        ;
 
@@ -290,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();
        }