From b25ba7a3166b32cb8c0744220baa7d98ef58783d Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Wed, 14 Jun 2023 12:48:22 +0200 Subject: [PATCH 1/4] Notification#BeginExecuteNotification(): track state change notifications --- lib/icinga/notification.cpp | 10 ++++++++++ lib/icinga/notification.ti | 4 ++++ 2 files changed, 14 insertions(+) diff --git a/lib/icinga/notification.cpp b/lib/icinga/notification.cpp index fb7bc1c70..05628969c 100644 --- a/lib/icinga/notification.cpp +++ b/lib/icinga/notification.cpp @@ -452,6 +452,16 @@ void Notification::BeginExecuteNotification(NotificationType type, const CheckRe /* collect all notified users */ allNotifiedUsers.insert(user); + switch (type) { + case NotificationProblem: + case NotificationRecovery: { + auto [host, service] = GetHostService(checkable); + GetLastNotifiedStatePerUser()->Set(userName, service ? service->GetState() : host->GetState()); + } + default: + ; + } + /* store all notified users for later recovery checks */ if (type == NotificationProblem && !notifiedProblemUsers->Contains(userName)) notifiedProblemUsers->Add(userName); diff --git a/lib/icinga/notification.ti b/lib/icinga/notification.ti index 485360757..be0784613 100644 --- a/lib/icinga/notification.ti +++ b/lib/icinga/notification.ti @@ -90,6 +90,10 @@ class Notification : CustomVarObject < NotificationNameComposer default {{{ return 0; }}} }; + [state, no_user_view, no_user_modify] Dictionary::Ptr last_notified_state_per_user { + default {{{ return new Dictionary(); }}} + }; + [config, navigation] name(Endpoint) command_endpoint (CommandEndpointRaw) { navigate {{{ return Endpoint::GetByName(GetCommandEndpointRaw()); From 2cff763295da543f399f775b0c49edac705a4b0c Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Wed, 14 Jun 2023 16:04:38 +0200 Subject: [PATCH 2/4] Cluster-sync Notification#last_notified_state_per_user --- doc/19-technical-concepts.md | 36 ++++++++++++++++++++++ lib/icinga/clusterevents.cpp | 60 ++++++++++++++++++++++++++++++++++++ lib/icinga/clusterevents.hpp | 3 ++ lib/icinga/notification.cpp | 8 ++++- lib/icinga/notification.hpp | 2 ++ 5 files changed, 108 insertions(+), 1 deletion(-) diff --git a/doc/19-technical-concepts.md b/doc/19-technical-concepts.md index 1da32b6f2..74a2756ca 100644 --- a/doc/19-technical-concepts.md +++ b/doc/19-technical-concepts.md @@ -1514,6 +1514,42 @@ Message updates will be dropped when: * Notification does not exist. * Origin endpoint's zone is not allowed to access this checkable. +#### event::UpdateLastNotifiedStatePerUser + +> Location: `clusterevents.cpp` + +##### Message Body + +Key | Value +----------|--------- +jsonrpc | 2.0 +method | event::UpdateLastNotifiedStatePerUser +params | Dictionary + +##### Params + +Key | Type | Description +-------------|--------|------------------ +notification | String | Notification name +user | String | User name +state | Number | Checkable state the user just got a problem notification for + +Used to sync the state of a notification object within the same HA zone. + +##### Functions + +Event Sender: `Notification::OnLastNotifiedStatePerUserUpdated` +Event Receiver: `LastNotifiedStatePerUserUpdatedAPIHandler` + +##### Permissions + +The receiver will not process messages from not configured endpoints. + +Message updates will be dropped when: + +* Notification does not exist. +* Origin endpoint is not within the local zone. + #### event::SetForceNextCheck > Location: `clusterevents.cpp` diff --git a/lib/icinga/clusterevents.cpp b/lib/icinga/clusterevents.cpp index 6c4daa8b8..95616d2e3 100644 --- a/lib/icinga/clusterevents.cpp +++ b/lib/icinga/clusterevents.cpp @@ -29,6 +29,7 @@ REGISTER_APIFUNCTION(SetStateBeforeSuppression, event, &ClusterEvents::StateBefo REGISTER_APIFUNCTION(SetSuppressedNotifications, event, &ClusterEvents::SuppressedNotificationsChangedAPIHandler); REGISTER_APIFUNCTION(SetSuppressedNotificationTypes, event, &ClusterEvents::SuppressedNotificationTypesChangedAPIHandler); REGISTER_APIFUNCTION(SetNextNotification, event, &ClusterEvents::NextNotificationChangedAPIHandler); +REGISTER_APIFUNCTION(UpdateLastNotifiedStatePerUser, event, &ClusterEvents::LastNotifiedStatePerUserUpdatedAPIHandler); REGISTER_APIFUNCTION(SetForceNextCheck, event, &ClusterEvents::ForceNextCheckChangedAPIHandler); REGISTER_APIFUNCTION(SetForceNextNotification, event, &ClusterEvents::ForceNextNotificationChangedAPIHandler); REGISTER_APIFUNCTION(SetAcknowledgement, event, &ClusterEvents::AcknowledgementSetAPIHandler); @@ -50,6 +51,7 @@ void ClusterEvents::StaticInitialize() Checkable::OnSuppressedNotificationsChanged.connect(&ClusterEvents::SuppressedNotificationsChangedHandler); Notification::OnSuppressedNotificationsChanged.connect(&ClusterEvents::SuppressedNotificationTypesChangedHandler); Notification::OnNextNotificationChanged.connect(&ClusterEvents::NextNotificationChangedHandler); + Notification::OnLastNotifiedStatePerUserUpdated.connect(&ClusterEvents::LastNotifiedStatePerUserUpdatedHandler); Checkable::OnForceNextCheckChanged.connect(&ClusterEvents::ForceNextCheckChangedHandler); Checkable::OnForceNextNotificationChanged.connect(&ClusterEvents::ForceNextNotificationChangedHandler); Checkable::OnNotificationsRequested.connect(&ClusterEvents::SendNotificationsHandler); @@ -529,6 +531,64 @@ Value ClusterEvents::NextNotificationChangedAPIHandler(const MessageOrigin::Ptr& return Empty; } +void ClusterEvents::LastNotifiedStatePerUserUpdatedHandler(const Notification::Ptr& notification, const String& user, uint_fast8_t state, const MessageOrigin::Ptr& origin) +{ + auto listener (ApiListener::GetInstance()); + + if (!listener) { + return; + } + + Dictionary::Ptr params = new Dictionary(); + params->Set("notification", notification->GetName()); + params->Set("user", user); + params->Set("state", state); + + Dictionary::Ptr message = new Dictionary(); + message->Set("jsonrpc", "2.0"); + message->Set("method", "event::UpdateLastNotifiedStatePerUser"); + message->Set("params", params); + + listener->RelayMessage(origin, notification, message, true); +} + +Value ClusterEvents::LastNotifiedStatePerUserUpdatedAPIHandler(const MessageOrigin::Ptr& origin, const Dictionary::Ptr& params) +{ + auto endpoint (origin->FromClient->GetEndpoint()); + + if (!endpoint) { + Log(LogNotice, "ClusterEvents") + << "Discarding 'last notified state of user updated' message from '" << origin->FromClient->GetIdentity() << "': Invalid endpoint origin (client not allowed)."; + + return Empty; + } + + if (origin->FromZone && origin->FromZone != Zone::GetLocalZone()) { + Log(LogNotice, "ClusterEvents") + << "Discarding 'last notified state of user updated' message from '" + << origin->FromClient->GetIdentity() << "': Unauthorized access."; + + return Empty; + } + + auto notification (Notification::GetByName(params->Get("notification"))); + + if (!notification) { + return Empty; + } + + auto state (params->Get("state")); + + if (!state.IsNumber()) { + return Empty; + } + + notification->GetLastNotifiedStatePerUser()->Set(params->Get("user"), state); + Notification::OnLastNotifiedStatePerUserUpdated(notification, params->Get("user"), state, origin); + + return Empty; +} + void ClusterEvents::ForceNextCheckChangedHandler(const Checkable::Ptr& checkable, const MessageOrigin::Ptr& origin) { ApiListener::Ptr listener = ApiListener::GetInstance(); diff --git a/lib/icinga/clusterevents.hpp b/lib/icinga/clusterevents.hpp index 4cdadacc9..2e6e092f2 100644 --- a/lib/icinga/clusterevents.hpp +++ b/lib/icinga/clusterevents.hpp @@ -41,6 +41,9 @@ public: static void NextNotificationChangedHandler(const Notification::Ptr& notification, const MessageOrigin::Ptr& origin); static Value NextNotificationChangedAPIHandler(const MessageOrigin::Ptr& origin, const Dictionary::Ptr& params); + static void LastNotifiedStatePerUserUpdatedHandler(const Notification::Ptr& notification, const String& user, uint_fast8_t state, const MessageOrigin::Ptr& origin); + static Value LastNotifiedStatePerUserUpdatedAPIHandler(const MessageOrigin::Ptr& origin, const Dictionary::Ptr& params); + static void ForceNextCheckChangedHandler(const Checkable::Ptr& checkable, const MessageOrigin::Ptr& origin); static Value ForceNextCheckChangedAPIHandler(const MessageOrigin::Ptr& origin, const Dictionary::Ptr& params); diff --git a/lib/icinga/notification.cpp b/lib/icinga/notification.cpp index 05628969c..8a254b8fc 100644 --- a/lib/icinga/notification.cpp +++ b/lib/icinga/notification.cpp @@ -23,6 +23,7 @@ std::map Notification::m_StateFilterMap; std::map Notification::m_TypeFilterMap; boost::signals2::signal Notification::OnNextNotificationChanged; +boost::signals2::signal Notification::OnLastNotifiedStatePerUserUpdated; String NotificationNameComposer::MakeName(const String& shortName, const Object::Ptr& context) const { @@ -456,7 +457,12 @@ void Notification::BeginExecuteNotification(NotificationType type, const CheckRe case NotificationProblem: case NotificationRecovery: { auto [host, service] = GetHostService(checkable); - GetLastNotifiedStatePerUser()->Set(userName, service ? service->GetState() : host->GetState()); + uint_fast8_t state = service ? service->GetState() : host->GetState(); + + if (state != (uint_fast8_t)GetLastNotifiedStatePerUser()->Get(userName)) { + GetLastNotifiedStatePerUser()->Set(userName, state); + OnLastNotifiedStatePerUserUpdated(this, userName, state, nullptr); + } } default: ; diff --git a/lib/icinga/notification.hpp b/lib/icinga/notification.hpp index ec9164f19..1c6a2f44d 100644 --- a/lib/icinga/notification.hpp +++ b/lib/icinga/notification.hpp @@ -13,6 +13,7 @@ #include "remote/endpoint.hpp" #include "remote/messageorigin.hpp" #include "base/array.hpp" +#include namespace icinga { @@ -92,6 +93,7 @@ public: static String NotificationHostStateToString(HostState state); static boost::signals2::signal OnNextNotificationChanged; + static boost::signals2::signal OnLastNotifiedStatePerUserUpdated; void Validate(int types, const ValidationUtils& utils) override; From 44e9c6f40dbe737a96b7e8f59c3c5a0c88d6f2f4 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Wed, 14 Jun 2023 17:00:56 +0200 Subject: [PATCH 3/4] Notification#BeginExecuteNotification(): discard likely duplicate problem notifications --- lib/icinga/notification.cpp | 16 +++++++ lib/icinga/notification.hpp | 1 - test/CMakeLists.txt | 3 ++ test/icinga-notification.cpp | 92 ++++++++++++++++++++++++++++++++++++ 4 files changed, 111 insertions(+), 1 deletion(-) diff --git a/lib/icinga/notification.cpp b/lib/icinga/notification.cpp index 8a254b8fc..9533d229b 100644 --- a/lib/icinga/notification.cpp +++ b/lib/icinga/notification.cpp @@ -440,6 +440,22 @@ void Notification::BeginExecuteNotification(NotificationType type, const CheckRe } } + if (type == NotificationProblem && !reminder && !checkable->GetVolatile()) { + auto [host, service] = GetHostService(checkable); + uint_fast8_t state = service ? service->GetState() : host->GetState(); + + if (state == (uint_fast8_t)GetLastNotifiedStatePerUser()->Get(userName)) { + auto stateStr (service ? NotificationServiceStateToString(service->GetState()) : NotificationHostStateToString(host->GetState())); + + Log(LogNotice, "Notification") + << "Notification object '" << notificationName << "': We already notified user '" << userName << "' for a " << stateStr + << " problem. Likely after that another state change notification was filtered out by config. Not sending duplicate '" + << stateStr << "' notification."; + + continue; + } + } + Log(LogInformation, "Notification") << "Sending " << (reminder ? "reminder " : "") << "'" << NotificationTypeToString(type) << "' notification '" << notificationName << "' for user '" << userName << "'"; diff --git a/lib/icinga/notification.hpp b/lib/icinga/notification.hpp index 1c6a2f44d..2c41a13e7 100644 --- a/lib/icinga/notification.hpp +++ b/lib/icinga/notification.hpp @@ -107,7 +107,6 @@ public: static const std::map& GetStateFilterMap(); static const std::map& GetTypeFilterMap(); -protected: void OnConfigLoaded() override; void OnAllConfigLoaded() override; void Start(bool runtimeCreated) override; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 8919de304..5e054c48e 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -138,6 +138,9 @@ add_boost_test(base icinga_notification/strings icinga_notification/state_filter icinga_notification/type_filter + icinga_notification/no_filter_problem_no_duplicate + icinga_notification/filter_problem_no_duplicate + icinga_notification/volatile_filter_problem_duplicate icinga_macros/simple icinga_legacytimeperiod/simple icinga_legacytimeperiod/advanced diff --git a/test/icinga-notification.cpp b/test/icinga-notification.cpp index 5cf3f49e8..4b3fe5f73 100644 --- a/test/icinga-notification.cpp +++ b/test/icinga-notification.cpp @@ -1,11 +1,74 @@ /* Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+ */ +#include "icinga/host.hpp" #include "icinga/notification.hpp" +#include "icinga/notificationcommand.hpp" +#include "icinga/service.hpp" +#include "icinga/user.hpp" #include #include using namespace icinga; +struct DuplicateDueToFilterHelper +{ + Host::Ptr h = new Host(); + Service::Ptr s = new Service(); + User::Ptr u = new User(); + NotificationCommand::Ptr nc = new NotificationCommand(); + Notification::Ptr n = new Notification(); + unsigned int called = 0; + + DuplicateDueToFilterHelper(int typeFilter, int stateFilter) + { + h->SetName("example.com", true); + h->Register(); + + s->SetShortName("disk", true); + h->AddService(s); + + u->SetName("jdoe", true); + u->SetTypeFilter(~0); + u->SetStateFilter(~0); + u->Register(); + + nc->SetName("mail", true); + nc->SetExecute(new Function("", [this]() { ++called; }), true); + nc->Register(); + + n->SetFieldByName("host_name", "example.com", false, DebugInfo()); + n->SetFieldByName("service_name", "disk", false, DebugInfo()); + n->SetFieldByName("command", "mail", false, DebugInfo()); + n->SetUsersRaw(new Array({"jdoe"}), true); + n->SetTypeFilter(typeFilter); + n->SetStateFilter(stateFilter); + n->OnAllConfigLoaded(); // link Service + } + + ~DuplicateDueToFilterHelper() + { + h->Unregister(); + u->Unregister(); + nc->Unregister(); + } + + void SendStateNotification(ServiceState state, bool isSent) + { + auto calledBefore (called); + + s->SetStateRaw(state, true); + Application::GetTP().Start(); + + n->BeginExecuteNotification( + state == ServiceOK ? NotificationRecovery : NotificationProblem, + nullptr, false, false, "", "" + ); + + Application::GetTP().Stop(); + BOOST_CHECK_EQUAL(called > calledBefore, isSent); + } +}; + BOOST_AUTO_TEST_SUITE(icinga_notification) BOOST_AUTO_TEST_CASE(strings) @@ -102,4 +165,33 @@ BOOST_AUTO_TEST_CASE(type_filter) std::cout << "#4 Notification type: " << ftype << " against " << notification->GetTypeFilter() << " must fail." << std::endl; BOOST_CHECK(!(notification->GetTypeFilter() & ftype)); } + +BOOST_AUTO_TEST_CASE(no_filter_problem_no_duplicate) +{ + DuplicateDueToFilterHelper helper (~0, ~0); + + helper.SendStateNotification(ServiceCritical, true); + helper.SendStateNotification(ServiceWarning, true); + helper.SendStateNotification(ServiceCritical, true); +} + +BOOST_AUTO_TEST_CASE(filter_problem_no_duplicate) +{ + DuplicateDueToFilterHelper helper (~0, ~StateFilterWarning); + + helper.SendStateNotification(ServiceCritical, true); + helper.SendStateNotification(ServiceWarning, false); + helper.SendStateNotification(ServiceCritical, false); +} + +BOOST_AUTO_TEST_CASE(volatile_filter_problem_duplicate) +{ + DuplicateDueToFilterHelper helper (~0, ~StateFilterWarning); + + helper.s->SetVolatile(true, true); + helper.SendStateNotification(ServiceCritical, true); + helper.SendStateNotification(ServiceWarning, false); + helper.SendStateNotification(ServiceCritical, true); +} + BOOST_AUTO_TEST_SUITE_END() From 97cd05db7a3c8d84a2ea57d9ef7a8826b2552b18 Mon Sep 17 00:00:00 2001 From: "Alexander A. Klimov" Date: Tue, 12 Dec 2023 15:55:04 +0100 Subject: [PATCH 4/4] Notification#BeginExecuteNotification(): on recovery clear last_notified_state_per_user --- doc/19-technical-concepts.md | 34 +++++++++++++++++++++++ lib/icinga/clusterevents.cpp | 53 ++++++++++++++++++++++++++++++++++++ lib/icinga/clusterevents.hpp | 3 ++ lib/icinga/notification.cpp | 25 +++++++++-------- lib/icinga/notification.hpp | 1 + test/CMakeLists.txt | 2 ++ test/icinga-notification.cpp | 18 ++++++++++++ 7 files changed, 125 insertions(+), 11 deletions(-) diff --git a/doc/19-technical-concepts.md b/doc/19-technical-concepts.md index 74a2756ca..b3a056170 100644 --- a/doc/19-technical-concepts.md +++ b/doc/19-technical-concepts.md @@ -1550,6 +1550,40 @@ Message updates will be dropped when: * Notification does not exist. * Origin endpoint is not within the local zone. +#### event::ClearLastNotifiedStatePerUser + +> Location: `clusterevents.cpp` + +##### Message Body + +Key | Value +----------|--------- +jsonrpc | 2.0 +method | event::ClearLastNotifiedStatePerUser +params | Dictionary + +##### Params + +Key | Type | Description +-------------|--------|------------------ +notification | String | Notification name + +Used to sync the state of a notification object within the same HA zone. + +##### Functions + +Event Sender: `Notification::OnLastNotifiedStatePerUserCleared` +Event Receiver: `LastNotifiedStatePerUserClearedAPIHandler` + +##### Permissions + +The receiver will not process messages from not configured endpoints. + +Message updates will be dropped when: + +* Notification does not exist. +* Origin endpoint is not within the local zone. + #### event::SetForceNextCheck > Location: `clusterevents.cpp` diff --git a/lib/icinga/clusterevents.cpp b/lib/icinga/clusterevents.cpp index 95616d2e3..fe5167b78 100644 --- a/lib/icinga/clusterevents.cpp +++ b/lib/icinga/clusterevents.cpp @@ -30,6 +30,7 @@ REGISTER_APIFUNCTION(SetSuppressedNotifications, event, &ClusterEvents::Suppress REGISTER_APIFUNCTION(SetSuppressedNotificationTypes, event, &ClusterEvents::SuppressedNotificationTypesChangedAPIHandler); REGISTER_APIFUNCTION(SetNextNotification, event, &ClusterEvents::NextNotificationChangedAPIHandler); REGISTER_APIFUNCTION(UpdateLastNotifiedStatePerUser, event, &ClusterEvents::LastNotifiedStatePerUserUpdatedAPIHandler); +REGISTER_APIFUNCTION(ClearLastNotifiedStatePerUser, event, &ClusterEvents::LastNotifiedStatePerUserClearedAPIHandler); REGISTER_APIFUNCTION(SetForceNextCheck, event, &ClusterEvents::ForceNextCheckChangedAPIHandler); REGISTER_APIFUNCTION(SetForceNextNotification, event, &ClusterEvents::ForceNextNotificationChangedAPIHandler); REGISTER_APIFUNCTION(SetAcknowledgement, event, &ClusterEvents::AcknowledgementSetAPIHandler); @@ -52,6 +53,7 @@ void ClusterEvents::StaticInitialize() Notification::OnSuppressedNotificationsChanged.connect(&ClusterEvents::SuppressedNotificationTypesChangedHandler); Notification::OnNextNotificationChanged.connect(&ClusterEvents::NextNotificationChangedHandler); Notification::OnLastNotifiedStatePerUserUpdated.connect(&ClusterEvents::LastNotifiedStatePerUserUpdatedHandler); + Notification::OnLastNotifiedStatePerUserCleared.connect(&ClusterEvents::LastNotifiedStatePerUserClearedHandler); Checkable::OnForceNextCheckChanged.connect(&ClusterEvents::ForceNextCheckChangedHandler); Checkable::OnForceNextNotificationChanged.connect(&ClusterEvents::ForceNextNotificationChangedHandler); Checkable::OnNotificationsRequested.connect(&ClusterEvents::SendNotificationsHandler); @@ -589,6 +591,57 @@ Value ClusterEvents::LastNotifiedStatePerUserUpdatedAPIHandler(const MessageOrig return Empty; } +void ClusterEvents::LastNotifiedStatePerUserClearedHandler(const Notification::Ptr& notification, const MessageOrigin::Ptr& origin) +{ + auto listener (ApiListener::GetInstance()); + + if (!listener) { + return; + } + + Dictionary::Ptr params = new Dictionary(); + params->Set("notification", notification->GetName()); + + Dictionary::Ptr message = new Dictionary(); + message->Set("jsonrpc", "2.0"); + message->Set("method", "event::ClearLastNotifiedStatePerUser"); + message->Set("params", params); + + listener->RelayMessage(origin, notification, message, true); +} + +Value ClusterEvents::LastNotifiedStatePerUserClearedAPIHandler(const MessageOrigin::Ptr& origin, const Dictionary::Ptr& params) +{ + auto endpoint (origin->FromClient->GetEndpoint()); + + if (!endpoint) { + Log(LogNotice, "ClusterEvents") + << "Discarding 'last notified state of user cleared' message from '" + << origin->FromClient->GetIdentity() << "': Invalid endpoint origin (client not allowed)."; + + return Empty; + } + + if (origin->FromZone && origin->FromZone != Zone::GetLocalZone()) { + Log(LogNotice, "ClusterEvents") + << "Discarding 'last notified state of user cleared' message from '" + << origin->FromClient->GetIdentity() << "': Unauthorized access."; + + return Empty; + } + + auto notification (Notification::GetByName(params->Get("notification"))); + + if (!notification) { + return Empty; + } + + notification->GetLastNotifiedStatePerUser()->Clear(); + Notification::OnLastNotifiedStatePerUserCleared(notification, origin); + + return Empty; +} + void ClusterEvents::ForceNextCheckChangedHandler(const Checkable::Ptr& checkable, const MessageOrigin::Ptr& origin) { ApiListener::Ptr listener = ApiListener::GetInstance(); diff --git a/lib/icinga/clusterevents.hpp b/lib/icinga/clusterevents.hpp index 2e6e092f2..8daf86ab3 100644 --- a/lib/icinga/clusterevents.hpp +++ b/lib/icinga/clusterevents.hpp @@ -44,6 +44,9 @@ public: static void LastNotifiedStatePerUserUpdatedHandler(const Notification::Ptr& notification, const String& user, uint_fast8_t state, const MessageOrigin::Ptr& origin); static Value LastNotifiedStatePerUserUpdatedAPIHandler(const MessageOrigin::Ptr& origin, const Dictionary::Ptr& params); + static void LastNotifiedStatePerUserClearedHandler(const Notification::Ptr& notification, const MessageOrigin::Ptr& origin); + static Value LastNotifiedStatePerUserClearedAPIHandler(const MessageOrigin::Ptr& origin, const Dictionary::Ptr& params); + static void ForceNextCheckChangedHandler(const Checkable::Ptr& checkable, const MessageOrigin::Ptr& origin); static Value ForceNextCheckChangedAPIHandler(const MessageOrigin::Ptr& origin, const Dictionary::Ptr& params); diff --git a/lib/icinga/notification.cpp b/lib/icinga/notification.cpp index 9533d229b..ab8d42b8c 100644 --- a/lib/icinga/notification.cpp +++ b/lib/icinga/notification.cpp @@ -24,6 +24,7 @@ std::map Notification::m_TypeFilterMap; boost::signals2::signal Notification::OnNextNotificationChanged; boost::signals2::signal Notification::OnLastNotifiedStatePerUserUpdated; +boost::signals2::signal Notification::OnLastNotifiedStatePerUserCleared; String NotificationNameComposer::MakeName(const String& shortName, const Object::Ptr& context) const { @@ -232,6 +233,13 @@ void Notification::BeginExecuteNotification(NotificationType type, const CheckRe << "notifications of type '" << notificationTypeName << "' for notification object '" << notificationName << "'."; + if (type == NotificationRecovery) { + auto states (GetLastNotifiedStatePerUser()); + + states->Clear(); + OnLastNotifiedStatePerUserCleared(this, nullptr); + } + Checkable::Ptr checkable = GetCheckable(); if (!force) { @@ -469,19 +477,14 @@ void Notification::BeginExecuteNotification(NotificationType type, const CheckRe /* collect all notified users */ allNotifiedUsers.insert(user); - switch (type) { - case NotificationProblem: - case NotificationRecovery: { - auto [host, service] = GetHostService(checkable); - uint_fast8_t state = service ? service->GetState() : host->GetState(); + if (type == NotificationProblem) { + auto [host, service] = GetHostService(checkable); + uint_fast8_t state = service ? service->GetState() : host->GetState(); - if (state != (uint_fast8_t)GetLastNotifiedStatePerUser()->Get(userName)) { - GetLastNotifiedStatePerUser()->Set(userName, state); - OnLastNotifiedStatePerUserUpdated(this, userName, state, nullptr); - } + if (state != (uint_fast8_t)GetLastNotifiedStatePerUser()->Get(userName)) { + GetLastNotifiedStatePerUser()->Set(userName, state); + OnLastNotifiedStatePerUserUpdated(this, userName, state, nullptr); } - default: - ; } /* store all notified users for later recovery checks */ diff --git a/lib/icinga/notification.hpp b/lib/icinga/notification.hpp index 2c41a13e7..1b6cbedb1 100644 --- a/lib/icinga/notification.hpp +++ b/lib/icinga/notification.hpp @@ -94,6 +94,7 @@ public: static boost::signals2::signal OnNextNotificationChanged; static boost::signals2::signal OnLastNotifiedStatePerUserUpdated; + static boost::signals2::signal OnLastNotifiedStatePerUserCleared; void Validate(int types, const ValidationUtils& utils) override; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 5e054c48e..24eb2198c 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -141,6 +141,8 @@ add_boost_test(base icinga_notification/no_filter_problem_no_duplicate icinga_notification/filter_problem_no_duplicate icinga_notification/volatile_filter_problem_duplicate + icinga_notification/no_recovery_filter_no_duplicate + icinga_notification/recovery_filter_duplicate icinga_macros/simple icinga_legacytimeperiod/simple icinga_legacytimeperiod/advanced diff --git a/test/icinga-notification.cpp b/test/icinga-notification.cpp index 4b3fe5f73..a0aeb7df8 100644 --- a/test/icinga-notification.cpp +++ b/test/icinga-notification.cpp @@ -194,4 +194,22 @@ BOOST_AUTO_TEST_CASE(volatile_filter_problem_duplicate) helper.SendStateNotification(ServiceCritical, true); } +BOOST_AUTO_TEST_CASE(no_recovery_filter_no_duplicate) +{ + DuplicateDueToFilterHelper helper (~0, ~0); + + helper.SendStateNotification(ServiceCritical, true); + helper.SendStateNotification(ServiceOK, true); + helper.SendStateNotification(ServiceCritical, true); +} + +BOOST_AUTO_TEST_CASE(recovery_filter_duplicate) +{ + DuplicateDueToFilterHelper helper (~NotificationRecovery, ~0); + + helper.SendStateNotification(ServiceCritical, true); + helper.SendStateNotification(ServiceOK, false); + helper.SendStateNotification(ServiceCritical, true); +} + BOOST_AUTO_TEST_SUITE_END()