Repository: MatterMiners/tardis

Issue:
Auditor plugin crashes when drone does not reach AvailableState
In some cases, it can happen that a drone is started but never reaches the `AvailableState`. However, during clean-up, it reaches the `DownState`. This case is currently not handled correctly in the auditor plugin, see

https://github.com/MatterMiners/tardis/blob/2bf9dcbeeeb548da01161a26c83d0809ecd165d6/tardis/plugins/auditor.py#L78-L88

A new record is created when the drone reaches `AvailableState`. The record is then updated with the stop-time once the drone reaches `DownState`. If the drone never reached the `AvailableState`, there is also no record to be updated. In this case, AUDITOR returns a HTTP error `400 BAD REQUEST (The server cannot or will not process the request due to something that is perceived to be a client error).

```
Jul 13 12:20:02 monopol.bfg.privat docker-COBalD-Tardis-atlhei[3808458]: cobald.runtime.tardis.plugins.auditor: 2023-07-13 10:20:02 Drone: {'site_name': 'NEMO', 'machine_type': 'tardis_c40m100', 'obs_machine_meta_data_translation_mapping': {'Cores': 1, 'Memory': 1000, 'Disk': 1000}, 'remote_resource_uuid': 16996522, 'c
reated': datetime.datetime(2023, 7, 12, 20, 4, 53, 130170), 'updated': datetime.datetime(2023, 7, 13, 10, 20, 2, 340694), 'drone_uuid': 'nemo-f25919f1d0', 'resource_status': <ResourceStatus.Deleted: 4>} has changed state to DownState
Jul 13 12:20:02 monopol.bfg.privat docker-COBalD-Tardis-atlhei[3808458]: cobald.runtime.runner.asyncio: 2023-07-13 10:20:02 runner aborted: <cobald.daemon.runners.asyncio_runner.AsyncioRunner object at 0x7f17fab74040>
Jul 13 12:20:02 monopol.bfg.privat docker-COBalD-Tardis-atlhei[3808458]: Traceback (most recent call last):
Jul 13 12:20:02 monopol.bfg.privat docker-COBalD-Tardis-atlhei[3808458]:   File "/usr/local/lib/python3.8/site-packages/cobald/daemon/runners/base_runner.py", line 68, in run
Jul 13 12:20:02 monopol.bfg.privat docker-COBalD-Tardis-atlhei[3808458]:     await self.manage_payloads()
Jul 13 12:20:02 monopol.bfg.privat docker-COBalD-Tardis-atlhei[3808458]:   File "/usr/local/lib/python3.8/site-packages/cobald/daemon/runners/asyncio_runner.py", line 54, in manage_payloads
Jul 13 12:20:02 monopol.bfg.privat docker-COBalD-Tardis-atlhei[3808458]:     await self._payload_failure
Jul 13 12:20:02 monopol.bfg.privat docker-COBalD-Tardis-atlhei[3808458]:   File "/usr/local/lib/python3.8/site-packages/cobald/daemon/runners/asyncio_runner.py", line 40, in _monitor_payload
Jul 13 12:20:02 monopol.bfg.privat docker-COBalD-Tardis-atlhei[3808458]:     result = await payload()
Jul 13 12:20:02 monopol.bfg.privat docker-COBalD-Tardis-atlhei[3808458]:   File "/usr/local/lib/python3.8/site-packages/tardis/resources/drone.py", line 123, in run
Jul 13 12:20:02 monopol.bfg.privat docker-COBalD-Tardis-atlhei[3808458]:     await current_state.run(self)
Jul 13 12:20:02 monopol.bfg.privat docker-COBalD-Tardis-atlhei[3808458]:   File "/usr/local/lib/python3.8/site-packages/tardis/resources/dronestates.py", line 288, in run
Jul 13 12:20:02 monopol.bfg.privat docker-COBalD-Tardis-atlhei[3808458]:     await drone.set_state(await cls.run_processing_pipeline(drone))
Jul 13 12:20:02 monopol.bfg.privat docker-COBalD-Tardis-atlhei[3808458]:   File "/usr/local/lib/python3.8/site-packages/tardis/resources/drone.py", line 143, in set_state
Jul 13 12:20:02 monopol.bfg.privat docker-COBalD-Tardis-atlhei[3808458]:     await self.notify_plugins()
Jul 13 12:20:02 monopol.bfg.privat docker-COBalD-Tardis-atlhei[3808458]:   File "/usr/local/lib/python3.8/site-packages/tardis/resources/drone.py", line 153, in notify_plugins
Jul 13 12:20:02 monopol.bfg.privat docker-COBalD-Tardis-atlhei[3808458]:     await plugin.notify(self.state, self.resource_attributes)
Jul 13 12:20:02 monopol.bfg.privat docker-COBalD-Tardis-atlhei[3808458]:   File "/usr/local/lib/python3.8/site-packages/tardis/plugins/auditor.py", line 88, in notify
Jul 13 12:20:02 monopol.bfg.privat docker-COBalD-Tardis-atlhei[3808458]:     await self._client.update(record)
Jul 13 12:20:02 monopol.bfg.privat docker-COBalD-Tardis-atlhei[3808458]: RuntimeError: Reqwest Error: HTTP status client error (400 Bad Request) for url (http://10.18.0.12:8001/update)
```

I think this is the correct behaviour from auditor, as updating a non-existing record does not make sense.

So we should handle this exception somehow or try to find another solution.

Discussion:
giffels: Thanks a lot for the contribution. Without checking any details yet, could you add a unittest for that use-case as well, please?
QuantumDancer: Thanks for the instructions regarding the unit testing. I've now added tests that should cover all three cases. Please let me know, I should change something else.

Patch:
diff --git a/tardis/plugins/auditor.py b/tardis/plugins/auditor.py
index b2d120e..0976a39 100644
--- a/tardis/plugins/auditor.py
+++ b/tardis/plugins/auditor.py
@@ -85,7 +85,18 @@ class Auditor(Plugin):
                 .replace(tzinfo=self._local_timezone)
                 .astimezone(pytz.utc)
             )
-            await self._client.update(record)
+            try:
+                await self._client.update(record)
+            except RuntimeError as e:
+                if str(e).startswith(
+                    "Reqwest Error: HTTP status client error (400 Bad Request)"
+                ):
+                    self.logger.debug(
+                        f"Could not update record {record.record_id}, "
+                        "it probably does not exist in the database"
+                    )
+                else:
+                    raise
 
     def construct_record(self, resource_attributes: AttributeDict):
         """


Test patch:
diff --git a/tests/plugins_t/test_auditor.py b/tests/plugins_t/test_auditor.py
index b1c7ce5..faf4b81 100644
--- a/tests/plugins_t/test_auditor.py
+++ b/tests/plugins_t/test_auditor.py
@@ -143,6 +143,46 @@ class TestAuditor(TestCase):
             self.assertEqual(self.client.add.call_count, 1)
             self.assertEqual(self.client.update.call_count, 1)
 
+        # test exception handling
+        self.client.update.side_effect = RuntimeError(
+            "Reqwest Error: HTTP status client error (400 Bad Request) "
+            "for url (http://127.0.0.1:8000/update)"
+        )
+        run_async(
+            self.plugin.notify,
+            state=DownState(),
+            resource_attributes=self.test_param,
+        )
+
+        self.client.update.side_effect = RuntimeError(
+            "Reqwest Error: HTTP status client error (403 Forbidden) "
+            "for url (http://127.0.0.1:8000/update)"
+        )
+        with self.assertRaises(RuntimeError):
+            run_async(
+                self.plugin.notify,
+                state=DownState(),
+                resource_attributes=self.test_param,
+            )
+
+        self.client.update.side_effect = RuntimeError("Does not match RegEx")
+        with self.assertRaises(RuntimeError):
+            run_async(
+                self.plugin.notify,
+                state=DownState(),
+                resource_attributes=self.test_param,
+            )
+
+        self.client.update.side_effect = ValueError("Other exception")
+        with self.assertRaises(ValueError):
+            run_async(
+                self.plugin.notify,
+                state=DownState(),
+                resource_attributes=self.test_param,
+            )
+
+        self.client.update.side_effect = None
+
     def test_construct_record(self):
         record = self.plugin.construct_record(resource_attributes=self.test_param)