Merge pull request #247 from giffels/change/plugin-drone-state-handling

Change drone state initialisation and notification of plugins

docs/source/changelog.rst

@@ -1,12 +1,12 @@
-.. Created by changelog.py at 2022-05-06, command
+.. Created by changelog.py at 2022-05-11, command
    '/Users/giffler/.cache/pre-commit/repor6pnmwlm/py_env-default/bin/changelog docs/source/changes compile --output=docs/source/changelog.rst'
    based on the format of 'https://keepachangelog.com/'
 
 #########
 CHANGELOG
 #########
 
-[Unreleased] - 2022-05-06
+[Unreleased] - 2022-05-11
 =========================
 
 Added
@@ -19,6 +19,7 @@ Changed
 -------
 
 * SSHExecutor respects the remote MaxSessions via queueing
+* Change drone state initialisation and notification of plugins
 
 Fixed
 -----

docs/source/changes/247.change_drone_state_initialisation.yaml

@@ -0,0 +1,10 @@
+category: changed
+summary: "Change drone state initialisation and notification of plugins"
+description: |
+  The initialisation procedure and the notification of the plugins is changed to fix a bug occurring on restarts of 
+  Drones. A newly created Drone is now initialised with ``state = None`` and all plugins are notified first state
+  change ``None`` -> ``RequestState``. The Drone is now inserted in the `SqliteRegistry` when it state changes to 
+  ``RequestState`` and all subsequent changes are DB updates. So, failing duplicated inserts due to the unique 
+  requirement of the ``drone_uuid`` are prevented in case a Drone changes back to ``BootingState`` again.
+pull_requests:
+  - 247

tardis/plugins/sqliteregistry.py

@@ -27,7 +27,7 @@ def __init__(self):
         self._db_file = configuration.Plugins.SqliteRegistry.db_file
         self._deploy_db_schema()
         self._dispatch_on_state = dict(
-            BootingState=self.insert_resource, DownState=self.delete_resource
+            RequestState=self.insert_resource, DownState=self.delete_resource
         )
 
         for site in configuration.Sites:

tardis/resources/drone.py

@@ -28,7 +28,7 @@ def __init__(
         plugins: Optional[List[Plugin]] = None,
         remote_resource_uuid=None,
         drone_uuid=None,
-        state: RequestState = RequestState(),
+        state: Optional[State] = None,
         created: float = None,
         updated: float = None,
     ):
@@ -93,6 +93,12 @@ def site_agent(self) -> SiteAgent:
         return self._site_agent
 
     async def run(self):
+        if self.state is None:
+            # The state of a newly created Drone is None, since the plugins need
+            # to be notified on the first state change. As calling the
+            # ``set_state`` coroutine is not possible in the constructor, we
+            # initiate the first state change here
+            await self.set_state(RequestState())
         while True:
             current_state = self.state
             await current_state.run(self)

tardis/resources/poolfactory.py

@@ -6,7 +6,6 @@
 from ..agents.siteagent import SiteAgent
 from ..configuration.configuration import Configuration
 from ..resources.drone import Drone
-from ..resources.dronestates import RequestState
 
 from cobald.composite.weighted import WeightedComposite
 from cobald.composite.factory import FactoryPool
@@ -100,7 +99,7 @@ def create_drone(
     plugins: Optional[Iterable[Plugin]] = None,
     remote_resource_uuid=None,
     drone_uuid=None,
-    state: State = RequestState(),
+    state: Optional[State] = None,
     created: float = None,
     updated: float = None,
 ):

tests/plugins_t/test_sqliteregistry.py

@@ -2,7 +2,7 @@
 
 from tardis.resources.dronestates import BootingState
 from tardis.resources.dronestates import IntegrateState
-from tardis.resources.dronestates import DownState
+from tardis.resources.dronestates import RequestState, DownState
 from tardis.interfaces.state import State
 from tardis.plugins.sqliteregistry import SqliteRegistry
 from tardis.utilities.attributedict import AttributeDict
@@ -48,15 +48,15 @@ def setUpClass(cls):
                 "remote_resource_uuid"
             ],
             "drone_uuid": cls.test_resource_attributes["drone_uuid"],
-            "state": str(BootingState()),
+            "state": str(RequestState()),
             "created": cls.test_resource_attributes["created"],
             "updated": cls.test_resource_attributes["updated"],
         }
 
         cls.test_notify_result = (
             cls.test_resource_attributes["remote_resource_uuid"],
             cls.test_resource_attributes["drone_uuid"],
-            str(BootingState()),
+            str(RequestState()),
             cls.test_resource_attributes["site_name"],
             cls.test_resource_attributes["machine_type"],
             str(cls.test_resource_attributes["created"]),
@@ -182,7 +182,7 @@ def test_double_schema_deployment(self):
     def test_get_resources(self):
         self.registry.add_site(self.test_site_name)
         self.registry.add_machine_types(self.test_site_name, self.test_machine_type)
-        run_async(self.registry.notify, BootingState(), self.test_resource_attributes)
+        run_async(self.registry.notify, RequestState(), self.test_resource_attributes)
 
         self.assertListEqual(
             self.registry.get_resources(
@@ -208,13 +208,13 @@ def fetch_all():
         self.registry.add_site(self.test_site_name)
         self.registry.add_machine_types(self.test_site_name, self.test_machine_type)
 
-        run_async(self.registry.notify, BootingState(), self.test_resource_attributes)
+        run_async(self.registry.notify, RequestState(), self.test_resource_attributes)
 
         self.assertEqual([self.test_notify_result], fetch_all())
 
         with self.assertRaises(sqlite3.IntegrityError) as ie:
             run_async(
-                self.registry.notify, BootingState(), self.test_resource_attributes
+                self.registry.notify, RequestState(), self.test_resource_attributes
             )
         self.assertTrue("unique" in str(ie.exception).lower())
 
@@ -250,7 +250,7 @@ def fetch_all():
             self.registry.add_site(site_name)
             self.registry.add_machine_types(site_name, self.test_machine_type)
 
-        bind_parameters = {"state": "BootingState"}
+        bind_parameters = {"state": "RequestState"}
         bind_parameters.update(self.test_resource_attributes)
 
         run_async(self.registry.insert_resource, bind_parameters)

tests/resources_t/test_drone.py

@@ -3,7 +3,7 @@
 from tardis.interfaces.plugin import Plugin
 from tardis.interfaces.state import State
 from tardis.resources.drone import Drone
-from tardis.resources.dronestates import RequestState, DownState
+from tardis.resources.dronestates import DownState
 from tardis.utilities.attributedict import AttributeDict
 
 from logging import DEBUG
@@ -140,7 +140,7 @@ def test_set_state(self):
 
     def test_state(self):
         self.assertEqual(self.drone.state, self.drone._state)
-        self.assertIsInstance(self.drone.state, RequestState)
+        self.assertIsNone(self.drone.state)
 
     def test_notify_plugins(self):
         self.drone.register_plugins(self.mock_plugin)

tests/resources_t/test_poolfactory.py

@@ -126,26 +126,24 @@ def test_create_composite(
     @patch("tardis.resources.poolfactory.BatchSystemAgent")
     @patch("tardis.resources.poolfactory.SiteAgent")
     def test_create_drone(self, mock_site_agent, mock_batch_system_agent, mock_drone):
-        self.assertEqual(
-            create_drone(
-                site_agent=mock_site_agent, batch_system_agent=mock_batch_system_agent
-            ),
-            mock_drone(),
+        create_drone(
+            site_agent=mock_site_agent, batch_system_agent=mock_batch_system_agent
         )
 
-        mock_drone.has_call(
+        self.assertListEqual(
+            mock_drone.mock_calls,
             [
                 call(
                     site_agent=mock_site_agent,
                     batch_system_agent=mock_batch_system_agent,
                     plugins=None,
                     remote_resource_uuid=None,
                     drone_uuid=None,
-                    state=RequestState(),
+                    state=None,
                     created=None,
                     updated=None,
                 )
-            ]
+            ],
         )
 
     def test_load_plugins(self):