Checkable: Don't recalculate next_check for remotely generated cr

[yhabteab]

Currently, when processing a CheckResult, it will first trigger an OnNextCheckChanged event, which is sent to all connected endpoints. Then, when Checkable#ProcessCheckResult() returns, an OnNewCheckResult event is fired, which is of course also sent to all connected endpoints.

Next, the other endpoints receive the event::SetNextCheck cluster event followed by event::CheckResultand invoke
checkable#SetNextCheck() and Checkable#ProcessCheckResult() with the newly received check. So they also try to recalculate the next check themselves and invalidate the previously received next check timestamp from the source endpoint. Since each endpoint calculates it relative to time#now (recomputing the next check relative to the last check/cr#schedule_end does not work for active checks either, as each endpoint randomly initialises its own scheduling offset), the recalculated next check will always differ by a split second/millisecond on each of them. As a consequence, two Icinga DB HA instances will generate two different checksums for the same state and causes the state histories tables to be fully resynchronised after a takeover/Icinga 2 reload.

Before

~ diff <(curl -sSku root:icinga 'https://localhost:5666/v1/objects/hosts/icinga2?pretty=1') <(curl -sSku root:icinga 'https://localhost:5665/v1/objects/hosts/icinga2?pretty=1')
94,95c94,95
<                 "next_check": 1709202335.3899999,
<                 "next_update": 1709202350.4348836,
---
>                 "next_check": 1709202334.593121,
>                 "next_update": 1709202349.6380048,
99,100c99,100
<                 "package": "_cluster",
<                 "paused": false,
---
>                 "package": "_etc",
>                 "paused": true,
110c110
<                     "path": "/Users/yhabteab/Workspace/icinga2-2/prefix/var/lib/icinga2/api/zones/master/_etc/hosts.conf"
---
>                     "path": "/Users/yhabteab/Workspace/icinga2/prefix/etc/icinga2/zones.d/master/hosts.conf"

After

~ diff <(curl -sSku root:icinga 'https://localhost:5666/v1/objects/hosts/icinga2?pretty=1') <(curl -sSku root:icinga 'https://localhost:5665/v1/objects/hosts/icinga2?pretty=1')
99,100c99,100
<                 "package": "_cluster",
<                 "paused": false,
---
>                 "package": "_etc",
>                 "paused": true,
110c110
<                     "path": "/Users/yhabteab/Workspace/icinga2-2/prefix/var/lib/icinga2/api/zones/master/_etc/hosts.conf"
---
>                     "path": "/Users/yhabteab/Workspace/icinga2/prefix/etc/icinga2/zones.d/master/hosts.conf"

[Al2Klimov]

Does this work with CRs from command endpoints?

[yhabteab]

Does this work with CRs from command endpoints?

How do these CRs differ from any other remote generated ones? I don't see why they shouldn't work.

[Al2Klimov]

A command endpoint is NOT in the zone of the checkable it checks. So it would send a 'next check changed' message along with the CR, but the master would ignore it: Discarding 'next check changed' message for checkable '...' from '...': Unauthorized access And the master wouldn't update the next check by itself as !origin is false. The CR's origin is the command endpoint.

[julianbrost]

A command endpoint is NOT in the zone of the checkable it checks.

It may or may not be in that zone. You can also use command_endpoint to pin the check execution to a particular node within a HA zone.

[yhabteab]

A command endpoint is NOT in the zone of the checkable it checks.

It may or may not be in that zone. You can also use command_endpoint to pin the check execution to a particular node within a HA zone.

Why should the master discard the updates? If it generally does not accept updates from that zone, how do you think the expected CR will be processed then? When that particular endpoint is responsible for executing the checks and generating CRs of a given checkable, under no circumstances will the master reject these updates.

[Al2Klimov]

• ClusterEvents::CheckResultAPIHandler() allows checkable zone, its parents and the command endpoint:

  icinga2/lib/icinga/clusterevents.cpp

  Line 171 in 9e31b8b

  if (origin->FromZone && !origin->FromZone->CanAccessObject(checkable) && endpoint != checkable->GetCommandEndpoint()) {

• ClusterEvents::NextCheckChangedAPIHandler() allows checkable zone and its parents:

  icinga2/lib/icinga/clusterevents.cpp

  Line 236 in 9e31b8b

  if (origin->FromZone && !origin->FromZone->CanAccessObject(checkable)) {

[yhabteab]

• ClusterEvents::CheckResultAPIHandler() allows checkable zone, its parents and the command endpoint:

But that receiver doesn't pass the origin to ProcessCheckResult() if the message is a result of a command endpoint.

icinga2/lib/icinga/clusterevents.cpp

Lines 178 to 179 in 9e31b8b

	if (!checkable->IsPaused() && Zone::GetLocalZone() == checkable->GetZone() && endpoint == checkable->GetCommandEndpoint())
	checkable->ProcessCheckResult(cr);

[Al2Klimov]

Please test it with out-of-zone command endpoint, just to be sure.

[yhabteab]

Master <-> satellite:

~/Workspace/icinga2 (next-check-cluster-sync-issue ✗) diff <(curl -sSku root:icinga 'https://localhost:5667/v1/objects/services/satellite!cluster?pretty=1') <(curl -sSku root:icinga 'https://localhost:5666/v1/objects/services/satellite!cluster?pretty=1')
223c223
<                 "package": "_etc",
---
>                 "package": "_cluster",
234c234
<                  "path": "/Users/yhabteab/Workspace/icinga2/prefix/etc/icinga2/zones.d/satellite/host.conf"
---
>                  "path": "/Users/yhabteab/ClionProjects/icinga2/prefix/var/lib/icinga2/api/zones/satellite/_etc/host.conf"

[Al2Klimov]

Command Endpoints aren't problematic!

icinga2/lib/icinga/clusterevents.cpp

Lines 178 to 181 in bca1a84

	if (!checkable->IsPaused() && Zone::GetLocalZone() == checkable->GetZone() && endpoint == checkable->GetCommandEndpoint())
	checkable->ProcessCheckResult(cr);
	else
	checkable->ProcessCheckResult(cr, origin);

[julianbrost]

From the current PR description:

As a consequence, two Icinga DB HA instances will generate two different checksums for the same state and causes the state histories to be fully resynchronised after a takeover/Icinga 2 reload.

This should not be related to the host/service history and there's no full resynchronization of that either. I think you mean the host/service state tables.