From cb70cf4c5f5e5dfca5f232ea48918de7848d1d25 Mon Sep 17 00:00:00 2001 From: utkarshbhatthere Date: Tue, 30 Aug 2022 10:36:53 +0000 Subject: [PATCH] Adds support for scaling down multisite rgw system Adds implementation for relation-departed hooks to cleanly remove participant sites from the multisite system. The replication between zones is stopped and both zones split up to continue as separate master zones. Change-Id: I420f7933db55f3004f752949b5c09b1b79774f64 func-test-pr: https://github.com/openstack-charmers/zaza-openstack-tests/pull/863 --- hooks/hooks.py | 47 ++++++++++++++++++++++ hooks/multisite.py | 75 ++++++++++++++++++++++++++++++++---- hooks/utils.py | 4 +- unit_tests/test_hooks.py | 41 +++++++++++++++++++- unit_tests/test_multisite.py | 30 +++++++++++++++ 5 files changed, 188 insertions(+), 9 deletions(-) diff --git a/hooks/hooks.py b/hooks/hooks.py index eaea048e..7c5e9cd0 100755 --- a/hooks/hooks.py +++ b/hooks/hooks.py @@ -821,6 +821,53 @@ def master_relation_joined(relation_id=None): secret=secret) +@hooks.hook('master-relation-departed') +@hooks.hook('slave-relation-departed') +def multisite_relation_departed(): + if not is_leader(): + log('Cannot remove multisite relation, this unit is not the leader') + return + + if not ready_for_service(legacy=False): + raise RuntimeError("Leader unit not ready for service.") + + zone = config('zone') + zonegroup = config('zonegroup') + realm = config('realm') + + # If config zone/zonegroup not present on site, + # remove-relation is called prematurely + if not multisite.is_multisite_configured(zone=zone, + zonegroup=zonegroup): + log('Multisite is not configured, skipping scaledown.') + return + + zonegroup_info = multisite.get_zonegroup_info(zonegroup) + # remove other zones from zonegroup + for zone_info in zonegroup_info['zones']: + if zone_info['name'] is not zone: + multisite.remove_zone_from_zonegroup( + zone_info['name'], zonegroup + ) + + # modify self as master zone. + multisite.modify_zone(zone, default=True, master=True, + zonegroup=zonegroup) + + # Update period. + multisite.update_period( + fatal=True, zonegroup=zonegroup, + zone=zone, realm=realm + ) + + # Verify multisite is not configured. + if multisite.is_multisite_configured(zone=zone, + zonegroup=zonegroup): + status_set(WORKLOAD_STATES.BLOCKED, + "Failed to do a clean scaledown.") + raise RuntimeError("Residual multisite config at local site.") + + @hooks.hook('slave-relation-changed') def slave_relation_changed(relation_id=None, unit=None): if not is_leader(): diff --git a/hooks/multisite.py b/hooks/multisite.py index 590fdeb0..fc4200f6 100644 --- a/hooks/multisite.py +++ b/hooks/multisite.py @@ -370,7 +370,60 @@ def modify_zone(name, endpoints=None, default=False, master=False, return None -def update_period(fatal=True, zonegroup=None, zone=None): +def remove_zone_from_zonegroup(zone, zonegroup): + """Remove RADOS Gateway zone from provided parent zonegroup + + Removal is different from deletion, this operation removes zone/zonegroup + affiliation but does not delete the actual zone. + + :param zonegroup: parent zonegroup name + :type zonegroup: str + :param zone: zone name + :type zone: str + :return: modified zonegroup config + :rtype: dict + """ + cmd = [ + RGW_ADMIN, '--id={}'.format(_key_name()), + 'zonegroup', 'remove', + '--rgw-zonegroup={}'.format(zonegroup), + '--rgw-zone={}'.format(zone), + ] + try: + result = _check_output(cmd) + return json.loads(result) + except (TypeError, subprocess.CalledProcessError) as exc: + raise RuntimeError( + "Error removing zone {} from zonegroup {}. Result: {}" + .format(zone, zonegroup, result)) from exc + + +def add_zone_to_zonegroup(zone, zonegroup): + """Add RADOS Gateway zone to provided zonegroup + + :param zonegroup: parent zonegroup name + :type zonegroup: str + :param zone: zone name + :type zone: str + :return: modified zonegroup config + :rtype: dict + """ + cmd = [ + RGW_ADMIN, '--id={}'.format(_key_name()), + 'zonegroup', 'add', + '--rgw-zonegroup={}'.format(zonegroup), + '--rgw-zone={}'.format(zone), + ] + try: + result = _check_output(cmd) + return json.loads(result) + except (TypeError, subprocess.CalledProcessError) as exc: + raise RuntimeError( + "Error adding zone {} from zonegroup {}. Result: {}" + .format(zone, zonegroup, result)) from exc + + +def update_period(fatal=True, zonegroup=None, zone=None, realm=None): """Update RADOS Gateway configuration period :param fatal: In failure case, whether CalledProcessError is to be raised. @@ -379,6 +432,8 @@ def update_period(fatal=True, zonegroup=None, zone=None): :type zonegroup: str :param zone: zone name :type zone: str + :param realm: realm name + :type realm: str """ cmd = [ RGW_ADMIN, '--id={}'.format(_key_name()), @@ -388,6 +443,8 @@ def update_period(fatal=True, zonegroup=None, zone=None): cmd.append('--rgw-zonegroup={}'.format(zonegroup)) if zone is not None: cmd.append('--rgw-zone={}'.format(zone)) + if realm is not None: + cmd.append('--rgw-realm={}'.format(realm)) if fatal: _check_call(cmd) else: @@ -641,17 +698,21 @@ def is_multisite_configured(zone, zonegroup): :rtype: Boolean """ - if zone not in list_zones(): - hookenv.log("No local zone found with name ({})".format(zonegroup), - level=hookenv.ERROR) + local_zones = list_zones() + if zone not in local_zones: + hookenv.log("zone {} not found in local zones {}" + .format(zone, local_zones), level=hookenv.ERROR) return False - if zonegroup not in list_zonegroups(): - hookenv.log("No zonegroup found with name ({})".format(zonegroup), - level=hookenv.ERROR) + local_zonegroups = list_zonegroups() + if zonegroup not in local_zonegroups: + hookenv.log("zonegroup {} not found in local zonegroups {}" + .format(zonegroup, local_zonegroups), level=hookenv.ERROR) return False sync_status = get_sync_status() + hookenv.log("Multisite sync status {}".format(sync_status), + level=hookenv.DEBUG) if sync_status is not None: return ('data sync source:' in sync_status) diff --git a/hooks/utils.py b/hooks/utils.py index 00695154..80da68fc 100644 --- a/hooks/utils.py +++ b/hooks/utils.py @@ -219,7 +219,9 @@ def check_optional_config_and_relations(configs): leader_get('restart_nonce')) # Any realm or zonegroup config is present, multisite checks can be done. - if (config('realm') or config('zonegroup')): + # zone config can't be used because it's used by default. + if config('realm') or config('zonegroup') or relation_ids('master') \ + or relation_ids('slave'): # All of Realm, zonegroup, and zone must be configured. if not all(multisite_config): return ('blocked', diff --git a/unit_tests/test_hooks.py b/unit_tests/test_hooks.py index 416ea84f..cec082cf 100644 --- a/unit_tests/test_hooks.py +++ b/unit_tests/test_hooks.py @@ -11,7 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - +import json from unittest.mock import ( patch, call, MagicMock, ANY ) @@ -75,6 +75,25 @@ TO_PATCH = [ ] +# Stub Methods +def get_zonegroup_stub(): + # populate dummy zones info + zone_one = {} + zone_one['id'] = "test_zone_id_one" + zone_one['name'] = "testzone" + + zone_two = {} + zone_two['id'] = "test_zone_id_two" + zone_two['name'] = "testzone_two" + + # populate dummy zonegroup info + zonegroup = {} + zonegroup['name'] = "testzonegroup" + zonegroup['master_zone'] = "test_zone_id_one" + zonegroup['zones'] = [zone_one, zone_two] + return zonegroup + + class CephRadosGWTests(CharmTestCase): def setUp(self): @@ -793,6 +812,26 @@ class MasterMultisiteTests(CephRadosMultisiteTests): ) self.multisite.list_realms.assert_not_called() + @patch.object(json, 'loads') + def test_multisite_relation_departed(self, json_loads): + for k, v in self._complete_config.items(): + self.test_config.set(k, v) + self.is_leader.return_value = True + # Multisite is configured at first but then disabled. + self.multisite.is_multisite_configured.side_effect = [True, False] + self.multisite.get_zonegroup_info.return_value = get_zonegroup_stub() + # json.loads() raises TypeError for mock objects. + json_loads.returnvalue = [] + ceph_hooks.multisite_relation_departed() + + self.multisite.modify_zone.assert_called_once_with( + 'testzone', default=True, master=True, zonegroup='testzonegroup' + ) + self.multisite.update_period.assert_called_once_with( + fatal=True, zonegroup='testzonegroup', + zone='testzone', realm='testrealm' + ) + class SlaveMultisiteTests(CephRadosMultisiteTests): diff --git a/unit_tests/test_multisite.py b/unit_tests/test_multisite.py index 5374e422..cb030bec 100644 --- a/unit_tests/test_multisite.py +++ b/unit_tests/test_multisite.py @@ -14,6 +14,7 @@ import inspect import os +import json from unittest import mock import multisite @@ -34,6 +35,7 @@ def get_zonegroup_stub(): # populate dummy zonegroup info zonegroup = {} zonegroup['name'] = "test_zonegroup" + zonegroup['master_zone'] = "test_zone_id" zonegroup['zones'] = [zone] return zonegroup @@ -441,6 +443,34 @@ class TestMultisiteHelpers(CharmTestCase): '--rgw-zonegroup=test_zonegroup', ]) + @mock.patch.object(json, 'loads') + def test_remove_zone_from_zonegroup(self, json_loads): + # json.loads() raises TypeError for mock objects. + json_loads.returnvalue = [] + multisite.remove_zone_from_zonegroup( + 'test_zone', 'test_zonegroup', + ) + + self.subprocess.check_output.assert_called_with([ + 'radosgw-admin', '--id=rgw.testhost', + 'zonegroup', 'remove', '--rgw-zonegroup=test_zonegroup', + '--rgw-zone=test_zone', + ], stderr=mock.ANY) + + @mock.patch.object(json, 'loads') + def test_add_zone_from_zonegroup(self, json_loads): + # json.loads() raises TypeError for mock objects. + json_loads.returnvalue = [] + multisite.add_zone_to_zonegroup( + 'test_zone', 'test_zonegroup', + ) + + self.subprocess.check_output.assert_called_with([ + 'radosgw-admin', '--id=rgw.testhost', + 'zonegroup', 'add', '--rgw-zonegroup=test_zonegroup', + '--rgw-zone=test_zone', + ], stderr=mock.ANY) + @mock.patch.object(multisite, 'list_zonegroups') @mock.patch.object(multisite, 'get_local_zone') @mock.patch.object(multisite, 'list_buckets')