parse duration once in command and handle exceptions

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 = {

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,
                 )

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:

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