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

Fixed raritan_pdu_plugs.py #753

Closed
wants to merge 2 commits into from
Closed

Conversation

Woodstock68
Copy link
Contributor

Fixed state of powered off plugs.

Powered off plugs always had a critical state ignoring the discovered state and existing PDU plug state rules.

Fixed state of powered off plugs compared to discovered state or requested state rule.
Copy link

github-actions bot commented Aug 29, 2024

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@Woodstock68
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA or my organization already has a signed CLA.

@@ -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"])
state, state_info = data["state"]
Copy link
Contributor

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.

removed repeated yield lines
if state_info != required_state:
yield 2, "Expected: %s" % required_state
Copy link
Contributor

@logan-connolly logan-connolly Oct 9, 2024

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
Contributor

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 (!!).

CheckmkCI pushed a commit that referenced this pull request Oct 11, 2024
Currently, the check disregards the custom parameters set by the user,
always returning the state defined by the SNMP mapping. With this
change, the service now compares the current state with the required
state defined by the user in the check parameters. If there is a
mismatch, a "CRIT" result will be yielded, comparing the actual and
expected plug state. Otherwise, an "OK" result will be yielded with the
plug state.

If these parameters are not explicitly set for a service, the service
will compare the current state with the state set during service
discovery.

CMK-18917

Closes: #753
Co-authored-by: Logan Connolly <[email protected]>
Change-Id: I778f8d4aaf73ee9dad5e684ade88bf7141142b5f
CheckmkCI pushed a commit that referenced this pull request Oct 11, 2024
Currently, the check disregards the custom parameters set by the user,
always returning the state defined by the SNMP mapping. With this
change, the service now compares the current state with the required
state defined by the user in the check parameters. If there is a
mismatch, a "CRIT" result will be yielded, comparing the actual and
expected plug state. Otherwise, an "OK" result will be yielded with the
plug state.

If these parameters are not explicitly set for a service, the service
will compare the current state with the state set during service
discovery.

CMK-18917

Closes: #753
Co-authored-by: Logan Connolly <[email protected]>
Change-Id: I778f8d4aaf73ee9dad5e684ade88bf7141142b5f
@CheckmkCI CheckmkCI closed this in 02a2c19 Oct 11, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Oct 11, 2024
@mo-ki
Copy link
Contributor

mo-ki commented Oct 14, 2024

Hi @Woodstock68,
Thank you very much for your contribution! As you can see, @logan-connolly merged your change :-)

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

Successfully merging this pull request may close these issues.

4 participants