openstack
/
charm-ceph-mon
Code Issues Proposed changes

Ensure broker requests are re-processed on upgrade-charm 

When broker-request caching was added, it broke functionality
that ensured that clients were updated on charm-upgrade, this
change enables a bypass of that cache functionality and uses
it to re-process broker requests in the upgrade-charm hook.

Depends-On: https://review.opendev.org/c/openstack/charms.ceph/+/848311
Func-Test-Pr: https://github.com/openstack-charmers/zaza-openstack-tests/pull/1066
Closes-Bug: #1968369
Change-Id: Ibdad1fd5976fdf2d5f3384f1b120b0d5dda34947
This commit is contained in:
Chris MacNaughton 2022-06-29 15:08:15 -05:00 committed by Luciano Lo Giudice
parent b31de1f027
commit 88d37461dc
4 changed files with 53 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/ceph_client.py
View File

@ -139,7 +139,7 @@ class CephClientProvides(Object):
return request_id in self._stored.processed
def _handle_broker_request(
self, relation, unit, add_legacy_response=False):
self, relation, unit, add_legacy_response=False, force=False):
"""Retrieve broker request from relation, process, return response data.
:param event: Operator event for the relation
@ -149,6 +149,8 @@ class CephClientProvides(Object):
new way.
:type add_legacy_response: bool
:returns: Dictionary of response data ready for use with relation_set.
:param force: Whether to re-process broker requests.
:type force: bool
:rtype: dict
"""
def _get_broker_req_id(request):
@ -186,7 +188,7 @@ class CephClientProvides(Object):
broker_req_id))
return {}
if self._req_already_treated(broker_req_id):
if self._req_already_treated(broker_req_id) and not force:
logger.debug(
"Ignoring already executed broker request {}".format(
broker_req_id))

60
src/ceph_hooks.py
View File

@ -595,10 +595,10 @@ def attempt_mon_cluster_bootstrap():
return True
def notify_relations():
notify_osds()
notify_radosgws()
notify_rbd_mirrors()
def notify_relations(reprocess_broker_requests=False):
notify_osds(reprocess_broker_requests=reprocess_broker_requests)
notify_radosgws(reprocess_broker_requests=reprocess_broker_requests)
notify_rbd_mirrors(reprocess_broker_requests=reprocess_broker_requests)
notify_prometheus()
@ -614,22 +614,29 @@ def notify_prometheus():
module_enabled=module_enabled)
def notify_osds():
def notify_osds(reprocess_broker_requests=False):
for relid in relation_ids('osd'):
for unit in related_units(relid):
osd_relation(relid=relid, unit=unit)
osd_relation(
relid=relid, unit=unit,
reprocess_broker_requests=reprocess_broker_requests)
def notify_radosgws():
def notify_radosgws(reprocess_broker_requests=False):
for relid in relation_ids('radosgw'):
for unit in related_units(relid):
radosgw_relation(relid=relid, unit=unit)
radosgw_relation(
relid=relid, unit=unit,
reprocess_broker_requests=reprocess_broker_requests)
def notify_rbd_mirrors():
def notify_rbd_mirrors(reprocess_broker_requests=False):
for relid in relation_ids('rbd-mirror'):
for unit in related_units(relid):
rbd_mirror_relation(relid=relid, unit=unit, recurse=False)
rbd_mirror_relation(
relid=relid, unit=unit,
recurse=False,
reprocess_broker_requests=reprocess_broker_requests)
def req_already_treated(request_id, relid, req_unit):
@ -738,7 +745,7 @@ def retrieve_client_broker_requests():
def handle_broker_request(relid, unit, add_legacy_response=False,
recurse=True):
recurse=True, force=False):
"""Retrieve broker request from relation, process, return response data.
:param relid: Realtion ID
@ -752,6 +759,9 @@ def handle_broker_request(relid, unit, add_legacy_response=False,
not. Mainly used to handle recursion when called from
notify_rbd_mirrors()
:type recurse: bool
:param force: Process broker requests even if they have already been
processed.
:type force: bool
:returns: Dictionary of response data ready for use with relation_set.
:rtype: dict
"""
@ -784,7 +794,7 @@ def handle_broker_request(relid, unit, add_legacy_response=False,
level=DEBUG)
return {}
if req_already_treated(broker_req_id, relid, unit):
if req_already_treated(broker_req_id, relid, unit) and not force:
log("Ignoring already executed broker request {}".format(
broker_req_id),
level=DEBUG)
@ -820,7 +830,7 @@ def handle_broker_request(relid, unit, add_legacy_response=False,
@hooks.hook('osd-relation-joined')
@hooks.hook('osd-relation-changed')
def osd_relation(relid=None, unit=None):
def osd_relation(relid=None, unit=None, reprocess_broker_requests=False):
if ceph.is_quorum():
log('mon cluster in quorum - providing fsid & keys')
public_addr = get_public_addr()
@ -855,7 +865,8 @@ def osd_relation(relid=None, unit=None):
)
}
data.update(handle_broker_request(relid, unit))
data.update(handle_broker_request(
relid, unit, force=reprocess_broker_requests))
relation_set(relation_id=relid,
relation_settings=data)
@ -968,7 +979,7 @@ def dashboard_relation(relid=None):
@hooks.hook('radosgw-relation-changed')
@hooks.hook('radosgw-relation-joined')
def radosgw_relation(relid=None, unit=None):
def radosgw_relation(relid=None, unit=None, reprocess_broker_requests=False):
# Install radosgw for admin tools
apt_install(packages=filter_installed_packages(['radosgw']))
if not unit:
@ -997,13 +1008,16 @@ def radosgw_relation(relid=None, unit=None):
# Old style global radosgw key
data['radosgw_key'] = ceph.get_radosgw_key()
data.update(handle_broker_request(relid, unit))
data.update(handle_broker_request(
relid, unit, force=reprocess_broker_requests))
relation_set(relation_id=relid, relation_settings=data)
@hooks.hook('rbd-mirror-relation-joined')
@hooks.hook('rbd-mirror-relation-changed')
def rbd_mirror_relation(relid=None, unit=None, recurse=True):
def rbd_mirror_relation(
relid=None, unit=None, recurse=True,
reprocess_broker_requests=False):
'''
Handle the rbd mirror relation
@ -1029,7 +1043,8 @@ def rbd_mirror_relation(relid=None, unit=None, recurse=True):
return ceph.list_pools_detail()
# handle broker requests first to get a updated pool map
data = (handle_broker_request(relid, unit, recurse=recurse))
data = (handle_broker_request(
relid, unit, recurse=recurse, force=reprocess_broker_requests))
data.update({
'auth': 'cephx',
'ceph-public-address': get_public_addr(),
@ -1061,7 +1076,8 @@ def rbd_mirror_relation(relid=None, unit=None, recurse=True):
@hooks.hook('mds-relation-changed')
@hooks.hook('mds-relation-joined')
def mds_relation_joined(relid=None, unit=None):
def mds_relation_joined(
relid=None, unit=None, reprocess_broker_requests=False):
if ready_for_service():
log('mon cluster in quorum and osds bootstrapped '
'- providing mds client with keys')
@ -1078,7 +1094,9 @@ def mds_relation_joined(relid=None, unit=None):
ceph.get_mds_key(name=mds_name),
'auth': 'cephx',
'ceph-public-address': public_addr}
data.update(handle_broker_request(relid, unit))
data.update(
handle_broker_request(
relid, unit, force=reprocess_broker_requests))
relation_set(relation_id=relid, relation_settings=data)
@ -1131,7 +1149,7 @@ def upgrade_charm():
# NOTE(jamespage):
# Reprocess broker requests to ensure that any cephx
# key permission changes are applied
notify_relations()
notify_relations(reprocess_broker_requests=True)
@hooks.hook('nrpe-external-master-relation-joined')

2
tests/tests.yaml
View File

@ -19,6 +19,7 @@ tests:
- zaza.openstack.charm_tests.ceph.tests.CephTest
- zaza.openstack.charm_tests.ceph.osd.tests.SecurityTest
- zaza.openstack.charm_tests.ceph.tests.CephPrometheusTest
- zaza.openstack.charm_tests.ceph.mon.tests.CephPermissionUpgradeTest
- zaza.openstack.charm_tests.ceph.tests.CheckPoolTypes
- zaza.openstack.charm_tests.ceph.tests.CephLowLevelTest
- zaza.openstack.charm_tests.ceph.tests.CephTest
@ -35,3 +36,4 @@ tests:
# Tests from quincy.
- zaza.openstack.charm_tests.ceph.tests.CephAuthTest
- zaza.openstack.charm_tests.ceph.tests.CephMonActionsTest
- zaza.openstack.charm_tests.ceph.mon.tests.CephPermissionUpgradeTest

13
unit_tests/test_ceph_hooks.py
View File

@ -240,7 +240,8 @@ class CephHooksTestCase(test_utils.CharmTestCase):
ceph_hooks.upgrade_charm()
mocks["apt_install"].assert_called_with(
"lockfile-progs", fatal=True)
mock_notify_radosgws.assert_called_once_with()
mock_notify_radosgws.assert_called_once_with(
reprocess_broker_requests=True)
mock_ceph.update_monfs.assert_called_once_with()
mock_notify_prometheus.assert_called_once_with()
mock_service_pause.assert_called_with('ceph-create-keys')
@ -255,9 +256,11 @@ class CephHooksTestCase(test_utils.CharmTestCase):
ceph_hooks.notify_rbd_mirrors()
mock_relation_ids.assert_called_once_with('rbd-mirror')
mock_related_units.assert_called_once_with('arelid')
mock_rbd_mirror_relation.assert_called_once_with(relid='arelid',
unit='aunit',
recurse=False)
mock_rbd_mirror_relation.assert_called_once_with(
relid='arelid',
unit='aunit',
recurse=False,
reprocess_broker_requests=False)
@patch.object(ceph_hooks, 'uuid')
@patch.object(ceph_hooks, 'relation_set')
@ -892,7 +895,7 @@ class RBDMirrorRelationTestCase(test_utils.CharmTestCase):
]
ceph_hooks.rbd_mirror_relation('rbd-mirror:51', 'ceph-rbd-mirror/0')
self.handle_broker_request.assert_called_with(
'rbd-mirror:51', 'ceph-rbd-mirror/0', recurse=True)
'rbd-mirror:51', 'ceph-rbd-mirror/0', recurse=True, force=False)
self.relation_set.assert_called_with(
relation_id='rbd-mirror:51',
relation_settings=base_relation_settings)