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

Fixes #5788

etc/init.d/icinga2.cmake

@@ -61,11 +61,13 @@ start() {
 	chown $ICINGA2_USER:$ICINGA2_COMMAND_GROUP $ICINGA2_STATE_DIR/run/icinga2/cmd
 	chmod 2755 $ICINGA2_STATE_DIR/run/icinga2/cmd
 
-        echo "Starting Icinga 2: "
+	echo "Starting Icinga 2: "
-        $DAEMON -c $ICINGA2_CONFIG_FILE -Z -d -e $ICINGA2_ERROR_LOG -u $ICINGA2_USER -g $ICINGA2_GROUP
+	if ! $DAEMON -c $ICINGA2_CONFIG_FILE -d -e $ICINGA2_ERROR_LOG -u $ICINGA2_USER -g $ICINGA2_GROUP; then
-
+		echo "Error starting Icinga."
-        echo "Done"
+		exit 1
-        echo
+	else
+		echo "Done"
+	fi
 }
 
 # Restart Icinga 2
@@ -104,17 +106,13 @@ stop() {
 # Reload Icinga 2
 reload() {
 	printf "Reloading Icinga 2: "
-	if [ ! -e $ICINGA2_PID_FILE ]; then
-		echo "The PID file '$ICINGA2_PID_FILE' does not exist."
-		exit 1
-	fi
 
 	pid=`cat $ICINGA2_PID_FILE`
-
+	if kill -HUP $pid >/dev/null 2>&1; then
-	if ! kill -HUP $pid >/dev/null 2>&1; then
-		echo "Failed - Icinga 2 is not running."
-	else
 		echo "Done"
+	else
+		echo "Error: Icinga not running"
+		exit 3
 	fi
 }
 
@@ -162,7 +160,6 @@ case "$1" in
         start
         ;;
   reload)
-	checkconfig reload fail
 	reload
 	;;
   checkconfig)

icinga-app/icinga.cpp

@@ -67,7 +67,7 @@ static String LoadAppType(const String& typeSpec)
 	return typeSpec.SubStr(index + 1);
 }
 
-static bool LoadConfigFiles(const String& appType, ValidationType validate)
+static bool LoadConfigFiles(const String& appType)
 {
 	ConfigCompilerContext::GetInstance()->Reset();
 
@@ -88,7 +88,7 @@ static bool LoadConfigFiles(const String& appType, ValidationType validate)
 	ConfigItem::Ptr item = builder->Compile();
 	item->Register();
 
-	bool result = ConfigItem::ActivateItems(validate);
+	bool result = ConfigItem::ValidateItems();
 
 	int warnings = 0, errors = 0;
 
@@ -187,6 +187,33 @@ static bool Daemonize(const String& stderrFile)
 	return true;
 }
 
+/**
+ * Terminate another process and wait till it has ended
+ *
+ * @params target PID of the process to end
+ */
+static void TerminateAndWaitForEnd(pid_t target)
+{
+#ifndef _WIN32
+	// allow 30 seconds timeout
+	double timeout = Utility::GetTime() + 30;
+
+	int ret = kill(target, SIGTERM);
+
+	while (Utility::GetTime() < timeout && (ret == 0 || errno != ESRCH)) {
+		Utility::Sleep(0.1);
+		ret = kill(target, 0);
+	}
+
+	// timeout and the process still seems to live: kill it
+	if (ret == 0 || errno != ESRCH)
+		kill(target, SIGKILL);
+
+#else
+	// TODO: implement this for Win32
+#endif /* _WIN32 */
+}
+
 int Main(void)
 {
 	int argc = Application::GetArgC();
@@ -250,10 +277,10 @@ int Main(void)
 		("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")
-		("no-validate,Z", "skip validating the configuration")
 		("debug,x", "enable debugging")
 		("errorlog,e", po::value<std::string>(), "log fatal errors to the specified log file (only works in combination with --daemonize)")
 #ifndef _WIN32
+		("reload-internal", po::value<int>(), "used internally to implement config reload: do not call manually, send SIGHUP instead")
 		("daemonize,d", "detach from the controlling terminal")
 		("user,u", po::value<std::string>(), "user to run Icinga as")
 		("group,g", po::value<std::string>(), "group to run Icinga as")
@@ -406,6 +433,29 @@ int Main(void)
 		return EXIT_FAILURE;
 	}
 
+	if (!g_AppParams.count("validate") && !g_AppParams.count("reload-internal")) {
+		pid_t runningpid = Application::ReadPidFile(Application::GetPidPath());
+		if (runningpid >= 0) {
+			Log(LogCritical, "icinga-app", "Another instance of Icinga already running with PID " + Convert::ToString(runningpid));
+			return EXIT_FAILURE;
+		}
+	}
+
+	if (!LoadConfigFiles(appType))
+		return EXIT_FAILURE;
+
+	if (g_AppParams.count("validate")) {
+		Log(LogInformation, "icinga-app", "Finished validating the configuration file(s).");
+		return EXIT_SUCCESS;
+	}
+
+	if(g_AppParams.count("reload-internal")) {
+		int parentpid = g_AppParams["reload-internal"].as<int>();
+		Log(LogInformation, "icinga-app", "Terminating previous instance of Icinga (PID " + Convert::ToString(parentpid) + ")");
+		TerminateAndWaitForEnd(parentpid);
+		Log(LogInformation, "icinga-app", "Previous instance has ended, taking over now.");
+	}
+
 	if (g_AppParams.count("daemonize")) {
 		String errorLog;
 
@@ -416,22 +466,12 @@ int Main(void)
 		Logger::DisableConsoleLog();
 	}
 
-	ValidationType validate = ValidateStart;
+	// activate config only after daemonization: it starts threads and that is not compatible with fork()
-
+	if (!ConfigItem::ActivateItems()) {
-	if (g_AppParams.count("validate"))
+		Log(LogCritical, "icinga-app", "Error activating configuration.");
-		validate = ValidateOnly;
-
-	if (g_AppParams.count("no-validate"))
-		validate = ValidateNone;
-
-	if (!LoadConfigFiles(appType, validate))
 		return EXIT_FAILURE;
-
-	if (validate == ValidateOnly) {
-		Log(LogInformation, "icinga-app", "Finished validating the configuration file(s).");
-		return EXIT_SUCCESS;
 	}
-
+	
 #ifndef _WIN32
 	struct sigaction sa;
 	memset(&sa, 0, sizeof(sa));

lib/base/application.cpp

@@ -26,6 +26,7 @@
 #include "base/utility.h"
 #include "base/debug.h"
 #include "base/type.h"
+#include "base/convert.h"
 #include "base/scriptvariable.h"
 #include "icinga-version.h"
 #include <sstream>
@@ -46,6 +47,7 @@ REGISTER_TYPE(Application);
 
 Application *Application::m_Instance = NULL;
 bool Application::m_ShuttingDown = false;
+bool Application::m_RequestRestart = false;
 bool Application::m_Restarting = false;
 bool Application::m_Debugging = false;
 int Application::m_ArgC;
@@ -219,7 +221,8 @@ void Application::RunEventLoop(void) const
 
 	double lastLoop = Utility::GetTime();
 
-	while (!m_ShuttingDown && !m_Restarting) {
+mainloop:
+	while (!m_ShuttingDown && !m_RequestRestart) {
 		/* Watches for changes to the system time. Adjusts timers if necessary. */
 		Utility::Sleep(2.5);
 
@@ -239,7 +242,20 @@ void Application::RunEventLoop(void) const
 
 		lastLoop = now;
 	}
+		
+	if (m_RequestRestart) {
+		m_RequestRestart = false;         // we are now handling the request, once is enough
 
+		// are we already restarting? ignore request if we already are
+		if (m_Restarting)
+			goto mainloop;
+
+		m_Restarting = true;
+		StartReloadProcess();
+
+		goto mainloop;
+	}
+	
 	Log(LogInformation, "base", "Shutting down Icinga...");
 	DynamicObject::StopObjects();
 	Application::GetInstance()->OnShutdown();
@@ -259,6 +275,37 @@ void Application::OnShutdown(void)
 	/* Nothing to do here. */
 }
 
+void Application::StartReloadProcess(void) const
+{
+	Log(LogInformation, "base", "Got reload command: Starting new instance.");
+
+	// prepare arguments
+	std::vector<String> args;
+	args.push_back(GetExePath(m_ArgV[0]));
+
+	for (int i=1; i < Application::GetArgC(); i++) {
+		if (std::string(Application::GetArgV()[i]) != "--reload-internal")
+			args.push_back(Application::GetArgV()[i]);
+		else
+			i++;     // the next parameter after --reload-internal is the pid, remove that too
+	}
+	args.push_back("--reload-internal");
+	args.push_back(Convert::ToString(Utility::GetPid()));
+
+	Process::Ptr process = make_shared<Process>(args);
+
+	process->SetTimeout(300);
+
+	process->Run(boost::bind(&Application::ReloadProcessCallback, _1));
+}
+
+void Application::ReloadProcessCallback(const ProcessResult& pr)
+{
+	if (pr.ExitStatus != 0)
+		Log(LogCritical, "base", "Found error in config: reloading aborted");
+	m_Restarting=false;
+}
+
 /**
  * Signals the application to shut down during the next
  * execution of the event loop.
@@ -274,7 +321,7 @@ void Application::RequestShutdown(void)
  */
 void Application::RequestRestart(void)
 {
-	m_Restarting = true;
+	m_RequestRestart = true;
 }
 
 /**
@@ -563,29 +610,6 @@ int Application::Run(void)
 
 	result = Main();
 
-	if (m_Restarting) {
-		Log(LogInformation, "base", "Restarting application.");
-
-#ifndef _WIN32
-		String exePath = GetExePath(m_ArgV[0]);
-
-		int fdcount = getdtablesize();
-
-		for (int i = 3; i < fdcount; i++)
-			(void) close(i);
-
-		(void) execv(exePath.CStr(), m_ArgV);
-#else /* _WIN32 */
-		STARTUPINFO si;
-		PROCESS_INFORMATION pi;
-		memset(&si, 0, sizeof(si));
-		si.cb = sizeof(si);
-		CreateProcess(NULL, GetCommandLine(), NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi);
-#endif /* _WIN32 */
-
-		_exit(0);
-	}
-
 	return result;
 }
 
@@ -638,7 +662,7 @@ void Application::UpdatePidFile(const String& filename)
 	}
 #endif /* _WIN32 */
 
-	fprintf(m_PidFile, "%d", Utility::GetPid());
+	fprintf(m_PidFile, "%d\n", Utility::GetPid());
 	fflush(m_PidFile);
 }
 
@@ -656,6 +680,60 @@ void Application::ClosePidFile(void)
 	m_PidFile = NULL;
 }
 
+/**
+ * Checks if another process currently owns the pidfile and read it
+ *
+ * @param filename The name of the PID file.
+ * @returns -1: no process owning the pidfile, pid of the process otherwise
+ */
+pid_t Application::ReadPidFile(const String& filename)
+{
+	FILE *pidfile = fopen(filename.CStr(), "r");
+
+	if (pidfile == NULL)
+		return -1;
+
+#ifndef _WIN32
+	int fd = fileno(pidfile);
+
+	struct flock lock;
+
+	lock.l_start = 0;
+	lock.l_len = 0;
+	lock.l_type = F_WRLCK;
+	lock.l_whence = SEEK_SET;
+
+	if (fcntl(fd, F_GETLK, &lock) < 0) {
+		int error = errno;
+		fclose(pidfile);
+		BOOST_THROW_EXCEPTION(posix_error()
+		    << boost::errinfo_api_function("fcntl")
+		    << boost::errinfo_errno(error));
+	}
+
+	if (lock.l_type == F_UNLCK) {
+		// nobody has locked the file: no icinga running
+		fclose(pidfile);
+		return -1;
+	}
+#endif /* _WIN32 */
+
+	pid_t runningpid;
+	int res = fscanf(pidfile, "%d", &runningpid);
+	fclose(pidfile);
+
+	// bogus result?
+	if (res != 1)
+		return -1;
+
+#ifdef _WIN32
+	// TODO: add check if the read pid is still running or not
+#endif /* _WIN32 */
+
+	return runningpid;
+}
+
+
 /**
  * Retrieves the path of the installation prefix.
  *

lib/base/application.h

@@ -24,6 +24,7 @@
 #include "base/application.th"
 #include "base/threadpool.h"
 #include "base/dynamicobject.h"
+#include "base/process.h"
 
 namespace icinga {
 
@@ -69,6 +70,7 @@ public:
 
 	void UpdatePidFile(const String& filename);
 	void ClosePidFile(void);
+	static pid_t ReadPidFile(const String& filename);
 
 	static String GetExePath(const String& argv0);
 
@@ -108,6 +110,8 @@ protected:
 
 	void RunEventLoop(void) const;
 
+	void StartReloadProcess(void) const;
+
 	virtual void OnShutdown(void);
 
 private:
@@ -115,6 +119,7 @@ private:
 
 	static bool m_ShuttingDown; /**< Whether the application is in the process of
 				  shutting down. */
+	static bool m_RequestRestart;
 	static bool m_Restarting;
 	static int m_ArgC; /**< The number of command-line arguments. */
 	static char **m_ArgV; /**< Command-line arguments. */
@@ -134,6 +139,8 @@ private:
 
 	static void SigAbrtHandler(int signum);
 	static void ExceptionHandler(void);
+
+	static void ReloadProcessCallback(const ProcessResult& pr);
 };
 
 }

lib/base/streamlogger.cpp

@@ -42,6 +42,15 @@ void StreamLogger::Start(void)
 	m_Tty = false;
 }
 
+void StreamLogger::Stop(void)
+{
+	Logger::Stop();
+
+	// make sure we flush the log data on shutdown, even if we don't call the destructor
+	if (m_Stream)
+		m_Stream->flush();
+}
+
 /**
  * Destructor for the StreamLogger class.
  */

lib/base/streamlogger.h

@@ -39,6 +39,7 @@ public:
 	DECLARE_PTR_TYPEDEFS(StreamLogger);
 
 	virtual void Start(void);
+	virtual void Stop(void);
 	~StreamLogger(void);
 
 	void BindStream(std::ostream *stream, bool ownsStream);

lib/config/configitem.cpp

@@ -274,29 +274,23 @@ void ConfigItem::ValidateItem(void)
 	m_Validated = true;
 }
 
-bool ConfigItem::ActivateItems(ValidationType validate)
+bool ConfigItem::ValidateItems(void)
 {
 	if (ConfigCompilerContext::GetInstance()->HasErrors())
 		return false;
 
-	if (ConfigCompilerContext::GetInstance()->HasErrors())
-		return false;
-
 	ParallelWorkQueue upq;
 
-	if (validate != ValidateNone) {
+	Log(LogInformation, "config", "Validating config items (step 1)...");
-		Log(LogInformation, "config", "Validating config items (step 1)...");
 
-		BOOST_FOREACH(const ItemMap::value_type& kv, m_Items) {
+	BOOST_FOREACH(const ItemMap::value_type& kv, m_Items) {
-			upq.Enqueue(boost::bind(&ConfigItem::ValidateItem, kv.second));
+		upq.Enqueue(boost::bind(&ConfigItem::ValidateItem, kv.second));
-		}
+	}
 
-		upq.Join();
+	upq.Join();
 
-		if (ConfigCompilerContext::GetInstance()->HasErrors())
+	if (ConfigCompilerContext::GetInstance()->HasErrors())
-			return false;
+		return false;
-	} else
-		Log(LogInformation, "config", "Skipping validating config items (step 1)...");
 
 	Log(LogInformation, "config", "Committing config items");
 
@@ -328,35 +322,32 @@ bool ConfigItem::ActivateItems(ValidationType validate)
 	Log(LogInformation, "config", "Evaluating 'object' rules...");
 	ObjectRule::EvaluateRules();
 
-	if (validate != ValidateNone) {
+	Log(LogInformation, "config", "Validating config items (step 2)...");
-		Log(LogInformation, "config", "Validating config items (step 2)...");
 
-		BOOST_FOREACH(const ItemMap::value_type& kv, m_Items) {
+	BOOST_FOREACH(const ItemMap::value_type& kv, m_Items) {
-			upq.Enqueue(boost::bind(&ConfigItem::ValidateItem, kv.second));
+		upq.Enqueue(boost::bind(&ConfigItem::ValidateItem, kv.second));
-		}
+	}
 
-		upq.Join();
+	upq.Join();
-	} else
-		Log(LogInformation, "config", "Skipping validating config items (step 2)...");
 
 	ConfigItem::DiscardItems();
 	ConfigType::DiscardTypes();
 
-	if (validate != ValidateNone) {
+	/* log stats for external parsers */
-		/* log stats for external parsers */
+	BOOST_FOREACH(const DynamicType::Ptr& type, DynamicType::GetTypes()) {
-		BOOST_FOREACH(const DynamicType::Ptr& type, DynamicType::GetTypes()) {
+		int count = std::distance(type->GetObjects().first, type->GetObjects().second);
-			int count = std::distance(type->GetObjects().first, type->GetObjects().second);
+		if (count > 0)
-			if (count > 0)
+			Log(LogInformation, "config", "Checked " + Convert::ToString(count) + " " + type->GetName() + "(s).");
-				Log(LogInformation, "config", "Checked " + Convert::ToString(count) + " " + type->GetName() + "(s).");
-		}
 	}
 
+	return !ConfigCompilerContext::GetInstance()->HasErrors();
+}
+
+bool ConfigItem::ActivateItems(void)
+{
 	if (ConfigCompilerContext::GetInstance()->HasErrors())
 		return false;
 
-	if (validate == ValidateOnly)
-		return true;
-
 	/* restore the previous program state */
 	try {
 		DynamicObject::RestoreObjects(Application::GetStatePath());
@@ -366,6 +357,8 @@ bool ConfigItem::ActivateItems(ValidationType validate)
 
 	Log(LogInformation, "config", "Triggering Start signal for config items");
 
+	ParallelWorkQueue upq;
+
 	BOOST_FOREACH(const DynamicType::Ptr& type, DynamicType::GetTypes()) {
 		BOOST_FOREACH(const DynamicObject::Ptr& object, type->GetObjects()) {
 			if (object->IsActive())

lib/config/configitem.h

@@ -27,13 +27,6 @@
 namespace icinga
 {
 
-enum ValidationType
-{
-	ValidateNone,
-	ValidateOnly,
-	ValidateStart
-};
-
 /**
  * A configuration item. Non-abstract configuration items can be used to
  * create configuration objects at runtime.
@@ -70,7 +63,8 @@ public:
 
 	void ValidateItem(void);
 
-	static bool ActivateItems(ValidationType validate);
+	static bool ValidateItems(void);
+	static bool ActivateItems(void);
 	static void DiscardItems(void);
 
 private: