Skip to content
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

IcingaDB Check: Multiple Responsible Instances #10188

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

oxzi
Copy link
Member

@oxzi oxzi commented Oct 11, 2024

By design, only one Icinga 2 instance should be responsible in the HA context. If this promise is broken, the Icinga 2 IcingaDB check should report it.

The code did not check for invalid data in icingadb:telemetry:heartbeat. With this change, it will go CRITICAL with a descriptive message and report the actual number of icingadb_responsible_instances in the performance data.


ref/IP/55850

@oxzi oxzi added area/icingadb New backend ref/IP labels Oct 11, 2024
@oxzi oxzi requested review from julianbrost and lippserd October 11, 2024 15:22
@cla-bot cla-bot bot added the cla/signed label Oct 11, 2024
Al2Klimov
Al2Klimov previously approved these changes Nov 14, 2024
lib/icingadb/icingadbchecktask.cpp Outdated Show resolved Hide resolved
@oxzi oxzi force-pushed the icingadb-heartbeat-both-responsible branch from 0e45b0c to b4e1066 Compare November 15, 2024 10:37
@oxzi oxzi requested a review from Al2Klimov November 15, 2024 10:37
@Al2Klimov Al2Klimov added this to the 2.15.0 milestone Nov 15, 2024
@yhabteab
Copy link
Member

yhabteab commented Nov 15, 2024

I don't think producing a check output counts as logging, thus you might want to rephrase your commit message and PR title.

@oxzi oxzi force-pushed the icingadb-heartbeat-both-responsible branch from b4e1066 to f57d074 Compare November 15, 2024 11:39
@oxzi oxzi changed the title IcingaDB: Log if both instances are responsible IcingaDB Check: Multiple Responsible Instances Nov 15, 2024
@oxzi
Copy link
Member Author

oxzi commented Nov 15, 2024

@yhabteab: You are totally right. The old description was in deed misleading and a bit flippant. Please take another look.

@oxzi oxzi requested a review from yhabteab November 15, 2024 11:41
By design, only one Icinga 2 instance should be responsible in the HA
context. If this promise is broken, the Icinga 2 IcingaDB check should
report it.

The code did not check for invalid data in icingadb:telemetry:heartbeat.
With this change, it will go CRITICAL with a descriptive message and
report the actual number of icingadb_responsible_instances in the
performance data.
@oxzi oxzi force-pushed the icingadb-heartbeat-both-responsible branch from f57d074 to 0bbe7a9 Compare November 15, 2024 11:56
@oxzi
Copy link
Member Author

oxzi commented Nov 15, 2024

I have just rebased this branch on the master branch. There is no change in this PR's diff.

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

Successfully merging this pull request may close these issues.

3 participants