From 1e006cb66f815ad267f7514ffc7f9e56fc75606e Mon Sep 17 00:00:00 2001 From: HgO Date: Tue, 12 Jul 2022 00:27:17 +0200 Subject: [PATCH] parse duration once in command and handle exceptions --- matrix_alertbot/alertmanager.py | 8 +- matrix_alertbot/command.py | 16 +++- tests/test_alertmanager.py | 33 ++----- tests/test_command.py | 155 +++++++++++++++++++++++++++++--- 4 files changed, 171 insertions(+), 41 deletions(-) diff --git a/matrix_alertbot/alertmanager.py b/matrix_alertbot/alertmanager.py index 5bf53f8..eb4cf2f 100644 --- a/matrix_alertbot/alertmanager.py +++ b/matrix_alertbot/alertmanager.py @@ -4,7 +4,6 @@ from datetime import datetime, timedelta from typing import Dict, List import aiohttp -import pytimeparse2 from aiohttp import ClientError from aiohttp_prometheus_exporter.trace import PrometheusTraceConfig from diskcache import Cache @@ -44,7 +43,7 @@ class AlertmanagerClient: async def create_silence( self, fingerprint: str, - duration: str, + seconds: int, user: str, matchers: List[AlertMatcher], ) -> str: @@ -56,9 +55,10 @@ class AlertmanagerClient: {"name": label, "value": value, "isRegex": False, "isEqual": True} for label, value in alert["labels"].items() ] + start_time = datetime.now() - duration_seconds = pytimeparse2.parse(duration) - duration_delta = timedelta(seconds=duration_seconds) + + duration_delta = timedelta(seconds=seconds) end_time = start_time + duration_delta silence = { diff --git a/matrix_alertbot/command.py b/matrix_alertbot/command.py index 058b8db..6f7214c 100644 --- a/matrix_alertbot/command.py +++ b/matrix_alertbot/command.py @@ -1,6 +1,7 @@ import logging from typing import List +import pytimeparse2 from diskcache import Cache from nio import AsyncClient, MatrixRoom @@ -8,8 +9,8 @@ from matrix_alertbot.alertmanager import AlertmanagerClient from matrix_alertbot.chat_functions import send_text_to_room from matrix_alertbot.config import Config from matrix_alertbot.errors import ( - AlertNotFoundError, AlertmanagerError, + AlertNotFoundError, SilenceNotFoundError, ) from matrix_alertbot.matcher import AlertMatcher, AlertRegexMatcher @@ -90,6 +91,16 @@ class Command: f"Receiving a command to create a silence for a duration of {duration}" ) + duration_seconds = pytimeparse2.parse(duration) + if duration_seconds is None: + logger.error(f"Unable to create silence: Invalid duration '{duration}'") + await send_text_to_room( + self.client, + self.room.room_id, + f"I tried really hard, but I can't convert this duration to a number of seconds: {duration}.", + ) + return + logger.debug(f"Read alert fingerprints for event {self.event_id} from cache") if self.event_id not in self.cache: @@ -105,10 +116,11 @@ class Command: logger.debug( f"Create silence for alert with fingerprint {alert_fingerprint} for a duration of {duration}" ) + try: await self.alertmanager.create_silence( alert_fingerprint, - duration, + duration_seconds, self.room.user_name(self.sender), matchers, ) diff --git a/tests/test_alertmanager.py b/tests/test_alertmanager.py index cae48f2..bca89e1 100644 --- a/tests/test_alertmanager.py +++ b/tests/test_alertmanager.py @@ -222,29 +222,12 @@ class AlertmanagerClientTestCase(unittest.IsolatedAsyncioTestCase): ) async with aiotools.closing_async(alertmanager): silence = await alertmanager.create_silence( - "fingerprint1", "1d", "user", [] + "fingerprint1", 86400, "user", [] ) self.assertEqual("silence1", silence) fake_timedelta.assert_called_once_with(seconds=86400) - @patch("matrix_alertbot.alertmanager.timedelta", side_effect=FakeTimeDelta) - async def test_create_silence_with_complex_duration( - self, fake_timedelta: Mock - ) -> None: - async with FakeAlertmanagerServer() as fake_alertmanager_server: - port = fake_alertmanager_server.port - alertmanager = AlertmanagerClient( - f"http://localhost:{port}", self.fake_cache - ) - async with aiotools.closing_async(alertmanager): - silence = await alertmanager.create_silence( - "fingerprint1", "1w 3d", "user", [] - ) - - self.assertEqual("silence1", silence) - fake_timedelta.assert_called_once_with(seconds=864000) - @patch("matrix_alertbot.alertmanager.timedelta", side_effect=FakeTimeDelta) async def test_create_silence_with_matchers(self, fake_timedelta: Mock) -> None: matchers = [AlertMatcher(label="alertname", value="alert1")] @@ -257,7 +240,7 @@ class AlertmanagerClientTestCase(unittest.IsolatedAsyncioTestCase): async with aiotools.closing_async(alertmanager): silence = await alertmanager.create_silence( "fingerprint1", - "1d", + 86400, "user", matchers, ) @@ -281,7 +264,7 @@ class AlertmanagerClientTestCase(unittest.IsolatedAsyncioTestCase): async with aiotools.closing_async(alertmanager): silence = await alertmanager.create_silence( "fingerprint1", - "1d", + 86400, "user", matchers, ) @@ -304,7 +287,7 @@ class AlertmanagerClientTestCase(unittest.IsolatedAsyncioTestCase): with self.assertRaises(AlertMismatchError): await alertmanager.create_silence( "fingerprint1", - "1d", + 86400, "user", matchers, ) @@ -321,7 +304,7 @@ class AlertmanagerClientTestCase(unittest.IsolatedAsyncioTestCase): with self.assertRaises(AlertMismatchError): await alertmanager.create_silence( "fingerprint1", - "1d", + 86400, "user", matchers, ) @@ -340,7 +323,7 @@ class AlertmanagerClientTestCase(unittest.IsolatedAsyncioTestCase): with self.assertRaises(AlertMismatchError): await alertmanager.create_silence( "fingerprint1", - "1d", + 86400, "user", matchers, ) @@ -353,7 +336,7 @@ class AlertmanagerClientTestCase(unittest.IsolatedAsyncioTestCase): ) async with aiotools.closing_async(alertmanager): with self.assertRaises(AlertNotFoundError): - await alertmanager.create_silence("fingerprint1", "1d", "user", []) + await alertmanager.create_silence("fingerprint1", 86400, "user", []) async def test_create_silence_raise_alertmanager_error(self) -> None: async with FakeAlertmanagerServerWithErrorCreateSilence() as fake_alertmanager_server: @@ -365,7 +348,7 @@ class AlertmanagerClientTestCase(unittest.IsolatedAsyncioTestCase): await alertmanager.get_alert("fingerprint1") with self.assertRaises(AlertmanagerServerError): - await alertmanager.create_silence("fingerprint1", "1d", "user", []) + await alertmanager.create_silence("fingerprint1", 86400, "user", []) async def test_delete_silences_without_matchers(self) -> None: async with FakeAlertmanagerServer() as fake_alertmanager_server: diff --git a/tests/test_command.py b/tests/test_command.py index 8992481..b16cc83 100644 --- a/tests/test_command.py +++ b/tests/test_command.py @@ -8,7 +8,11 @@ from diskcache import Cache import matrix_alertbot.callback from matrix_alertbot.alertmanager import AlertmanagerClient from matrix_alertbot.command import Command -from matrix_alertbot.errors import AlertmanagerError +from matrix_alertbot.errors import ( + AlertmanagerError, + AlertNotFoundError, + SilenceNotFoundError, +) from matrix_alertbot.matcher import AlertMatcher, AlertRegexMatcher from tests.utils import make_awaitable @@ -22,6 +26,14 @@ async def create_silence_raise_alertmanager_error( return "silence1" +async def create_silence_raise_alert_not_found_error( + fingerprint: str, duration: str, user: str, matchers: List[AlertMatcher] +) -> str: + if fingerprint == "fingerprint1": + raise AlertNotFoundError + return "silence1" + + async def delete_silence_raise_alertmanager_error( fingerprint: str, matchers: List[AlertMatcher] ) -> List[str]: @@ -30,6 +42,14 @@ async def delete_silence_raise_alertmanager_error( return ["silence1"] +async def delete_silence_raise_silence_not_found_error( + fingerprint: str, matchers: List[AlertMatcher] +) -> List[str]: + if fingerprint == "fingerprint1": + raise SilenceNotFoundError + return ["silence1"] + + class CommandTestCase(unittest.IsolatedAsyncioTestCase): def setUp(self) -> None: # Create a Command object and give it some Mock'd objects to use @@ -165,7 +185,7 @@ class CommandTestCase(unittest.IsolatedAsyncioTestCase): # Check that we attempted to create silences self.fake_alertmanager.create_silence.assert_has_calls( [ - call(fingerprint, "1d", self.fake_sender, []) + call(fingerprint, 86400, self.fake_sender, []) for fingerprint in self.fake_fingerprints ] ) @@ -203,7 +223,7 @@ class CommandTestCase(unittest.IsolatedAsyncioTestCase): [ call( fingerprint, - "1d", + 86400, self.fake_sender, matchers, ) @@ -228,7 +248,7 @@ class CommandTestCase(unittest.IsolatedAsyncioTestCase): self.fake_cache, self.fake_alertmanager, self.fake_config, - "ack 1w 2d", + "ack 1w 3d", self.fake_room, self.fake_sender, self.fake_event_id, @@ -238,14 +258,14 @@ class CommandTestCase(unittest.IsolatedAsyncioTestCase): # Check that we attempted to create silences self.fake_alertmanager.create_silence.assert_has_calls( [ - call(fingerprint, "1w 2d", self.fake_sender, []) + call(fingerprint, 864000, self.fake_sender, []) for fingerprint in self.fake_fingerprints ] ) fake_send_text_to_room.assert_called_once_with( self.fake_client, self.fake_room.room_id, - "Created 2 silences with a duration of 1w 2d.", + "Created 2 silences with a duration of 1w 3d.", ) @patch.object(matrix_alertbot.command, "send_text_to_room") @@ -264,7 +284,7 @@ class CommandTestCase(unittest.IsolatedAsyncioTestCase): self.fake_cache, self.fake_alertmanager, self.fake_config, - "ack 1w 2d alertname=alert1 severity=critical", + "ack 1w 3d alertname=alert1 severity=critical", self.fake_room, self.fake_sender, self.fake_event_id, @@ -276,7 +296,7 @@ class CommandTestCase(unittest.IsolatedAsyncioTestCase): [ call( fingerprint, - "1w 2d", + 864000, self.fake_sender, matchers, ) @@ -286,7 +306,7 @@ class CommandTestCase(unittest.IsolatedAsyncioTestCase): fake_send_text_to_room.assert_called_once_with( self.fake_client, self.fake_room.room_id, - "Created 2 silences with a duration of 1w 2d.", + "Created 2 silences with a duration of 1w 3d.", ) @patch.object(matrix_alertbot.command, "send_text_to_room") @@ -315,7 +335,7 @@ class CommandTestCase(unittest.IsolatedAsyncioTestCase): # Check that we attempted to create silences self.fake_alertmanager.create_silence.assert_has_calls( [ - call(fingerprint, "1d", self.fake_sender, []) + call(fingerprint, 86400, self.fake_sender, []) for fingerprint in self.fake_fingerprints ] ) @@ -325,6 +345,79 @@ class CommandTestCase(unittest.IsolatedAsyncioTestCase): "Created 1 silences with a duration of 1d.", ) + @patch.object(matrix_alertbot.command, "send_text_to_room") + async def test_ack_raise_alert_not_found_error( + self, fake_send_text_to_room: Mock + ) -> None: + """Tests the callback for InviteMemberEvents""" + # Tests that the bot attempts to join a room after being invited to it + + command = Command( + self.fake_client, + self.fake_cache, + self.fake_alertmanager, + self.fake_config, + "ack", + self.fake_room, + self.fake_sender, + self.fake_event_id, + ) + + self.fake_alertmanager.create_silence.side_effect = ( + create_silence_raise_alert_not_found_error + ) + await command._ack() + + # Check that we attempted to create silences + self.fake_alertmanager.create_silence.assert_has_calls( + [ + call(fingerprint, 86400, self.fake_sender, []) + for fingerprint in self.fake_fingerprints + ] + ) + fake_send_text_to_room.assert_has_calls( + [ + call( + self.fake_client, + self.fake_room.room_id, + "Sorry, I couldn't find 1 alerts, therefore I couldn't create their silence.", + ), + call( + self.fake_client, + self.fake_room.room_id, + "Created 1 silences with a duration of 1d.", + ), + ] + ) + + @patch.object(matrix_alertbot.command, "send_text_to_room") + async def test_ack_with_invalid_duration( + self, fake_send_text_to_room: Mock + ) -> None: + """Tests the callback for InviteMemberEvents""" + # Tests that the bot attempts to join a room after being invited to it + + command = Command( + self.fake_client, + self.fake_cache, + self.fake_alertmanager, + self.fake_config, + "ack duration", + self.fake_room, + self.fake_sender, + self.fake_event_id, + ) + + await command._ack() + + # Check that we attempted to create silences + self.fake_alertmanager.create_silence.assert_not_called() + fake_send_text_to_room.assert_called_once_with( + self.fake_client, + self.fake_room.room_id, + "I tried really hard, but I can't convert this duration to a number of seconds: duration.", + ) + @patch.object(matrix_alertbot.command, "send_text_to_room") async def test_ack_with_event_not_found_in_cache( self, fake_send_text_to_room: Mock @@ -437,6 +530,48 @@ class CommandTestCase(unittest.IsolatedAsyncioTestCase): self.fake_client, self.fake_room.room_id, "Removed 1 silences." ) + @patch.object(matrix_alertbot.command, "send_text_to_room") + async def test_unack_raise_silence_not_found_error( + self, fake_send_text_to_room: Mock + ) -> None: + """Tests the callback for InviteMemberEvents""" + # Tests that the bot attempts to join a room after being invited to it + + command = Command( + self.fake_client, + self.fake_cache, + self.fake_alertmanager, + self.fake_config, + "unack", + self.fake_room, + self.fake_sender, + self.fake_event_id, + ) + + self.fake_alertmanager.delete_silences.side_effect = ( + delete_silence_raise_silence_not_found_error + ) + await command._unack() + + # Check that we attempted to create silences + self.fake_alertmanager.delete_silences.assert_has_calls( + [call(fingerprint, []) for fingerprint in self.fake_fingerprints] + ) + fake_send_text_to_room.assert_has_calls( + [ + call( + self.fake_client, + self.fake_room.room_id, + "Sorry, I couldn't find 1 alerts, therefore I couldn't remove their silences.", + ), + call( + self.fake_client, + self.fake_room.room_id, + "Removed 1 silences.", + ), + ] + ) + @patch.object(matrix_alertbot.command, "send_text_to_room") async def test_unack_with_event_not_found_in_cache( self, fake_send_text_to_room: Mock