From cfef9fdadc4d24ae4d30995a21220b43bee0ba58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Fu=C3=9F?= Date: Tue, 8 Sep 2020 17:25:45 +0200 Subject: [PATCH 1/6] Introduce redundancy groups for Dependency Objects Traditional behaviour was to regard all dependecies as cumulative (e.g., the parent considered unreachable if any one dependency is violated), commit ed5892238916ab667a4c9d904bd73acd3ed162f2 made all dependencies regarded redundant (e.g., the parent considered unreachable only if all dependency are violated). This may lead to unrelated services (or even hosts vs. services) inadvertantly regarded to be redundant to each other. Most importantly, applying the explicit "disable-host-service-checks" dependency described in the "Monitoring Basics" chapter will defeat all other dependencies. This commit introduces a new "redundancy_group" attribute for dependencies. Specifying a redundancy_group causes a dependency to be regarded as redundant only inside that redundancy group. Dependencies lacking a redundancy_group attribute are regarded as essential for the parent. This allows for both cumulative and redundant dependencies and even a combination (cumulation of redundancies, like SSH depeding on both LDAP and DNS to function, while operating redundant LDAP servers as well as redundant DNS resolvers). This commit lacks changes to the tests. --- doc/09-object-types.md | 11 +++++++++ lib/icinga/checkable-dependency.cpp | 36 +++++++++++++++++++++-------- lib/icinga/dependency.ti | 2 ++ 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/doc/09-object-types.md b/doc/09-object-types.md index cdb6db5fb..af8075772 100644 --- a/doc/09-object-types.md +++ b/doc/09-object-types.md @@ -201,6 +201,7 @@ Configuration Attributes: parent\_service\_name | Object name | **Optional.** The parent service. If omitted, this dependency object is treated as host dependency. child\_host\_name | Object name | **Required.** The child host. child\_service\_name | Object name | **Optional.** The child service. If omitted, this dependency object is treated as host dependency. + redundancy\_group | String | **Optional.** Puts the dependency into a group of mutually redundant ones. See discussion below. disable\_checks | Boolean | **Optional.** Whether to disable checks (i.e., don't schedule active checks and drop passive results) when this dependency fails. Defaults to false. disable\_notifications | Boolean | **Optional.** Whether to disable notifications when this dependency fails. Defaults to true. ignore\_soft\_states | Boolean | **Optional.** Whether to ignore soft states for the reachability calculation. Defaults to true. @@ -218,6 +219,16 @@ Up Down ``` +Redundancy groups: + +Sometimes, you want a dependencies to accumulate (e.g., the parent considered reachable only if no dependency is violated), sometimes you want them to be regarded as redundant (e.g., the parent considered unreachable only if no dependency is fulfilled) or even a mixture of both. Think of a host connected to both a network and a storage switch vs. a host connected to redundant routers or a service like SSH depeding on both LDAP and DNS to function, while operating redundant LDAP servers as well as redundant DNS resolvers. + +Behaviour prior to 2.12.0 was to regard all dependecies as cumulative; 2.12.0 made all dependencies regareded redundant. +This may lead to unrelated services inadvertantly regarded to be redundant to each other. + +Specifying a `redundancy_group` causes a dependency to be regarded as redundant only inside that redundancy group. +Dependencies lacking a `redundancy_group` attribute are regarded as essential for the parent. + When using [apply rules](03-monitoring-basics.md#using-apply) for dependencies, you can leave out certain attributes which will be automatically determined by Icinga 2. diff --git a/lib/icinga/checkable-dependency.cpp b/lib/icinga/checkable-dependency.cpp index 5ce92886c..736553a48 100644 --- a/lib/icinga/checkable-dependency.cpp +++ b/lib/icinga/checkable-dependency.cpp @@ -74,25 +74,43 @@ bool Checkable::IsReachable(DependencyType dt, Dependency::Ptr *failedDependency auto deps = GetDependencies(); - int countDeps = deps.size(); - int countFailed = 0; + std::unordered_map violated; // key: redundancy group, value: nullptr if satisfied, violating dependency otherwise for (const Dependency::Ptr& dep : deps) { - if (!dep->IsAvailable(dt)) { - countFailed++; + std::string redundancy_group = dep->GetRedundancyGroup(); - if (failedDependency) - *failedDependency = dep; + if (!dep->IsAvailable(dt)) { + if (redundancy_group.empty()) { + Log(LogDebug, "Checkable") + << "Non-redundant dependency '" << dep->GetName() << "' failed for checkable '" << GetName() << "': Marking as unreachable."; + + if (failedDependency) + *failedDependency = dep; + + return false; + } + + // tentatively mark this dependency group as failed unless it is already marked; + // so it either passed before (don't overwrite) or already failed (so don't care) + if (violated.find(redundancy_group) == violated.end()) + violated.insert(std::make_pair(redundancy_group, dep)); + } else if (!redundancy_group.empty()) { + // definitely mark this dependency group as passed + violated.insert(std::make_pair(redundancy_group, nullptr)); } } - /* If there are dependencies, and all of them failed, mark as unreachable. */ - if (countDeps > 0 && countFailed == countDeps) { + auto violator = std::find_if(violated.begin(), violated.end(), [](const std::pair&v) { return v.second != nullptr; }); + if (violator != violated.end()) { Log(LogDebug, "Checkable") - << "All dependencies have failed for checkable '" << GetName() << "': Marking as unreachable."; + << "All dependencies in redundancy group '" << violator->first << "' have failed for checkable '" << GetName() << "': Marking as unreachable."; + + if (failedDependency) + *failedDependency = violator->second; return false; } + if (failedDependency) *failedDependency = nullptr; diff --git a/lib/icinga/dependency.ti b/lib/icinga/dependency.ti index 3fc832522..41de7ba23 100644 --- a/lib/icinga/dependency.ti +++ b/lib/icinga/dependency.ti @@ -77,6 +77,8 @@ class Dependency : CustomVarObject < DependencyNameComposer }}} }; + [config] String redundancy_group; + [config, navigation] name(TimePeriod) period (PeriodRaw) { navigate {{{ return TimePeriod::GetByName(GetPeriodRaw()); From 5bba609e60ddc64b9200bfa140b26106ae63ebf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Fu=C3=9F?= Date: Tue, 5 Oct 2021 16:44:20 +0200 Subject: [PATCH 2/6] Add missing #include --- lib/icinga/checkable-dependency.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/icinga/checkable-dependency.cpp b/lib/icinga/checkable-dependency.cpp index 736553a48..80f567356 100644 --- a/lib/icinga/checkable-dependency.cpp +++ b/lib/icinga/checkable-dependency.cpp @@ -3,6 +3,7 @@ #include "icinga/service.hpp" #include "icinga/dependency.hpp" #include "base/logger.hpp" +#include using namespace icinga; From 20d7e1b5e66ca8f3e5b44667e37d0b08633b4e5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edgar=20Fu=C3=9F?= Date: Tue, 5 Oct 2021 16:51:32 +0200 Subject: [PATCH 3/6] Fix use of std::unordered_map::insert() as pointed out by Nathaniel Wesley Filardo in GitHup Pull Request #8999 --- lib/icinga/checkable-dependency.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/icinga/checkable-dependency.cpp b/lib/icinga/checkable-dependency.cpp index 80f567356..104d3444c 100644 --- a/lib/icinga/checkable-dependency.cpp +++ b/lib/icinga/checkable-dependency.cpp @@ -93,10 +93,13 @@ bool Checkable::IsReachable(DependencyType dt, Dependency::Ptr *failedDependency // tentatively mark this dependency group as failed unless it is already marked; // so it either passed before (don't overwrite) or already failed (so don't care) - if (violated.find(redundancy_group) == violated.end()) - violated.insert(std::make_pair(redundancy_group, dep)); + // note that std::unordered_map::insert() will not overwrite an existing entry + violated.insert(std::make_pair(redundancy_group, dep)); } else if (!redundancy_group.empty()) { // definitely mark this dependency group as passed + // as std::unordered_map::insert() will not overwrite an existing entry, erase it first + // in C++17, one could use std::unorderer_map::insert_or_assign() instead + violated.erase(redundancy_group); violated.insert(std::make_pair(redundancy_group, nullptr)); } } From 396a71c6a0b203555c33f5381aa18880ca14280a Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Tue, 21 Feb 2023 17:49:26 +0100 Subject: [PATCH 4/6] Repair unit tests --- test/icinga-dependencies.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/test/icinga-dependencies.cpp b/test/icinga-dependencies.cpp index b31f540b1..929b6ca0d 100644 --- a/test/icinga-dependencies.cpp +++ b/test/icinga-dependencies.cpp @@ -70,14 +70,26 @@ BOOST_AUTO_TEST_CASE(multi_parent) /* Test the reachability from this point. * parentHost1 is DOWN, parentHost2 is UP. - * Expected result: childHost is reachable. + * Expected result: childHost is unreachable. */ parentHost1->SetStateRaw(ServiceCritical); // parent Host 1 DOWN parentHost2->SetStateRaw(ServiceOK); // parent Host 2 UP + BOOST_CHECK(childHost->IsReachable() == false); + + /* The only DNS server is DOWN. + * Expected result: childHost is unreachable. + */ + dep1->SetRedundancyGroup("DNS"); + BOOST_CHECK(childHost->IsReachable() == false); + + /* 1/2 DNS servers is DOWN. + * Expected result: childHost is reachable. + */ + dep2->SetRedundancyGroup("DNS"); BOOST_CHECK(childHost->IsReachable() == true); - /* parentHost1 is DOWN, parentHost2 is DOWN. + /* Both DNS servers are DOWN. * Expected result: childHost is unreachable. */ parentHost1->SetStateRaw(ServiceCritical); // parent Host 1 DOWN From 35248b1b63631840f62934043ed36aee648840f8 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Mon, 3 Apr 2023 13:39:08 +0200 Subject: [PATCH 5/6] Code style --- lib/icinga/checkable-dependency.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/icinga/checkable-dependency.cpp b/lib/icinga/checkable-dependency.cpp index 104d3444c..58d6b578b 100644 --- a/lib/icinga/checkable-dependency.cpp +++ b/lib/icinga/checkable-dependency.cpp @@ -96,15 +96,11 @@ bool Checkable::IsReachable(DependencyType dt, Dependency::Ptr *failedDependency // note that std::unordered_map::insert() will not overwrite an existing entry violated.insert(std::make_pair(redundancy_group, dep)); } else if (!redundancy_group.empty()) { - // definitely mark this dependency group as passed - // as std::unordered_map::insert() will not overwrite an existing entry, erase it first - // in C++17, one could use std::unorderer_map::insert_or_assign() instead - violated.erase(redundancy_group); - violated.insert(std::make_pair(redundancy_group, nullptr)); + violated[redundancy_group] = nullptr; } } - auto violator = std::find_if(violated.begin(), violated.end(), [](const std::pair&v) { return v.second != nullptr; }); + auto violator = std::find_if(violated.begin(), violated.end(), [](auto& v) { return v.second != nullptr; }); if (violator != violated.end()) { Log(LogDebug, "Checkable") << "All dependencies in redundancy group '" << violator->first << "' have failed for checkable '" << GetName() << "': Marking as unreachable."; From bcd16cbfb05666bb8cd74cd65d5c723c4e8d66c3 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Wed, 5 Apr 2023 14:46:35 +0200 Subject: [PATCH 6/6] Rewrite Dependency redundancy groups docs --- doc/03-monitoring-basics.md | 21 +++++++++++++++++++++ doc/09-object-types.md | 12 +----------- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/doc/03-monitoring-basics.md b/doc/03-monitoring-basics.md index 79ef8cc5d..0c5281272 100644 --- a/doc/03-monitoring-basics.md +++ b/doc/03-monitoring-basics.md @@ -2738,6 +2738,27 @@ apply Dependency "internet" to Service { } ``` +### Redundancy Groups + +Sometimes you want dependencies to accumulate, +i.e. to consider the parent reachable only if no dependency is violated. +Sometimes you want them to be regarded as redundant, +i.e. to consider the parent unreachable only if no dependency is fulfilled. +Think of a host connected to both a network and a storage switch vs. a host connected to redundant routers. + +Sometimes you even want a mixture of both. +Think of a service like SSH depeding on both LDAP and DNS to function, +while operating redundant LDAP servers as well as redundant DNS resolvers. + +Before v2.12, Icinga regarded all dependecies as cumulative. +In v2.12 and v2.13, Icinga regarded all dependencies redundant. +The latter led to unrelated services being inadvertantly regarded to be redundant to each other. + +v2.14 restored the former behavior and allowed to override it. +I.e. all dependecies are regarded as essential for the parent by default. +Specifying the `redundancy_group` attribute for two dependecies of a child object with the equal value +causes them to be regarded as redundant (only inside that redundancy group). + diff --git a/doc/09-object-types.md b/doc/09-object-types.md index af8075772..6763b6a87 100644 --- a/doc/09-object-types.md +++ b/doc/09-object-types.md @@ -201,7 +201,7 @@ Configuration Attributes: parent\_service\_name | Object name | **Optional.** The parent service. If omitted, this dependency object is treated as host dependency. child\_host\_name | Object name | **Required.** The child host. child\_service\_name | Object name | **Optional.** The child service. If omitted, this dependency object is treated as host dependency. - redundancy\_group | String | **Optional.** Puts the dependency into a group of mutually redundant ones. See discussion below. + redundancy\_group | String | **Optional.** Puts the dependency into a group of [mutually redundant ones](03-monitoring-basics.md#dependencies-redundancy-groups). disable\_checks | Boolean | **Optional.** Whether to disable checks (i.e., don't schedule active checks and drop passive results) when this dependency fails. Defaults to false. disable\_notifications | Boolean | **Optional.** Whether to disable notifications when this dependency fails. Defaults to true. ignore\_soft\_states | Boolean | **Optional.** Whether to ignore soft states for the reachability calculation. Defaults to true. @@ -219,16 +219,6 @@ Up Down ``` -Redundancy groups: - -Sometimes, you want a dependencies to accumulate (e.g., the parent considered reachable only if no dependency is violated), sometimes you want them to be regarded as redundant (e.g., the parent considered unreachable only if no dependency is fulfilled) or even a mixture of both. Think of a host connected to both a network and a storage switch vs. a host connected to redundant routers or a service like SSH depeding on both LDAP and DNS to function, while operating redundant LDAP servers as well as redundant DNS resolvers. - -Behaviour prior to 2.12.0 was to regard all dependecies as cumulative; 2.12.0 made all dependencies regareded redundant. -This may lead to unrelated services inadvertantly regarded to be redundant to each other. - -Specifying a `redundancy_group` causes a dependency to be regarded as redundant only inside that redundancy group. -Dependencies lacking a `redundancy_group` attribute are regarded as essential for the parent. - When using [apply rules](03-monitoring-basics.md#using-apply) for dependencies, you can leave out certain attributes which will be automatically determined by Icinga 2.