New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Acknowledgment notifications should only be send if problem notification has been send #6527

Merged
merged 4 commits into from Sep 12, 2018

Conversation

Projects
None yet
2 participants
@N-o-X
Member

N-o-X commented Aug 7, 2018

I'm not really sure, if always_send_acknowledgements should default to true.
@dnsmichi what's your opinion on that?

Update @dnsmichi: We'll go without a configuration option. Check the upgrading docs.

fixes #6047
fixes #6611

@N-o-X N-o-X added this to the 2.10.0 milestone Aug 7, 2018

@N-o-X N-o-X requested a review from dnsmichi Aug 7, 2018

@dnsmichi

This comment has been minimized.

Member

dnsmichi commented Aug 24, 2018

I don't like the configuration option on the notification object, although you cannot change the behaviour. Icinga 1.x behaves the same as Icinga 2 here by default. This needs more discussion after vacation.

@N-o-X N-o-X force-pushed the feature/acknowledgment-notifications-6047 branch 2 times, most recently from b571aa2 to 7075f89 Sep 4, 2018

@dnsmichi

This comment has been minimized.

Member

dnsmichi commented Sep 12, 2018

I needed to draw the differences and changes between versions. The patch also involves a change with Recovery notifications, this should be highlighted in a separate issue (for the changelog).

Need to simplify the patch for the problem filter, test it again, and then write upgrading docs.

@dnsmichi

This comment has been minimized.

Member

dnsmichi commented Sep 12, 2018

icinga2_notifications_2 9_2 10_acks

icinga2_notifications_2 9_2 10_recovery

@dnsmichi

This comment has been minimized.

Member

dnsmichi commented Sep 12, 2018

@N-o-X the patch with CheckNotificationUserFilters may cause errors. This also checks for time periods and state filters again. In this specific case we only want to check whether the user type filter contains Problem, and this can be done with a simple logical and.

@dnsmichi

This comment has been minimized.

Member

dnsmichi commented Sep 12, 2018

I've constructed a test case for this, please always include test protocols and configs in theses PRs :)

Tests

Acknowledgement

I'm simulating a timeperiod where at first start, the problem notification is not sent to this specific user. Later, before the acknowledgement happens, I'm stopping Icinga 2, disabling the timeperiod and starting Icinga 2 again.

Test Config

vim /usr/local/icinga2/etc/icinga2/tests/6527.conf

object NotificationCommand "6527" {
  command = [ "/bin/echo", "6527" ]
}

object TimePeriod "6527" {
  ranges = {
    "wednesday" = "17:00-19:00"
  }
}

object User "6527-1" {
  types = [ Problem, Acknowledgement ]

  /* provoke a test case where we don't receive a notification in a time period. */
  period = "6527"

}
object User "6527-2" {
  types = [ Acknowledgement ]
}

object Notification "6527" {
  command = "6527"
  users = [ "6527-1", "6527-2" ]
  host_name = "6527"
}

object Host "6527" {
  check_command = "dummy"
  check_interval = 1d
  retry_interval = 1d
  enable_active_checks = false
}

Test Run

  • Send passive check results via Icinga Web 2
  • Trigger the hard state with notifications for the problem. Verify the history, a notification was sent, but no contacts affected.

screen shot 2018-09-12 at 16 30 40

  • Stop Icinga 2, edit 6527.conf and comment out the period = ... line
  • Start Icinga 2 and acknowledge the problem.

screen shot 2018-09-12 at 16 31 04

Verify the logs.

icinga2 daemon -x debug | grep 6527

[2018-09-12 16:31:09 +0200] information/Checkable: Checking for configured notifications for object '6527'
[2018-09-12 16:31:09 +0200] debug/Checkable: Checkable '6527' has 2 notification(s).

[2018-09-12 16:31:09 +0200] notice/Notification: Attempting to send  notifications for notification object '6527!6527'.
[2018-09-12 16:31:09 +0200] notice/Notification: We did not notify user '6527-1' (Problem types enabled) for a problem before. Not sending acknowledgement notification.

[2018-09-12 16:31:09 +0200] information/Notification: Sending 'Acknowledgement' notification '6527!6527' for user '6527-2'
[2018-09-12 16:31:09 +0200] notice/Notification: Attempting to send  notifications for notification object '6527!test-ce'.
[2018-09-12 16:31:09 +0200] notice/Process: Running command '/bin/echo' '6527': PID 35157
[2018-09-12 16:31:09 +0200] information/Notification: Completed sending 'Acknowledgement' notification '6527!6527' for checkable '6527' and user '6527-2'.
[2018-09-12 16:31:09 +0200] notice/Process: PID 35157 ('/bin/echo' '6527') terminated with exit code 0

Recovery

Test Config

Use the above configuration, but add more users with recovery details.

object NotificationCommand "6527" {
  command = [ "/bin/echo", "6527" ]
}

object TimePeriod "6527" {
  ranges = {
    "wednesday" = "17:00-19:00"
  }
}

object User "6527-1" {
  types = [ Problem, Acknowledgement ]

  /* provoke a test case where we don't receive a notification in a time period. */
  period = "6527"

}
object User "6527-2" {
  types = [ Acknowledgement ]
}

object User "6527-3" {
  types = [ Problem, Recovery ]

  /* provoke a test case where we don't receive a notification in a time period. */
  //period = "6527"

}

object User "6527-4" {
  types = [ Recovery ]
}

object Notification "6527" {
  command = "6527"
  users = [ "6527-1", "6527-2", "6527-3", "6527-4" ]
  host_name = "6527"
}

object Host "6527" {
  check_command = "dummy"
  check_interval = 1d
  retry_interval = 1d
  enable_active_checks = false
}

Test Run

This is easy, same procedure as above, except for sending an UP check result instead of an Acknowledgement.

icinga2 daemon -x debug | grep 6527


[2018-09-12 16:50:57 +0200] debug/Checkable: Checkable '6527' has 2 notification(s).
[2018-09-12 16:50:57 +0200] notice/Notification: Attempting to send  notifications for notification object '6527!6527'.
[2018-09-12 16:50:57 +0200] information/Notification: Sending 'Recovery' notification '6527!6527' for user '6527-4'
[2018-09-12 16:50:57 +0200] notice/Process: Running command '/bin/echo' '6527': PID 37829
[2018-09-12 16:50:57 +0200] information/Notification: Completed sending 'Recovery' notification '6527!6527' for checkable '6527' and user '6527-4'.

[2018-09-12 16:50:57 +0200] notice/Notification: We did not notify user '6527-3' (Problem types enabled) for a problem before. Not sending recovery notification.

screen shot 2018-09-12 at 16 50 36
screen shot 2018-09-12 at 17 03 04

@dnsmichi dnsmichi force-pushed the feature/acknowledgment-notifications-6047 branch from 7075f89 to 354e3c9 Sep 12, 2018

@dnsmichi

All fine 🥇

@dnsmichi dnsmichi merged commit 71b5c5b into master Sep 12, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@dnsmichi dnsmichi deleted the feature/acknowledgment-notifications-6047 branch Sep 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment