Fix db init notifications

Ensures that leader does not respond to db init
notifications to avoid infitinite looping after
leader switches to a different unit.

Also ensures that leader only restarts its neutron-server
once on db init.

Closes-Bug: #1893008

Change-Id: I59b9d5e0caab62b72380879bf16cb0fd8703bb32
(cherry picked from commit 104626a19f)

hooks/neutron_api_utils.py

@@ -53,6 +53,7 @@ from charmhelpers.core.hookenv import (
     relation_get,
     relation_set,
     local_unit,
+    is_leader,
 )
 
 from charmhelpers.fetch import (
@@ -295,7 +296,8 @@ def check_local_db_actions_complete():
 
     NOTE: this must only be called from peer relation context.
     """
-    if not is_db_initialised():
+    # leader must not respond to notifications
+    if is_leader() or not is_db_initialised():
         return
 
     settings = relation_get() or {}
@@ -314,8 +316,10 @@ def check_local_db_actions_complete():
                     "initialisation", level=DEBUG)
                 service_restart('neutron-server')
 
-            # Echo notification
+            # Echo notification and ensure init key unset since we are not
-            relation_set(**{NEUTRON_DB_INIT_ECHO_RKEY: init_id})
+            # leader anymore.
+            relation_set(**{NEUTRON_DB_INIT_ECHO_RKEY: init_id,
+                            NEUTRON_DB_INIT_RKEY: None})
 
 
 def api_port(service):
@@ -789,19 +793,23 @@ def migrate_neutron_database(upgrade=False):
                'head']
         subprocess.check_output(cmd)
 
+    if not is_unit_paused_set():
+        log("Restarting neutron-server following database migration",
+            level=DEBUG)
+        service_restart('neutron-server')
+
     cluster_rids = relation_ids('cluster')
     if cluster_rids:
         # Notify peers so that services get restarted
         log("Notifying peer(s) that db is initialised and restarting services",
             level=DEBUG)
+        # Use the same uuid for all notifications in this cycle to make
+        # them easier to identify.
+        n_id = uuid.uuid4()
         for r_id in cluster_rids:
-            if not is_unit_paused_set():
+            # Notify peers that they should also restart their services
-                service_restart('neutron-server')
-
-            # Notify peers that tey should also restart their services
             shared_db_rel_id = (relation_ids('shared-db') or [None])[0]
-            id = "{}-{}-{}".format(local_unit(), shared_db_rel_id,
+            id = "{}-{}-{}".format(local_unit(), shared_db_rel_id, n_id)
-                                   uuid.uuid4())
             relation_set(relation_id=r_id, **{NEUTRON_DB_INIT_RKEY: id})
 
 

unit_tests/test_neutron_api_utils.py

@@ -655,31 +655,37 @@ class TestNeutronAPIUtils(CharmTestCase):
                'icehouse']
         self.subprocess.check_output.assert_called_with(cmd)
 
+    @patch.object(nutils, 'relation_ids')
     @patch.object(nutils, 'relation_set')
-    @patch.object(nutils, 'is_db_initialised', lambda: False)
     @patch.object(nutils, 'relation_get')
+    @patch.object(nutils, 'is_leader')
+    @patch.object(nutils, 'is_db_initialised')
     @patch.object(nutils, 'local_unit', lambda *args: 'unit/0')
-    def test_check_local_db_actions_complete_by_self(self, mock_relation_get,
+    def test_check_local_db_actions_complete_leader(self,
-                                                     mock_relation_set):
+                                                    mock_is_db_initialised,
-        mock_relation_get.return_value = {}
+                                                    mock_is_leader,
+                                                    mock_relation_get,
+                                                    mock_relation_set,
+                                                    mock_relation_ids):
+        mock_is_leader.return_value = True
         nutils.check_local_db_actions_complete()
-        self.assertFalse(mock_relation_set.called)
+        mock_relation_get.assert_not_called()
-
+        mock_relation_set.assert_not_called()
-        mock_relation_get.return_value = {nutils.NEUTRON_DB_INIT_RKEY:
+        self.service_restart.assert_not_called()
-                                          'unit/0-1234'}
-        nutils.check_local_db_actions_complete()
-        self.assertFalse(mock_relation_set.called)
 
     @patch.object(nutils, 'relation_ids')
     @patch.object(nutils, 'relation_set')
     @patch.object(nutils, 'relation_get')
+    @patch.object(nutils, 'is_leader')
     @patch.object(nutils, 'is_db_initialised')
     @patch.object(nutils, 'local_unit', lambda *args: 'unit/0')
-    def test_check_local_db_actions_complete(self,
+    def test_check_local_db_actions_complete_non_leader(self,
-                                             mock_is_db_initialised,
+                                                        mock_is_db_initialised,
-                                             mock_relation_get,
+                                                        mock_is_leader,
-                                             mock_relation_set,
+                                                        mock_relation_get,
-                                             mock_relation_ids):
+                                                        mock_relation_set,
+                                                        mock_relation_ids):
+        mock_is_leader.return_value = False
         shared_db_rel_id = 'shared-db:1'
         mock_relation_ids.return_value = [shared_db_rel_id]
         mock_is_db_initialised.return_value = True
@@ -697,7 +703,8 @@ class TestNeutronAPIUtils(CharmTestCase):
         init_db_val = 'unit/1-{}-1234'.format(shared_db_rel_id)
         r_settings = {nutils.NEUTRON_DB_INIT_RKEY: init_db_val}
         nutils.check_local_db_actions_complete()
-        calls = [call(**{nutils.NEUTRON_DB_INIT_ECHO_RKEY: init_db_val})]
+        calls = [call(**{nutils.NEUTRON_DB_INIT_ECHO_RKEY: init_db_val,
+                         nutils.NEUTRON_DB_INIT_RKEY: None})]
         mock_relation_set.assert_has_calls(calls)
         self.service_restart.assert_called_with('neutron-server')