Skip to content

Commit e4514ff

Browse files
committed
Checkable: Add test for state notifications after a suppression ends
1 parent 3ad708b commit e4514ff

File tree

6 files changed

+210
-22
lines changed

6 files changed

+210
-22
lines changed

lib/icinga/checkable-notification.cpp

+18-18
Original file line numberDiff line numberDiff line change
@@ -129,34 +129,34 @@ void Checkable::UnregisterNotification(const Notification::Ptr& notification)
129129
m_Notifications.erase(notification);
130130
}
131131

132-
static void FireSuppressedNotifications(Checkable* checkable)
132+
void Checkable::FireSuppressedNotifications()
133133
{
134-
if (!checkable->IsActive())
134+
if (!IsActive())
135135
return;
136136

137-
if (checkable->IsPaused())
137+
if (IsPaused())
138138
return;
139139

140-
if (!checkable->GetEnableNotifications())
140+
if (!GetEnableNotifications())
141141
return;
142142

143-
int suppressed_types (checkable->GetSuppressedNotifications());
143+
int suppressed_types (GetSuppressedNotifications());
144144
if (!suppressed_types)
145145
return;
146146

147147
int subtract = 0;
148148

149149
{
150-
LazyInit<bool> wasLastParentRecoveryRecent ([&checkable]() {
151-
auto cr (checkable->GetLastCheckResult());
150+
LazyInit<bool> wasLastParentRecoveryRecent ([this]() {
151+
auto cr (GetLastCheckResult());
152152

153153
if (!cr) {
154154
return true;
155155
}
156156

157157
auto threshold (cr->GetExecutionStart());
158158

159-
for (auto& dep : checkable->GetDependencies()) {
159+
for (auto& dep : GetDependencies()) {
160160
auto parent (dep->GetParent());
161161
ObjectLock oLock (parent);
162162

@@ -170,7 +170,7 @@ static void FireSuppressedNotifications(Checkable* checkable)
170170

171171
for (auto type : {NotificationProblem, NotificationRecovery, NotificationFlappingStart, NotificationFlappingEnd}) {
172172
if (suppressed_types & type) {
173-
if (type & (NotificationProblem|NotificationRecovery) && checkable->GetStateType() != StateTypeHard) {
173+
if (type & (NotificationProblem|NotificationRecovery) && GetStateType() != StateTypeHard) {
174174
/* If there are suppressed state notifications and the checkable currently is in a soft state,
175175
* wait with sending the notification until a hard state is reached.
176176
*
@@ -185,11 +185,11 @@ static void FireSuppressedNotifications(Checkable* checkable)
185185
continue;
186186
}
187187

188-
bool still_applies = checkable->NotificationReasonApplies(type);
188+
bool still_applies = NotificationReasonApplies(type);
189189

190190
if (still_applies) {
191-
if (!checkable->NotificationReasonSuppressed(type) && !checkable->IsLikelyToBeCheckedSoon() && !wasLastParentRecoveryRecent.Get()) {
192-
Checkable::OnNotificationsRequested(checkable, type, checkable->GetLastCheckResult(), "", "", nullptr);
191+
if (!NotificationReasonSuppressed(type) && !IsLikelyToBeCheckedSoon() && !wasLastParentRecoveryRecent.Get()) {
192+
Checkable::OnNotificationsRequested(this, type, GetLastCheckResult(), "", "", nullptr);
193193

194194
subtract |= type;
195195
}
@@ -201,28 +201,28 @@ static void FireSuppressedNotifications(Checkable* checkable)
201201
}
202202

203203
if (subtract) {
204-
ObjectLock olock (checkable);
204+
ObjectLock olock (this);
205205

206-
int suppressed_types_before (checkable->GetSuppressedNotifications());
206+
int suppressed_types_before (GetSuppressedNotifications());
207207
int suppressed_types_after (suppressed_types_before & ~subtract);
208208

209209
if (suppressed_types_after != suppressed_types_before) {
210-
checkable->SetSuppressedNotifications(suppressed_types_after);
210+
SetSuppressedNotifications(suppressed_types_after);
211211
}
212212
}
213213
}
214214

215215
/**
216216
* Re-sends all notifications previously suppressed by e.g. downtimes if the notification reason still applies.
217217
*/
218-
void Checkable::FireSuppressedNotifications(const Timer * const&)
218+
void Checkable::FireSuppressedNotificationsTimer(const Timer * const&)
219219
{
220220
for (auto& host : ConfigType::GetObjectsByType<Host>()) {
221-
::FireSuppressedNotifications(host.get());
221+
host->FireSuppressedNotifications();
222222
}
223223

224224
for (auto& service : ConfigType::GetObjectsByType<Service>()) {
225-
::FireSuppressedNotifications(service.get());
225+
service->FireSuppressedNotifications();
226226
}
227227
}
228228

lib/icinga/checkable.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ void Checkable::Start(bool runtimeCreated)
103103
boost::call_once(once, []() {
104104
l_CheckablesFireSuppressedNotifications = new Timer();
105105
l_CheckablesFireSuppressedNotifications->SetInterval(5);
106-
l_CheckablesFireSuppressedNotifications->OnTimerExpired.connect(&Checkable::FireSuppressedNotifications);
106+
l_CheckablesFireSuppressedNotifications->OnTimerExpired.connect(&Checkable::FireSuppressedNotificationsTimer);
107107
l_CheckablesFireSuppressedNotifications->Start();
108108

109109
l_CleanDeadlinedExecutions = new Timer();

lib/icinga/checkable.hpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,8 @@ class Checkable : public ObjectImpl<Checkable>
191191
bool NotificationReasonSuppressed(NotificationType type);
192192
bool IsLikelyToBeCheckedSoon();
193193

194+
void FireSuppressedNotifications();
195+
194196
static void IncreasePendingChecks();
195197
static void DecreasePendingChecks();
196198
static int GetPendingChecks();
@@ -222,7 +224,7 @@ class Checkable : public ObjectImpl<Checkable>
222224

223225
static void NotifyDowntimeEnd(const Downtime::Ptr& downtime);
224226

225-
static void FireSuppressedNotifications(const Timer * const&);
227+
static void FireSuppressedNotificationsTimer(const Timer * const&);
226228
static void CleanDeadlinedExecutions(const Timer * const&);
227229

228230
/* Comments */

lib/icinga/downtime.ti

+2-2
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,12 @@ class Downtime : ConfigObject < DowntimeNameComposer
2525
load_after Host;
2626
load_after Service;
2727

28-
[config, protected, required, navigation(host)] name(Host) host_name {
28+
[config, required, navigation(host)] name(Host) host_name {
2929
navigate {{{
3030
return Host::GetByName(GetHostName());
3131
}}}
3232
};
33-
[config, protected, navigation(service)] String service_name {
33+
[config, navigation(service)] String service_name {
3434
track {{{
3535
if (!oldValue.IsEmpty()) {
3636
Service::Ptr service = Service::GetByNamePair(GetHostName(), oldValue);

test/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ add_boost_test(base
133133
icinga_checkresult/service_3attempts
134134
icinga_checkresult/host_flapping_notification
135135
icinga_checkresult/service_flapping_notification
136+
icinga_checkresult/suppressed_notification
136137
icinga_dependencies/multi_parent
137138
icinga_notification/strings
138139
icinga_notification/state_filter

test/icinga-checkresult.cpp

+185
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
/* Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+ */
22

3+
#include "icinga/downtime.hpp"
34
#include "icinga/host.hpp"
5+
#include "icinga/service.hpp"
46
#include <BoostTestTargetConfig.h>
57
#include <iostream>
8+
#include <sstream>
9+
#include <utility>
10+
#include <vector>
611

712
using namespace icinga;
813

@@ -809,4 +814,184 @@ BOOST_AUTO_TEST_CASE(service_flapping_ok_over_bad_into_ok)
809814

810815
#endif /* I2_DEBUG */
811816
}
817+
818+
BOOST_AUTO_TEST_CASE(suppressed_notification)
819+
{
820+
/* Tests that suppressed notifications on a Checkable are sent after the suppression ends if and only if the first
821+
* hard state after the suppression is different from the last hard state before the suppression. The test works
822+
* by bringing a service in a defined hard state, creating a downtime, performing some state changes, removing the
823+
* downtime, bringing the service into another defined hard state (if not already) and checking the requested
824+
* notifications.
825+
*/
826+
827+
const std::vector<ServiceState> states {ServiceOK, ServiceWarning, ServiceCritical, ServiceUnknown};
828+
829+
for (bool isVolatile : {false, true}) {
830+
for (int checkAttempts : {1, 2}) {
831+
for (ServiceState initialState: states) {
832+
for (auto s1 : states) for (auto s2 : states) for (auto s3 : states) for (auto s4 : states) {
833+
const std::vector<ServiceState> sequence {s1, s2, s3, s4};
834+
835+
std::string testcase;
836+
{
837+
std::ostringstream buf;
838+
buf << "volatile=" << isVolatile
839+
<< " checkAttempts=" << checkAttempts
840+
<< " sequence={" << Service::StateToString(initialState);
841+
for (ServiceState s: sequence) {
842+
buf << " " << Service::StateToString(s);
843+
}
844+
buf << "}";
845+
testcase = buf.str();
846+
}
847+
std::cout << "Test case: " << testcase << std::endl;
848+
849+
// Create host and service for the test.
850+
Host::Ptr host = new Host();
851+
host->SetName("suppressed_notifications");
852+
host->Register();
853+
854+
Service::Ptr service = new Service();
855+
service->SetHostName(host->GetName());
856+
service->SetName("service");
857+
service->SetActive(true);
858+
service->SetVolatile(isVolatile);
859+
service->SetMaxCheckAttempts(checkAttempts);
860+
service->Activate();
861+
service->SetAuthority(true);
862+
service->SetEnableActiveChecks(false); // TODO: maybe needed due to LikelyToBeCheckedSoon
863+
service->Register();
864+
865+
dynamic_pointer_cast<ConfigObject>(host)->OnAllConfigLoaded();
866+
dynamic_pointer_cast<ConfigObject>(service)->OnAllConfigLoaded();
867+
868+
// Bring service into the initial hard state.
869+
for (int i = 0; i < checkAttempts; i++) {
870+
std::cout << " ProcessCheckResult("
871+
<< Service::StateToString(initialState) << ")" << std::endl;
872+
service->ProcessCheckResult(MakeCheckResult(initialState));
873+
}
874+
BOOST_CHECK(service->GetState() == initialState);
875+
BOOST_CHECK(service->GetStateType() == StateTypeHard);
876+
877+
// Keep track of all notifications requested from now on.
878+
std::vector<std::pair<NotificationType, ServiceState>> requestedNotifications;
879+
boost::signals2::scoped_connection c (Checkable::OnNotificationsRequested.connect([&](
880+
const Checkable::Ptr& checkable, NotificationType type, const CheckResult::Ptr& cr,
881+
const String&, const String&, const MessageOrigin::Ptr&
882+
) {
883+
BOOST_CHECK_EQUAL(checkable, service);
884+
std::cout << " -> OnNotificationsRequested(" << Notification::NotificationTypeToString(type)
885+
<< ", " << Service::StateToString(cr->GetState()) << ")" << std::endl;
886+
requestedNotifications.emplace_back(type, cr->GetState());
887+
}));
888+
889+
// Helper to assert which notifications were requested. Implicitly clears the stored notifications.
890+
auto assertNotifications = [&](
891+
const std::vector<std::pair<NotificationType, ServiceState>>& expected,
892+
const std::string& extraMessage
893+
) {
894+
// Pretty-printer for the vectors of requested and expected notifications.
895+
auto pretty = [](const std::vector<std::pair<NotificationType, ServiceState>>& vec) {
896+
std::ostringstream s;
897+
898+
s << "{";
899+
bool first = true;
900+
for (const auto &v : vec) {
901+
if (first) {
902+
first = false;
903+
} else {
904+
s << ", ";
905+
}
906+
s << Notification::NotificationTypeToString(v.first)
907+
<< "/" << Service::StateToString(v.second);
908+
}
909+
s << "}";
910+
911+
return s.str();
912+
};
913+
914+
BOOST_CHECK_MESSAGE(requestedNotifications == expected, "expected=" << pretty(expected)
915+
<< " got=" << pretty(requestedNotifications)
916+
<< (extraMessage.empty() ? "" : " ") << extraMessage);
917+
918+
requestedNotifications.clear();
919+
};
920+
921+
// Start a downtime for the service.
922+
std::cout << " Downtime Start" << std::endl;
923+
Downtime::Ptr downtime = new Downtime();
924+
downtime->SetHostName(host->GetName());
925+
downtime->SetServiceName(service->GetName());
926+
downtime->SetName("downtime");
927+
downtime->SetFixed(true);
928+
downtime->SetStartTime(Utility::GetTime() - 3600);
929+
downtime->SetEndTime(Utility::GetTime() + 3600);
930+
service->RegisterDowntime(downtime);
931+
downtime->Register();
932+
dynamic_pointer_cast<ConfigObject>(downtime)->OnAllConfigLoaded();
933+
downtime->TriggerDowntime(Utility::GetTime());
934+
935+
BOOST_CHECK(service->IsInDowntime());
936+
937+
// Process check results for the state sequence.
938+
for (ServiceState s: sequence) {
939+
std::cout << " ProcessCheckResult(" << Service::StateToString(s) << ")" << std::endl;
940+
service->ProcessCheckResult(MakeCheckResult(s));
941+
BOOST_CHECK(service->GetState() == s);
942+
if (checkAttempts == 1) {
943+
BOOST_CHECK(service->GetStateType() == StateTypeHard);
944+
}
945+
}
946+
947+
assertNotifications({}, "(no notifications in downtime)");
948+
949+
if (service->GetSuppressedNotifications()) {
950+
BOOST_CHECK_EQUAL(service->GetStateBeforeSuppression(), initialState);
951+
}
952+
953+
// Remove the downtime.
954+
std::cout << " Downtime End" << std::endl;
955+
service->UnregisterDowntime(downtime);
956+
downtime->Unregister();
957+
BOOST_CHECK(!service->IsInDowntime());
958+
959+
if (service->GetStateType() == icinga::StateTypeSoft) {
960+
// When the current state is a soft state, no notification should be sent just yet.
961+
std::cout << " FireSuppressedNotifications()" << std::endl;
962+
service->FireSuppressedNotifications();
963+
964+
assertNotifications({}, testcase + " (should not fire in soft state)");
965+
966+
// Repeat the last check result until reaching a hard state.
967+
for (int i = 0; i < checkAttempts && service->GetStateType() == StateTypeSoft; i++) {
968+
std::cout << " ProcessCheckResult(" << Service::StateToString(sequence.back()) << ")"
969+
<< std::endl;
970+
service->ProcessCheckResult(MakeCheckResult(sequence.back()));
971+
BOOST_CHECK(service->GetState() == sequence.back());
972+
}
973+
}
974+
975+
// The service should be in a hard state now and notifications should now be sent if applicable.
976+
BOOST_CHECK(service->GetStateType() == StateTypeHard);
977+
978+
std::cout << " FireSuppressedNotifications()" << std::endl;
979+
service->FireSuppressedNotifications();
980+
981+
if (initialState != sequence.back()) {
982+
NotificationType t = sequence.back() == ServiceOK ? NotificationRecovery : NotificationProblem;
983+
assertNotifications({{t, sequence.back()}}, testcase);
984+
} else {
985+
assertNotifications({}, testcase);
986+
}
987+
988+
// Remove host and service.
989+
service->Unregister();
990+
host->Unregister();
991+
}
992+
}
993+
}
994+
}
995+
}
996+
812997
BOOST_AUTO_TEST_SUITE_END()

0 commit comments

Comments
 (0)