Destroy services from DB on stop()

When manila services are stopped or restarted via stop(), the DB
entries are not deleted, they are destroyed only in kill() method. In
cluster deployments, where multiple instances of manila-scheduler are
deployed via PODs, unique hostname is derived from node name. However
if pods are deployed again and launched on new hosts/nodes, the old
entries of manila-scheduler remains as it is. Fix via removing DB
entries after service stop() and create new entries again ins start().

Closes-bug: #1990839

manila/data/manager.py

@@ -59,8 +59,9 @@ def __init__(self, service_name=None, *args, **kwargs):
         super(DataManager, self).__init__(*args, **kwargs)
         self.busy_tasks_shares = {}
 
-    def init_host(self):
+    def init_host(self, service_id=None):
         ctxt = context.get_admin_context()
+        self.service_id = service_id
         shares = self.db.share_get_all(ctxt)
         for share in shares:
             if share['task_state'] in constants.BUSY_COPYING_STATES:

manila/manager.py

@@ -100,13 +100,14 @@ def periodic_tasks(self, context, raise_on_error=False):
         """Tasks to be run at a periodic interval."""
         return self.run_periodic_tasks(context, raise_on_error=raise_on_error)
 
-    def init_host(self):
+    def init_host(self, service_id=None):
         """Handle initialization if this is a standalone service.
 
         A hook point for services to execute tasks before the services are made
         available (i.e. showing up on RPC and starting to accept RPC calls) to
         other components.  Child classes should override this method.
 
+        :param service_id: ID of the service where the manager is running.
         """
         pass
 

manila/service.py

@@ -134,12 +134,7 @@ def start(self):
         LOG.info('Starting %(topic)s node (version %(version_string)s)',
                  {'topic': self.topic, 'version_string': version_string})
         self.model_disconnected = False
-        self.manager.init_host()
         ctxt = context.get_admin_context()
-
-        if self.coordinator:
-            coordination.LOCK_COORDINATOR.start()
-
         try:
             service_ref = db.service_get_by_args(ctxt,
                                                  self.host,
@@ -148,6 +143,11 @@ def start(self):
         except exception.NotFound:
             self._create_service_ref(ctxt)
 
+        self.manager.init_host(service_id=self.service_id)
+
+        if self.coordinator:
+            coordination.LOCK_COORDINATOR.start()
+
         LOG.debug("Creating RPC server for service %s.", self.topic)
 
         target = messaging.Target(topic=self.topic, server=self.host)
@@ -227,12 +227,7 @@ def create(cls, host=None, binary=None, topic=None, manager=None,
         return service_obj
 
     def kill(self):
-        """Destroy the service object in the datastore."""
         self.stop()
-        try:
-            db.service_destroy(context.get_admin_context(), self.service_id)
-        except exception.NotFound:
-            LOG.warning('Service killed that has no database entry.')
 
     def stop(self):
         # Try to shut the connection down, but if we get any sort of
@@ -242,6 +237,11 @@ def stop(self):
         except Exception:
             pass
 
+        try:
+            db.service_destroy(context.get_admin_context(), self.service_id)
+        except exception.NotFound:
+            LOG.warning('Service killed that has no database entry.')
+
         if self.coordinator:
             try:
                 coordination.LOCK_COORDINATOR.stop()

manila/share/manager.py

@@ -339,7 +339,7 @@ def _ensure_share_instance_has_pool(self, ctxt, share_instance):
         return pool
 
     @add_hooks
-    def init_host(self, reexport=False):
+    def init_host(self, service_id=None, reexport=False):
         """Initialization for a standalone service."""
 
         # mark service alive by creating a probe
@@ -349,6 +349,7 @@ def init_host(self, reexport=False):
             LOG.error("Probe not created: %(e)s", {'e': six.text_type(e)})
 
         ctxt = context.get_admin_context()
+        self.service_id = service_id
         driver_host_pair = "{}@{}".format(
             self.driver.__class__.__name__,
             self.host)

manila/tests/test_service.py

@@ -88,21 +88,19 @@ def test_service_enabled_on_create_based_on_flag(self):
         binary = 'manila-fake'
         app = service.Service.create(host=host, binary=binary)
         app.start()
-        app.stop()
         ref = db.service_get(context.get_admin_context(), app.service_id)
-        db.service_destroy(context.get_admin_context(), app.service_id)
         self.assertFalse(ref['disabled'])
+        app.stop()
 
     def test_service_disabled_on_create_based_on_flag(self):
         self.flags(enable_new_services=False)
         host = 'foo'
         binary = 'manila-fake'
         app = service.Service.create(host=host, binary=binary)
         app.start()
-        app.stop()
         ref = db.service_get(context.get_admin_context(), app.service_id)
-        db.service_destroy(context.get_admin_context(), app.service_id)
         self.assertTrue(ref['disabled'])
+        app.stop()
 
 
 def fake_service_get_by_args(*args, **kwargs):

releasenotes/notes/bug-1990839-destroy-services-from-DB-on-service.stop-c4fe2a6e312a1651.yaml

@@ -0,0 +1,9 @@
+---
+fixes:
+  - |
+    In cluster deployments, where multiple instances of manila-scheduler are
+    deployed via PODs, unique hostname is derived from node name. However if
+    pods are deployed again and launched on new hosts/nodes, the old entries
+    of manila-scheduler remains as it is. Fixed it by deleting DB entries
+    after service.stop() and creating new entries again in service.start().
+    Launchpad bug `1990839 <https://bugs.launchpad.net/manila/+bug/1990839>`_