mirror of https://github.com/Icinga/icinga2.git
Merge pull request #10360 from Icinga/dependency-cycle-detection
Rework dependency cycle detection
This commit is contained in:
Julian Brost
GitHub
commit
e6ad2199fc
|
|
@ -38,6 +38,7 @@ set(base_SOURCES
|
|||
filelogger.cpp filelogger.hpp filelogger-ti.hpp
|
||||
function.cpp function.hpp function-ti.hpp function-script.cpp functionwrapper.hpp
|
||||
initialize.cpp initialize.hpp
|
||||
intrusive-ptr.hpp
|
||||
io-engine.cpp io-engine.hpp
|
||||
journaldlogger.cpp journaldlogger.hpp journaldlogger-ti.hpp
|
||||
json.cpp json.hpp json-script.cpp
|
||||
|
|
|
|||
|
|
@ -9,11 +9,13 @@
|
|||
#include "base/dictionary.hpp"
|
||||
#include <shared_mutex>
|
||||
#include <unordered_map>
|
||||
#include <boost/signals2.hpp>
|
||||
|
||||
namespace icinga
|
||||
{
|
||||
|
||||
class ConfigObject;
|
||||
class ConfigItems;
|
||||
|
||||
class ConfigType
|
||||
{
|
||||
|
|
@ -48,6 +50,13 @@ for (const auto& object : objects) {
|
|||
|
||||
int GetObjectCount() const;
|
||||
|
||||
/**
|
||||
* Signal that allows hooking into the config loading process just before ConfigObject::OnAllConfigLoaded() is
|
||||
* called for a bunch of objects. A vector of pointers to these objects is passed as an argument. All elements
|
||||
* are of the object type the signal is called on.
|
||||
*/
|
||||
boost::signals2::signal<void (const ConfigItems&)> BeforeOnAllConfigLoaded;
|
||||
|
||||
private:
|
||||
typedef std::unordered_map<String, intrusive_ptr<ConfigObject> > ObjectMap;
|
||||
typedef std::vector<intrusive_ptr<ConfigObject> > ObjectVector;
|
||||
|
|
|
|||
|
|
@ -0,0 +1,22 @@
|
|||
/* Icinga 2 | (c) 2025 Icinga GmbH | GPLv2+ */
|
||||
|
||||
#pragma once
|
||||
|
||||
#include "base/i2-base.hpp"
|
||||
#include <memory>
|
||||
#include <boost/smart_ptr/intrusive_ptr.hpp>
|
||||
#include <boost/version.hpp>
|
||||
|
||||
// std::hash is only implemented starting from Boost 1.74. Implement it ourselves for older version to allow using
|
||||
// boost::intrusive_ptr inside std::unordered_set<> or as the key of std::unordered_map<>.
|
||||
// https://github.com/boostorg/smart_ptr/commit/5a18ffdc5609a0e64b63e47cb81c4f0847e0c087
|
||||
#if BOOST_VERSION < 107400
|
||||
template<class T>
|
||||
struct std::hash<boost::intrusive_ptr<T>>
|
||||
{
|
||||
std::size_t operator()(const boost::intrusive_ptr<T>& ptr) const noexcept
|
||||
{
|
||||
return std::hash<T*>{}(ptr.get());
|
||||
}
|
||||
};
|
||||
#endif /* BOOST_VERSION < 107400 */
|
||||
|
|
@ -5,6 +5,7 @@
|
|||
|
||||
#include "base/i2-base.hpp"
|
||||
#include "base/debug.hpp"
|
||||
#include "base/intrusive-ptr.hpp"
|
||||
#include <boost/smart_ptr/intrusive_ptr.hpp>
|
||||
#include <atomic>
|
||||
#include <cstddef>
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@
|
|||
#define SHARED_H
|
||||
|
||||
#include "base/atomic.hpp"
|
||||
#include "base/intrusive-ptr.hpp"
|
||||
#include <boost/smart_ptr/intrusive_ptr.hpp>
|
||||
#include <cstdint>
|
||||
#include <utility>
|
||||
|
|
|
|||
|
|
@ -256,17 +256,6 @@ bool DaemonUtility::LoadConfigFiles(const std::vector<std::string>& configs,
|
|||
upq.SetName("DaemonUtility::LoadConfigFiles");
|
||||
bool result = ConfigItem::CommitItems(ascope.GetContext(), upq, newItems);
|
||||
|
||||
if (result) {
|
||||
try {
|
||||
Dependency::AssertNoCycles();
|
||||
} catch (...) {
|
||||
Log(LogCritical, "config")
|
||||
<< DiagnosticInformation(boost::current_exception(), false);
|
||||
|
||||
result = false;
|
||||
}
|
||||
}
|
||||
|
||||
if (!result) {
|
||||
ConfigCompilerContext::GetInstance()->CancelObjectsFile();
|
||||
return false;
|
||||
|
|
|
|||
|
|
@ -499,6 +499,23 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue
|
|||
auto items (itemsByType.find(type.get()));
|
||||
|
||||
if (items != itemsByType.end()) {
|
||||
auto configType = dynamic_cast<ConfigType*>(type.get());
|
||||
|
||||
// Skip the call if no handlers are connected (signal::empty()) or there are no items (vector::empty()).
|
||||
if (configType && !configType->BeforeOnAllConfigLoaded.empty() && !items->second.empty()) {
|
||||
// Call the signal in the WorkQueue so that if an exception is thrown, it is caught by the WorkQueue
|
||||
// and then reported like any other config validation error.
|
||||
upq.Enqueue([&configType, &items]() {
|
||||
configType->BeforeOnAllConfigLoaded(ConfigItems(items->second));
|
||||
});
|
||||
|
||||
upq.Join();
|
||||
|
||||
if (upq.HasExceptions()) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
upq.ParallelFor(items->second, [¬ified_items](const ItemPair& ip) {
|
||||
const ConfigItem::Ptr& item = ip.first;
|
||||
|
||||
|
|
@ -525,6 +542,10 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue
|
|||
});
|
||||
|
||||
upq.Join();
|
||||
|
||||
if (upq.HasExceptions()) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -534,9 +555,6 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue
|
|||
<< "Sent OnAllConfigLoaded to " << notified_items << " items of type '" << type->GetName() << "'.";
|
||||
#endif /* I2_DEBUG */
|
||||
|
||||
if (upq.HasExceptions())
|
||||
return false;
|
||||
|
||||
notified_items = 0;
|
||||
for (auto loadDep : type->GetLoadDependencies()) {
|
||||
auto items (itemsByType.find(loadDep));
|
||||
|
|
|
|||
|
|
@ -101,6 +101,39 @@ private:
|
|||
static bool CommitNewItems(const ActivationContext::Ptr& context, WorkQueue& upq, std::vector<ConfigItem::Ptr>& newItems);
|
||||
};
|
||||
|
||||
/**
|
||||
* Helper class for exposing config items being committed to the ConfigType::BeforeOnAllConfigLoaded callback.
|
||||
*
|
||||
* This class wraps a reference to an internal data structure used in ConfigItem::CommitNewItems() and provides
|
||||
* functions useful for the callbacks without exposing the internals of CommitNewItems().
|
||||
*/
|
||||
class ConfigItems
|
||||
{
|
||||
explicit ConfigItems(std::vector<std::pair<ConfigItem::Ptr, bool>>& items) : m_Items(items) {}
|
||||
|
||||
std::vector<std::pair<ConfigItem::Ptr, bool>>& m_Items;
|
||||
|
||||
friend ConfigItem;
|
||||
|
||||
public:
|
||||
/**
|
||||
* ForEachObject<T>(f) calls f(t) for each object T::Ptr t in vector of underlying config items.
|
||||
*
|
||||
* @tparam T ConfigObject type to iterate over
|
||||
* @tparam F Callback functor type (usually automatically deduced from func)
|
||||
* @param func Functor accepting T::Ptr as an argument to be called for each object
|
||||
*/
|
||||
template<typename T, typename F>
|
||||
void ForEachObject(F func) const
|
||||
{
|
||||
for (const auto& item : m_Items) {
|
||||
if (typename T::Ptr obj = dynamic_pointer_cast<T>(item.first->GetObject()); obj) {
|
||||
func(std::move(obj));
|
||||
}
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
}
|
||||
|
||||
#endif /* CONFIGITEM_H */
|
||||
|
|
|
|||
|
|
@ -9,106 +9,163 @@
|
|||
#include "base/exception.hpp"
|
||||
#include <map>
|
||||
#include <sstream>
|
||||
#include <unordered_map>
|
||||
#include <utility>
|
||||
#include <variant>
|
||||
|
||||
using namespace icinga;
|
||||
|
||||
REGISTER_TYPE(Dependency);
|
||||
|
||||
bool Dependency::m_AssertNoCyclesForIndividualDeps = false;
|
||||
INITIALIZE_ONCE(&Dependency::StaticInitialize);
|
||||
|
||||
struct DependencyCycleNode
|
||||
void Dependency::StaticInitialize()
|
||||
{
|
||||
bool Visited = false;
|
||||
bool OnStack = false;
|
||||
};
|
||||
|
||||
struct DependencyStackFrame
|
||||
{
|
||||
ConfigObject::Ptr Node;
|
||||
bool Implicit;
|
||||
|
||||
inline DependencyStackFrame(ConfigObject::Ptr node, bool implicit = false) : Node(std::move(node)), Implicit(implicit)
|
||||
{ }
|
||||
};
|
||||
|
||||
struct DependencyCycleGraph
|
||||
{
|
||||
std::map<Checkable::Ptr, DependencyCycleNode> Nodes;
|
||||
std::vector<DependencyStackFrame> Stack;
|
||||
};
|
||||
|
||||
static void AssertNoDependencyCycle(const Checkable::Ptr& checkable, DependencyCycleGraph& graph, bool implicit = false);
|
||||
|
||||
static void AssertNoParentDependencyCycle(const Checkable::Ptr& parent, DependencyCycleGraph& graph, bool implicit)
|
||||
{
|
||||
if (graph.Nodes[parent].OnStack) {
|
||||
std::ostringstream oss;
|
||||
oss << "Dependency cycle:\n";
|
||||
|
||||
for (auto& frame : graph.Stack) {
|
||||
oss << frame.Node->GetReflectionType()->GetName() << " '" << frame.Node->GetName() << "'";
|
||||
|
||||
if (frame.Implicit) {
|
||||
oss << " (implicit)";
|
||||
}
|
||||
|
||||
oss << "\n-> ";
|
||||
}
|
||||
|
||||
oss << parent->GetReflectionType()->GetName() << " '" << parent->GetName() << "'";
|
||||
|
||||
if (implicit) {
|
||||
oss << " (implicit)";
|
||||
}
|
||||
|
||||
BOOST_THROW_EXCEPTION(ScriptError(oss.str()));
|
||||
}
|
||||
|
||||
AssertNoDependencyCycle(parent, graph, implicit);
|
||||
ConfigType::Get<Dependency>()->BeforeOnAllConfigLoaded.connect(&BeforeOnAllConfigLoadedHandler);
|
||||
}
|
||||
|
||||
static void AssertNoDependencyCycle(const Checkable::Ptr& checkable, DependencyCycleGraph& graph, bool implicit)
|
||||
/**
|
||||
* Helper class to search for cycles in the dependency graph.
|
||||
*
|
||||
* State is stored inside the class and no synchronization is done,
|
||||
* hence instances of this class must not be used concurrently.
|
||||
*/
|
||||
class DependencyCycleChecker
|
||||
{
|
||||
auto& node (graph.Nodes[checkable]);
|
||||
struct Node
|
||||
{
|
||||
bool Visited = false;
|
||||
bool OnStack = false;
|
||||
std::vector<Dependency::Ptr> ExtraDependencies;
|
||||
};
|
||||
|
||||
if (!node.Visited) {
|
||||
node.Visited = true;
|
||||
node.OnStack = true;
|
||||
graph.Stack.emplace_back(checkable, implicit);
|
||||
std::unordered_map<Checkable::Ptr, Node> m_Nodes;
|
||||
|
||||
for (auto& dep : checkable->GetDependencies()) {
|
||||
graph.Stack.emplace_back(dep);
|
||||
AssertNoParentDependencyCycle(dep->GetParent(), graph, false);
|
||||
graph.Stack.pop_back();
|
||||
}
|
||||
// Stack representing the path currently visited by AssertNoCycle(). Dependency::Ptr represents an edge from its
|
||||
// child to parent, Service::Ptr represents the implicit dependency of that service to its host.
|
||||
std::vector<std::variant<Dependency::Ptr, Service::Ptr>> m_Stack;
|
||||
|
||||
{
|
||||
auto service (dynamic_pointer_cast<Service>(checkable));
|
||||
public:
|
||||
/**
|
||||
* Add a dependency to this DependencyCycleChecker that will be considered by AssertNoCycle() in addition to
|
||||
* dependencies already registered to the checkables. This allows checking if additional dependencies would cause
|
||||
* a cycle before actually registering them to the checkables.
|
||||
*
|
||||
* @param dependency Dependency to additionally consider during the cycle search.
|
||||
*/
|
||||
void AddExtraDependency(Dependency::Ptr dependency)
|
||||
{
|
||||
auto& node = m_Nodes[dependency->GetChild()];
|
||||
node.ExtraDependencies.emplace_back(std::move(dependency));
|
||||
}
|
||||
|
||||
if (service) {
|
||||
AssertNoParentDependencyCycle(service->GetHost(), graph, true);
|
||||
/**
|
||||
* Searches the dependency graph for cycles and throws an exception if one is found.
|
||||
*
|
||||
* Only the part of the graph that's reachable from the starting node when traversing dependencies towards the
|
||||
* parents is searched. In order to check that there are no cycles in the whole dependency graph, this method
|
||||
* has to be called for every checkable. For this, the method can be called on the same DependencyCycleChecker
|
||||
* instance multiple times, in which case parts of the graph aren't searched twice. However, if the graph structure
|
||||
* changes, a new DependencyCycleChecker instance must be used.
|
||||
*
|
||||
* @param checkable Starting node for the cycle search.
|
||||
* @throws ScriptError A dependency cycle was found.
|
||||
*/
|
||||
void AssertNoCycle(const Checkable::Ptr& checkable)
|
||||
{
|
||||
auto& node = m_Nodes[checkable];
|
||||
|
||||
if (node.OnStack) {
|
||||
std::ostringstream s;
|
||||
s << "Dependency cycle:";
|
||||
for (const auto& obj : m_Stack) {
|
||||
Checkable::Ptr child, parent;
|
||||
Dependency::Ptr dependency;
|
||||
|
||||
if (std::holds_alternative<Dependency::Ptr>(obj)) {
|
||||
dependency = std::get<Dependency::Ptr>(obj);
|
||||
parent = dependency->GetParent();
|
||||
child = dependency->GetChild();
|
||||
} else {
|
||||
const auto& service = std::get<Service::Ptr>(obj);
|
||||
parent = service->GetHost();
|
||||
child = service;
|
||||
}
|
||||
|
||||
auto quoted = [](const String& str) { return std::quoted(str.GetData()); };
|
||||
s << "\n\t- " << child->GetReflectionType()->GetName() << " " << quoted(child->GetName()) << " depends on ";
|
||||
if (child == parent) {
|
||||
s << "itself";
|
||||
} else {
|
||||
s << parent->GetReflectionType()->GetName() << " " << quoted(parent->GetName());
|
||||
}
|
||||
if (dependency) {
|
||||
s << " (Dependency " << quoted(dependency->GetShortName()) << " " << dependency->GetDebugInfo() << ")";
|
||||
} else {
|
||||
s << " (implicit)";
|
||||
}
|
||||
}
|
||||
BOOST_THROW_EXCEPTION(ScriptError(s.str()));
|
||||
}
|
||||
|
||||
if (node.Visited) {
|
||||
return;
|
||||
}
|
||||
node.Visited = true;
|
||||
|
||||
node.OnStack = true;
|
||||
|
||||
// Implicit dependency of each service to its host
|
||||
if (auto service (dynamic_pointer_cast<Service>(checkable)); service) {
|
||||
m_Stack.emplace_back(service);
|
||||
AssertNoCycle(service->GetHost());
|
||||
m_Stack.pop_back();
|
||||
}
|
||||
|
||||
// Explicitly configured dependency objects
|
||||
for (const auto& dep : checkable->GetDependencies()) {
|
||||
m_Stack.emplace_back(dep);
|
||||
AssertNoCycle(dep->GetParent());
|
||||
m_Stack.pop_back();
|
||||
}
|
||||
|
||||
// Additional dependencies to consider
|
||||
for (const auto& dep : node.ExtraDependencies) {
|
||||
m_Stack.emplace_back(dep);
|
||||
AssertNoCycle(dep->GetParent());
|
||||
m_Stack.pop_back();
|
||||
}
|
||||
|
||||
graph.Stack.pop_back();
|
||||
node.OnStack = false;
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
void Dependency::AssertNoCycles()
|
||||
/**
|
||||
* Checks that adding these new dependencies to the configuration does not introduce any cycles.
|
||||
*
|
||||
* This is done as an optimization: cycles are checked once for all dependencies in a batch of config objects instead
|
||||
* of individually per dependency in Dependency::OnAllConfigLoaded(). For runtime updates, this function may still be
|
||||
* called for single objects.
|
||||
*
|
||||
* @param items Config items containing Dependency objects added to the running configuration.
|
||||
*/
|
||||
void Dependency::BeforeOnAllConfigLoadedHandler(const ConfigItems& items)
|
||||
{
|
||||
DependencyCycleGraph graph;
|
||||
DependencyCycleChecker checker;
|
||||
|
||||
for (auto& host : ConfigType::GetObjectsByType<Host>()) {
|
||||
AssertNoDependencyCycle(host, graph);
|
||||
}
|
||||
// Resolve parent/child names to Checkable::Ptr and temporarily add the edges to the checker.
|
||||
// The dependencies are later registered to the checkables by Dependency::OnAllConfigLoaded().
|
||||
items.ForEachObject<Dependency>([&checker](Dependency::Ptr dependency) {
|
||||
dependency->InitChildParentReferences();
|
||||
checker.AddExtraDependency(std::move(dependency));
|
||||
});
|
||||
|
||||
for (auto& service : ConfigType::GetObjectsByType<Service>()) {
|
||||
AssertNoDependencyCycle(service, graph);
|
||||
}
|
||||
|
||||
m_AssertNoCyclesForIndividualDeps = true;
|
||||
// It's sufficient to search for cycles starting from newly added dependencies only: if a newly added dependency is
|
||||
// part of a cycle, that cycle is reachable from both the child and the parent of that dependency. The cycle search
|
||||
// is started from the parent as a slight optimization as that will traverse fewer edges if there is no cycle.
|
||||
items.ForEachObject<Dependency>([&checker](const Dependency::Ptr& dependency) {
|
||||
checker.AssertNoCycle(dependency->GetParent());
|
||||
});
|
||||
}
|
||||
|
||||
String DependencyNameComposer::MakeName(const String& shortName, const Object::Ptr& context) const
|
||||
|
|
@ -160,10 +217,8 @@ void Dependency::OnConfigLoaded()
|
|||
SetStateFilter(FilterArrayToInt(GetStates(), Notification::GetStateFilterMap(), defaultFilter));
|
||||
}
|
||||
|
||||
void Dependency::OnAllConfigLoaded()
|
||||
void Dependency::InitChildParentReferences()
|
||||
{
|
||||
ObjectImpl<Dependency>::OnAllConfigLoaded();
|
||||
|
||||
Host::Ptr childHost = Host::GetByName(GetChildHostName());
|
||||
|
||||
if (childHost) {
|
||||
|
|
@ -187,21 +242,17 @@ void Dependency::OnAllConfigLoaded()
|
|||
|
||||
if (!m_Parent)
|
||||
BOOST_THROW_EXCEPTION(ScriptError("Dependency '" + GetName() + "' references a parent host/service which doesn't exist.", GetDebugInfo()));
|
||||
}
|
||||
|
||||
void Dependency::OnAllConfigLoaded()
|
||||
{
|
||||
ObjectImpl<Dependency>::OnAllConfigLoaded();
|
||||
|
||||
// InitChildParentReferences() has to be called before.
|
||||
VERIFY(m_Child && m_Parent);
|
||||
|
||||
m_Child->AddDependency(this);
|
||||
m_Parent->AddReverseDependency(this);
|
||||
|
||||
if (m_AssertNoCyclesForIndividualDeps) {
|
||||
DependencyCycleGraph graph;
|
||||
|
||||
try {
|
||||
AssertNoDependencyCycle(m_Parent, graph);
|
||||
} catch (...) {
|
||||
m_Child->RemoveDependency(this);
|
||||
m_Parent->RemoveReverseDependency(this);
|
||||
throw;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
void Dependency::Stop(bool runtimeRemoved)
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@
|
|||
|
||||
#include "icinga/i2-icinga.hpp"
|
||||
#include "icinga/dependency-ti.hpp"
|
||||
#include "config/configitem.hpp"
|
||||
|
||||
namespace icinga
|
||||
{
|
||||
|
|
@ -22,6 +23,8 @@ class Service;
|
|||
class Dependency final : public ObjectImpl<Dependency>
|
||||
{
|
||||
public:
|
||||
static void StaticInitialize();
|
||||
|
||||
DECLARE_OBJECT(Dependency);
|
||||
DECLARE_OBJECTNAME(Dependency);
|
||||
|
||||
|
|
@ -36,9 +39,8 @@ public:
|
|||
|
||||
static void EvaluateApplyRules(const intrusive_ptr<Host>& host);
|
||||
static void EvaluateApplyRules(const intrusive_ptr<Service>& service);
|
||||
static void AssertNoCycles();
|
||||
|
||||
/* Note: Only use them for unit test mocks. Prefer OnConfigLoaded(). */
|
||||
/* Note: Only use them for unit test mocks. Prefer InitChildParentReferences(). */
|
||||
void SetParent(intrusive_ptr<Checkable> parent);
|
||||
void SetChild(intrusive_ptr<Checkable> child);
|
||||
|
||||
|
|
@ -46,15 +48,16 @@ protected:
|
|||
void OnConfigLoaded() override;
|
||||
void OnAllConfigLoaded() override;
|
||||
void Stop(bool runtimeRemoved) override;
|
||||
void InitChildParentReferences();
|
||||
|
||||
private:
|
||||
Checkable::Ptr m_Parent;
|
||||
Checkable::Ptr m_Child;
|
||||
|
||||
static bool m_AssertNoCyclesForIndividualDeps;
|
||||
|
||||
static bool EvaluateApplyRuleInstance(const Checkable::Ptr& checkable, const String& name, ScriptFrame& frame, const ApplyRule& rule, bool skipFilter);
|
||||
static bool EvaluateApplyRule(const Checkable::Ptr& checkable, const ApplyRule& rule, bool skipFilter = false);
|
||||
|
||||
static void BeforeOnAllConfigLoadedHandler(const ConfigItems& items);
|
||||
};
|
||||
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue