Don't set Notification#no_more_notifications on custom notifications

[Al2Klimov]

... as it has nothing to do with reminder notifications.

fixes #6333

Edit

Imagine you get a problem notification. The above if branch does SetNoMoreNotifications(true), so that (the only) GetNoMoreNotifications() in NotificationComponent prevent reminders if none configured.

Now, if you send a custom notification, w/o my change the else branch does SetNoMoreNotifications(false), so that the NotificationComponent sends a reminder next time despite none configured.

[Al2Klimov]

@cla-bot check

[Al2Klimov]

What unit test cases would you like to prove this didn't break anything?

[yhabteab]

Firstly the referenced issues are very different ones. The OP of #7758 is reporting that the no_more_notifications is still set even after the Host has recovered while in Downtime and the Downtime has ended (either cancelled manually or ended automatically). This can even be found in the debug logs he provided.

Host goes down:

[2020-01-22 16:45:10 +0100] notice/ClusterEvents: Discarding 'send custom notification' message for checkable 'test-host-notification-bug-3' from 'xyz-mon-sat1.xyz.de': Unauthorized access.
[2020-01-22 16:45:10 +0100] information/Checkable: Checkable 'test-host-notification-bug-3' has 1 notification(s). Checking filters for type 'Problem', sends will be logged.
[2020-01-22 16:45:10 +0100] notice/Notification: Attempting to send notifications of type 'Problem' for notification object 'test-host-notification-bug-3!test-host-notification-bug-delay'.
[2020-01-22 16:45:10 +0100] notice/Notification: Not sending notifications for notification object 'test-host-notification-bug-3!test-host-notification-bug-delay': before specified begin time (10 seconds)

A moment later the notification is send and sets no_more_notifications to true.

[2020-01-22 16:45:25 +0100] notice/NotificationComponent: Attempting to send reminder notification 'test-host-notification-bug-3!test-host-notification-bug-delay'.
[2020-01-22 16:45:25 +0100] notice/Notification: Attempting to send reminder notifications of type 'Problem' for notification object 'test-host-notification-bug-3!test-host-notification-bug-delay'.
[2020-01-22 16:45:25 +0100] debug/Notification: User 'test-user-notification-bug' notification 'test-host-notification-bug-3!test-host-notification-bug-delay', Type 'Problem', TypeFilter: Problem (FType=32, TypeFilter=104)
[2020-01-22 16:45:25 +0100] debug/Notification: User 'test-user-notification-bug' notification 'test-host-notification-bug-3!test-host-notification-bug-delay', State 'Down', StateFilter: Critical and Down (FState=32, StateFilter=36)
[2020-01-22 16:45:25 +0100] information/Notification: Sending reminder 'Problem' notification 'test-host-notification-bug-3!test-host-notification-bug-delay' for user 'test-user-notification-bug'

During the downtime, the host recovers and the Recovery notification is suppressed. Afterwards, the downtime is ended/canceled and no notification is set due to filter types and ends up here and leaves no_more_notifications unchanged.

icinga2/lib/icinga/notification.cpp

Lines 329 to 346 in 2be08aa

	if (!(ftype & GetTypeFilter())) {
	Log(LogNotice, "Notification")
	<< "Not sending " << (reminder ? "reminder " : "") << "notifications for notification object '"
	<< notificationName << "': type '"
	<< NotificationTypeToString(type) << "' does not match type filter: "
	<< NotificationFilterToString(GetTypeFilter(), GetTypeFilterMap()) << ".";
	/* Ensure to reset no_more_notifications on Recovery notifications,
	* even if the admin did not configure them in the filter.
	*/
	{
	ObjectLock olock(this);
	if (type == NotificationRecovery && GetInterval() <= 0)
	SetNoMoreNotifications(false);
	}
	return;
	}

2020-01-22 16:53:05 +0100] notice/Notification: Attempting to send notifications of type 'DowntimeEnd' for notification object 'test-host-notification-bug-3!test-host-notification-bug-delay'.
[2020-01-22 16:53:05 +0100] notice/Notification: Not sending notifications for notification object 'test-host-notification-bug-3!test-host-notification-bug-delay': type 'DowntimeEnd' does not match type filter: Custom, Problem and Recovery.

So, how is your PR supposed to solve the problem? That would make it even worse and wouldn't reset it if the filter types would contain DowntimeEnd.

The OP of #9046, on the other hand, reports that sending custom notifications implicitly resets the "no_more_notifications" flag and causes problem notifications to be sent again while interval is set to 0.

[Al2Klimov]

#9046 test

This PR

sends the custom notification and that's it.

master

sends another problem one afterwards.

[yhabteab]

fixes doesn't fix #7758

Please remove this!

[Al2Klimov]

Because it says nothing (could be every other not fixed issue number) or because it fixes the mentioned issue?

[yhabteab]

Because it says nothing (could be every other not fixed issue number)

Exactly! It has nothing to do with PR at all!