tyler.durden
/
icinga2
mirror of https://github.com/Icinga/icinga2.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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.
This commit is contained in:
Julian Brost 2024-10-11 12:59:04 +02:00
parent 5e9e0bbcdf
commit 7d0a43f926
1 changed files with 3 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
lib/icinga/checkable-notification.cpp
View File

@ -203,7 +203,7 @@ void Checkable::FireSuppressedNotifications()
* If any of these conditions is not met, processing the suppressed notification is further delayed. * If any of these conditions is not met, processing the suppressed notification is further delayed.
*/ */
if (!state_suppressed && GetStateType() == StateTypeHard && !IsLikelyToBeCheckedSoon() && !wasLastParentRecoveryRecent.Get()) { if (!state_suppressed && GetStateType() == StateTypeHard && !IsLikelyToBeCheckedSoon() && !wasLastParentRecoveryRecent.Get()) {
if (NotificationReasonApplies(type)) { if (cr->GetState() != GetStateBeforeSuppression()) {
Checkable::OnNotificationsRequested(this, type, cr, "", "", nullptr); Checkable::OnNotificationsRequested(this, type, cr, "", "", nullptr);
} }
subtract |= NotificationRecovery|NotificationProblem; subtract |= NotificationRecovery|NotificationProblem;
@ -266,12 +266,12 @@ bool Checkable::NotificationReasonApplies(NotificationType type)
case NotificationProblem: case NotificationProblem:
{ {
auto cr (GetLastCheckResult()); auto cr (GetLastCheckResult());
return cr && !IsStateOK(cr->GetState()) && cr->GetState() != GetStateBeforeSuppression(); return cr && !IsStateOK(cr->GetState());
} }
case NotificationRecovery: case NotificationRecovery:
{ {
auto cr (GetLastCheckResult()); auto cr (GetLastCheckResult());
return cr && IsStateOK(cr->GetState()) && cr->GetState() != GetStateBeforeSuppression(); return cr && IsStateOK(cr->GetState());
} }
case NotificationFlappingStart: case NotificationFlappingStart:
return IsFlapping(); return IsFlapping();