Only notify users on recovery who have been notified on not-OK before

Also ensure that type NotificationRecovery always
passes the state filter (missing `OK` is totally fine).

Also fix that notification delays set the correct
next notification time to the begin time window.

fixes #7579
fixes #7623
fixes #6547

diff --git a/doc/4-monitoring-basics.md b/doc/4-monitoring-basics.md
index f37fc16ec7a2f18a4ef57cbf25c7a38d0f853ad5..fcca50ba2914f26b4a353f2a26221d33218a8bc2 100644 (file)
--- a/doc/4-monitoring-basics.md
+++ b/doc/4-monitoring-basics.md
@@ -610,8 +610,6 @@ to the defined notifications. That way you'll save duplicated attributes in each
 
 The time period `24x7` is shipped as example configuration with Icinga 2.
 
-
-
 Use the `apply` keyword to create `Notification` objects for your services:
 
     apply Notification "notify-cust-xy-mysql" to Service {
@@ -628,6 +626,11 @@ Instead of assigning users to notifications, you can also add the `user_groups`
 attribute with a list of user groups to the `Notification` object. Icinga 2 will
 send notifications to all group members.
 
+> **Note**
+>
+> Only users who have been notified of a problem before  (`Warning`, `Critical`, `Unknown`
+> states for services, `Down` for hosts) will receive `Recovery` notifications.
+
 ### <a id="notification-escalations"></a> Notification Escalations
 
 When a problem notification is sent and a problem still exists at the time of re-notification

diff --git a/lib/icinga/notification.cpp b/lib/icinga/notification.cpp
index 350a3deae9accde288ba650462172118f5f64921..a5c561aeb15adda35030c9152bc4aaefb91ca49a 100644 (file)
--- a/lib/icinga/notification.cpp
+++ b/lib/icinga/notification.cpp
@@ -245,6 +245,14 @@ void Notification::BeginExecuteNotification(NotificationType type, const CheckRe
                        if (times && times->Contains("begin") && now < checkable->GetLastHardStateChange() + times->Get("begin")) {
                                Log(LogNotice, "Notification")
                                    << "Not sending notifications for notification object '" << GetName() << "': before escalation range";
+
+                               /* we need to adjust the next notification time
+                                * to now + begin delaying the first notification
+                                */
+                               double nextProposedNotification = now + times->Get("begin") + 1.0;
+                               if (GetNextNotification() > nextProposedNotification)
+                                       SetNextNotification(nextProposedNotification);
+
                                return;
                        }
 
@@ -262,25 +270,29 @@ void Notification::BeginExecuteNotification(NotificationType type, const CheckRe
 
                if (!(ftype & GetTypeFilter())) {
                        Log(LogNotice, "Notification")
-                           << "Not sending notifications for notification object '" << GetName() << "': type filter does not match";
+                           << "Not sending notifications for notification object '" << GetName() << "': type filter does not match '"
+                           << NotificationTypeToString(type) << "'";
                        return;
                }
 
-               Host::Ptr host;
-               Service::Ptr service;
-               tie(host, service) = GetHostService(checkable);
+               /* ensure that recovery notifications are always sent, no state filter checks necessary */
+               if (type != NotificationRecovery) {
+                       Host::Ptr host;
+                       Service::Ptr service;
+                       tie(host, service) = GetHostService(checkable);
 
-               unsigned long fstate;
+                       unsigned long fstate;
 
-               if (service)
-                       fstate = ServiceStateToFilter(service->GetState());
-               else
-                       fstate = HostStateToFilter(host->GetState());
+                       if (service)
+                               fstate = ServiceStateToFilter(service->GetState());
+                       else
+                               fstate = HostStateToFilter(host->GetState());
 
-               if (!(fstate & GetStateFilter())) {
-                       Log(LogNotice, "Notification")
-                           << "Not sending notifications for notification object '" << GetName() << "': state filter does not match";
-                       return;
+                       if (!(fstate & GetStateFilter())) {
+                               Log(LogNotice, "Notification")
+                                   << "Not sending notifications for notification object '" << GetName() << "': state filter does not match";
+                               return;
+                       }
                }
        }
 
@@ -307,19 +319,47 @@ void Notification::BeginExecuteNotification(NotificationType type, const CheckRe
        Service::OnNotificationSendStart(this, checkable, allUsers, type, cr, author, text);
 
        std::set<User::Ptr> allNotifiedUsers;
+
        BOOST_FOREACH(const User::Ptr& user, allUsers) {
-               if (!user->GetEnableNotifications() || !CheckNotificationUserFilters(type, user, force))
+               String userName = user->GetName();
+
+               if (!user->GetEnableNotifications()) {
+                       Log(LogNotice, "Notification")
+                           << "Disabled notifications for user '" << userName << "'. Not sending notification.";
                        continue;
+               }
+
+               if (!CheckNotificationUserFilters(type, user, force)) {
+                       Log(LogNotice, "Notification")
+                           << "Notification filters for user '" << userName << "' not matched. Not sending notification.";
+                       continue;
+               }
+
+               /* on recovery, check if user was notified before */
+               if (type == NotificationRecovery) {
+                       if (m_NotifiedUsers.find(userName) == m_NotifiedUsers.end()) {
+                               Log(LogNotice, "Notification")
+                                   << "We did not notify user '" << userName << "' before. Not sending recovery notification.";
+                               continue;
+                       }
+               }
 
                Log(LogInformation, "Notification")
-                   << "Sending notification for user '" << user->GetName() << "'";
+                   << "Sending notification for user '" << userName << "'";
 
                Utility::QueueAsyncCallback(boost::bind(&Notification::ExecuteNotificationHelper, this, type, user, cr, force, author, text));
 
                /* collect all notified users */
                allNotifiedUsers.insert(user);
+
+               /* store all notified users for later recovery checks */
+               m_NotifiedUsers.insert(userName);
        }
 
+       /* if this was a recovery notification, reset all notified users */
+       if (type == NotificationRecovery)
+               ResetNotifiedUsers();
+
        /* used in db_ido for notification history */
        Service::OnNotificationSentToAllUsers(this, checkable, allNotifiedUsers, type, cr, author, text);
 }
@@ -347,23 +387,26 @@ bool Notification::CheckNotificationUserFilters(NotificationType type, const Use
                        return false;
                }
 
-               Checkable::Ptr checkable = GetCheckable();
-               Host::Ptr host;
-               Service::Ptr service;
-               tie(host, service) = GetHostService(checkable);
+               /* check state filters it this is not a recovery notification */
+               if (type != NotificationRecovery) {
+                       Checkable::Ptr checkable = GetCheckable();
+                       Host::Ptr host;
+                       Service::Ptr service;
+                       tie(host, service) = GetHostService(checkable);
 
-               unsigned long fstate;
+                       unsigned long fstate;
 
-               if (service)
-                               fstate = ServiceStateToFilter(service->GetState());
-               else
-                               fstate = HostStateToFilter(host->GetState());
+                       if (service)
+                                       fstate = ServiceStateToFilter(service->GetState());
+                       else
+                                       fstate = HostStateToFilter(host->GetState());
 
-               if (!(fstate & user->GetStateFilter())) {
-                       Log(LogNotice, "Notification")
-                           << "Not sending notifications for notification object '"
-                           << GetName() << " and user '" << user->GetName() << "': state filter does not match";
-                       return false;
+                       if (!(fstate & user->GetStateFilter())) {
+                               Log(LogNotice, "Notification")
+                                   << "Not sending notifications for notification object '"
+                                   << GetName() << " and user '" << user->GetName() << "': state filter does not match";
+                               return false;
+                       }
                }
        }
 
@@ -403,6 +446,11 @@ void Notification::ExecuteNotificationHelper(NotificationType type, const User::
        }
 }
 
+void Notification::ResetNotifiedUsers(void)
+{
+       m_NotifiedUsers.clear();
+}
+
 int icinga::ServiceStateToFilter(ServiceState state)
 {
        switch (state) {
@@ -471,4 +519,3 @@ void Notification::ValidateFilters(const String& location, const Dictionary::Ptr
                    location + ": Type filter is invalid.");
        }
 }
-

diff --git a/lib/icinga/notification.hpp b/lib/icinga/notification.hpp
index 9d596cfb9fc54796f750d51cb77819c369055796..64cd8f7ab9608455d63e20136550aef04a38a8b0 100644 (file)
--- a/lib/icinga/notification.hpp
+++ b/lib/icinga/notification.hpp
@@ -97,6 +97,8 @@ public:
 
        bool CheckNotificationUserFilters(NotificationType type, const User::Ptr& user, bool force);
 
+       void ResetNotifiedUsers(void);
+
        static String NotificationTypeToString(NotificationType type);
 
        static boost::signals2::signal<void (const Notification::Ptr&, double, const MessageOrigin&)> OnNextNotificationChanged;
@@ -111,6 +113,8 @@ protected:
        virtual void Stop(void);
 
 private:
+       std::set<String> m_NotifiedUsers;
+
        void ExecuteNotificationHelper(NotificationType type, const User::Ptr& user, const CheckResult::Ptr& cr, bool force, const String& author = "", const String& text = "");
 
        static void EvaluateApplyRuleOneInstance(const intrusive_ptr<Checkable>& checkable, const String& name, const Dictionary::Ptr& locals, const ApplyRule& rule);