From e97e3607e26d14d6f2da47ab6b6b6a28c098105a Mon Sep 17 00:00:00 2001 From: utkarshbhatthere Date: Sat, 20 Aug 2022 00:30:42 +0530 Subject: [PATCH] Adds existence verification for config values Multisite config values (realm, zonegroup, zone) are written to ceph.conf as the defaults without verifying their existence, this causes failure for commands which use the default values. Closes-Bug: #1987127 Change-Id: I0ab4df34f0000339227e5d5b80352355ea7bd36e --- hooks/ceph_radosgw_context.py | 15 ++++++++--- hooks/hooks.py | 2 ++ hooks/multisite.py | 33 +++++++++++++++++++++++-- unit_tests/test_ceph_radosgw_context.py | 17 +++++++++++++ 4 files changed, 62 insertions(+), 5 deletions(-) diff --git a/hooks/ceph_radosgw_context.py b/hooks/ceph_radosgw_context.py index be991ca1..c951fb61 100644 --- a/hooks/ceph_radosgw_context.py +++ b/hooks/ceph_radosgw_context.py @@ -18,6 +18,7 @@ import socket import tempfile import shutil +import multisite from charmhelpers.contrib.openstack import context from charmhelpers.contrib.hahelpers.cluster import ( determine_api_port, @@ -307,9 +308,17 @@ class MonContext(context.CephContext): if self.context_complete(ctxt): # Multi-site zone configuration is optional, # so add after assessment - ctxt['rgw_zone'] = config('zone') - ctxt['rgw_zonegroup'] = config('zonegroup') - ctxt['rgw_realm'] = config('realm') + zone = config('zone') + zonegroup = config('zonegroup') + realm = config('realm') + log("config: zone {} zonegroup {} realm {}" + .format(zone, zonegroup, realm), level=DEBUG) + if zone in multisite.plain_list('zone'): + ctxt['rgw_zone'] = zone + if zonegroup in multisite.plain_list('zonegroup'): + ctxt['rgw_zonegroup'] = zonegroup + if realm in multisite.plain_list('realm'): + ctxt['rgw_realm'] = realm return ctxt return {} diff --git a/hooks/hooks.py b/hooks/hooks.py index 2f563098..eaea048e 100755 --- a/hooks/hooks.py +++ b/hooks/hooks.py @@ -810,6 +810,7 @@ def master_relation_joined(relation_id=None): 'Mutation detected. Restarting {}.'.format(service_name()), 'INFO') multisite.update_period(zonegroup=zonegroup, zone=zone) + CONFIGS.write_all() service_restart(service_name()) leader_set(restart_nonce=str(uuid.uuid4())) else: @@ -898,6 +899,7 @@ def slave_relation_changed(relation_id=None, unit=None): 'Mutation detected. Restarting {}.'.format(service_name()), 'INFO') multisite.update_period(zonegroup=zonegroup, zone=zone) + CONFIGS.write_all() service_restart(service_name()) leader_set(restart_nonce=str(uuid.uuid4())) else: diff --git a/hooks/multisite.py b/hooks/multisite.py index 5815cba7..590fdeb0 100644 --- a/hooks/multisite.py +++ b/hooks/multisite.py @@ -81,6 +81,35 @@ def _list(key): return [] +def plain_list(key): + """Simple Implementation for list_*, where execution may fail expectedly. + + On failure, retries are not attempted and empty list is returned. + + :param key: string for required resource (zone, zonegroup, realm, user) + :type key: str + :return: list of specified entities found + :rtype: list + """ + cmd = [ + RGW_ADMIN, '--id={}'.format(_key_name()), + key, 'list' + ] + try: + result = json.loads(subprocess.check_output( + cmd, stderr=subprocess.PIPE + ).decode('UTF-8')) + hookenv.log("Results: {}".format(result), level=hookenv.DEBUG) + if isinstance(result, dict): + return result['{}s'.format(key)] + else: + return result + except subprocess.CalledProcessError: + return [] + except TypeError: + return [] + + @decorators.retry_on_exception(num_retries=5, base_delay=3, exc_type=ValueError) def list_zones(retry_on_empty=False): @@ -646,7 +675,7 @@ def get_local_zone(zonegroup): if zonegroup_info is None: hookenv.log("Failed to fetch zonegroup ({}) info".format(zonegroup), level=hookenv.ERROR) - return None + return None, None # zonegroup info always contains self name and zones list so fetching # directly is safe. @@ -660,7 +689,7 @@ def get_local_zone(zonegroup): "No local zone configured for zonegroup ({})".format(zonegroup), level=hookenv.ERROR ) - return None + return None, None def rename_multisite_config(zonegroups, new_zonegroup_name, diff --git a/unit_tests/test_ceph_radosgw_context.py b/unit_tests/test_ceph_radosgw_context.py index 9cad52e4..cfd07e07 100644 --- a/unit_tests/test_ceph_radosgw_context.py +++ b/unit_tests/test_ceph_radosgw_context.py @@ -33,6 +33,7 @@ TO_PATCH = [ 'determine_api_port', 'cmp_pkgrevno', 'leader_get', + 'multisite', 'utils', ] @@ -83,6 +84,17 @@ class MonContextTest(CharmTestCase): self.test_config.set('zonegroup', 'zonegroup1') self.test_config.set('realm', 'realmX') + @staticmethod + def plain_list_stub(key): + if key == "zone": + return ["default"] + if key == "zonegroup": + return ["zonegroup1"] + if key == "realm": + return ["realmX"] + else: + return [] + @patch.object(ceph, 'config', lambda *args: '{"client.radosgw.gateway": {"rgw init timeout": 60}}') @patch.object(context, 'ensure_host_resolvable_v6') @@ -104,6 +116,7 @@ class MonContextTest(CharmTestCase): self.relation_get.side_effect = _relation_get self.relation_ids.return_value = ['mon:6'] self.related_units.return_value = ['ceph/0', 'ceph/1', 'ceph/2'] + self.multisite.plain_list = self.plain_list_stub self.determine_api_port.return_value = 70 expect = { 'auth_supported': 'cephx', @@ -156,6 +169,7 @@ class MonContextTest(CharmTestCase): self.relation_get.side_effect = _relation_get self.relation_ids.return_value = ['mon:6'] + self.multisite.plain_list = self.plain_list_stub self.related_units.return_value = ['ceph-proxy/0'] self.determine_api_port.return_value = 70 expect = { @@ -219,6 +233,7 @@ class MonContextTest(CharmTestCase): self.relation_get.side_effect = _relation_get self.relation_ids.return_value = ['mon:6'] self.related_units.return_value = ['ceph/0', 'ceph/1', 'ceph/2'] + self.multisite.plain_list = self.plain_list_stub self.determine_api_port.return_value = 70 expect = { 'auth_supported': 'none', @@ -264,6 +279,7 @@ class MonContextTest(CharmTestCase): self.relation_ids.return_value = ['mon:6'] self.related_units.return_value = ['ceph/0', 'ceph/1', 'ceph/2'] self.determine_api_port.return_value = 70 + self.multisite.plain_list = self.plain_list_stub expect = { 'auth_supported': 'cephx', 'hostname': 'testhost', @@ -365,6 +381,7 @@ class MonContextTest(CharmTestCase): self.relation_get.side_effect = _relation_get self.relation_ids.return_value = ['mon:6'] self.related_units.return_value = ['ceph/0', 'ceph/1', 'ceph/2'] + self.multisite.plain_list = self.plain_list_stub self.determine_api_port.return_value = 70 expect = { 'auth_supported': 'cephx',