TimePeriod: consult GetIncludes() and GetExcludes() in IsInside(), not UpdateRegion() #7855

Al2Klimov · 2020-02-26T14:43:52Z

~~fixes~~ doesn't fix #7398

Edit

The solution

2/3 commits just remove unused methods. 769de8d is your friend.

Instead of (error prone) merging the timeperiod's own calculated segments (e.g. 1640000000 - 1670000000) with the ones of the included/excluded timeperiods (e.g. 1660000000 - 1690000000) for using the merge result in one IsInside() call, this PR keeps each timeperiod's own calculated segments isolated from each other.

Due to this IsInside() has to call included/excluded timeperiods' IsInside() now.

The problem

~~IMAO Yonas already summarised the problem clearly in #7855 (comment) .~~

Edit II

"error prone" means that the order of periodically updated timeperiods matter. I.e. ideally the timeperiods used for including/excluding are updated first, then the ones refering to them. In the opposite case you have a problem as the computed ranges of parent_TP are merged with the computed ranges of child_TP before the latter are updated.

Al2Klimov · 2021-08-04T12:22:56Z

@cla-bot check

Al2Klimov · 2022-04-04T11:33:48Z

And maybe it also fixes #8741.

davixd · 2022-05-03T15:10:43Z

I'm very interested for solving #8741. Thatswhy I try to bring focus back on this issue. I have some news which maybe would help for further debug:

This bug affected me already since icinga2-2.12.3-1.el7.
With icinga2-2.13.2-1.el7 the bug was gone.
On 2. May I updated to icinga2-2.13.3-1.el7 and the bug is back. Yesterday was the 1. May (holiday). Notifications arrived at 2. May in the morning with icinga2-2.13.2-1.el7, right after update to icinga2-2.13.3-1.el7, the notifications are missing again, as it treats 2. May as a holiday again.

yhabteab · 2022-06-22T08:38:59Z

Test cases for #7398

Config

object TimePeriod "24x7" {
  ranges = { wednesday = "00:00-23:59" }
}

object TimePeriod "offDuty" {
  ranges = { wednesday = "10:00-17:00" }
}

object TimePeriod "preferred_tp" {
  ranges = { wednesday = "09:00-11:00" }
}

object TimePeriod "onDuty" {
  excludes = ["offDuty"]
  //includes = ["preferred_tp"]
  ranges = { wednesday = "00:00-23:59" }
}

object User "john" {
  email = "[email protected]"
  period = "24x7"
}

object Host "test" {
  address = "127.0.0.1"
  check_command = "hostalive"
}

apply Notification "notify-hosts" to Host {
  command = "mail-host-notification"
  period = "onDuty"

  users = ["john"]

  states = [Up, Down]
  types = [ Problem, Acknowledgement, Recovery, Custom, FlappingStart, FlappingEnd, DowntimeStart, DowntimeEnd, DowntimeRemoved ]

  assign where true
}

Before

[2022-06-22 09:57:51 +0200] information/Notification: Completed sending 'DowntimeStart' notification 'test!notify-hosts' for checkable 'test' and user 'john' using command 'mail-host-notification'.

[2022-06-22 09:57:51 +0200] notice/Notification: Attempting to send notifications of type 'DowntimeStart' for notification object 'test!notify-hosts'.
[2022-06-22 09:57:51 +0200] debug/Notification: Type 'DowntimeStart', TypeFilter: Acknowledgement, Custom, DowntimeEnd, DowntimeRemoved, DowntimeStart, FlappingEnd, FlappingStart, Problem and Recovery (FType=1, TypeFilter=511)
[2022-06-22 09:57:51 +0200] debug/Notification: User 'john' notification 'test!notify-hosts', Type 'DowntimeStart', TypeFilter: Acknowledgement, Custom, DowntimeEnd, DowntimeRemoved, DowntimeStart, FlappingEnd, FlappingStart, Problem and Recovery (FType=1, TypeFilter=511)
[2022-06-22 09:57:51 +0200] debug/Notification: User 'john' notification 'test!notify-hosts', State 'Down', StateFilter: Critical, Down, OK, Unknown, Up and Warning (FState=32, StateFilter=-1)
[2022-06-22 09:57:51 +0200] information/Notification: Sending 'DowntimeStart' notification 'test!notify-hosts' for user 'john'
[2022-06-22 09:57:51 +0200] notice/ApiListener: Relaying 'event::NotificationSentToAllUsers' message
[2022-06-22 09:57:51 +0200] information/Downtime: Triggering downtime 'test!d8b19bc4-25ed-4a5b-8372-fae70be6473d' for checkable 'test'.
[2022-06-22 09:57:51 +0200] debug/ApiListener: Sent update for object 'test!d8b19bc4-25ed-4a5b-8372-fae70be6473d': {"config":"object Downtime \"d8b19bc4-25ed-4a5b-8372-fae70be6473d\" ignore_on_error {\n\tauthor = \"icingaadmin\"\n\tcomment = \"Cluster upgrade maintenance\"\n\tconfig_owner = \"\"\n\tduration = 0.000000\n\tend_time = 1655888271.000000\n\tentry_time = 1655884671.609968\n\tfixed = true\n\thost_name = \"test\"\n\tparent = \"\"\n\tscheduled_by = \"\"\n\tstart_time = 1655884671.000000\n\ttriggered_by = \"\"\n\tversion = 1655884671.610063\n\tzone = \"master\"\n}\n","modified_attributes":{},"name":"test!d8b19bc4-25ed-4a5b-8372-fae70be6473d","original_attributes":[],"type":"Downtime","version":1655884671.610063,"zone":"master"}
[2022-06-22 09:57:51 +0200] notice/ApiListener: Relaying 'config::UpdateObject' message
[2022-06-22 09:57:51 +0200] information/ConfigObjectUtility: Created and activated object 'test!d8b19bc4-25ed-4a5b-8372-fae70be6473d' of type 'Downtime'.
[2022-06-22 09:57:51 +0200] notice/Process: Running command '/Users/yhabteab/Workspace/icinga2/prefix/etc/icinga2/scripts/mail-host-notification.sh' '-4' '127.0.0.1' '-6' '' '-b' 'icingaadmin' '-c' 'Cluster upgrade maintenance' '-d' '2022-06-22 09:57:51 +0200' '-l' 'test' '-n' 'test' '-o' 'execvpe(/Users/yhabteab/Workspace/icinga2/prefix/lib/nagios/plugins/check_ping) failed: No such file or directory' '-r' '[email protected]' '-s' 'DOWN' '-t' 'DOWNTIMESTART': PID 48059
[2022-06-22 09:57:51 +0200] notice/WorkQueue: Stopped WorkQueue threads for 'ConfigObjectUtility::CreateObject'
[2022-06-22 09:57:51 +0200] notice/ApiListener: Relaying 'event::NotificationSentUser' message
[2022-06-22 09:57:51 +0200] information/Notification: Completed sending 'DowntimeStart' notification 'test!notify-hosts' for checkable 'test' and user 'john' using command 'mail-host-notification'.

After

[2022-06-22 10:33:58 +0200] notice/Notification: Not sending notifications for notification object 'test!notify-hosts': not in timeperiod 'onDuty'

[2022-06-22 10:33:58 +0200] information/Checkable: Checkable 'test' has 1 notification(s). Checking filters for type 'DowntimeStart', sends will be logged.
[2022-06-22 10:33:58 +0200] notice/ApiListener: Relaying 'event::SetForceNextNotification' message
[2022-06-22 10:33:58 +0200] notice/Notification: Attempting to send notifications of type 'DowntimeStart' for notification object 'test!notify-hosts'.
[2022-06-22 10:33:58 +0200] notice/Notification: Not sending notifications for notification object 'test!notify-hosts': not in timeperiod 'onDuty'
[2022-06-22 10:33:58 +0200] information/Downtime: Triggering downtime 'test!61b2d5f8-9de9-4403-a51d-1c6ca87cf166' for checkable 'test'.
[2022-06-22 10:33:58 +0200] debug/ApiListener: Sent update for object 'test!61b2d5f8-9de9-4403-a51d-1c6ca87cf166': {"config":"object Downtime \"61b2d5f8-9de9-4403-a51d-1c6ca87cf166\" ignore_on_error {\n\tauthor = \"icingaadmin\"\n\tcomment = \"Cluster upgrade maintenance\"\n\tconfig_owner = \"\"\n\tduration = 0.000000\n\tend_time = 1655890437.000000\n\tentry_time = 1655886838.060954\n\tfixed = true\n\thost_name = \"test\"\n\tparent = \"\"\n\tscheduled_by = \"\"\n\tstart_time = 1655886837.000000\n\ttriggered_by = \"\"\n\tversion = 1655886838.061191\n\tzone = \"master\"\n}\n","modified_attributes":{},"name":"test!61b2d5f8-9de9-4403-a51d-1c6ca87cf166","original_attributes":[],"type":"Downtime","version":1655886838.061191,"zone":"master"}

Now, comment out the includes = ["preferred_tp"] attribute of onDuty TimePeriod.

[2022-06-22 10:35:44 +0200] information/Notification: Completed sending 'DowntimeStart' notification 'test!notify-hosts' for checkable 'test' and user 'john' using command 'mail-host-notification'.

[2022-06-22 10:35:44 +0200] information/Checkable: Checkable 'test' has 1 notification(s). Checking filters for type 'DowntimeStart', sends will be logged.
[2022-06-22 10:35:44 +0200] notice/ApiListener: Relaying 'event::SetForceNextNotification' message
[2022-06-22 10:35:44 +0200] notice/Notification: Attempting to send notifications of type 'DowntimeStart' for notification object 'test!notify-hosts'.
[2022-06-22 10:35:44 +0200] debug/Notification: Type 'DowntimeStart', TypeFilter: Acknowledgement, Custom, DowntimeEnd, DowntimeRemoved, DowntimeStart, FlappingEnd, FlappingStart, Problem and Recovery (FType=1, TypeFilter=511)
[2022-06-22 10:35:44 +0200] debug/Notification: User 'john' notification 'test!notify-hosts', Type 'DowntimeStart', TypeFilter: Acknowledgement, Custom, DowntimeEnd, DowntimeRemoved, DowntimeStart, FlappingEnd, FlappingStart, Problem and Recovery (FType=1, TypeFilter=511)
[2022-06-22 10:35:44 +0200] debug/Notification: User 'john' notification 'test!notify-hosts', State 'Down', StateFilter: Critical, Down, OK, Unknown, Up and Warning (FState=32, StateFilter=-1)
[2022-06-22 10:35:44 +0200] information/Notification: Sending 'DowntimeStart' notification 'test!notify-hosts' for user 'john'
[2022-06-22 10:35:44 +0200] notice/ApiListener: Relaying 'event::NotificationSentToAllUsers' message
[2022-06-22 10:35:44 +0200] information/Downtime: Triggering downtime 'test!c8039a8c-2824-4e60-b315-5c7a14d175fb' for checkable 'test'.
[2022-06-22 10:35:44 +0200] debug/ApiListener: Sent update for object 'test!c8039a8c-2824-4e60-b315-5c7a14d175fb': {"config":"object Downtime \"c8039a8c-2824-4e60-b315-5c7a14d175fb\" ignore_on_error {\n\tauthor = \"icingaadmin\"\n\tcomment = \"Cluster upgrade maintenance\"\n\tconfig_owner = \"\"\n\tduration = 0.000000\n\tend_time = 1655890543.000000\n\tentry_time = 1655886944.024694\n\tfixed = true\n\thost_name = \"test\"\n\tparent = \"\"\n\tscheduled_by = \"\"\n\tstart_time = 1655886943.000000\n\ttriggered_by = \"\"\n\tversion = 1655886944.024791\n\tzone = \"master\"\n}\n","modified_attributes":{},"name":"test!c8039a8c-2824-4e60-b315-5c7a14d175fb","original_attributes":[],"type":"Downtime","version":1655886944.024791,"zone":"master"}
[2022-06-22 10:35:44 +0200] information/ConfigObjectUtility: Created and activated object 'test!c8039a8c-2824-4e60-b315-5c7a14d175fb' of type 'Downtime'.
[2022-06-22 10:35:44 +0200] notice/ApiListener: Relaying 'config::UpdateObject' message
[2022-06-22 10:35:44 +0200] notice/Process: Running command '/Users/yhabteab/Workspace/icinga2/prefix/etc/icinga2/scripts/mail-host-notification.sh' '-4' '127.0.0.1' '-6' '' '-b' 'icingaadmin' '-c' 'Cluster upgrade maintenance' '-d' '2022-06-22 10:35:44 +0200' '-l' 'test' '-n' 'test' '-o' 'execvpe(/Users/yhabteab/Workspace/icinga2/prefix/lib/nagios/plugins/check_ping) failed: No such file or directory' '-r' '[email protected]' '-s' 'DOWN' '-t' 'DOWNTIMESTART': PID 70037
[2022-06-22 10:35:44 +0200] notice/WorkQueue: Stopped WorkQueue threads for 'ConfigObjectUtility::CreateObject'
[2022-06-22 10:35:44 +0200] notice/ApiListener: Relaying 'event::NotificationSentUser' message
[2022-06-22 10:35:44 +0200] information/Notification: Completed sending 'DowntimeStart' notification 'test!notify-hosts' for checkable 'test' and user 'john' using command 'mail-host-notification'.

yhabteab · 2022-06-22T12:18:38Z

Running tests for #8741: http://10.27.0.228/icingaweb2/dashboard

Al2Klimov · 2023-02-22T14:51:25Z

ref/IP/44756

julianbrost · 2023-02-23T15:19:00Z

The code change looks non-trivial and the referenced issue is quite lengty, so please add a summary of the problem and how this is fixed by this PR to the description.

julianbrost · 2023-03-10T16:40:02Z

IMAO Yonas already summarised the problem clearly in #7855 (comment) .

That comment only contains a test. Or do you consider the example configuration to be a sufficient description of the problem?

Apart from that, I can't really follow that test either. Let's consider the time periods on 2022-06-22 (which indeed was a Wednesday) at 09:57:51:

offDuty: it's not 10:00-17:00, so it's outside of that time period
onDuty: it's inside 00:00-23:59 and it's outside of the excluded offDutytime period, so we are insideonDuty`

And according to the "Before" log, a notification gets sent, which looks correct to me.

Now the "After" log is from 10:33:58, that means the offDuty period was entered, therefore the exclude in onDuty takes effect and no notification is sent, as it's now outside this time period, so also looks correct to me.

For the second "After" test, the time period preferred_tp becomes relevant. It's a time between 09:00-11:00, so we are inside that time period. onDuty now additionally includes this time period so that for the current time, it has both active includes and excludes. prefer_includes is not given explicitly, therefore the default true is used, i.e. we are inside onDuty. And indeed, the notification gets sent, which again looks correct to me.

So there are only correct cases in this test, so it doesn't demonstrate any change. Apart from that, the test is strange because there was a time period transition between the times the old and the new version was tested.

Al2Klimov · 2023-03-20T10:57:19Z

And from pure code design PoV?

Instead of (error prone) merging the timeperiod's own calculated segments (e.g. 1640000000 - 1670000000) with the ones of the included/excluded timeperiods (e.g. 1660000000 - 1690000000) for using the merge result in one IsInside() call, this PR keeps each timeperiod's own calculated segments isolated from each other.

Due to this IsInside() has to call included/excluded timeperiods' IsInside() now.

Al2Klimov · 2023-04-11T13:23:03Z

Intermediate result

The only thing changed here -TimePeriod- doesn’t have a getcurrent/next/whatever interface based on the segments, so only IsInside() matters. IMAO no harm possibility.

Al2Klimov · 2023-04-11T13:32:17Z

@yhabteab Btw. no idea why you and me got different test results with notifications and checks, both(!) use only IsInside(). Maybe yet another rebase will fix your tests...

refs #7398

…t UpdateRegion() refs #7398

refs #7398

Al2Klimov · 2023-04-11T15:57:21Z

I'm very interested for solving #8741. Thatswhy I try to bring focus back on this issue. I have some news which maybe would help for further debug:

This bug affected me already since icinga2-2.12.3-1.el7. With icinga2-2.13.2-1.el7 the bug was gone. On 2. May I updated to icinga2-2.13.3-1.el7 and the bug is back. Yesterday was the 1. May (holiday). Notifications arrived at 2. May in the morning with icinga2-2.13.2-1.el7, right after update to icinga2-2.13.3-1.el7, the notifications are missing again, as it treats 2. May as a holiday again.

@davixd Would you like to test whether this fix solves your specific problem? Here you are your Icinga version plus my three commits:

https://nextcloud.icinga.com/index.php/s/RMLGwtxc4bQsCqC

davixd · 2023-04-11T23:31:49Z

@Al2Klimov
Sure! Thanks for the work.
I have at company a dev environment which is a equivalent to our prod environment. There I will be able to deploy the icinga version and to test it. I will be back with the results as fast as possible.

julianbrost · 2023-04-12T11:55:29Z

@Al2Klimov What about demonstrating the issue/fix on the TimePeriod level as we discussed (preferably as a unit test)?

Al2Klimov · 2023-04-12T15:02:11Z

I honestly -but abjectly- tried. This is hard-to-catch fish.

BOOST_AUTO_TEST_CASE(ak)
{
	TimePeriod::Ptr tp0900_1100 = new TimePeriod();
	tp0900_1100->SetName("tp0900_1100");
	tp0900_1100->SetRanges(new Dictionary({{ "thursday", "09:00-11:00" }}), true);

	TimePeriod::Ptr tp1000_1120 = new TimePeriod();
	tp1000_1120->SetName("tp1000_1120");
	tp1000_1120->SetRanges(new Dictionary({{ "thursday", "10:00-11:20" }}), true);

	TimePeriod::Ptr tp0000_2400 = new TimePeriod();
	tp0000_2400->SetName("tp0000_2400");
	tp0000_2400->SetRanges(new Dictionary({{ "thursday", "00:00-24:00" }}), true);
	tp0000_2400->SetIncludes(new Array({"tp0900_1100"}), true);
	tp0000_2400->SetExcludes(new Array({"tp1000_1120"}), true);

	TimePeriod::Ptr tps[] = {tp0900_1100, tp1000_1120, tp0000_2400};
	Function::Ptr update = new Function("LegacyTimePeriod", LegacyTimePeriod::ScriptFunc, {"tp", "begin", "end"});

	for (auto& tp : tps) {
		tp->SetUpdate(update, true);
		tp->Register();
	}

	Defer unregister ([&tps]() {
		for (auto& tp : tps) {
			tp->Unregister();
		}
	});

	for (auto& tp : tps) {
		tp->UpdateRegion(make_time_t("2023-04-06 00:00:00"), make_time_t("2023-04-07 00:00:00"), true);
	}

	BOOST_CHECK(!tp0000_2400->IsInside(make_time_t("2023-04-06 11:10:00")));
}

And you said yourself that it's unlikely the customer's problem. So let's cancel our attempt and demount the fishing rod.

davixd · 2023-04-12T17:03:20Z

@Al2Klimov

the packages seems not to be updatable with the official repo from Icinga2 for CentOS7:

[18:59:27 INFRA root@monitoring-dev-1-1 7855c7]# ll
total 5020
-rw-r--r--. 1 root root   27932 Apr 11 15:37 icinga2-2.13.3+3.g5c20af2c3-1.el7.x86_64.rpm
-rw-r--r--. 1 root root 4815424 Apr 11 15:37 icinga2-bin-2.13.3+3.g5c20af2c3-1.el7.x86_64.rpm
-rw-r--r--. 1 root root  160456 Apr 11 15:37 icinga2-common-2.13.3+3.g5c20af2c3-1.el7.x86_64.rpm
-rw-r--r--. 1 root root  121476 Apr 11 15:37 icinga2-ido-mysql-2.13.3+3.g5c20af2c3-1.el7.x86_64.rpm
-rw-r--r--. 1 root root    7204 Apr 11 15:37 vim-icinga2-2.13.3+3.g5c20af2c3-1.el7.x86_64.rpm
[18:59:28 INFRA root@monitoring-dev-1-1 7855c7]#

[18:57:04 INFRA root@monitoring-dev-1-1 7855c7]# yum localinstall *.rpm
Loaded plugins: fastestmirror, langpacks, rhnplugin, versionlock
This system is receiving updates from RHN Classic or Red Hat Satellite.
Examining icinga2-2.13.3+3.g5c20af2c3-1.el7.x86_64.rpm: icinga2-2.13.3+3.g5c20af2c3-1.el7.x86_64
icinga2-2.13.3+3.g5c20af2c3-1.el7.x86_64.rpm: does not update installed package.
Examining icinga2-bin-2.13.3+3.g5c20af2c3-1.el7.x86_64.rpm: icinga2-bin-2.13.3+3.g5c20af2c3-1.el7.x86_64
icinga2-bin-2.13.3+3.g5c20af2c3-1.el7.x86_64.rpm: does not update installed package.
Examining icinga2-common-2.13.3+3.g5c20af2c3-1.el7.x86_64.rpm: icinga2-common-2.13.3+3.g5c20af2c3-1.el7.x86_64
icinga2-common-2.13.3+3.g5c20af2c3-1.el7.x86_64.rpm: does not update installed package.
Examining icinga2-ido-mysql-2.13.3+3.g5c20af2c3-1.el7.x86_64.rpm: icinga2-ido-mysql-2.13.3+3.g5c20af2c3-1.el7.x86_64
icinga2-ido-mysql-2.13.3+3.g5c20af2c3-1.el7.x86_64.rpm: does not update installed package.
Examining vim-icinga2-2.13.3+3.g5c20af2c3-1.el7.x86_64.rpm: vim-icinga2-2.13.3+3.g5c20af2c3-1.el7.x86_64
vim-icinga2-2.13.3+3.g5c20af2c3-1.el7.x86_64.rpm: does not update installed package.
Nothing to do

Installed on my CentOS 7.9 dev system is:

[19:00:37 INFRA root@monitoring-dev-1-1 7855c7]# rpm -qa |grep icinga2
icinga2-common-2.13.7-1.el7.x86_64
icinga2-bin-2.13.7-1.el7.x86_64
icinga2-2.13.7-1.el7.x86_64
vim-icinga2-2.13.7-1.el7.x86_64
icinga2-ido-mysql-2.13.7-1.el7.x86_64
[19:00:39 INFRA root@monitoring-dev-1-1 7855c7]#

Any suggestions?

Al2Klimov · 2023-04-13T07:59:36Z

What's your current Icinga 2 version?

davixd · 2023-04-13T08:52:30Z

icinga2-2.13.7-1.el7.x86_64

Al2Klimov · 2023-04-13T09:35:57Z

At best you put all these in a new, persistent directory. Then run createrepo /new/persistent/directory. Finally create /etc/yum.repos.d/NAME_IS_UP_TO_YOU.repo like this:

[NAME_IS_UP_TO_YOU]
baseurl=file:///new/persistent/directory
enabled=1
gpgcheck=0

Now you should be able to upgrade just the icinga2 packages as usual and to undo the upgrade by yum history undo last before you disable that new repo again (if you wish).

julianbrost · 2023-04-13T09:41:06Z

@Al2Klimov Your packages are based on 2.13.3, I don't see why that downgrade should be intended here (and that's why yum rejects them).

julianbrost · 2023-04-13T09:43:32Z

I honestly -but abjectly- tried. This is hard-to-catch fish.

What is the code block with the test case trying to show? Does that test pass both with and without this PR?

davixd · 2023-04-13T10:04:52Z

@julianbrost
That was the magic keyword here: downgrade. I have overlooked that provided packages are older.

@Al2Klimov @julianbrost
After a downgrade session, I was able to install your provides packages:

=================================================================================================================================================== Package                    Arch            Version                             Repository                                                    Size
===================================================================================================================================================Updating:
 icinga2                    x86_64          2.13.3+3.g5c20af2c3-1.el7           /icinga2-2.13.3+3.g5c20af2c3-1.el7.x86_64                     54 k
 icinga2-bin                x86_64          2.13.3+3.g5c20af2c3-1.el7           /icinga2-bin-2.13.3+3.g5c20af2c3-1.el7.x86_64                 20 M
 icinga2-common             x86_64          2.13.3+3.g5c20af2c3-1.el7           /icinga2-common-2.13.3+3.g5c20af2c3-1.el7.x86_64             733 k
 icinga2-ido-mysql          x86_64          2.13.3+3.g5c20af2c3-1.el7           /icinga2-ido-mysql-2.13.3+3.g5c20af2c3-1.el7.x86_64          559 k
 vim-icinga2                x86_64          2.13.3+3.g5c20af2c3-1.el7           /vim-icinga2-2.13.3+3.g5c20af2c3-1.el7.x86_64                 17 k

Transaction Summary
===================================================================================================================================================Upgrade  5 Packages

So now I can test it with Icinga2 2.13.3+3. I be back with the results.

Al2Klimov · 2023-04-13T10:56:40Z

C'mon! I've built fresh 2.13.7+3 packages for you: #7855 (comment)

And yes, the unit test result (pass IIRC) doesn’t change with this PR.

davixd · 2023-04-13T13:25:10Z

Perfectly, its up and running:

So I confirm to start the tests with icinga2-2.13.7+3.

Thanks. And I be back with the results.

davixd · 2023-04-14T14:06:51Z

I'm back with the results. Unfortunately #7855 / 2.13.7+3 does not fix #8741.

My tests and results:

Test1:

I have added 13. April as a holiday and did excluded it for User david and included it for user ouser
Right after I did /usr/sbin/icinga2 daemon -C and systemctl restart icinga2
I picked up a ssh Check in the Icinga2Web-GUI and generated a not forced message
The message was correctly send out only to ouser (during holiday)

Test2:

I have removed 13. April as a holiday again
Right after I did /usr/sbin/icinga2 daemon -C and systemctl restart icinga2
I picked up the same ssh Check in the Icinga2Web-GUI and generated a not forced message
The message was correctly send out only to david (during a regular business day)

Test3:

I have added again 13. April as a holiday
Right after I did /usr/sbin/icinga2 daemon -C and systemctl restart icinga2
I picked up the same ssh Check in the Icinga2Web-GUI and generated a not forced message
The message was again send out correctly only to ouser (during holiday)

Test4:

holiday setting (13. April) stayed in between 13. April and 14. April (till the next day)
no manual reload or restart was executed!
I picked up the same ssh Check in the Icinga2Web-GUI and generated a not forced message
The message was send out incorrectly and only to ouser (which should get it only during holiday), while 14. April is not a holiday anymore

Test5:

no setting change (13. April is still marked as holiday)
active systemctl restart icinga2 at 14. April
I picked up the same ssh Check in the Icinga2Web-GUI and generated a not forced message
The message was send out incorrectly and only to ouser, while 14. April is not a holiday anymore
I repeated a second systemctl restart icinga2 at 14. April
I picked up the same ssh Check in the Icinga2Web-GUI and generated a not forced message
The message was send out again incorrectly and only to ouser, while 14. April is not a holiday anymore, its stocked at that point.
date on the VM is correctly synced with chronyd (NTP):

[15:56:07 INFRA root@monitoring-dev-1-1 conf.d]# date
Fri Apr 14 15:56:07 CEST 2023
[15:56:07 INFRA root@monitoring-dev-1-1 conf.d]#

Notification history:

My config:

timeperiods.conf:

object TimePeriod "bereitschaft-h" {
  display_name = "Bereitschaft-h"
  excludes = [ "feiertage-de-fix", "feiertage-de-dynamic" ]
  ranges = {
    "monday"    = "06:00-22:00"
    "tuesday"   = "06:00-22:00"
    "wednesday" = "06:00-22:00"
    "thursday"  = "06:00-22:00"
    "friday"    = "06:00-22:00"
  }
}

object TimePeriod "bereitschaft-o" {
  display_name = "Bereitschaft-o"
  includes = [ "feiertage-de-fix", "feiertage-de-dynamic" ]
  ranges = {
    "monday"    = "00:00-06:00,22:00-24:00"
    "tuesday"   = "00:00-06:00,22:00-24:00"
    "wednesday" = "00:00-06:00,22:00-24:00"
    "thursday"  = "00:00-06:00,22:00-24:00"
    "friday"    = "00:00-06:00,22:00-24:00"
    "saturday"  = "00:00-24:00"
    "sunday"    = "00:00-24:00"
  }
}


#####################
#Includes and Excludes#
#####################
object TimePeriod "feiertage-de-fix" {
  ranges = {
    "january 1" = "06:00-22:00"                 //new year's day
    "march 8" = "06:00-22:00"                   //international womens day
    "april 13" = "06:00-22:00"                  //pull 7855
    "may 1" = "06:00-22:00"                     //Tag der Arbeit
    "may 05" = "06:00-22:00"
    "october 03" = "06:00-22:00"                //Tag der deutschen Einheit
    "december 24" = "06:00-22:00"               //christmas holidays
    "december 25" = "06:00-22:00"               //christmas holidays
    "december 26" = "06:00-22:00"               //christmas holidays
    "december 31" = "06:00-22:00"               //day before new year
  }
}

object TimePeriod "feiertage-de-dynamic" {
  ranges = {
    "2022-04-15" = "06:00-22:00"
    "2022-04-18" = "06:00-22:00"
    "2022-05-26" = "06:00-22:00"
    "2022-06-06" = "06:00-22:00"
  }
}

users.conf:

object User "david" {
import "david"

  display_name = "david"
  enable_notifications = true
  groups = [ "bereitschaft-h" ]
  email = "[email protected]"
  period = "bereitschaft-h"
}

object User "ouser" {
import "ouser"

  display_name = "O User"
  enable_notifications = true
  groups = [ "bereitschaft-o" ]
  email = "[email protected]"
  period = "bereitschaft-o"
}

If you need further tests, please do not hesitate to ask. I will help as I can to pin point this bug.

Best regards
David

Al2Klimov · 2023-06-20T10:22:19Z

Updated OP with more relation to

ref/IP/44756

Al2Klimov · 2023-06-20T10:28:02Z

@davixd 💡 Please could you re-try your test cases with copied&pasted TimePeriods under different names and after installing my packages and restarting Icinga, so that the old TPs' states (state file) don’t affect them?

Al2Klimov · 2024-10-14T13:13:01Z

⚠️ Thing to consider

Cyclic references between objects should be forbidden #10189

Al2Klimov added this to the 2.13.0 milestone Feb 26, 2020

Al2Klimov self-assigned this Feb 26, 2020

Al2Klimov marked this pull request as ready for review February 26, 2020 16:41

Al2Klimov removed their assignment Feb 26, 2020

Al2Klimov marked this pull request as draft December 14, 2020 17:45

julianbrost removed this from the 2.13.0 milestone May 31, 2021

cla-bot bot added the cla/signed label Aug 4, 2021

Al2Klimov added the bug Something isn't working label Aug 10, 2021

Al2Klimov marked this pull request as ready for review August 10, 2021 19:04

Al2Klimov requested a review from yhabteab April 4, 2022 11:33

yhabteab force-pushed the bugfix/timeperiod-exclusions-7398 branch from 810df35 to cef46e5 Compare June 22, 2022 09:03

Al2Klimov removed the request for review from yhabteab January 31, 2023 10:11

Al2Klimov added the ref/IP label Feb 22, 2023

Al2Klimov added this to the 2.14.0 milestone Feb 22, 2023

Al2Klimov requested a review from julianbrost February 23, 2023 16:30

lippserd assigned Al2Klimov Mar 30, 2023

Al2Klimov added 3 commits April 11, 2023 15:32

Drop TimePeriod#FindNextTransition()

272e798

refs #7398

TimePeriod: consult GetIncludes() and GetExcludes() in IsInside(), no…

e93f632

…t UpdateRegion() refs #7398

Drop TimePeriod#Merge()

50cd84e

refs #7398

Al2Klimov force-pushed the bugfix/timeperiod-exclusions-7398 branch from cef46e5 to 50cd84e Compare April 11, 2023 13:32

Al2Klimov assigned davixd and unassigned Al2Klimov Apr 12, 2023

julianbrost removed this from the 2.14.0 milestone May 3, 2023

Al2Klimov unassigned davixd Jun 6, 2023

Al2Klimov assigned davixd Jul 13, 2023

Al2Klimov mentioned this pull request Aug 22, 2023

Timeperiod broken - not allowed segment for the next day are added #9781

Closed

yhabteab mentioned this pull request Jan 29, 2024

Fix broken TimePeriod/ScheduledDowntimes #9983

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TimePeriod: consult GetIncludes() and GetExcludes() in IsInside(), not UpdateRegion() #7855

TimePeriod: consult GetIncludes() and GetExcludes() in IsInside(), not UpdateRegion() #7855

Al2Klimov commented Feb 26, 2020 •

edited

Loading

Al2Klimov commented Aug 4, 2021

Al2Klimov commented Apr 4, 2022

davixd commented May 3, 2022 •

edited

Loading

yhabteab commented Jun 22, 2022 •

edited

Loading

yhabteab commented Jun 22, 2022

Al2Klimov commented Feb 22, 2023

julianbrost commented Feb 23, 2023

julianbrost commented Mar 10, 2023

Al2Klimov commented Mar 20, 2023

Al2Klimov commented Apr 11, 2023

Al2Klimov commented Apr 11, 2023

Al2Klimov commented Apr 11, 2023

davixd commented Apr 11, 2023

julianbrost commented Apr 12, 2023

Al2Klimov commented Apr 12, 2023

davixd commented Apr 12, 2023

Al2Klimov commented Apr 13, 2023

davixd commented Apr 13, 2023

Al2Klimov commented Apr 13, 2023

julianbrost commented Apr 13, 2023

julianbrost commented Apr 13, 2023

davixd commented Apr 13, 2023

Al2Klimov commented Apr 13, 2023

davixd commented Apr 13, 2023 •

edited

Loading

davixd commented Apr 14, 2023 •

edited

Loading

Al2Klimov commented Jun 20, 2023

Al2Klimov commented Jun 20, 2023

Al2Klimov commented Oct 14, 2024

TimePeriod: consult GetIncludes() and GetExcludes() in IsInside(), not UpdateRegion() #7855

Are you sure you want to change the base?

TimePeriod: consult GetIncludes() and GetExcludes() in IsInside(), not UpdateRegion() #7855

Conversation

Al2Klimov commented Feb 26, 2020 • edited Loading

Edit

The solution

The problem

Edit II

Al2Klimov commented Aug 4, 2021

Al2Klimov commented Apr 4, 2022

davixd commented May 3, 2022 • edited Loading

yhabteab commented Jun 22, 2022 • edited Loading

Before

After

yhabteab commented Jun 22, 2022

Al2Klimov commented Feb 22, 2023

julianbrost commented Feb 23, 2023

julianbrost commented Mar 10, 2023

Al2Klimov commented Mar 20, 2023

Al2Klimov commented Apr 11, 2023

Intermediate result

Al2Klimov commented Apr 11, 2023

Al2Klimov commented Apr 11, 2023

davixd commented Apr 11, 2023

julianbrost commented Apr 12, 2023

Al2Klimov commented Apr 12, 2023

davixd commented Apr 12, 2023

Al2Klimov commented Apr 13, 2023

davixd commented Apr 13, 2023

Al2Klimov commented Apr 13, 2023

julianbrost commented Apr 13, 2023

julianbrost commented Apr 13, 2023

davixd commented Apr 13, 2023

Al2Klimov commented Apr 13, 2023

davixd commented Apr 13, 2023 • edited Loading

davixd commented Apr 14, 2023 • edited Loading

Al2Klimov commented Jun 20, 2023

Al2Klimov commented Jun 20, 2023

Al2Klimov commented Oct 14, 2024

⚠️ Thing to consider

Al2Klimov commented Feb 26, 2020 •

edited

Loading

davixd commented May 3, 2022 •

edited

Loading

yhabteab commented Jun 22, 2022 •

edited

Loading

davixd commented Apr 13, 2023 •

edited

Loading

davixd commented Apr 14, 2023 •

edited

Loading