Skip to content

Commit

Permalink
NAS-130890 / 25.04 / Fix UPS events (#14391)
Browse files Browse the repository at this point in the history
* Modify alert.oneshot_delete to support alert name or list of alert names

* Switch ups.dismiss_alerts to use alert.oneshot_delete with a list

* For test robustness stagger onbatt and lowbattery
  • Loading branch information
bmeagherix authored Sep 3, 2024
1 parent 27a325d commit cfee630
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 22 deletions.
48 changes: 30 additions & 18 deletions src/middlewared/middlewared/plugins/alert.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
ProThreadedAlertService,
)
from middlewared.alert.base import UnavailableException, AlertService as _AlertService
from middlewared.schema import accepts, Any, Bool, Datetime, Dict, Int, List, Patch, returns, Ref, Str
from middlewared.schema import accepts, Any, Bool, Datetime, Dict, Int, List, OROperator, Patch, returns, Ref, Str
from middlewared.service import (
ConfigService, CRUDService, Service, ValidationErrors,
job, periodic, private,
Expand Down Expand Up @@ -932,34 +932,46 @@ async def oneshot_create(self, job, klass, args):
await self.middleware.call("alert.send_alerts")

@private
@accepts(Str("klass"), Any("query", null=True, default=None))
@accepts(
OROperator(
Str("klass"),
List('klass', items=[Str('klassname')], default=None),
),
Any("query", null=True, default=None))
@job(lock="process_alerts", transient=True)
async def oneshot_delete(self, job, klass, query):
"""
Deletes one-shot alerts of specified `klass`, passing `query` to `klass.delete` method.
Deletes one-shot alerts of specified `klass` or klasses, passing `query`
to `klass.delete` method.
It's not an error if no alerts matching delete `query` exist.
:param klass: one-shot alert class name (without the `AlertClass` suffix).
:param klass: either one-shot alert class name (without the `AlertClass` suffix), or list thereof.
:param query: `query` that will be passed to `klass.delete` method.
"""

try:
klass = AlertClass.class_by_name[klass]
except KeyError:
raise CallError(f"Invalid alert source: {klass!r}")

if not issubclass(klass, OneShotAlertClass):
raise CallError(f"Alert class {klass!r} is not a one-shot alert source")
if isinstance(klass, list):
klasses = klass
else:
klasses = [klass]

related_alerts, unrelated_alerts = bisect(lambda a: (a.node, a.klass) == (self.node, klass),
self.alerts)
left_alerts = await klass(self.middleware).delete(related_alerts, query)
deleted = False
for deleted_alert in related_alerts:
if deleted_alert not in left_alerts:
self.alerts.remove(deleted_alert)
deleted = True
for klassname in klasses:
try:
klass = AlertClass.class_by_name[klassname]
except KeyError:
raise CallError(f"Invalid alert source: {klassname!r}")

if not issubclass(klass, OneShotAlertClass):
raise CallError(f"Alert class {klassname!r} is not a one-shot alert source")

related_alerts, unrelated_alerts = bisect(lambda a: (a.node, a.klass) == (self.node, klass),
self.alerts)
left_alerts = await klass(self.middleware).delete(related_alerts, query)
for deleted_alert in related_alerts:
if deleted_alert not in left_alerts:
self.alerts.remove(deleted_alert)
deleted = True

if deleted:
# We need to flush alerts to the database immediately after deleting oneshot alerts.
Expand Down
4 changes: 2 additions & 2 deletions src/middlewared/middlewared/plugins/ups.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,8 @@ async def alerts_mapping(self):

@private
async def dismiss_alerts(self):
for alert in (await self.alerts_mapping()).values():
await self.middleware.call('alert.oneshot_delete', alert)
alerts = list((await self.alerts_mapping()).values())
await self.middleware.call('alert.oneshot_delete', alerts)

@private
@accepts(
Expand Down
5 changes: 3 additions & 2 deletions tests/api2/test_530_ups.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,10 +226,11 @@ def test__ups_onbatt_to_online(ups_running, dummy_ups_driver_configured):

def test__ups_online_to_onbatt_lowbattery(ups_running, dummy_ups_driver_configured):
assert 'UPSOnBattery' not in [alert['klass'] for alert in call('alert.list')]
write_fake_data({'battery.charge': 10, 'ups.status': 'OB LB'})
write_fake_data({'battery.charge': 90, 'ups.status': 'OB'})
alert = wait_for_alert('UPSOnBattery')
assert alert
assert 'battery.charge: 10' in alert['formatted'], alert
assert 'battery.charge: 90' in alert['formatted'], alert
write_fake_data({'battery.charge': 10, 'ups.status': 'OB LB'})
alert = wait_for_alert('UPSBatteryLow')
assert alert
assert 'battery.charge: 10' in alert['formatted'], alert
Expand Down

0 comments on commit cfee630

Please sign in to comment.