diff --git a/hooks/keystone_hooks.py b/hooks/keystone_hooks.py index fda34745..79da4f6f 100755 --- a/hooks/keystone_hooks.py +++ b/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) diff --git a/hooks/keystone_utils.py b/hooks/keystone_utils.py index c1055f17..2774f991 100644 --- a/hooks/keystone_utils.py +++ b/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): diff --git a/unit_tests/test_keystone_hooks.py b/unit_tests/test_keystone_hooks.py index 1f38aab7..2079bb40 100644 --- a/unit_tests/test_keystone_hooks.py +++ b/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') diff --git a/unit_tests/test_keystone_utils.py b/unit_tests/test_keystone_utils.py index 3ce32261..e1f79717 100644 --- a/unit_tests/test_keystone_utils.py +++ b/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') - def test_send_id_notifications(self, mock_is_elected_leader, + @patch.object(utils, 'is_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, - relation_settings=checksums) + mock_relation_set.assert_has_calls([ + 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, - relation_settings=checksums) + mock_relation_set.assert_has_calls([ + 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(