Fix sending notifications for volatile checks on OK->OK changes

volatile checks make state changes behave like HARD state changes.
Though OK -> OK transitions must not be notified.

Increased log information for notifications too.

fixes #8063

lib/icinga/checkable-check.cpp

@@ -381,10 +381,9 @@ void Checkable::ProcessCheckResult(const CheckResult::Ptr& cr, const MessageOrig
 	if (stateChange && old_stateType == StateTypeHard && GetStateType() == StateTypeHard)
 		hardChange = true;
 
-	if (GetVolatile())
-		hardChange = true;
+	bool is_volatile = GetVolatile();
 
-	if (hardChange) {
+	if (hardChange || is_volatile) {
 		SetLastHardStateRaw(new_state);
 		SetLastHardStateChange(now);
 	}
@@ -412,6 +411,9 @@ void Checkable::ProcessCheckResult(const CheckResult::Ptr& cr, const MessageOrig
 	if (old_state == ServiceOK && old_stateType == StateTypeSoft)
 		send_notification = false; /* Don't send notifications for SOFT-OK -> HARD-OK. */
 
+	if (is_volatile && old_state == ServiceOK && new_state == ServiceOK)
+		send_notification = false; /* Don't send notifications for volatile OK -> OK changes. */
+
 	bool send_downtime_notification = (GetLastInDowntime() != in_downtime);
 	SetLastInDowntime(in_downtime);
 
@@ -458,17 +460,17 @@ void Checkable::ProcessCheckResult(const CheckResult::Ptr& cr, const MessageOrig
 	String old_state_str = (service ? Service::StateToString(old_state) : Host::StateToString(Host::CalculateState(old_state)));
 	String new_state_str = (service ? Service::StateToString(new_state) : Host::StateToString(Host::CalculateState(new_state)));
 
-	if (hardChange) {
+	if (hardChange || is_volatile) {
 		OnStateChange(this, cr, StateTypeHard, origin);
 		Log(LogNotice, "Checkable")
-		    << "State Change: Checkable " << GetName() << " hard state change from " << old_state_str << " to " << new_state_str << " detected.";
+		    << "State Change: Checkable " << GetName() << " hard state change from " << old_state_str << " to " << new_state_str << " detected." << (is_volatile ? " Checkable is volatile." : "");
 	} else if (stateChange) {
 		OnStateChange(this, cr, StateTypeSoft, origin);
 		Log(LogNotice, "Checkable")
 		    << "State Change: Checkable " << GetName() << " soft state change from " << old_state_str << " to " << new_state_str << " detected.";
 	}
 
-	if (GetStateType() == StateTypeSoft || hardChange || recovery)
+	if (GetStateType() == StateTypeSoft || hardChange || recovery || is_volatile)
 		ExecuteEventHandler();
 
 	if (send_downtime_notification)

lib/icinga/notification.cpp

@@ -231,6 +231,9 @@ void Notification::BeginExecuteNotification(NotificationType type, const CheckRe
 {
 	ASSERT(!OwnsLock());
 
+	Log(LogInformation, "Notification")
+	    << "Attempting to send notifications for notification object '" << GetName() << "'.";
+
 	Checkable::Ptr checkable = GetCheckable();
 
 	if (!force) {
@@ -350,7 +353,7 @@ void Notification::BeginExecuteNotification(NotificationType type, const CheckRe
 		}
 
 		Log(LogInformation, "Notification")
-		    << "Sending notification for user '" << userName << "'";
+		    << "Sending notification '" << GetName() << "' for user '" << userName << "'";
 
 		Utility::QueueAsyncCallback(boost::bind(&Notification::ExecuteNotificationHelper, this, type, user, cr, force, author, text));
 
@@ -444,10 +447,10 @@ void Notification::ExecuteNotificationHelper(NotificationType type, const User::
 		Service::OnNotificationSentToUser(this, GetCheckable(), user, type, cr, author, text, command->GetName());
 
 		Log(LogInformation, "Notification")
-		    << "Completed sending notification for object '" << GetCheckable()->GetName() << "'";
+		    << "Completed sending notification '" << GetName() << "' for checkable '" << GetCheckable()->GetName() << "'";
 	} catch (const std::exception& ex) {
 		Log(LogWarning, "Notification")
-		    << "Exception occured during notification for object '"
+		    << "Exception occured during notification for checkable '"
 		    << GetCheckable()->GetName() << "': " << DiagnosticInformation(ex);
 	}
 }

test/config/8063.conf

@@ -0,0 +1,53 @@
+object CheckCommand "8063-my-disk" {
+  import "plugin-check-command"
+
+  command = [ PluginDir + "/check_disk" ]
+
+  arguments = {
+    "-w" = "$disk_wfree$%"
+    "-c" = "$disk_cfree$%"
+    "-W" = "$disk_inode_wfree$%"
+    "-K" = "$disk_inode_cfree$%"
+    "-p" = "$disk_partitions$"
+    "-x" = "$disk_partitions_excluded$"
+  }
+
+  vars.disk_wfree = 20
+  vars.disk_cfree = 10
+}
+
+object Host "8063-my-server" {
+  import "generic-host"
+  address = "127.0.0.1"
+  address6 = "::1"
+
+  vars.local_disks["basic-partitions"] = {
+    disk_partitions = [ "/", "/tmp", "/var", "/home", "/run/user/1000/gvfs" ]
+  }
+}
+
+apply Service "8063-" for (disk => config in host.vars.local_disks) {
+  import "generic-service"
+  check_command = "8063-my-disk"
+  check_interval = 5s
+  retry_interval = 5s
+
+  volatile = true
+  vars.volatile_check = true
+
+  vars += config
+
+  vars.disk_wfree = 10
+  vars.disk_cfree = 5
+
+  assign where host.vars.local_disks
+}
+
+apply Notification "disk-notification" to Service {
+  import "test-mail-service-notification"
+
+  users = [ "test-icingaadmin" ]
+
+  assign where service.vars.volatile_check == true
+}
+