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

introduce further handling and notifiers for ALARM status #2658

Merged
merged 19 commits into from
Oct 21, 2024

Conversation

desertwitch
Copy link
Contributor

@desertwitch desertwitch commented Oct 15, 2024

As outlined in #2657, dstate.c sets ALARM as a common denominator status when encountering ups.alarm variables reported through the different device-/manufacturer-specific implementations within the UPS drivers. Previously a user had to either rely on audible alarms by the UPS or (in an unattended setting) periodically check the UPS status and variables manually to be informed of potentially severe UPS device alarm states/conditions.

This pull request introduces ALARM status handling in upsmon as the remaining piece of the puzzle to effectively act upon UPS device alarms as raised by the UPS drivers. This includes notifiers ALARM and NOTALARM for when the status is encountered and dissipates, respectively, allowing the user to now configure notifications and tasks e.g. through upssched.

By default upsmon will treat any encountered ALARM status as (potentially) severe enough to warrant an UPS being promoted to critical in a no-communication situation. This can, however, be unwarranted due to there being no common standard for what constitutes an alarm. In fact mundane alarm states were previously observed with UPS devices in HE/ECO mode, which could lead to unwarranted shutdown situations on minor connection hiccups. For this reason a 0|1 setting ALARMCRITICAL was introduced into upsmon.conf, for such users to be able to prevent upsmon from treating the ALARM status as overly severe when it's not.

As for the default behaviour (of considering such an UPS as volatile) erring on the side of caution seemed to be reasonable, as e.g. an alarm for some failing circuitry and the affected UPS later dropping offline should be concern enough for the UPS to be considered immediately critical and the system pre-emptively being shutdown e.g. by reduction/depletion of MINSUPPLIES.

fixes #2657

@desertwitch
Copy link
Contributor Author

desertwitch commented Oct 15, 2024

For further improvements down the road, perhaps we should also consider tracking ups.alarm variables and produce logs for newly registered alarms and later cleared again alarms respectively (although this would be more suitable in direction of dstate.c and/or the drivers, rather than within upsmon, but it would be nice to have a unified/common denominator solution in spite of different driver-side implementations... similar to as done with the ALARM status, so perhaps in dstate.c as well?). This would complement the upsmon side of handling things (as introduced by this PR) well, where the status is handled but not the individual alarm messages/variables. But this is, I think, outside the scope of this upsmon-specific PR.

@jimklimov
Copy link
Member

Thanks, I hope to have some time to look at it this week. So far it started very intensively.

@desertwitch desertwitch marked this pull request as draft October 16, 2024 06:43
@desertwitch desertwitch changed the title upsmon: introduce handling and notifiers for ALARM status introduce further handling and notifiers for ALARM status Oct 16, 2024
@desertwitch desertwitch marked this pull request as ready for review October 16, 2024 07:15
@desertwitch
Copy link
Contributor Author

Thanks Jim, it's no stress at all, I've taken the liberty to broaden the scope here a bit where it made sense. 😉

@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@AppVeyorBot
Copy link

@jimklimov jimklimov merged commit 87403c6 into networkupstools:master Oct 21, 2024
27 of 31 checks passed
@desertwitch desertwitch deleted the upsmon-handle-alarm branch October 21, 2024 18:40
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.

upsmon: ALARM status should be handled/handleable
3 participants