From b94c2b685a3e70daed0be7b83199b7183e3e1983 Mon Sep 17 00:00:00 2001 From: Redouane Kachach Date: Tue, 11 Oct 2022 10:10:16 +0200 Subject: [PATCH] Addressing adking review comments Signed-off-by: Redouane Kachach --- doc/mgr/rgw.rst | 12 +-- src/pybind/mgr/cephadm/serve.py | 50 ++++++----- src/pybind/mgr/rgw/module.py | 89 ++++++++++--------- .../ceph/deployment/service_spec.py | 6 ++ src/python-common/ceph/rgw/rgwam_core.py | 13 +-- 5 files changed, 97 insertions(+), 73 deletions(-) diff --git a/doc/mgr/rgw.rst b/doc/mgr/rgw.rst index 5632fa64f71..97624fcac62 100644 --- a/doc/mgr/rgw.rst +++ b/doc/mgr/rgw.rst @@ -77,9 +77,9 @@ the `ceph rgw realm tokens` output: } ] -User can use the token to create and synchronize a secondary zones -on another cluster with the master zone by using `ceph rgw zone create` -command and proving the corresponding token. +User can use the token to pull a realm to create secondary zone on a +different cluster that syncs with the master zone on the primary cluster +by using `ceph rgw zone create` command and providing the corresponding token. Following is an example of zone spec file: @@ -101,9 +101,9 @@ Following is an example of zone spec file: ceph rgw zone create -i zone-spec.yaml -.. note:: The spec file used by RGW has the same format as the one used by cephadm. Thus, - the user can provide any cephadm rgw supported parameter as any other advanced - configuration items such as SSL certificates etc. +.. note:: The spec file used by RGW has the same format as the one used by the orchestrator. Thus, + the user can provide any orchestration supported rgw parameters including advanced + configuration features such as SSL certificates etc. Commands -------- diff --git a/src/pybind/mgr/cephadm/serve.py b/src/pybind/mgr/cephadm/serve.py index 49c614115d4..7e46fccb540 100644 --- a/src/pybind/mgr/cephadm/serve.py +++ b/src/pybind/mgr/cephadm/serve.py @@ -561,6 +561,35 @@ class CephadmServe: self.mgr.set_health_warning('CEPHADM_FAILED_SET_OPTION', f'Failed to set {len(options_failed_to_set)} option(s)', len( options_failed_to_set), options_failed_to_set) + def _update_rgw_endpoints(self, rgw_spec: RGWSpec) -> None: + + if not rgw_spec.update_endpoints or rgw_spec.rgw_realm_token is None: + return + + ep = [] + protocol = 'https' if rgw_spec.ssl else 'http' + for s in self.mgr.cache.get_daemons_by_service(rgw_spec.service_name()): + if s.ports: + for p in s.ports: + ep.append(f'{protocol}://{s.hostname}:{p}') + zone_update_cmd = { + 'prefix': 'rgw zone update', + 'realm_name': rgw_spec.rgw_realm, + 'zonegroup_name': rgw_spec.rgw_zonegroup, + 'zone_name': rgw_spec.rgw_zone, + 'realm_token': rgw_spec.rgw_realm_token, + 'endpoints': ep, + } + self.log.debug(f'rgw cmd: {zone_update_cmd}') + rc, out, err = self.mgr.mon_command(zone_update_cmd) + rgw_spec.update_endpoints = (rc != 0) # keep trying on failure + if rc != 0: + self.log.error(f'Error when trying to update rgw zone {err}') + self.mgr.set_health_warning('CEPHADM_RGW', 'Cannot update rgw endpoints', 1, + [f'Cannot update rgw endpoints for daemon {rgw_spec.service_name()}']) + else: + self.mgr.remove_health_warning('CEPHADM_RGW') + def _apply_service(self, spec: ServiceSpec) -> bool: """ Schedule a service. Deploy new daemons or remove old ones, depending @@ -815,26 +844,7 @@ class CephadmServe: self.mgr._schedule_daemon_action(active_mgr.name(), 'restart') if service_type == 'rgw': - rgw_spec = cast(RGWSpec, spec) - if rgw_spec.update_endpoints and rgw_spec.rgw_realm_token is not None: - ep = [] - for s in self.mgr.cache.get_daemons_by_service(rgw_spec.service_name()): - if s.ports: - for p in s.ports: - ep.append(f'http://{s.hostname}:{p}') - zone_update_cmd = { - 'prefix': 'rgw zone update', - 'realm_name': rgw_spec.rgw_realm, - 'zonegroup_name': rgw_spec.rgw_zonegroup, - 'zone_name': rgw_spec.rgw_zone, - 'realm_token': rgw_spec.rgw_realm_token, - 'endpoints': ep, - } - self.log.debug(f'rgw cmd: {zone_update_cmd}') - rc, out, err = self.mgr.mon_command(zone_update_cmd) - rgw_spec.update_endpoints = (rc != 0) # keep trying on failure - if rc != 0: - self.log.error(f'Error when trying to update rgw zone {err}.. keep trying') + self._update_rgw_endpoints(cast(RGWSpec, spec)) # remove any? def _ok_to_stop(remove_daemons: List[orchestrator.DaemonDescription]) -> bool: diff --git a/src/pybind/mgr/rgw/module.py b/src/pybind/mgr/rgw/module.py index b402b37f413..e47b9b5a83b 100644 --- a/src/pybind/mgr/rgw/module.py +++ b/src/pybind/mgr/rgw/module.py @@ -128,29 +128,30 @@ class Module(orchestrator.OrchestratorClientMixin, MgrModule): start_radosgw: Optional[bool] = True, inbuf: Optional[str] = None): """Bootstrap new rgw realm, zonegroup, and zone""" - try: - if inbuf: - rgw_specs = self._parse_rgw_specs(inbuf) - elif (realm_name and zonegroup_name and zone_name): - placement_spec = PlacementSpec.from_string(placement) if placement else None - rgw_specs = [RGWSpec(rgw_realm=realm_name, - rgw_zonegroup=zonegroup_name, - rgw_zone=zone_name, - rgw_frontend_port=port, - placement=placement_spec)] - else: - return HandleCommandResult(retval=-errno.EINVAL, stdout='', stderr='Invalid arguments: -h or --help for usage') + if inbuf: + rgw_specs = self._parse_rgw_specs(inbuf) + elif (realm_name and zonegroup_name and zone_name): + placement_spec = PlacementSpec.from_string(placement) if placement else None + rgw_specs = [RGWSpec(rgw_realm=realm_name, + rgw_zonegroup=zonegroup_name, + rgw_zone=zone_name, + rgw_frontend_port=port, + placement=placement_spec)] + else: + err_msg = 'Invalid arguments: either pass a spec with -i or provide the realm, zonegroup and zone.' + return HandleCommandResult(retval=-errno.EINVAL, stdout='', stderr=err_msg) + + try: for spec in rgw_specs: RGWAM(self.env).realm_bootstrap(spec, start_radosgw) - except RGWAMException as e: self.log.error('cmd run exception: (%d) %s' % (e.retcode, e.message)) return (e.retcode, e.message, e.stderr) return HandleCommandResult(retval=0, stdout="Realm(s) created correctly. Please, use 'ceph rgw realm tokens' to get the token.", stderr='') - def _parse_rgw_specs(self, inbuf: Optional[str] = None): + def _parse_rgw_specs(self, inbuf: Optional[str] = None) -> List[RGWSpec]: """Parse RGW specs from a YAML file.""" # YAML '---' document separator with no content generates # None entries in the output. Let's skip them silently. @@ -158,7 +159,6 @@ class Module(orchestrator.OrchestratorClientMixin, MgrModule): specs = [o for o in yaml_objs if o is not None] rgw_specs = [] for spec in specs: - # TODO(rkachach): should we use a new spec instead of RGWSpec here! rgw_spec = RGWSpec.from_json(spec) rgw_spec.validate() rgw_specs.append(rgw_spec) @@ -193,22 +193,26 @@ class Module(orchestrator.OrchestratorClientMixin, MgrModule): @CLICommand('rgw realm tokens', perm='r') def list_realm_tokens(self): - realms_info = [] - for realm_info in RGWAM(self.env).get_realms_info(): - if not realm_info['master_zone_id']: - realms_info.append({'realm': realm_info['realm_name'], 'token': 'realm has no master zone'}) - elif not realm_info['endpoint']: - realms_info.append({'realm': realm_info['realm_name'], 'token': 'master zone has no endpoint'}) - elif not (realm_info['access_key'] and realm_info['secret']): - realms_info.append({'realm': realm_info['realm_name'], 'token': 'master zone has no access/secret keys'}) - else: - keys = ['realm_name', 'realm_id', 'is_primary', 'endpoint', 'access_key', 'secret'] - realm_token = RealmToken(**{k: realm_info[k] for k in keys}) - realm_token_b = realm_token.to_json().encode('utf-8') - realm_token_s = base64.b64encode(realm_token_b).decode('utf-8') - realms_info.append({'realm': realm_info['realm_name'], 'token': realm_token_s}) + try: + realms_info = [] + for realm_info in RGWAM(self.env).get_realms_info(): + if not realm_info['master_zone_id']: + realms_info.append({'realm': realm_info['realm_name'], 'token': 'realm has no master zone'}) + elif not realm_info['endpoint']: + realms_info.append({'realm': realm_info['realm_name'], 'token': 'master zone has no endpoint'}) + elif not (realm_info['access_key'] and realm_info['secret']): + realms_info.append({'realm': realm_info['realm_name'], 'token': 'master zone has no access/secret keys'}) + else: + keys = ['realm_name', 'realm_id', 'is_primary', 'endpoint', 'access_key', 'secret'] + realm_token = RealmToken(**{k: realm_info[k] for k in keys}) + realm_token_b = realm_token.to_json().encode('utf-8') + realm_token_s = base64.b64encode(realm_token_b).decode('utf-8') + realms_info.append({'realm': realm_info['realm_name'], 'token': realm_token_s}) + except RGWAMException as e: + self.log.error(f'cmd run exception: ({e.retcode}) {e.message}') + return (e.retcode, e.message, e.stderr) - return HandleCommandResult(retval=0, stdout=json.dumps(realms_info), stderr='') + return HandleCommandResult(retval=0, stdout=json.dumps(realms_info, indent=4), stderr='') @CLICommand('rgw zone update', perm='rw') def update_zone_info(self, realm_name: str, zonegroup_name: str, zone_name: str, realm_token: str, endpoints: List[str]): @@ -233,23 +237,24 @@ class Module(orchestrator.OrchestratorClientMixin, MgrModule): start_radosgw: Optional[bool] = True, inbuf: Optional[str] = None): """Bootstrap new rgw zone that syncs with existing zone""" - try: - if inbuf: - rgw_specs = self._parse_rgw_specs(inbuf) - elif (zone_name and realm_token): - placement_spec = PlacementSpec.from_string(placement) if placement else None - rgw_specs = [RGWSpec(rgw_realm_token=realm_token, - rgw_zone=zone_name, - rgw_frontend_port=port, - placement=placement_spec)] - else: - return HandleCommandResult(retval=-errno.EINVAL, stdout='', stderr='Invalid arguments: -h or --help for usage') + if inbuf: + rgw_specs = self._parse_rgw_specs(inbuf) + elif (zone_name and realm_token): + placement_spec = PlacementSpec.from_string(placement) if placement else None + rgw_specs = [RGWSpec(rgw_realm_token=realm_token, + rgw_zone=zone_name, + rgw_frontend_port=port, + placement=placement_spec)] + else: + err_msg = 'Invalid arguments: either pass a spec with -i or provide the zone_name and realm_token.' + return HandleCommandResult(retval=-errno.EINVAL, stdout='', stderr=err_msg) + + try: for rgw_spec in rgw_specs: retval, out, err = RGWAM(self.env).zone_create(rgw_spec, start_radosgw) if retval != 0: break - except RGWAMException as e: self.log.error('cmd run exception: (%d) %s' % (e.retcode, e.message)) return (e.retcode, e.message, e.stderr) diff --git a/src/python-common/ceph/deployment/service_spec.py b/src/python-common/ceph/deployment/service_spec.py index 83e3c57c84b..66afb16f096 100644 --- a/src/python-common/ceph/deployment/service_spec.py +++ b/src/python-common/ceph/deployment/service_spec.py @@ -890,10 +890,16 @@ class RGWSpec(ServiceSpec): extra_container_args=extra_container_args, custom_configs=custom_configs) #: The RGW realm associated with this service. Needs to be manually created + #: if the spec is being applied directly to cephdam. In case of rgw module + #: the realm is created automatically. self.rgw_realm: Optional[str] = rgw_realm #: The RGW zonegroup associated with this service. Needs to be manually created + #: if the spec is being applied directly to cephdam. In case of rgw module + #: the zonegroup is created automatically. self.rgw_zonegroup: Optional[str] = rgw_zonegroup #: The RGW zone associated with this service. Needs to be manually created + #: if the spec is being applied directly to cephdam. In case of rgw module + #: the zone is created automatically. self.rgw_zone: Optional[str] = rgw_zone #: Port of the RGW daemons self.rgw_frontend_port: Optional[int] = rgw_frontend_port diff --git a/src/python-common/ceph/rgw/rgwam_core.py b/src/python-common/ceph/rgw/rgwam_core.py index 2bfaf97e55a..0f207b331be 100644 --- a/src/python-common/ceph/rgw/rgwam_core.py +++ b/src/python-common/ceph/rgw/rgwam_core.py @@ -195,7 +195,8 @@ class RealmOp: params = ['realm', 'list'] output = RGWAdminJSONCmd(ze).run(params) return output.get('realms') or [] - except RGWAMException: + except RGWAMException as e: + logging.info(f'Exception while listing realms {e.message}') # in case the realm list is empty an exception is raised return [] @@ -229,7 +230,8 @@ class ZonegroupOp: params = ['zonegroup', 'list'] output = RGWAdminJSONCmd(ze).run(params) return output.get('zonegroups') or [] - except RGWAMException: + except RGWAMException as e: + logging.info(f'Exception while listing zonegroups {e.message}') return [] def get(self, zonegroup: EntityKey = None): @@ -262,7 +264,8 @@ class ZoneOp: params = ['zone', 'list'] output = RGWAdminJSONCmd(ze).run(params) return output.get('zones') or [] - except RGWAMException: + except RGWAMException as e: + logging.info(f'Exception while listing zones {e.message}') return [] def get(self, zone: EntityKey): @@ -483,7 +486,7 @@ class RGWAM: uid_prefix='user-sys', is_system=True) sys_user = RGWUser(sys_user_info) - logging.info('Created system user: %s' % sys_user.uid) + logging.info(f'Created system user: {sys_user.uid} on {realm.name}/{zonegroup.name}/{zone.name}') access_key = sys_user.keys[0].access_key if sys_user and sys_user.keys else '' secret_key = sys_user.keys[0].secret_key if sys_user and sys_user.keys else '' sys_user.add_key(access_key, secret_key) @@ -495,7 +498,7 @@ class RGWAM: try: user_info = self.user_op().create(zone, zg, uid=uid, is_system=False) user = RGWUser(user_info) - logging.info('Created regular user: %s' % user.uid) + logging.info('Created regular user {user.uid} on {realm.name}/{zonegroup.name}/{zone.name}') return user except RGWAMException as e: raise RGWAMException('failed to create user', e)