From bec5e363d767b06fa5124f1fe45600c85b23dff2 Mon Sep 17 00:00:00 2001 From: Frode Nordahl Date: Thu, 14 Mar 2019 09:32:13 +0100 Subject: [PATCH] Ensure clients are updated with RBD features bitmap The ``client-relation`` hook did not compute RBD features based on presence of ``rbd-mirror`` relation as it should. Also, at present, units joining the ``client-relation`` before any ``rbd-mirror`` relations have completed would not get the appropriate RBD features bitmap set on the relation. Change-Id: Ic9e60fc91ecc467dead099c73ecc71d80c907fba --- hooks/ceph_hooks.py | 10 ++++++++-- unit_tests/test_ceph_hooks.py | 21 ++++++++++++++++----- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/hooks/ceph_hooks.py b/hooks/ceph_hooks.py index 759bd73f..b96dc817 100755 --- a/hooks/ceph_hooks.py +++ b/hooks/ceph_hooks.py @@ -713,6 +713,11 @@ def rbd_mirror_relation(relid=None, unit=None, recurse=True): relation_set(relation_id=relid, relation_settings=data) + # make sure clients are updated with the appropriate RBD features + # bitmap. + if recurse: + notify_client() + @hooks.hook('mds-relation-changed') @hooks.hook('mds-relation-joined') @@ -768,8 +773,9 @@ def client_relation(relid=None, unit=None): data = {'key': ceph.get_named_key(service_name), 'auth': config('auth-supported'), 'ceph-public-address': public_addr} - if config('default-rbd-features'): - data['rbd-features'] = config('default-rbd-features') + rbd_features = get_rbd_features() + if rbd_features: + data['rbd-features'] = rbd_features if not unit: unit = remote_unit() data.update( diff --git a/unit_tests/test_ceph_hooks.py b/unit_tests/test_ceph_hooks.py index 4600915c..553afef6 100644 --- a/unit_tests/test_ceph_hooks.py +++ b/unit_tests/test_ceph_hooks.py @@ -327,6 +327,7 @@ class RelatedUnitsTestCase(unittest.TestCase): call('osd:23') ]) + @patch.object(ceph_hooks, 'get_rbd_features') @patch.object(ceph_hooks, 'relation_set') @patch.object(ceph_hooks, 'handle_broker_request') @patch.object(ceph_hooks, 'config') @@ -341,11 +342,13 @@ class RelatedUnitsTestCase(unittest.TestCase): _get_named_key, _config, _handle_broker_request, - _relation_set): + _relation_set, + _get_rbd_features): _remote_service_name.return_value = 'glance' config = copy.deepcopy(CHARM_CONFIG) _config.side_effect = lambda key: config[key] _handle_broker_request.return_value = {} + _get_rbd_features.return_value = None ceph_hooks.client_relation(relid='rel1', unit='glance/0') _ready_for_service.assert_called_once_with() _get_public_addr.assert_called_once_with() @@ -359,7 +362,7 @@ class RelatedUnitsTestCase(unittest.TestCase): 'auth': False, 'ceph-public-address': _get_public_addr() }) - config.update({'default-rbd-features': 42}) + _get_rbd_features.return_value = 42 _relation_set.reset_mock() ceph_hooks.client_relation(relid='rel1', unit='glance/0') _relation_set.assert_called_once_with( @@ -371,6 +374,7 @@ class RelatedUnitsTestCase(unittest.TestCase): 'rbd-features': 42, }) + @patch.object(ceph_hooks, 'get_rbd_features') @patch.object(ceph_hooks, 'config') @patch.object(ceph_hooks.ceph, 'get_named_key') @patch.object(ceph_hooks, 'get_public_addr') @@ -394,7 +398,8 @@ class RelatedUnitsTestCase(unittest.TestCase): remote_service_name, get_public_addr, get_named_key, - _config): + _config, + _get_rbd_features): # Check for LP #1738154 ready_for_service.return_value = True process_requests.return_value = 'AOK' @@ -404,6 +409,7 @@ class RelatedUnitsTestCase(unittest.TestCase): is_quorum.return_value = True config = copy.deepcopy(CHARM_CONFIG) _config.side_effect = lambda key: config[key] + _get_rbd_features.return_value = None ceph_hooks.client_relation(relid='rel1', unit='glance/0') relation_set.assert_called_once_with( relation_id='rel1', @@ -714,7 +720,8 @@ class RBDMirrorRelationTestCase(test_utils.CharmTestCase): self.get_public_addr.return_value = '198.51.100.10' self.ceph.list_pools_detail.return_value = {'pool': {}} - def test_rbd_mirror_relation(self): + @patch.object(ceph_hooks, 'notify_client') + def test_rbd_mirror_relation(self, _notify_client): self.handle_broker_request.return_value = {} base_relation_settings = { 'auth': self.test_config.get('auth-supported'), @@ -730,12 +737,16 @@ class RBDMirrorRelationTestCase(test_utils.CharmTestCase): relation_settings=base_relation_settings) self.test_relation.set( {'unique_id': None}) - ceph_hooks.rbd_mirror_relation('rbd-mirror:52', 'ceph-rbd-mirror/0') + _notify_client.assert_called_once_with() + _notify_client.reset_mock() + ceph_hooks.rbd_mirror_relation('rbd-mirror:52', 'ceph-rbd-mirror/0', + recurse=False) self.relation_set.assert_called_with( relation_id='rbd-mirror:52', relation_settings=base_relation_settings) self.test_relation.set( {'unique_id': json.dumps('otherSideIsReactiveEndpoint')}) + self.assertFalse(_notify_client.called) ceph_hooks.rbd_mirror_relation('rbd-mirror:53', 'ceph-rbd-mirror/0') self.ceph.get_rbd_mirror_key.assert_called_once_with( 'rbd-mirror.otherSideIsReactiveEndpoint')