Skip to content

Commit

Permalink
Checkable: Don't skip redundancy group checks for parent dependencies
Browse files Browse the repository at this point in the history
Previously, all parent dependencies were recursively checked right at
the beginning of `Checkable::IsReachable()` for reachability and
immediately returned `false` if one of them fails. However, since the
introduction of `redundancy_group`, that failed parent might be within a
redundancy group, which shouldn't necessarily cause the dependency to
fail completely. This PR changes this insofar as not all parents are
checked at the beginning of the method, but only a single parent per
a dependency object is evaluated.
  • Loading branch information
yhabteab committed Nov 12, 2024
1 parent 9a8620d commit 4ad59ed
Showing 1 changed file with 11 additions and 12 deletions.
23 changes: 11 additions & 12 deletions lib/icinga/checkable-dependency.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,6 @@ bool Checkable::IsReachable(DependencyType dt, Dependency::Ptr *failedDependency
return false;
}

for (const Checkable::Ptr& checkable : GetParents()) {
if (!checkable->IsReachable(dt, failedDependency, rstack + 1))
return false;
}

/* implicit dependency on host if this is a service */
const auto *service = dynamic_cast<const Service *>(this);
if (service && (dt == DependencyState || dt == DependencyNotification)) {
Expand All @@ -78,10 +73,14 @@ bool Checkable::IsReachable(DependencyType dt, Dependency::Ptr *failedDependency
std::unordered_map<std::string, Dependency::Ptr> violated; // key: redundancy group, value: nullptr if satisfied, violating dependency otherwise

for (const Dependency::Ptr& dep : deps) {
std::string redundancy_group = dep->GetRedundancyGroup();
bool parentReachable{true};
if (Checkable::Ptr parent = dep->GetParent(); parent.get() != this) {
parentReachable = parent->IsReachable(dt, failedDependency, rstack);
}

if (!dep->IsAvailable(dt)) {
if (redundancy_group.empty()) {
std::string redundancyGroup = dep->GetRedundancyGroup();
if (!parentReachable || !dep->IsAvailable(dt)) {
if (redundancyGroup.empty()) {
Log(LogDebug, "Checkable")
<< "Non-redundant dependency '" << dep->GetName() << "' failed for checkable '" << GetName() << "': Marking as unreachable.";

Expand All @@ -92,11 +91,11 @@ 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)
// so it either passed before (don't overwrite) or already failed (so don't care)
// 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()) {
violated[redundancy_group] = nullptr;
violated.insert(std::make_pair(redundancyGroup, dep));
} else if (!redundancyGroup.empty()) {
violated[redundancyGroup] = nullptr;
}
}

Expand Down

0 comments on commit 4ad59ed

Please sign in to comment.