Fix cluster config sync for (non-)authoritative configs

Details: https://dev.icinga.org/issues/10819#note-39

refs #10819

lib/cli/daemonutility.cpp

@@ -60,25 +60,40 @@ static void IncludeZoneDirRecursive(const String& path, const String& package, b
 
 static void IncludeNonLocalZone(const String& zonePath, const String& package, bool& success)
 {
-	String etcPath = Application::GetZonesDir() + "/" + Utility::BaseName(zonePath);
+	/* Note: This include function must not call RegisterZoneDir().
+	 * We do not need to copy it for cluster config sync. */
 
-	if (Utility::PathExists(etcPath) || Utility::PathExists(zonePath + "/.authoritative"))
+	String zoneName = Utility::BaseName(zonePath);
+
+	/* Check whether this node already has an authoritative config version
+	 * from zones.d in etc or api package directory, or a local marker file)
+	 */
+	if (ConfigCompiler::HasZoneConfigAuthority(zoneName) || Utility::PathExists(zonePath + "/.authoritative")) {
+		Log(LogWarning, "config")
+		    << "Ignoring non local config include for zone '" << zoneName << "': We already have an authoritative copy included.";
 		return;
+	}
 
-	IncludeZoneDirRecursive(zonePath, package, success);
+	std::vector<Expression *> expressions;
+	Utility::GlobRecursive(zonePath, "*.conf", boost::bind(&ConfigCompiler::CollectIncludes, boost::ref(expressions), _1, zoneName, package), GlobFile);
+	DictExpression expr(expressions);
+	if (!ExecuteExpression(&expr))
+		success = false;
 }
 
 static void IncludePackage(const String& packagePath, bool& success)
 {
+	/* Note: Package includes will register their zones
+	 * for config sync inside their generated config. */
 	String packageName = Utility::BaseName(packagePath);
-	
+
 	if (Utility::PathExists(packagePath + "/include.conf")) {
 		Expression *expr = ConfigCompiler::CompileFile(packagePath + "/include.conf",
 		    String(), packageName);
-		
+
 		if (!ExecuteExpression(expr))
 			success = false;
-			
+
 		delete expr;
 	}
 }
@@ -99,8 +114,9 @@ bool DaemonUtility::ValidateConfigFiles(const std::vector<std::string>& configs,
 		}
 	}
 
-	/* Load cluster config files - this should probably be in libremote but
+	/* Load cluster config files from /etc/icinga2/zones.d.
-	* unfortunately moving it there is somewhat non-trivial. */
+	 * This should probably be in libremote but
+	 * unfortunately moving it there is somewhat non-trivial. */
 	success = true;
 
 	String zonesEtcDir = Application::GetZonesDir();
@@ -110,16 +126,19 @@ bool DaemonUtility::ValidateConfigFiles(const std::vector<std::string>& configs,
 	if (!success)
 		return false;
 
-	String zonesVarDir = Application::GetLocalStateDir() + "/lib/icinga2/api/zones";
+	/* Load package config files - they may contain additional zones which
-	if (Utility::PathExists(zonesVarDir))
+	 * are authoritative on this node and are checked in HasZoneConfigAuthority(). */
-		Utility::Glob(zonesVarDir + "/*", boost::bind(&IncludeNonLocalZone, _1, "_cluster", boost::ref(success)), GlobDirectory);
+	String packagesVarDir = Application::GetLocalStateDir() + "/lib/icinga2/api/packages";
+	if (Utility::PathExists(packagesVarDir))
+		Utility::Glob(packagesVarDir + "/*", boost::bind(&IncludePackage, _1, boost::ref(success)), GlobDirectory);
 
 	if (!success)
 		return false;
 
-	String packagesVarDir = Application::GetLocalStateDir() + "/lib/icinga2/api/packages";
+	/* Load cluster synchronized configuration files */
-	if (Utility::PathExists(packagesVarDir))
+	String zonesVarDir = Application::GetLocalStateDir() + "/lib/icinga2/api/zones";
-		Utility::Glob(packagesVarDir + "/*", boost::bind(&IncludePackage, _1, boost::ref(success)), GlobDirectory);
+	if (Utility::PathExists(zonesVarDir))
+		Utility::Glob(zonesVarDir + "/*", boost::bind(&IncludeNonLocalZone, _1, "_cluster", boost::ref(success)), GlobDirectory);
 
 	if (!success)
 		return false;

lib/config/configcompiler.cpp

@@ -323,3 +323,22 @@ void ConfigCompiler::RegisterZoneDir(const String& tag, const String& ppath, con
 	m_ZoneDirs[zoneName].push_back(zf);
 }
 
+bool ConfigCompiler::HasZoneConfigAuthority(const String& zoneName)
+{
+	std::vector<ZoneFragment> zoneDirs = m_ZoneDirs[zoneName];
+
+	bool empty = zoneDirs.empty();
+
+	if (!empty) {
+		std::vector<String> paths;
+		BOOST_FOREACH(const ZoneFragment& zf, zoneDirs) {
+			paths.push_back(zf.Path);
+		}
+
+		Log(LogNotice, "ConfigCompiler")
+		    << "Registered authoritative config directories for zone '" << zoneName << "': " << Utility::NaturalJoin(paths);
+	}
+
+	return !empty;
+}
+

lib/config/configcompiler.hpp

@@ -118,6 +118,8 @@ public:
 	static std::vector<ZoneFragment> GetZoneDirs(const String& zone);
 	static void RegisterZoneDir(const String& tag, const String& ppath, const String& zoneName);
 
+	static bool HasZoneConfigAuthority(const String& zoneName);
+
 private:
 	boost::promise<boost::shared_ptr<Expression> > m_Promise;
 

lib/remote/apilistener-filesync.cpp

@@ -32,21 +32,6 @@ using namespace icinga;
 
 REGISTER_APIFUNCTION(Update, config, &ApiListener::ConfigUpdateHandler);
 
-bool ApiListener::IsConfigMaster(const Zone::Ptr& zone)
-{
-	std::vector<ZoneFragment> zoneDirs = ConfigCompiler::GetZoneDirs(zone->GetName());
-
-	std::vector<String> paths;
-	BOOST_FOREACH(const ZoneFragment& zf, zoneDirs) {
-		paths.push_back(zf.Path);
-	}
-
-	Log(LogNotice, "ApiListener")
-	    << "Registered config directories for zone '" << zone->GetName() << "': " << Utility::NaturalJoin(paths);
-
-	return zoneDirs.size() > 0;
-}
-
 void ApiListener::ConfigGlobHandler(Dictionary::Ptr& config, const String& path, const String& file)
 {
 	CONTEXT("Creating config update for file '" + file + "'");
@@ -141,10 +126,13 @@ void ApiListener::SyncZoneDir(const Zone::Ptr& zone) const
 		}
 	}
 
+	if (newConfig->GetLength() == 0)
+		return;
+
 	String oldDir = Application::GetLocalStateDir() + "/lib/icinga2/api/zones/" + zone->GetName();
 
 	Log(LogInformation, "ApiListener")
-	    << "Copying zone configuration files for zone '" << zone->GetName() << "' to  '" << oldDir << "'.";
+	    << "Copying " << newConfig->GetLength() << " zone configuration files for zone '" << zone->GetName() << "' to '" << oldDir << "'.";
 
 	Utility::MkDir(oldDir, 0700);
 
@@ -156,12 +144,6 @@ void ApiListener::SyncZoneDir(const Zone::Ptr& zone) const
 void ApiListener::SyncZoneDirs(void) const
 {
 	BOOST_FOREACH(const Zone::Ptr& zone, ConfigType::GetObjectsByType<Zone>()) {
-		if (!IsConfigMaster(zone)) {
-			Log(LogWarning, "ApiListener")
-			    << "Not syncing config update for zone '" << zone->GetName() << "' because we do not have an authoritative version of the zone's config.";
-			continue;
-		}
-
 		try {
 			SyncZoneDir(zone);
 		} catch (const std::exception&) {
@@ -250,7 +232,7 @@ Value ApiListener::ConfigUpdateHandler(const MessageOrigin::Ptr& origin, const D
 			continue;
 		}
 
-		if (IsConfigMaster(zone)) {
+		if (ConfigCompiler::HasZoneConfigAuthority(kv.first)) {
 			Log(LogWarning, "ApiListener")
 			    << "Ignoring config update for zone '" << kv.first << "' because we have an authoritative version of the zone's config.";
 			continue;

lib/remote/apilistener.hpp

@@ -135,7 +135,6 @@ private:
 	void SyncZoneDirs(void) const;
 	void SyncZoneDir(const Zone::Ptr& zone) const;
 
-	static bool IsConfigMaster(const Zone::Ptr& zone);
 	static void ConfigGlobHandler(Dictionary::Ptr& config, const String& path, const String& file);
 	void SendConfigUpdate(const JsonRpcConnection::Ptr& aclient);