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.

Sign up for GitHub

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

Fixed raritan_pdu_plugs.py #753

Closed
wants to merge 2 commits into from
Closed

Fixed raritan_pdu_plugs.py #753

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d44e421
Fixed raritan_pdu_plugs.py
Woodstock68 Aug 29, 2024
66b67be
Update raritan_pdu_plugs.py
Woodstock68 Sep 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions cmk/base/legacy_checks/raritan_pdu_plugs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@ def check_raritan_pdu_plugs(item, params, parsed):
if data.get("outlet_name"):
yield 0, data["outlet_name"]

required_state = params.get("required_state", params["discovered_state"])
discovered_state = params["discovered_state"]
state, state_info = data["state"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

state is now completely unused; even if the user did not configure anything. Is that intended? It does not seem right to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

state is now completely unused; even if the user did not configure anything. Is that intended? It does not seem right to me.

required_state (if there is a "PDU Plug state" rule for this plug) and discovered_state are both stored as text ("on"/"off").
This is already implemented and it is what I have do deal with. Therefore I have to compare to that string using state_info and not state.

But since state and state_info have a one-to-one mapping (0,"on" | 2,"off" | 3, "unknown") it really makes no difference.

In version 1.6 a plug got a critical state when its state was different from the one from service discovery.
This could be overruled by a "State of PDU Plugs" rule.

My aim was to restore this behavior.

yield state, "Status: %s" % state_info

required_state = params.get("required_state", params["discovered_state"])
if state_info != required_state:
yield 2, "Expected: %s" % required_state
Copy link
Member

@logan-connolly logan-connolly Oct 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mo-ki should we include the "CRIT -" and "OK -" in the summary? This should be picked up by the respective state code, right? and therefore redundant?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Same goes for the (!!).

yield 2, "CRIT - Status: %s (discovered: %s, required: %s) (!!)" % (state_info, discovered_state, required_state)
else:
yield 0, "OK - Status: %s" % state_info


check_info["raritan_pdu_plugs"] = LegacyCheckDefinition(
Expand Down