diff --git a/lib/base/initialize.hpp b/lib/base/initialize.hpp index adc995fe1b9..6c50b240875 100644 --- a/lib/base/initialize.hpp +++ b/lib/base/initialize.hpp @@ -23,6 +23,7 @@ enum class InitializePriority { RegisterBuiltinTypes, RegisterFunctions, RegisterTypes, + SortTypes, EvaluateConfigFragments, Default, FreezeNamespaces, diff --git a/lib/base/type.cpp b/lib/base/type.cpp index 1dcef39bfc9..f6539b7c447 100644 --- a/lib/base/type.cpp +++ b/lib/base/type.cpp @@ -1,9 +1,13 @@ /* Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+ */ #include "base/type.hpp" +#include "base/atomic.hpp" +#include "base/configobject.hpp" +#include "base/debug.hpp" #include "base/scriptglobal.hpp" #include "base/namespace.hpp" #include "base/objectlock.hpp" +#include using namespace icinga; @@ -32,6 +36,43 @@ INITIALIZE_ONCE_WITH_PRIORITY([]() { Type::Register(type); }, InitializePriority::RegisterTypeType); +static std::vector l_SortedByLoadDependencies; +static Atomic l_SortingByLoadDependenciesDone (false); + +INITIALIZE_ONCE_WITH_PRIORITY([] { + std::unordered_set visited; + + std::function visit; + // Please note that this callback does not detect any cyclic load dependencies, + // instead, it relies on the "sort_by_load_after" unit test to fail. + visit = ([&visit, &visited](Type* type) { + if (visited.find(type) != visited.end()) { + return; + } + visited.emplace(type); + + for (auto dependency : type->GetLoadDependencies()) { + visit(dependency); + } + + // We have managed to reach the final/top node in this dependency graph, + // so let's place them in reverse order to their final place. + l_SortedByLoadDependencies.emplace_back(type); + }); + + // Sort the types by their load_after dependencies in a Depth-First search manner. + for (const Type::Ptr& type : Type::GetAllTypes()) { + // Note that only those types that are assignable to the dynamic ConfigObject type can have "load_after" + // dependencies, otherwise they are just some Icinga 2 primitive types such as Number, String, etc. and + // we need to ignore them. + if (ConfigObject::TypeInstance->IsAssignableFrom(type)) { + visit(type.get()); + } + } + + l_SortingByLoadDependenciesDone.store(true); +}, InitializePriority::SortTypes); + String Type::ToString() const { return "type '" + GetName() + "'"; @@ -72,6 +113,12 @@ std::vector Type::GetAllTypes() return types; } +const std::vector& Type::GetConfigTypesSortedByLoadDependencies() +{ + VERIFY(l_SortingByLoadDependenciesDone.load()); + return l_SortedByLoadDependencies; +} + String Type::GetPluralName() const { String name = GetName(); diff --git a/lib/base/type.hpp b/lib/base/type.hpp index 7b8d1cacb64..ee8a9b37cd4 100644 --- a/lib/base/type.hpp +++ b/lib/base/type.hpp @@ -83,6 +83,21 @@ class Type : public Object static Type::Ptr GetByName(const String& name); static std::vector GetAllTypes(); + /** + * Returns a list of config types sorted by their "load_after" dependencies. + * + * All dependencies of a given type are listed at a lower index than that of the type itself. In other words, + * if a `Service` type load depends on the `Host` and `ApiListener` types, the Host and ApiListener types are + * guaranteed to appear first on the list. Nevertheless, the order of the Host and ApiListener types themselves + * is arbitrary if the two types are not dependent. + * + * It should be noted that this method will fail fatally when used prior to the completion + * of namespace initialization. + * + * @return std::vector + */ + static const std::vector& GetConfigTypesSortedByLoadDependencies(); + void SetField(int id, const Value& value, bool suppress_events = false, const Value& cookie = Empty) override; Value GetField(int id) const override; diff --git a/lib/config/configitem.cpp b/lib/config/configitem.cpp index bf4da81a4be..e8a50927543 100644 --- a/lib/config/configitem.cpp +++ b/lib/config/configitem.cpp @@ -444,74 +444,47 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue << "Committing " << total << " new items."; #endif /* I2_DEBUG */ - std::set types; - std::set completed_types; int itemsCount {0}; - for (const Type::Ptr& type : Type::GetAllTypes()) { - if (ConfigObject::TypeInstance->IsAssignableFrom(type)) - types.insert(type); - } - - while (types.size() != completed_types.size()) { - for (const Type::Ptr& type : types) { - if (completed_types.find(type) != completed_types.end()) - continue; + for (auto& type : Type::GetConfigTypesSortedByLoadDependencies()) { + std::atomic committed_items(0); - bool unresolved_dep = false; + { + auto items (itemsByType.find(type.get())); - /* skip this type (for now) if there are unresolved load dependencies */ - for (auto pLoadDep : type->GetLoadDependencies()) { - if (types.find(pLoadDep) != types.end() && completed_types.find(pLoadDep) == completed_types.end()) { - unresolved_dep = true; - break; + if (items != itemsByType.end()) { + for (const ItemPair& pair: items->second) { + newItems.emplace_back(pair.first); } - } - - if (unresolved_dep) - continue; - std::atomic committed_items(0); + upq.ParallelFor(items->second, [&committed_items](const ItemPair& ip) { + const ConfigItem::Ptr& item = ip.first; - { - auto items (itemsByType.find(type.get())); + if (!item->Commit(ip.second)) { + if (item->IsIgnoreOnError()) { + item->Unregister(); + } - if (items != itemsByType.end()) { - for (const ItemPair& pair: items->second) { - newItems.emplace_back(pair.first); + return; } - upq.ParallelFor(items->second, [&committed_items](const ItemPair& ip) { - const ConfigItem::Ptr& item = ip.first; + committed_items++; + }); - if (!item->Commit(ip.second)) { - if (item->IsIgnoreOnError()) { - item->Unregister(); - } - - return; - } - - committed_items++; - }); - - upq.Join(); - } + upq.Join(); } + } - itemsCount += committed_items; - - completed_types.insert(type); + itemsCount += committed_items; #ifdef I2_DEBUG - if (committed_items > 0) - Log(LogDebug, "configitem") - << "Committed " << committed_items << " items of type '" << type->GetName() << "'."; + if (committed_items > 0) + Log(LogDebug, "configitem") + << "Committed " << committed_items << " items of type '" << type->GetName() << "'."; #endif /* I2_DEBUG */ - if (upq.HasExceptions()) - return false; - } + if (upq.HasExceptions()) + return false; } #ifdef I2_DEBUG @@ -519,105 +492,83 @@ bool ConfigItem::CommitNewItems(const ActivationContext::Ptr& context, WorkQueue << "Committed " << itemsCount << " items."; #endif /* I2_DEBUG */ - completed_types.clear(); + for (auto& type : Type::GetConfigTypesSortedByLoadDependencies()) { + std::atomic notified_items(0); - while (types.size() != completed_types.size()) { - for (const Type::Ptr& type : types) { - if (completed_types.find(type) != completed_types.end()) - continue; + { + auto items (itemsByType.find(type.get())); - bool unresolved_dep = false; + if (items != itemsByType.end()) { + upq.ParallelFor(items->second, [¬ified_items](const ItemPair& ip) { + const ConfigItem::Ptr& item = ip.first; - /* skip this type (for now) if there are unresolved load dependencies */ - for (auto pLoadDep : type->GetLoadDependencies()) { - if (types.find(pLoadDep) != types.end() && completed_types.find(pLoadDep) == completed_types.end()) { - unresolved_dep = true; - break; - } - } + if (!item->m_Object) + return; - if (unresolved_dep) - continue; - - std::atomic notified_items(0); - - { - auto items (itemsByType.find(type.get())); - - if (items != itemsByType.end()) { - upq.ParallelFor(items->second, [¬ified_items](const ItemPair& ip) { - const ConfigItem::Ptr& item = ip.first; - - if (!item->m_Object) - return; + try { + item->m_Object->OnAllConfigLoaded(); + notified_items++; + } catch (const std::exception& ex) { + if (!item->m_IgnoreOnError) + throw; - try { - item->m_Object->OnAllConfigLoaded(); - notified_items++; - } catch (const std::exception& ex) { - if (!item->m_IgnoreOnError) - throw; + Log(LogNotice, "ConfigObject") + << "Ignoring config object '" << item->m_Name << "' of type '" << item->m_Type->GetName() << "' due to errors: " << DiagnosticInformation(ex); - Log(LogNotice, "ConfigObject") - << "Ignoring config object '" << item->m_Name << "' of type '" << item->m_Type->GetName() << "' due to errors: " << DiagnosticInformation(ex); + item->Unregister(); - item->Unregister(); - - { - std::unique_lock lock(item->m_Mutex); - item->m_IgnoredItems.push_back(item->m_DebugInfo.Path); - } + { + std::unique_lock lock(item->m_Mutex); + item->m_IgnoredItems.push_back(item->m_DebugInfo.Path); } - }); + } + }); - upq.Join(); - } + upq.Join(); } - - completed_types.insert(type); + } #ifdef I2_DEBUG - if (notified_items > 0) - Log(LogDebug, "configitem") - << "Sent OnAllConfigLoaded to " << notified_items << " items of type '" << type->GetName() << "'."; + if (notified_items > 0) + Log(LogDebug, "configitem") + << "Sent OnAllConfigLoaded to " << notified_items << " items of type '" << type->GetName() << "'."; #endif /* I2_DEBUG */ - if (upq.HasExceptions()) - return false; + if (upq.HasExceptions()) + return false; - notified_items = 0; - for (auto loadDep : type->GetLoadDependencies()) { - auto items (itemsByType.find(loadDep)); + notified_items = 0; + for (auto loadDep : type->GetLoadDependencies()) { + auto items (itemsByType.find(loadDep)); - if (items != itemsByType.end()) { - upq.ParallelFor(items->second, [&type, ¬ified_items](const ItemPair& ip) { - const ConfigItem::Ptr& item = ip.first; + if (items != itemsByType.end()) { + upq.ParallelFor(items->second, [&type, ¬ified_items](const ItemPair& ip) { + const ConfigItem::Ptr& item = ip.first; - if (!item->m_Object) - return; + if (!item->m_Object) + return; - ActivationScope ascope(item->m_ActivationContext); - item->m_Object->CreateChildObjects(type); - notified_items++; - }); - } + ActivationScope ascope(item->m_ActivationContext); + item->m_Object->CreateChildObjects(type); + notified_items++; + }); } + } - upq.Join(); + upq.Join(); #ifdef I2_DEBUG - if (notified_items > 0) - Log(LogDebug, "configitem") - << "Sent CreateChildObjects to " << notified_items << " items of type '" << type->GetName() << "'."; + if (notified_items > 0) + Log(LogDebug, "configitem") + << "Sent CreateChildObjects to " << notified_items << " items of type '" << type->GetName() << "'."; #endif /* I2_DEBUG */ - if (upq.HasExceptions()) - return false; + if (upq.HasExceptions()) + return false; - // Make sure to activate any additionally generated items - if (!CommitNewItems(context, upq, newItems)) - return false; - } + // Make sure to activate any additionally generated items + if (!CommitNewItems(context, upq, newItems)) + return false; } return true; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index e3772886abd..dd9724f0bf0 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -2,6 +2,59 @@ include(BoostTestTargets) +set(types_test_SOURCES + icingaapplication-fixture.cpp + base-type.cpp + ${base_OBJS} + $ + $ + $ + $ +) + +if(ICINGA2_WITH_CHECKER) + list(APPEND types_test_SOURCES $) +endif() + +if(ICINGA2_WITH_MYSQL) + list(APPEND types_test_SOURCES $ $) +endif() + +if(ICINGA2_WITH_PGSQL) + list(APPEND types_test_SOURCES $ $) +endif() + +if(ICINGA2_WITH_ICINGADB) + list(APPEND types_test_SOURCES $) +endif() + +if(ICINGA2_WITH_NOTIFICATION) + list(APPEND types_test_SOURCES $) +endif() + +if(ICINGA2_WITH_PERFDATA) + list(APPEND types_test_SOURCES $) +endif() + +if(ICINGA2_UNITY_BUILD) + mkunity_target(types test types_test_SOURCES) +endif() + +# In order to test the order of all Icinga 2 config type load dependencies, we need to link against all the libraries, +# but this results in boost signals e.g. in dbevents.cpp being triggered by icinga-checkresult.cpp test cases that +# only pass partially initialised objects. Therefore, the types test cases are decoupled from base and moved to a +# separate executable to not crash the base test cases. +add_boost_test(types + SOURCES test-runner.cpp ${types_test_SOURCES} + LIBRARIES ${base_DEPS} + TESTS + types/gettype + types/assign + types/byname + types/instantiate + types/sort_by_load_after +) + set(base_test_SOURCES icingaapplication-fixture.cpp base-array.cpp @@ -21,7 +74,6 @@ set(base_test_SOURCES base-string.cpp base-timer.cpp base-tlsutility.cpp - base-type.cpp base-utility.cpp base-value.cpp config-apply.cpp @@ -117,10 +169,6 @@ add_boost_test(base base_tlsutility/iscertuptodate_ok base_tlsutility/iscertuptodate_expiring base_tlsutility/iscertuptodate_old - base_type/gettype - base_type/assign - base_type/byname - base_type/instantiate base_utility/parse_version base_utility/compare_version base_utility/comparepasswords_works diff --git a/test/base-type.cpp b/test/base-type.cpp index 21bcf439d35..4a8d0deb4b7 100644 --- a/test/base-type.cpp +++ b/test/base-type.cpp @@ -5,11 +5,13 @@ #include "base/objectlock.hpp" #include "base/application.hpp" #include "base/type.hpp" +#include "icinga/host.hpp" +#include "icinga/service.hpp" #include using namespace icinga; -BOOST_AUTO_TEST_SUITE(base_type) +BOOST_AUTO_TEST_SUITE(types) BOOST_AUTO_TEST_CASE(gettype) { @@ -44,4 +46,33 @@ BOOST_AUTO_TEST_CASE(instantiate) BOOST_CHECK(p); } +BOOST_AUTO_TEST_CASE(sort_by_load_after) +{ + int totalDependencies{0}; + std::unordered_set previousTypes; + for (auto type : Type::GetConfigTypesSortedByLoadDependencies()) { + BOOST_CHECK_EQUAL(true, ConfigObject::TypeInstance->IsAssignableFrom(type)); + + totalDependencies += type->GetLoadDependencies().size(); + for (Type* dependency : type->GetLoadDependencies()) { + // Note, Type::GetConfigTypesSortedByLoadDependencies() does not detect any cyclic load dependencies, + // instead, it relies on this test case to fail. + BOOST_CHECK_MESSAGE(previousTypes.find(dependency) != previousTypes.end(), "type '" << type->GetName() + << "' depends on '"<< dependency->GetName() << "' type, but it's not loaded before"); + } + + previousTypes.emplace(type.get()); + } + + // The magic number 12 is the sum of host,service,comment,downtime and endpoint load_after dependencies. + BOOST_CHECK_MESSAGE(totalDependencies >= 12, "total size of load dependency must be at least 12"); + BOOST_CHECK_MESSAGE(previousTypes.find(Host::TypeInstance.get()) != previousTypes.end(), "Host type should be in the list"); + BOOST_CHECK_MESSAGE(previousTypes.find(Service::TypeInstance.get()) != previousTypes.end(), "Service type should be in the list"); + BOOST_CHECK_MESSAGE(previousTypes.find(Downtime::TypeInstance.get()) != previousTypes.end(), "Downtime type should be in the list"); + BOOST_CHECK_MESSAGE(previousTypes.find(Comment::TypeInstance.get()) != previousTypes.end(), "Comment type should be in the list"); + BOOST_CHECK_MESSAGE(previousTypes.find(Notification::TypeInstance.get()) != previousTypes.end(), "Notification type should be in the list"); + BOOST_CHECK_MESSAGE(previousTypes.find(Zone::TypeInstance.get()) != previousTypes.end(), "Zone type should be in the list"); + BOOST_CHECK_MESSAGE(previousTypes.find(Endpoint::TypeInstance.get()) != previousTypes.end(), "Endpoint type should be in the list"); +} + BOOST_AUTO_TEST_SUITE_END()