From 577e42e1373237a7336d218119b504a4d52563cc Mon Sep 17 00:00:00 2001 From: Michael Friedrich Date: Fri, 7 Jun 2019 11:32:27 +0200 Subject: [PATCH] Quality: Comments and logs in cluster config sync --- lib/remote/apilistener-filesync.cpp | 171 +++++++++++++++++----------- 1 file changed, 107 insertions(+), 64 deletions(-) diff --git a/lib/remote/apilistener-filesync.cpp b/lib/remote/apilistener-filesync.cpp index 6279be5cc..f1ed77576 100644 --- a/lib/remote/apilistener-filesync.cpp +++ b/lib/remote/apilistener-filesync.cpp @@ -21,8 +21,8 @@ REGISTER_APIFUNCTION(Update, config, &ApiListener::ConfigUpdateHandler); boost::mutex ApiListener::m_ConfigSyncStageLock; /** - * Entrypoint for updating all authoritative configs into var/lib/icinga2/api/zones - * + * Entrypoint for updating all authoritative configs from /etc/zones.d, packages, etc. + * into var/lib/icinga2/api/zones */ void ApiListener::SyncLocalZoneDirs() const { @@ -36,7 +36,7 @@ void ApiListener::SyncLocalZoneDirs() const } /** - * Sync a zone directory where we have an authoritative copy (zones.d, etc.) + * Sync a zone directory where we have an authoritative copy (zones.d, packages, etc.) * * This function collects the registered zone config dirs from * the config compiler and reads the file content into the config @@ -58,25 +58,27 @@ void ApiListener::SyncLocalZoneDir(const Zone::Ptr& zone) const String zoneName = zone->GetName(); - /* Load registered zone paths, e.g. '_etc', '_api' and user packages. */ + // Load registered zone paths, e.g. '_etc', '_api' and user packages. for (const ZoneFragment& zf : ConfigCompiler::GetZoneDirs(zoneName)) { ConfigDirInformation newConfigPart = LoadConfigDir(zf.Path); - /* Config files '*.conf'. */ + // Config files '*.conf'. { ObjectLock olock(newConfigPart.UpdateV1); for (const Dictionary::Pair& kv : newConfigPart.UpdateV1) { String path = "/" + zf.Tag + kv.first; + newConfigInfo.UpdateV1->Set(path, kv.second); newConfigInfo.Checksums->Set(path, GetChecksum(kv.second)); } } - /* Meta files. */ + // Meta files. { ObjectLock olock(newConfigPart.UpdateV2); for (const Dictionary::Pair& kv : newConfigPart.UpdateV2) { String path = "/" + zf.Tag + kv.first; + newConfigInfo.UpdateV2->Set(path, kv.second); newConfigInfo.Checksums->Set(path, GetChecksum(kv.second)); } @@ -85,6 +87,7 @@ void ApiListener::SyncLocalZoneDir(const Zone::Ptr& zone) const size_t sumUpdates = newConfigInfo.UpdateV1->GetLength() + newConfigInfo.UpdateV2->GetLength(); + // Return early if there are no updates. if (sumUpdates == 0) return; @@ -93,13 +96,13 @@ void ApiListener::SyncLocalZoneDir(const Zone::Ptr& zone) const Log(LogInformation, "ApiListener") << "Copying " << sumUpdates << " zone configuration files for zone '" << zoneName << "' to '" << productionZonesDir << "'."; - /* Purge files to allow deletion via zones.d. */ + // Purge files to allow deletion via zones.d. if (Utility::PathExists(productionZonesDir)) Utility::RemoveDirRecursive(productionZonesDir); Utility::MkDirP(productionZonesDir, 0700); - /* Copy content and add additional meta data. */ + // Copy content and add additional meta data. size_t numBytes = 0; /* Note: We cannot simply copy directories here. @@ -111,15 +114,19 @@ void ApiListener::SyncLocalZoneDir(const Zone::Ptr& zone) const { ObjectLock olock(newConfig); + for (const Dictionary::Pair& kv : newConfig) { String dst = productionZonesDir + "/" + kv.first; + Utility::MkDirP(Utility::DirName(dst), 0755); Log(LogInformation, "ApiListener") << "Updating configuration file: " << dst; String content = kv.second; + std::ofstream fp(dst.CStr(), std::ofstream::out | std::ostream::binary | std::ostream::trunc); + fp << content; fp.close(); @@ -127,11 +134,12 @@ void ApiListener::SyncLocalZoneDir(const Zone::Ptr& zone) const } } - /* Additional metadata. */ + // Additional metadata. String tsPath = productionZonesDir + "/.timestamp"; if (!Utility::PathExists(tsPath)) { std::ofstream fp(tsPath.CStr(), std::ofstream::out | std::ostream::trunc); + fp << std::fixed << Utility::GetTime(); fp.close(); } @@ -143,12 +151,14 @@ void ApiListener::SyncLocalZoneDir(const Zone::Ptr& zone) const fp.close(); } + // Checksums. String checksumsPath = productionZonesDir + "/.checksums"; if (Utility::PathExists(checksumsPath)) Utility::Remove(checksumsPath); std::ofstream fp(checksumsPath.CStr(), std::ofstream::out | std::ostream::trunc); + fp << std::fixed << JsonEncode(newConfigInfo.Checksums); fp.close(); @@ -173,13 +183,13 @@ void ApiListener::SendConfigUpdate(const JsonRpcConnection::Ptr& aclient) Zone::Ptr clientZone = endpoint->GetZone(); Zone::Ptr localZone = Zone::GetLocalZone(); - /* don't try to send config updates to our master */ + // Don't send config updates to parent zones if (!clientZone->IsChildOf(localZone)) return; Dictionary::Ptr configUpdateV1 = new Dictionary(); Dictionary::Ptr configUpdateV2 = new Dictionary(); - Dictionary::Ptr configUpdateChecksums = new Dictionary(); /* new since 2.11 */ + Dictionary::Ptr configUpdateChecksums = new Dictionary(); // new since 2.11 String zonesDir = GetApiZonesDir(); @@ -187,11 +197,11 @@ void ApiListener::SendConfigUpdate(const JsonRpcConnection::Ptr& aclient) String zoneName = zone->GetName(); String zoneDir = zonesDir + zoneName; - /* Only sync child and global zones. */ + // Only sync child and global zones. if (!zone->IsChildOf(clientZone) && !zone->IsGlobal()) continue; - /* Zone was configured, but there's no configuration directory. */ + // Zone was configured, but there's no configuration directory. if (!Utility::PathExists(zoneDir)) continue; @@ -203,7 +213,7 @@ void ApiListener::SendConfigUpdate(const JsonRpcConnection::Ptr& aclient) configUpdateV1->Set(zoneName, config.UpdateV1); configUpdateV2->Set(zoneName, config.UpdateV2); - configUpdateChecksums->Set(zoneName, config.Checksums); /* new since 2.11 */ + configUpdateChecksums->Set(zoneName, config.Checksums); // new since 2.11 } Dictionary::Ptr message = new Dictionary({ @@ -211,8 +221,8 @@ void ApiListener::SendConfigUpdate(const JsonRpcConnection::Ptr& aclient) { "method", "config::Update" }, { "params", new Dictionary({ { "update", configUpdateV1 }, - { "update_v2", configUpdateV2 }, /* Since 2.4.2. */ - { "checksums", configUpdateChecksums } /* Since 2.11.0. */ + { "update_v2", configUpdateV2 }, // Since 2.4.2. + { "checksums", configUpdateChecksums } // Since 2.11.0. }) } }); @@ -233,7 +243,7 @@ void ApiListener::SendConfigUpdate(const JsonRpcConnection::Ptr& aclient) */ Value ApiListener::ConfigUpdateHandler(const MessageOrigin::Ptr& origin, const Dictionary::Ptr& params) { - /* Verify permissions and trust relationship. */ + // Verify permissions and trust relationship. if (!origin->FromClient->GetEndpoint() || (origin->FromZone && !Zone::GetLocalZone()->IsChildOf(origin->FromZone))) return Empty; @@ -263,12 +273,12 @@ Value ApiListener::ConfigUpdateHandler(const MessageOrigin::Ptr& origin, const D << "Applying config update from endpoint '" << fromEndpointName << "' of zone '" << fromZoneName << "'."; - /* Config files. */ + // Config files. Dictionary::Ptr updateV1 = params->Get("update"); - /* Meta data files: .timestamp, etc. */ + // Meta data files: .timestamp, etc. Dictionary::Ptr updateV2 = params->Get("update_v2"); - /* New since 2.11.0. */ + // New since 2.11.0. Dictionary::Ptr checksums; if (params->Contains("checksums")) @@ -276,7 +286,7 @@ Value ApiListener::ConfigUpdateHandler(const MessageOrigin::Ptr& origin, const D bool configChange = false; - /* Keep track of the relative config paths for later validation and copying. */ + // Keep track of the relative config paths for later validation and copying. TODO: Find a better algorithm. std::vector relativePaths; /* @@ -290,11 +300,12 @@ Value ApiListener::ConfigUpdateHandler(const MessageOrigin::Ptr& origin, const D Utility::MkDirP(apiZonesStageDir, 0700); - /* Analyse and process the update. */ + // Analyse and process the update. ObjectLock olock(updateV1); + for (const Dictionary::Pair& kv : updateV1) { - /* Check for the configured zones. */ + // Check for the configured zones. String zoneName = kv.first; Zone::Ptr zone = Zone::GetByName(zoneName); @@ -302,39 +313,42 @@ Value ApiListener::ConfigUpdateHandler(const MessageOrigin::Ptr& origin, const D Log(LogWarning, "ApiListener") << "Ignoring config update from endpoint '" << fromEndpointName << "' for unknown zone '" << zoneName << "'."; + continue; } - /* Whether we already have configuration in zones.d. */ + // Ignore updates where we have an authoritive copy in etc/zones.d, packages, etc. if (ConfigCompiler::HasZoneConfigAuthority(zoneName)) { Log(LogInformation, "ApiListener") << "Ignoring config update from endpoint '" << fromEndpointName << "' for zone '" << zoneName << "' because we have an authoritative version of the zone's config."; + continue; } - /* Put the received configuration into our stage directory. */ + // Put the received configuration into our stage directory. String productionConfigZoneDir = GetApiZonesDir() + zoneName; String stageConfigZoneDir = GetApiZonesStageDir() + zoneName; Utility::MkDirP(productionConfigZoneDir, 0700); Utility::MkDirP(stageConfigZoneDir, 0700); - /* Merge the config information. */ + // Merge the config information. ConfigDirInformation newConfigInfo; newConfigInfo.UpdateV1 = kv.second; - /* Load metadata. */ + // Load metadata. if (updateV2) newConfigInfo.UpdateV2 = updateV2->Get(kv.first); - /* Load checksums. New since 2.11. */ + // Load checksums. New since 2.11. if (checksums) newConfigInfo.Checksums = checksums->Get(kv.first); - /* Load the current production config details. */ + // Load the current production config details. ConfigDirInformation productionConfigInfo = LoadConfigDir(productionConfigZoneDir); + // Merge updateV1 and updateV2 Dictionary::Ptr productionConfig = MergeConfigUpdate(productionConfigInfo); Dictionary::Ptr newConfig = MergeConfigUpdate(newConfigInfo); @@ -346,11 +360,21 @@ Value ApiListener::ConfigUpdateHandler(const MessageOrigin::Ptr& origin, const D << "Received configuration for zone '" << zoneName << "' from endpoint '" << fromEndpointName << "'. Comparing the checksums."; - /* TODO: Do this earlier in hello-handshakes. */ + // TODO: Do this earlier in hello-handshakes? if (CheckConfigChange(productionConfigInfo, newConfigInfo)) configChange = true; + } else { - /* TODO: Figure out whether we always need to rely on the timestamp flags when there are checksums involved. */ + /* Fallback to timestamp handling when the parent endpoint didn't send checks. + * This can happen when the satellite is 2.11 and the master is 2.10. + * + * TODO: Deprecate and remove this behaviour in 2.13+. + */ + + Log(LogWarning, "ApiListener") + << "Received configuration update without checksums from parent endpoint " + << fromEndpointName << ". This behaviour is deprecated. Please upgrade the parent endpoint to 2.11+"; + double productionTimestamp; if (!productionConfig->Contains("/.timestamp")) @@ -365,8 +389,9 @@ Value ApiListener::ConfigUpdateHandler(const MessageOrigin::Ptr& origin, const D else newTimestamp = newConfig->Get("/.timestamp"); - /* skip update if our configuration files are more recent */ + // Skip update if our configuration files are more recent. if (productionTimestamp >= newTimestamp) { + Log(LogInformation, "ApiListener") << "Our configuration is more recent than the received configuration update." << " Ignoring configuration file update for path '" << stageConfigZoneDir << "'. Current timestamp '" @@ -375,21 +400,26 @@ Value ApiListener::ConfigUpdateHandler(const MessageOrigin::Ptr& origin, const D << ") >= received timestamp '" << Utility::FormatDateTime("%Y-%m-%d %H:%M:%S %z", newTimestamp) << "' (" << newTimestamp << ")."; + } else { configChange = true; } - /* Keep another hack when there's a timestamp file missing. */ - ObjectLock olock(newConfig); - for (const Dictionary::Pair& kv : newConfig) { - /* This is super expensive with a string content comparison. */ - if (productionConfig->Get(kv.first) != kv.second) { - if (!Utility::Match("*/.timestamp", kv.first)) - configChange = true; + // Keep another hack when there's a timestamp file missing. + { + ObjectLock olock(newConfig); + + for (const Dictionary::Pair &kv : newConfig) { + + // This is super expensive with a string content comparison. + if (productionConfig->Get(kv.first) != kv.second) { + if (!Utility::Match("*/.timestamp", kv.first)) + configChange = true; + } } } - /* Update the .timestamp file. */ + // Update the .timestamp file. String tsPath = stageConfigZoneDir + "/.timestamp"; if (!Utility::PathExists(tsPath)) { std::ofstream fp(tsPath.CStr(), std::ofstream::out | std::ostream::trunc); @@ -398,17 +428,20 @@ Value ApiListener::ConfigUpdateHandler(const MessageOrigin::Ptr& origin, const D } } - /* Dump the received configuration for this zone into the stage directory. */ + // Dump the received configuration for this zone into the stage directory. size_t numBytes = 0; { ObjectLock olock(newConfig); + for (const Dictionary::Pair& kv : newConfig) { + /* Store the relative config file path for later validation and activation. - * IMPORTANT: Store this prior to any filters. */ + * IMPORTANT: Store this prior to any filters. + * */ relativePaths.push_back(zoneName + "/" + kv.first); - /* Ignore same config content. This is an expensive comparison. */ + // Ignore same config content. This is an expensive comparison. if (productionConfig->Get(kv.first) == kv.second) continue; @@ -417,11 +450,13 @@ Value ApiListener::ConfigUpdateHandler(const MessageOrigin::Ptr& origin, const D Log(LogInformation, "ApiListener") << "Stage: Updating received configuration file '" << path << "' for zone '" << zoneName << "'."; - /* Sync string content only. */ + // Sync string content only. String content = kv.second; - /* Generate a directory tree (zones/1/2/3 might not exist yet). */ + // Generate a directory tree (zones/1/2/3 might not exist yet). Utility::MkDirP(Utility::DirName(path), 0755); + + // Write the content to file. std::ofstream fp(path.CStr(), std::ofstream::out | std::ostream::binary | std::ostream::trunc); fp << content; fp.close(); @@ -434,9 +469,10 @@ Value ApiListener::ConfigUpdateHandler(const MessageOrigin::Ptr& origin, const D << "Applying configuration file update for path '" << stageConfigZoneDir << "' (" << numBytes << " Bytes)."; - /* If the update removes a path, delete it on disk and signal a config change. */ + // If the update removes a path, delete it on disk and signal a config change. { ObjectLock xlock(productionConfig); + for (const Dictionary::Pair& kv : productionConfig) { if (!newConfig->Contains(kv.first)) { configChange = true; @@ -450,9 +486,11 @@ Value ApiListener::ConfigUpdateHandler(const MessageOrigin::Ptr& origin, const D /* * We have processed all configuration files and stored them in the staging directory. + * * We need to store them locally for later analysis. A config change means * that we will validate the configuration in a separate process sandbox, * and only copy the configuration to production when everything is ok. + * * A successful validation also triggers the final restart. */ if (configChange) { @@ -494,18 +532,18 @@ void ApiListener::TryActivateZonesStageCallback(const ProcessResult& pr, fpStatus << pr.ExitStatus; fpStatus.close(); - /* validation went fine, copy stage and reload */ + // Validation went fine, copy stage and reload. if (pr.ExitStatus == 0) { Log(LogInformation, "ApiListener") << "Config validation for stage '" << apiZonesStageDir << "' was OK, replacing into '" << apiZonesDir << "' and triggering reload."; - /* Purge production before copying stage. */ + // Purge production before copying stage. if (Utility::PathExists(apiZonesDir)) Utility::RemoveDirRecursive(apiZonesDir); Utility::MkDirP(apiZonesDir, 0700); - /* Copy all synced configuration files from stage to production. */ + // Copy all synced configuration files from stage to production. for (const String& path : relativePaths) { Log(LogInformation, "ApiListener") << "Copying file '" << path << "' from config sync staging to production zones directory."; @@ -518,6 +556,7 @@ void ApiListener::TryActivateZonesStageCallback(const ProcessResult& pr, Utility::CopyFile(stagePath, currentPath); } + // Clear any failed deployment before ApiListener::Ptr listener = ApiListener::GetInstance(); if (listener) @@ -525,10 +564,11 @@ void ApiListener::TryActivateZonesStageCallback(const ProcessResult& pr, Application::RequestRestart(); + // All good, return early. return; } - /* Error case. */ + // Error case. Log(LogCritical, "ApiListener") << "Config validation failed for staged cluster config sync in '" << apiZonesStageDir << "'. Aborting. Logs: '" << logFile << "'"; @@ -565,7 +605,7 @@ void ApiListener::AsyncTryActivateZonesStage(const std::vector& relative args->Add("--validate"); - /* Set the ZonesStageDir. This creates our own local chroot without any additional automated zone includes. */ + // Set the ZonesStageDir. This creates our own local chroot without any additional automated zone includes. args->Add("--define"); args->Add("System.ZonesStageVarDir=" + GetApiZonesStageDir()); @@ -615,39 +655,43 @@ bool ApiListener::CheckConfigChange(const ConfigDirInformation& oldConfig, const Dictionary::Ptr oldChecksums = oldConfig.Checksums; Dictionary::Ptr newChecksums = newConfig.Checksums; - Log(LogCritical, "ApiListener") - << "Comparing old (" << oldChecksums->GetLength() << "): '" + // TODO: Figure out whether normal users need this for debugging. + Log(LogDebug, "ApiListener") + << "Checking for config change between stage and production. Old (" << oldChecksums->GetLength() << "): '" << JsonEncode(oldChecksums) - << "' to new (" << newChecksums->GetLength() << "): '" + << "' vs. new (" << newChecksums->GetLength() << "): '" << JsonEncode(newChecksums) << "'."; - /* Different length means that either one or the other side added or removed something. */ + // Different length means that either one or the other side added or removed something. */ if (oldChecksums->GetLength() != newChecksums->GetLength()) return true; - /* Both dictionaries have an equal size. */ + // Both dictionaries have an equal size. ObjectLock olock(oldChecksums); + for (const Dictionary::Pair& kv : oldChecksums) { String path = kv.first; String oldChecksum = kv.second; - /* Only use configuration files for checksum calculation. */ + + // TODO: Figure out if config changes only apply to '.conf'. Leaving this open for other config files. //if (!Utility::Match("*.conf", path)) // continue; /* Ignore internal files, especially .timestamp and .checksums. + * * If we don't, this results in "always change" restart loops. */ if (Utility::Match("/.*", path)) continue; - Log(LogCritical, "ApiListener") + Log(LogDebug, "ApiListener") << "Checking " << path << " for checksum: " << oldChecksum; - /* Check whether our key exists in the new checksums, and they have an equal value. */ + // Check whether our key exists in the new checksums, and they have an equal value. String newChecksum = newChecksums->Get(path); if (newChecksums->Get(path) != kv.second) { - Log(LogCritical, "ApiListener") + Log(LogDebug, "ApiListener") << "Path '" << path << "' doesn't match old checksum '" << newChecksum << "' with new checksum '" << oldChecksum << "'."; return true; @@ -684,7 +728,7 @@ ConfigDirInformation ApiListener::LoadConfigDir(const String& dir) */ void ApiListener::ConfigGlobHandler(ConfigDirInformation& config, const String& path, const String& file) { - /* Avoid loading the authoritative marker for syncs. */ + // Avoid loading the authoritative marker for syncs at all cost. if (Utility::BaseName(file) == ".authoritative") return; @@ -716,7 +760,7 @@ void ApiListener::ConfigGlobHandler(ConfigDirInformation& config, const String& /* Calculate a checksum for each file (and a global one later). * - * IMPORTANT: Ignore the .authoritative file, this will not be synced. + * IMPORTANT: Ignore the .authoritative file above, this must not be synced. * */ config.Checksums->Set(relativePath, GetChecksum(content)); } @@ -738,5 +782,4 @@ Dictionary::Ptr ApiListener::MergeConfigUpdate(const ConfigDirInformation& confi config.UpdateV2->CopyTo(result); return result; -} - +} \ No newline at end of file -- 2.40.0