From 069a56d84f3eae68afd3487554d35314626e30a0 Mon Sep 17 00:00:00 2001 From: Julian Brost Date: Fri, 11 Oct 2024 12:59:04 +0200 Subject: [PATCH] Use `Checkable::GetStateBeforeSuppression()` only where relevant This fixes an issue where recovery notifications get lost if they happen outside of a notification time period. Not all calls to `Checkable::NotificationReasonApplies()` need `GetStateBeforeSuppression()` to be checked. In fact, for one caller, `FireSuppressedNotifications()` in `lib/notification/notificationcomponent.cpp`, the state before suppression may not even be initialized properly, so that the default value of OK is used which can lead to incorrect return values. Note the difference between suppressions happening on the level of the `Checkable` object level and the `Notification` object level. Only the first sets the state before suppression in the `Checkable` object, but so far, also the latter used that value incorrectly. This commit moves the check of `GetStateBeforeSuppression()` from `Checkable::NotificationReasonApplies()` to the one place where it's actually relevant: `Checkable::FireSuppressedNotifications()`. This made the existing call to `NotificationReasonApplies()` unneccessary as it would always return true: the `type` argument is computed based on the current check result, so there's no need to check it against the current check result. --- lib/icinga/checkable-notification.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/icinga/checkable-notification.cpp b/lib/icinga/checkable-notification.cpp index 779ed4b7b0e..d537fb24b47 100644 --- a/lib/icinga/checkable-notification.cpp +++ b/lib/icinga/checkable-notification.cpp @@ -203,7 +203,7 @@ void Checkable::FireSuppressedNotifications() * If any of these conditions is not met, processing the suppressed notification is further delayed. */ if (!state_suppressed && GetStateType() == StateTypeHard && !IsLikelyToBeCheckedSoon() && !wasLastParentRecoveryRecent.Get()) { - if (NotificationReasonApplies(type)) { + if (cr->GetState() != GetStateBeforeSuppression()) { Checkable::OnNotificationsRequested(this, type, cr, "", "", nullptr); } subtract |= NotificationRecovery|NotificationProblem; @@ -266,12 +266,12 @@ bool Checkable::NotificationReasonApplies(NotificationType type) case NotificationProblem: { auto cr (GetLastCheckResult()); - return cr && !IsStateOK(cr->GetState()) && cr->GetState() != GetStateBeforeSuppression(); + return cr && !IsStateOK(cr->GetState()); } case NotificationRecovery: { auto cr (GetLastCheckResult()); - return cr && IsStateOK(cr->GetState()) && cr->GetState() != GetStateBeforeSuppression(); + return cr && IsStateOK(cr->GetState()); } case NotificationFlappingStart: return IsFlapping();