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 14, 2024
1 parent 67175c4 commit cab27fc
Showing 1 changed file with 4 additions and 8 deletions.
12 changes: 4 additions & 8 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,9 +73,10 @@ 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(dep->GetParent()->IsReachable(dt, failedDependency, rstack + 1));

if (!dep->IsAvailable(dt)) {
std::string redundancy_group = dep->GetRedundancyGroup();
if (!parentReachable || !dep->IsAvailable(dt)) {
if (redundancy_group.empty()) {
Log(LogDebug, "Checkable")
<< "Non-redundant dependency '" << dep->GetName() << "' failed for checkable '" << GetName() << "': Marking as unreachable.";
Expand All @@ -92,7 +88,7 @@ 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()) {
Expand Down

0 comments on commit cab27fc

Please sign in to comment.