From 89f12c23230d682893573f7e948e34517985f700 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Thu, 6 Mar 2025 10:51:54 +0100 Subject: [PATCH 1/4] Notification: Reset `no_more_notifications` only on recovery --- lib/icinga/notification.cpp | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/lib/icinga/notification.cpp b/lib/icinga/notification.cpp index bd3158ced..b16b43e95 100644 --- a/lib/icinga/notification.cpp +++ b/lib/icinga/notification.cpp @@ -223,6 +223,30 @@ void Notification::ResetNotificationNumber() SetNotificationNumber(0); } +/** + * Check whether the given notification type is a Recovery or FlappingEnd notification and the Checkable object + * has already recovered. + * + * For the latter case, if the Checkable has recovered while it was in a Flapping state, the recovery notification + * will be silently discarded and leave some internal states of the Notification object that depend on it in an + * inconsistent state. So, use this helper function whether to reset one of these states. + * + * @param checkable The checkable object the notification is for + * @param cr The current check result passed to the notification + * @param type The requested notification type + * + * @return bool + */ +static bool IsRecoveryOrFlappingEndAndCheckableIsOK(const Checkable::Ptr& checkable, const CheckResult::Ptr& cr, NotificationType type) +{ + if (type == NotificationRecovery) { + return true; + } + + // Check whether missed the Checkable recovery because of its Flapping state. + return type == NotificationFlappingEnd && cr && checkable->IsStateOK(cr->GetState()); +} + void Notification::BeginExecuteNotification(NotificationType type, const CheckResult::Ptr& cr, bool force, bool reminder, const String& author, const String& text) { String notificationName = GetName(); @@ -389,7 +413,7 @@ void Notification::BeginExecuteNotification(NotificationType type, const CheckRe if (type == NotificationProblem && GetInterval() <= 0) SetNoMoreNotifications(true); - else if (type != NotificationCustom) + else if (IsRecoveryOrFlappingEndAndCheckableIsOK(checkable, cr, type)) SetNoMoreNotifications(false); if (type == NotificationProblem && GetInterval() > 0) From 86365a4e2b6708321234bd7f4a2e3cd1700b6e81 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 12 Mar 2025 14:18:19 +0100 Subject: [PATCH 2/4] Notification: Clear last notified state per user on flapping end as well --- lib/icinga/notification.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/icinga/notification.cpp b/lib/icinga/notification.cpp index b16b43e95..b6e9cd511 100644 --- a/lib/icinga/notification.cpp +++ b/lib/icinga/notification.cpp @@ -257,15 +257,18 @@ void Notification::BeginExecuteNotification(NotificationType type, const CheckRe << "notifications of type '" << notificationTypeName << "' for notification object '" << notificationName << "'."; - if (type == NotificationRecovery) { + Checkable::Ptr checkable = GetCheckable(); + + // Clear the last notified problem state per user if we're sending a recovery notification or if we're sending a + // flapping end notification and the checkable is already in an OK state. This is necessary since we might have + // missed the recovery notification due to the flapping state. + if (IsRecoveryOrFlappingEndAndCheckableIsOK(checkable, cr, type)) { auto states (GetLastNotifiedStatePerUser()); states->Clear(); OnLastNotifiedStatePerUserCleared(this, nullptr); } - Checkable::Ptr checkable = GetCheckable(); - if (!force) { TimePeriod::Ptr tp = GetPeriod(); From 91663268766c8a816ca863f3bdbe2524a83b45b3 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 12 Mar 2025 14:19:22 +0100 Subject: [PATCH 3/4] Notification: Reset notified problem users on flapping end as well --- lib/icinga/notification.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/icinga/notification.cpp b/lib/icinga/notification.cpp index b6e9cd511..5b2964a84 100644 --- a/lib/icinga/notification.cpp +++ b/lib/icinga/notification.cpp @@ -520,7 +520,7 @@ void Notification::BeginExecuteNotification(NotificationType type, const CheckRe } /* if this was a recovery notification, reset all notified users */ - if (type == NotificationRecovery) + if (IsRecoveryOrFlappingEndAndCheckableIsOK(checkable, cr, type)) notifiedProblemUsers->Clear(); /* used in db_ido for notification history */ From 4596b44171797a0c7691194f75116998b3bd27e3 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 12 Mar 2025 14:20:01 +0100 Subject: [PATCH 4/4] Reset `no_more_notifications` on filter mismatch correctly Previously, if you enable flapping for a Checkable but the corresponding `Notification` object does not have `FlappingStart` or `FlappingEnd` types set, the `no_more_notifications` flag wasn't reset to false again. This commit ensures that this flag is always reset on `Recovery` even the type filter does not match including when we miss the `Recovery` due to Flapping state. --- lib/icinga/notification.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/icinga/notification.cpp b/lib/icinga/notification.cpp index 5b2964a84..23c06e480 100644 --- a/lib/icinga/notification.cpp +++ b/lib/icinga/notification.cpp @@ -365,7 +365,7 @@ void Notification::BeginExecuteNotification(NotificationType type, const CheckRe */ { ObjectLock olock(this); - if (type == NotificationRecovery && GetInterval() <= 0) + if (GetInterval() <= 0 && IsRecoveryOrFlappingEndAndCheckableIsOK(checkable, cr, type)) SetNoMoreNotifications(false); }