commit d875568ad78cb7e6caeccfe2c43a4f8819b3ab5a Author: Liam Young Date: Thu Jun 18 13:08:14 2020 +0000 Support ceph client over CMRs Support ceph client over CMRs of and only if permit-insecure-cmr config option has been set to true, otherwise go into a blocked state. To support CMR clients try and get client service name from relation data first before falling back to using the remote unit name. Using the remote unit name fails when the clients are connecting via a cross-model relation. The clients side change is here: https://github.com/juju/charm-helpers/pull/481 Change-Id: If9616170b8af9eac309dc6e8edd670fb5cfd8e0f Closes-Bug: #1780712 diff --git a/config.yaml b/config.yaml index 690f7f7..b348835 100644 --- a/config.yaml +++ b/config.yaml @@ -293,3 +293,10 @@ options: feature for upgraded clusters, the pg-autotune option should be set to 'true'. To disable the autotuner for new clusters, the pg-autotune option should be set to 'false'. + permit-insecure-cmr: + type: boolean + default: False + description: | + The charm does not segregate access to pools from different models properly, + this means that the correct charm settings can result with client model B + having access to the data from model A. diff --git a/hooks/ceph_hooks.py b/hooks/ceph_hooks.py index bcf6869..ccd5b56 100755 --- a/hooks/ceph_hooks.py +++ b/hooks/ceph_hooks.py @@ -46,6 +46,7 @@ from charmhelpers.core.hookenv import ( Hooks, UnregisteredHookError, service_name, relations_of_type, + relations, status_set, local_unit, application_version_set) @@ -608,6 +609,22 @@ def notify_mons(): relation_settings={'nonce': nonce}) +def get_client_application_name(relid, unit): + """Retrieve client application name from relation data. + + :param relid: Realtion ID + :type relid: str + :param unit: Remote unit name + :type unit: str + """ + if not unit: + unit = remote_unit() + app_name = relation_get(rid=relid, unit=unit).get( + 'application-name', + hookenv.remote_service_name(relid=relid)) + return app_name + + def handle_broker_request(relid, unit, add_legacy_response=False, recurse=True): """Retrieve broker request from relation, process, return response data. @@ -635,7 +652,7 @@ def handle_broker_request(relid, unit, add_legacy_response=False, log("Not leader - ignoring broker request", level=DEBUG) else: rsp = process_requests(settings['broker_req']) - unit_id = unit.replace('/', '-') + unit_id = settings.get('unit-name', unit).replace('/', '-') unit_response_key = 'broker-rsp-' + unit_id response.update({unit_response_key: rsp}) if add_legacy_response: @@ -757,6 +774,8 @@ def radosgw_relation(relid=None, unit=None): apt_install(packages=filter_installed_packages(['radosgw'])) if not unit: unit = remote_unit() + if is_unsupported_cmr(unit): + return # NOTE: radosgw needs some usage OSD storage, so defer key # provision until OSD units are detected. @@ -791,6 +810,8 @@ def rbd_mirror_relation(relid=None, unit=None, recurse=True): '- providing rbd-mirror client with keys') if not unit: unit = remote_unit() + if is_unsupported_cmr(unit): + return # handle broker requests first to get a updated pool map data = (handle_broker_request(relid, unit, recurse=recurse)) data.update({ @@ -829,6 +850,8 @@ def mds_relation_joined(relid=None, unit=None): rid=relid, unit=unit) if not unit: unit = remote_unit() + if is_unsupported_cmr(unit): + return public_addr = get_public_addr() data = { 'fsid': leader_get('fsid'), @@ -843,6 +866,8 @@ def mds_relation_joined(relid=None, unit=None): @hooks.hook('admin-relation-changed') @hooks.hook('admin-relation-joined') def admin_relation_joined(relid=None): + if is_unsupported_cmr(remote_unit()): + return if ceph.is_quorum(): name = relation_get('keyring-name') if name is None: @@ -865,7 +890,7 @@ def client_relation(relid=None, unit=None): if ready_for_service(): log('mon cluster in quorum and osds bootstrapped ' '- providing client with keys, processing broker requests') - service_name = hookenv.remote_service_name(relid=relid) + service_name = get_client_application_name(relid, unit) if not service_name: log('Unable to determine remote service name, deferring ' 'processing of broker requests') @@ -879,6 +904,8 @@ def client_relation(relid=None, unit=None): data['rbd-features'] = rbd_features if not unit: unit = remote_unit() + if is_unsupported_cmr(unit): + return data.update( handle_broker_request(relid, unit, add_legacy_response=True)) relation_set(relation_id=relid, @@ -988,9 +1015,45 @@ def update_nrpe_config(): VERSION_PACKAGE = 'ceph-common' +def is_cmr_unit(unit_name): + '''Is the remote unit connected via a cross model relation. + + :param unit_name: Name of unit + :type unit_name: str + :returns: Whether unit is connected via cmr + :rtype: bool + ''' + return unit_name.startswith('remote-') + + +def is_unsupported_cmr(unit_name): + '''If unit is connected via CMR and if that is supported. + + :param unit_name: Name of unit + :type unit_name: str + :returns: Whether unit is supported + :rtype: bool + ''' + unsupported = False + if unit_name and is_cmr_unit(unit_name): + unsupported = not config('permit-insecure-cmr') + if unsupported: + log("CMR detected and not supported", "ERROR") + return unsupported + + def assess_status(): '''Assess status of current unit''' application_version_set(get_upstream_version(VERSION_PACKAGE)) + if not config('permit-insecure-cmr'): + units = [unit + for rtype in relations() + for relid in relation_ids(reltype=rtype) + for unit in related_units(relid=relid) + if is_cmr_unit(unit)] + if units: + status_set("blocked", "Unsupported CMR relation") + return if is_unit_upgrading_set(): status_set("blocked", "Ready for do-release-upgrade and reboot. " @@ -1084,8 +1147,15 @@ def post_series_upgrade(): if __name__ == '__main__': - try: - hooks.execute(sys.argv) - except UnregisteredHookError as e: - log('Unknown hook {} - skipping.'.format(e)) + remote_block = False + remote_unit_name = remote_unit() + if remote_unit_name and is_cmr_unit(remote_unit_name): + remote_block = not config('permit-insecure-cmr') + if remote_block: + log("Not running hook, CMR detected and not supported", "ERROR") + else: + try: + hooks.execute(sys.argv) + except UnregisteredHookError as e: + log('Unknown hook {} - skipping.'.format(e)) assess_status() diff --git a/unit_tests/test_ceph_hooks.py b/unit_tests/test_ceph_hooks.py index 50dd5e0..b590df4 100644 --- a/unit_tests/test_ceph_hooks.py +++ b/unit_tests/test_ceph_hooks.py @@ -290,6 +290,23 @@ class CephHooksTestCase(test_utils.CharmTestCase): relation_settings={ 'nonce': 'FAKE-UUID'}) + @patch.object(ceph_hooks.hookenv, 'remote_service_name') + @patch.object(ceph_hooks, 'relation_get') + @patch.object(ceph_hooks, 'remote_unit') + def test_get_client_application_name(self, remote_unit, relation_get, + remote_service_name): + relation_get.return_value = { + 'application-name': 'glance'} + remote_unit.return_value = 'glance/0' + self.assertEqual( + ceph_hooks.get_client_application_name('rel:1', None), + 'glance') + relation_get.return_value = {} + remote_service_name.return_value = 'glance' + self.assertEqual( + ceph_hooks.get_client_application_name('rel:1', None), + 'glance') + @patch.object(ceph_hooks.ceph, 'list_pools') @patch.object(ceph_hooks, 'mgr_enable_module') @patch.object(ceph_hooks, 'emit_cephconf') @@ -414,11 +431,11 @@ class RelatedUnitsTestCase(unittest.TestCase): @patch.object(ceph_hooks, 'config') @patch.object(ceph_hooks.ceph, 'get_named_key') @patch.object(ceph_hooks, 'get_public_addr') - @patch.object(ceph_hooks.hookenv, 'remote_service_name') + @patch.object(ceph_hooks, 'get_client_application_name') @patch.object(ceph_hooks, 'ready_for_service') def test_client_relation(self, _ready_for_service, - _remote_service_name, + _get_client_application_name, _get_public_addr, _get_named_key, _config, @@ -426,7 +443,7 @@ class RelatedUnitsTestCase(unittest.TestCase): _relation_set, _get_rbd_features, _send_osd_settings): - _remote_service_name.return_value = 'glance' + _get_client_application_name.return_value = 'glance' config = copy.deepcopy(CHARM_CONFIG) _config.side_effect = lambda key: config[key] _handle_broker_request.return_value = {} diff --git a/unit_tests/test_status.py b/unit_tests/test_status.py index 01ba5c4..71b845e 100644 --- a/unit_tests/test_status.py +++ b/unit_tests/test_status.py @@ -35,6 +35,7 @@ TO_PATCH = [ 'config', 'ceph', 'is_relation_made', + 'relations', 'relation_ids', 'relation_get', 'related_units', @@ -184,3 +185,19 @@ class ServiceStatusTestCase(test_utils.CharmTestCase): hooks.assess_status() self.status_set.assert_called_with('blocked', mock.ANY) self.application_version_set.assert_called_with('10.2.2') + + def test_cmr_remote_unit(self): + self.test_config.set('permit-insecure-cmr', False) + self.relations.return_value = ['client'] + self.relation_ids.return_value = ['client:1'] + self.related_units.return_value = ['remote-1'] + hooks.assess_status() + self.status_set.assert_called_with( + 'blocked', + 'Unsupported CMR relation') + self.status_set.reset_mock() + self.test_config.set('permit-insecure-cmr', True) + hooks.assess_status() + self.assertFalse( + mock.call('blocked', 'Unsupported CMR relation') in + self.status_set.call_args_list)