Use the application data bag to set id and id_service notifications
When purely using relation-set from a leader, updates after the leader has changed can lead to old data being persisted on a relation in addition to newer data being set by the new leader. When this happens, there can be issues with services using old data to talk to other related services. This change introduces the use of the application data bag to ensure that all units related to keystone get the same data from the leader, regardless of leadership changes. While this change enables the application data bag for these relations, it still sends the per-unit relation data as well to maintain backwards compatibility. Charms that consume the identity-service and identity-notification relations will need an update to use the application data bag to complete this change. Partial-Bug: #1902264 Change-Id: Iadd795fec605e7704e5a6673906452279bbecb34
This commit is contained in:
Chris MacNaughton
parent
3143cb6638
commit
9b8b81a0bc
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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):
|
||||
|
|
|
|||
|
|
@ -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')
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
Loading…
Reference in New Issue