Merge "Use the application data bag to set id and id_service notifications"

hooks/keystone_hooks.py

@@ -427,7 +427,7 @@ def db_departed_or_broken():
 def identity_changed(relation_id=None, remote_unit=None):
     notifications_checksums = {}
     notifications_endpoints = {}
-    if is_elected_leader(CLUSTER_RES):
+    if is_leader():
         if not is_db_ready():
             log("identity-service-relation-changed hook fired before db "
                 "ready - deferring until db ready", level=WARNING)

hooks/keystone_utils.py

@@ -2240,6 +2240,20 @@ def send_id_service_notifications(data):
                 relation_settings={
                     'catalog_ttl': config('catalog-cache-expiration'),
                     'ep_changed': json.dumps(changed, sort_keys=True)})
+            # As a first step in resolving a race condition when updating
+            # identity and identity-notifications (LP: #1902264), start sending
+            # this data into the application data-bag instead of just the
+            # per-unit relation data. The consuming charms will need updating
+            # to look in the app data bag as well.
+            #
+            # After the charms consuming thie relation have been updated,
+            # it will be possible to remove the per-relation set above.
+            relation_set(
+                relation_id=rid,
+                relation_settings={
+                    'catalog_ttl': config('catalog-cache-expiration'),
+                    'ep_changed': json.dumps(changed, sort_keys=True)},
+                app=True)
 
 
 def send_notifications(checksum_data, endpoint_data, force=False):
@@ -2262,7 +2276,7 @@ def send_id_notifications(data, force=False):
     :param force: Determines whether a trigger value is set to ensure the
                   remote hook is fired.
     """
-    if not data or not is_elected_leader(CLUSTER_RES):
+    if not data or not is_leader():
         log("Not sending notifications (no data or not leader)", level=INFO)
         return
 
@@ -2311,6 +2325,17 @@ def send_id_notifications(data, force=False):
         level=DEBUG)
     for rid in rel_ids:
         relation_set(relation_id=rid, relation_settings=_notifications)
+        # As a first step in resolving a race condition when updating
+        # identity and identity-notifications (LP: #1902264), start sending
+        # this data into the application data-bag instead of just the
+        # per-unit relation data. The consuming charms will need updating
+        # to look in the app data bag as well.
+        #
+        # After the charms consuming thie relation have been updated,
+        # it will be possible to remove the per-relation set above.
+        relation_set(relation_id=rid,
+                     relation_settings=_notifications,
+                     app=True)
 
 
 def is_db_ready(use_current_context=False, db_rel=None):

unit_tests/test_keystone_hooks.py

@@ -466,7 +466,7 @@ class KeystoneRelationTests(CharmTestCase):
 
     @patch('keystone_utils.log')
     def test_identity_changed_no_leader(self, mock_log):
-        self.is_elected_leader.return_value = False
+        self.is_leader.return_value = False
         hooks.identity_changed(
             relation_id='identity-service:0',
             remote_unit='unit/0')

unit_tests/test_keystone_utils.py

@@ -893,32 +893,50 @@ class TestKeystoneUtils(CharmTestCase):
     @patch.object(utils, 'relation_set')
     @patch.object(utils, 'relation_get')
     @patch.object(utils, 'relation_ids')
-    @patch.object(utils, 'is_elected_leader')
+    @patch.object(utils, 'is_leader')
-    def test_send_id_notifications(self, mock_is_elected_leader,
+    def test_send_id_notifications(self, mock_is_leader,
                                    mock_relation_ids, mock_relation_get,
                                    mock_relation_set, mock_uuid):
         checksums = {'foo-endpoint-changed': 1}
         relation_id = 'testrel:0'
         mock_uuid.uuid4.return_value = '1234'
         mock_relation_ids.return_value = [relation_id]
-        mock_is_elected_leader.return_value = False
+        mock_is_leader.return_value = False
         utils.send_id_notifications(checksums)
         self.assertFalse(mock_relation_set.called)
 
-        mock_is_elected_leader.return_value = True
+        mock_is_leader.return_value = True
         utils.send_id_notifications({})
         self.assertFalse(mock_relation_set.called)
 
         utils.send_id_notifications(checksums)
         self.assertTrue(mock_relation_set.called)
-        mock_relation_set.assert_called_once_with(relation_id=relation_id,
+        mock_relation_set.assert_has_calls([
-                                                  relation_settings=checksums)
+            call(
+                relation_id=relation_id,
+                relation_settings=checksums,
+                app=True
+            ),
+            call(
+                relation_id=relation_id,
+                relation_settings=checksums
+            ),
+        ], any_order=True)
         mock_relation_set.reset_mock()
         utils.send_id_notifications(checksums, force=True)
         self.assertTrue(mock_relation_set.called)
         checksums['trigger'] = '1234'
-        mock_relation_set.assert_called_once_with(relation_id=relation_id,
+        mock_relation_set.assert_has_calls([
-                                                  relation_settings=checksums)
+            call(
+                relation_id=relation_id,
+                relation_settings=checksums,
+                app=True
+            ),
+            call(
+                relation_id=relation_id,
+                relation_settings=checksums
+            ),
+        ], any_order=True)
 
     @patch.object(utils, 'service_endpoint_dict')
     @patch.object(utils, 'relation_ids')
@@ -984,7 +1002,8 @@ class TestKeystoneUtils(CharmTestCase):
                     'ep_changed':
                         ('{"neutron": {"internal": '
                          '"http://neutron.demo.com:9696"}}')
-                }
+                },
+                app=True
             ),
             call(
                 relation_id='identity-service:2',
@@ -994,7 +1013,8 @@ class TestKeystoneUtils(CharmTestCase):
                         ('{"neutron": {"internal": '
                          '"http://neutron.demo.com:9696"},'
                          ' "placement": {"internal": "http://demo.com"}}')
-                }
+                },
+                app=True
             )
         ], any_order=True)
 
@@ -1009,7 +1029,8 @@ class TestKeystoneUtils(CharmTestCase):
                     'catalog_ttl': 60,
                     'ep_changed':
                         '{"neutron": {"internal": "http://demo.com"}}'
-                }
+                },
+                app=True
             ),
             call(
                 relation_id='identity-service:2',
@@ -1017,7 +1038,8 @@ class TestKeystoneUtils(CharmTestCase):
                     'catalog_ttl': 60,
                     'ep_changed':
                         '{"neutron": {"internal": "http://demo.com"}}'
-                }
+                },
+                app=True
             )
         ]
         mock_relation_set.assert_has_calls(
@@ -1036,7 +1058,8 @@ class TestKeystoneUtils(CharmTestCase):
                     'catalog_ttl': 60,
                     'ep_changed':
                         '{"neutron": {"internal": "http://demo.com"}}'
-                }
+                },
+                app=True
             ),
             call(
                 relation_id='identity-service:2',
@@ -1046,7 +1069,8 @@ class TestKeystoneUtils(CharmTestCase):
                         '{"neutron": {"internal": "http://demo.com"}, '
                         '"placement": {"internal": "http://demo.com"}}'
                     )
-                }
+                },
+                app=True
             )
         ]
         mock_relation_set.assert_has_calls(