From 248e0bcb7a5c93c88da3deca3c72b0d4d78010aa Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Mon, 15 Mar 2021 17:05:45 -0500 Subject: [PATCH] mgr/orchestrator: drop $realm.$zone naming convention - Let users name the rgw service(s) whatever they like - Make the rgw_zone and rgw_realm arguments optional, so that they can omit them and let radosgw start up in the generic single-cluster configuration (whichk, notably, has no realm). - Continue to set the rgw_realm and rgw_zone options for the daemon(s), but only when those values are specified. - Adjust the CLI accordingly. Drop the subcluster argument, which was only used to generate a service_id. - Adjust rook. This is actually a simplification and improved mapping onto the rook CRD, *except* that for octopus we enforced the realm.zone naming *and* realm==zone. I doubt a single user actually did this so it is not be worth dealing with, but we need a special case for where there is a . in the service name. Signed-off-by: Sage Weil (cherry picked from commit af0216d93fb522d239b10b5d7d18208ee02fed77) # Conflicts: # src/pybind/mgr/rook/rook_cluster.py - pacific branch does not have type annotation on apply_objectstore() --- src/cephadm/vstart-smoke.sh | 2 +- .../mgr/cephadm/services/cephadmservice.py | 29 ++++++----- src/pybind/mgr/cephadm/tests/test_cephadm.py | 23 ++++----- src/pybind/mgr/cephadm/tests/test_spec.py | 51 ++++++++++--------- src/pybind/mgr/orchestrator/module.py | 16 +++--- src/pybind/mgr/rook/rook_cluster.py | 19 ++++--- .../ceph/deployment/service_spec.py | 24 --------- .../ceph/tests/test_service_spec.py | 6 +-- 8 files changed, 76 insertions(+), 94 deletions(-) diff --git a/src/cephadm/vstart-smoke.sh b/src/cephadm/vstart-smoke.sh index 4eb879bdb85ef..ecdb59d1880a1 100755 --- a/src/cephadm/vstart-smoke.sh +++ b/src/cephadm/vstart-smoke.sh @@ -64,7 +64,7 @@ bin/ceph orch apply grafana 1 while ! bin/ceph dashboard get-grafana-api-url | grep $host ; do sleep 1 ; done -bin/ceph orch apply rgw myrealm myzone 1 +bin/ceph orch apply rgw foo --placement=1 bin/ceph orch ps bin/ceph orch ls diff --git a/src/pybind/mgr/cephadm/services/cephadmservice.py b/src/pybind/mgr/cephadm/services/cephadmservice.py index 51e7fbc2d921a..767d77671ba8c 100644 --- a/src/pybind/mgr/cephadm/services/cephadmservice.py +++ b/src/pybind/mgr/cephadm/services/cephadmservice.py @@ -661,19 +661,22 @@ class RgwService(CephService): def config(self, spec: RGWSpec, rgw_id: str) -> None: # type: ignore assert self.TYPE == spec.service_type - # ensure rgw_realm and rgw_zone is set for these daemons - ret, out, err = self.mgr.check_mon_command({ - 'prefix': 'config set', - 'who': f"{utils.name_to_config_section('rgw')}.{spec.service_id}", - 'name': 'rgw_zone', - 'value': spec.rgw_zone, - }) - ret, out, err = self.mgr.check_mon_command({ - 'prefix': 'config set', - 'who': f"{utils.name_to_config_section('rgw')}.{spec.rgw_realm}", - 'name': 'rgw_realm', - 'value': spec.rgw_realm, - }) + # set rgw_realm and rgw_zone, if present + if spec.rgw_realm: + ret, out, err = self.mgr.check_mon_command({ + 'prefix': 'config set', + 'who': f"{utils.name_to_config_section('rgw')}.{spec.service_id}", + 'name': 'rgw_realm', + 'value': spec.rgw_realm, + }) + if spec.rgw_zone: + ret, out, err = self.mgr.check_mon_command({ + 'prefix': 'config set', + 'who': f"{utils.name_to_config_section('rgw')}.{spec.service_id}", + 'name': 'rgw_zone', + 'value': spec.rgw_zone, + }) + ret, out, err = self.mgr.check_mon_command({ 'prefix': 'config set', 'who': f"{utils.name_to_config_section('rgw')}.{spec.service_id}", diff --git a/src/pybind/mgr/cephadm/tests/test_cephadm.py b/src/pybind/mgr/cephadm/tests/test_cephadm.py index cbc68aca9c445..8db49b269ba1a 100644 --- a/src/pybind/mgr/cephadm/tests/test_cephadm.py +++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py @@ -143,10 +143,6 @@ class TestCephadm(object): 'count': 1, 'hosts': ["test"] }, - 'spec': { - 'rgw_realm': 'r', - 'rgw_zone': 'z', - }, 'service_id': 'r.z', 'service_name': 'rgw.r.z', 'service_type': 'rgw', @@ -660,20 +656,20 @@ class TestCephadm(object): def test_rgw_update(self, cephadm_module): with with_host(cephadm_module, 'host1'): with with_host(cephadm_module, 'host2'): - with with_service(cephadm_module, RGWSpec(rgw_realm='realm', rgw_zone='zone1', unmanaged=True)): + with with_service(cephadm_module, RGWSpec(service_id="foo", unmanaged=True)): ps = PlacementSpec(hosts=['host1'], count=1) c = cephadm_module.add_rgw( - RGWSpec(rgw_realm='realm', rgw_zone='zone1', placement=ps)) + RGWSpec(service_id="foo", placement=ps)) [out] = wait(cephadm_module, c) - match_glob(out, "Deployed rgw.realm.zone1.host1.* on host 'host1'") + match_glob(out, "Deployed rgw.foo.* on host 'host1'") ps = PlacementSpec(hosts=['host1', 'host2'], count=2) r = CephadmServe(cephadm_module)._apply_service( - RGWSpec(rgw_realm='realm', rgw_zone='zone1', placement=ps)) + RGWSpec(service_id="foo", placement=ps)) assert r - assert_rm_daemon(cephadm_module, 'rgw.realm.zone1', 'host1') - assert_rm_daemon(cephadm_module, 'rgw.realm.zone1', 'host2') + assert_rm_daemon(cephadm_module, 'rgw.foo', 'host1') + assert_rm_daemon(cephadm_module, 'rgw.foo', 'host2') @mock.patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm( json.dumps([ @@ -706,7 +702,7 @@ class TestCephadm(object): (ServiceSpec('alertmanager'), CephadmOrchestrator.add_alertmanager), (ServiceSpec('rbd-mirror'), CephadmOrchestrator.add_rbd_mirror), (ServiceSpec('mds', service_id='fsname'), CephadmOrchestrator.add_mds), - (RGWSpec(rgw_realm='realm', rgw_zone='zone'), CephadmOrchestrator.add_rgw), + (RGWSpec(service_id="foo"), CephadmOrchestrator.add_rgw), (ServiceSpec('cephadm-exporter'), CephadmOrchestrator.add_cephadm_exporter), ] ) @@ -847,13 +843,14 @@ class TestCephadm(object): )] ) ), CephadmOrchestrator.apply_mds), - (RGWSpec(rgw_realm='realm', rgw_zone='zone'), CephadmOrchestrator.apply_rgw), + (RGWSpec(service_id='foo'), CephadmOrchestrator.apply_rgw), (RGWSpec( + service_id='bar', rgw_realm='realm', rgw_zone='zone', placement=PlacementSpec( hosts=[HostPlacementSpec( hostname='test', - name='realm.zone.a', + name='bar', network='' )] ) diff --git a/src/pybind/mgr/cephadm/tests/test_spec.py b/src/pybind/mgr/cephadm/tests/test_spec.py index 2018fa2cf3598..8eb8e8b7b9d55 100644 --- a/src/pybind/mgr/cephadm/tests/test_spec.py +++ b/src/pybind/mgr/cephadm/tests/test_spec.py @@ -72,8 +72,7 @@ from orchestrator import DaemonDescription, OrchestratorError "service_type": "rgw", "service_id": "default-rgw-realm.eu-central-1.1", "rgw_realm": "default-rgw-realm", - "rgw_zone": "eu-central-1", - "subcluster": "1" + "rgw_zone": "eu-central-1" }, { "service_type": "osd", @@ -296,40 +295,38 @@ def test_dd_octopus(dd_json): # https://tracker.ceph.com/issues/44934 ( RGWSpec( + service_id="foo", rgw_realm="default-rgw-realm", rgw_zone="eu-central-1", - subcluster='1', ), DaemonDescription( daemon_type='rgw', - daemon_id="default-rgw-realm.eu-central-1.1.ceph-001.ytywjo", + daemon_id="foo.ceph-001.ytywjo", hostname="ceph-001", ), True ), ( - # no subcluster + # no realm RGWSpec( - rgw_realm="default-rgw-realm", + service_id="foo.bar", rgw_zone="eu-central-1", ), DaemonDescription( daemon_type='rgw', - daemon_id="default-rgw-realm.eu-central-1.ceph-001.ytywjo", + daemon_id="foo.bar.ceph-001.ytywjo", hostname="ceph-001", ), True ), ( - # with tld + # no realm or zone RGWSpec( - rgw_realm="default-rgw-realm", - rgw_zone="eu-central-1", - subcluster='1', + service_id="bar", ), DaemonDescription( daemon_type='rgw', - daemon_id="default-rgw-realm.eu-central-1.1.host.domain.tld.ytywjo", + daemon_id="bar.host.domain.tld.ytywjo", hostname="host.domain.tld", ), True @@ -337,8 +334,7 @@ def test_dd_octopus(dd_json): ( # explicit naming RGWSpec( - rgw_realm="realm", - rgw_zone="zone", + service_id="realm.zone", ), DaemonDescription( daemon_type='rgw', @@ -351,9 +347,20 @@ def test_dd_octopus(dd_json): # without host RGWSpec( service_type='rgw', - rgw_realm="default-rgw-realm", - rgw_zone="eu-central-1", - subcluster='1', + service_id="foo", + ), + DaemonDescription( + daemon_type='rgw', + daemon_id="foo.hostname.ytywjo", + hostname=None, + ), + False + ), + ( + # without host (2) + RGWSpec( + service_type='rgw', + service_id="default-rgw-realm.eu-central-1.1", ), DaemonDescription( daemon_type='rgw', @@ -363,16 +370,14 @@ def test_dd_octopus(dd_json): False ), ( - # zone contains hostname - # https://tracker.ceph.com/issues/45294 + # service_id contains hostname + # (sort of) https://tracker.ceph.com/issues/45294 RGWSpec( - rgw_realm="default.rgw.realm", - rgw_zone="ceph.001", - subcluster='1', + service_id="default.rgw.realm.ceph.001", ), DaemonDescription( daemon_type='rgw', - daemon_id="default.rgw.realm.ceph.001.1.ceph.001.ytywjo", + daemon_id="default.rgw.realm.ceph.001.ceph.001.ytywjo", hostname="ceph.001", ), True diff --git a/src/pybind/mgr/orchestrator/module.py b/src/pybind/mgr/orchestrator/module.py index dc68d374f68cf..9980c87cd4fe6 100644 --- a/src/pybind/mgr/orchestrator/module.py +++ b/src/pybind/mgr/orchestrator/module.py @@ -906,9 +906,7 @@ Usage: @_cli_write_command('orch daemon add rgw') def _rgw_add(self, - realm_name: str, - zone_name: str, - subcluster: Optional[str] = None, + svc_id: str, port: Optional[int] = None, ssl: bool = False, placement: Optional[str] = None, @@ -918,9 +916,7 @@ Usage: raise OrchestratorValidationError('unrecognized command -i; -h or --help for usage') spec = RGWSpec( - rgw_realm=realm_name, - rgw_zone=zone_name, - subcluster=subcluster, + service_id=svc_id, rgw_frontend_port=port, ssl=ssl, placement=PlacementSpec.from_string(placement), @@ -1096,9 +1092,9 @@ Usage: @_cli_write_command('orch apply rgw') def _apply_rgw(self, - realm_name: str, - zone_name: str, - subcluster: Optional[str] = None, + svc_id: str, + realm_name: Optional[str] = None, + zone_name: Optional[str] = None, port: Optional[int] = None, ssl: bool = False, placement: Optional[str] = None, @@ -1111,9 +1107,9 @@ Usage: raise OrchestratorValidationError('unrecognized command -i; -h or --help for usage') spec = RGWSpec( + service_id=svc_id, rgw_realm=realm_name, rgw_zone=zone_name, - subcluster=subcluster, rgw_frontend_port=port, ssl=ssl, placement=PlacementSpec.from_string(placement), diff --git a/src/pybind/mgr/rook/rook_cluster.py b/src/pybind/mgr/rook/rook_cluster.py index 197e1318dd1a2..acf178c552240 100644 --- a/src/pybind/mgr/rook/rook_cluster.py +++ b/src/pybind/mgr/rook/rook_cluster.py @@ -416,14 +416,19 @@ class RookCluster(object): _update_fs, _create_fs) def apply_objectstore(self, spec): + assert spec.service_id is not None - # FIXME: service_id is $realm.$zone, but rook uses realm - # $crname and zone $crname. The '.' will confuse kubernetes. - # For now, assert that realm==zone. - (realm, zone) = spec.service_id.split('.', 1) - assert realm == zone - assert spec.subcluster is None - name = realm + name = spec.service_id + + if '.' in spec.service_id: + # rook does not like . in the name. this is could + # there because it is a legacy rgw spec that was named + # like $realm.$zone, except that I doubt there were any + # users of this code. Instead, focus on future users and + # translate . to - (fingers crossed!) instead. + name = spec.service_id.replace('.', '-') + + # FIXME: pass realm and/or zone through to the CR def _create_zone() -> cos.CephObjectStore: port = None diff --git a/src/python-common/ceph/deployment/service_spec.py b/src/python-common/ceph/deployment/service_spec.py index 2e5762c43542b..3af3fb632a816 100644 --- a/src/python-common/ceph/deployment/service_spec.py +++ b/src/python-common/ceph/deployment/service_spec.py @@ -657,7 +657,6 @@ class RGWSpec(ServiceSpec): placement: Optional[PlacementSpec] = None, rgw_realm: Optional[str] = None, rgw_zone: Optional[str] = None, - subcluster: Optional[str] = None, rgw_frontend_port: Optional[int] = None, rgw_frontend_ssl_certificate: Optional[List[str]] = None, rgw_frontend_ssl_key: Optional[List[str]] = None, @@ -667,18 +666,6 @@ class RGWSpec(ServiceSpec): config: Optional[Dict[str, str]] = None, ): assert service_type == 'rgw', service_type - if service_id: - a = service_id.split('.', 2) - rgw_realm = a[0] - if len(a) > 1: - rgw_zone = a[1] - if len(a) > 2: - subcluster = a[2] - else: - if subcluster: - service_id = '%s.%s.%s' % (rgw_realm, rgw_zone, subcluster) - else: - service_id = '%s.%s' % (rgw_realm, rgw_zone) super(RGWSpec, self).__init__( 'rgw', service_id=service_id, placement=placement, unmanaged=unmanaged, @@ -686,7 +673,6 @@ class RGWSpec(ServiceSpec): self.rgw_realm = rgw_realm self.rgw_zone = rgw_zone - self.subcluster = subcluster self.rgw_frontend_port = rgw_frontend_port self.rgw_frontend_ssl_certificate = rgw_frontend_ssl_certificate self.rgw_frontend_ssl_key = rgw_frontend_ssl_key @@ -710,16 +696,6 @@ class RGWSpec(ServiceSpec): ports.append(f"port={self.get_port()}") return f'beast {" ".join(ports)}' - def validate(self) -> None: - super(RGWSpec, self).validate() - - if not self.rgw_realm: - raise ServiceSpecValidationError( - 'Cannot add RGW: No realm specified') - if not self.rgw_zone: - raise ServiceSpecValidationError( - 'Cannot add RGW: No zone specified') - yaml.add_representer(RGWSpec, ServiceSpec.yaml_representer) diff --git a/src/python-common/ceph/tests/test_service_spec.py b/src/python-common/ceph/tests/test_service_spec.py index 1d4bbf1e95d43..b871e0ddc262b 100644 --- a/src/python-common/ceph/tests/test_service_spec.py +++ b/src/python-common/ceph/tests/test_service_spec.py @@ -161,7 +161,6 @@ placement: spec: rgw_realm: default-rgw-realm rgw_zone: eu-central-1 - subcluster: '1' --- service_type: osd service_id: osd_spec_default @@ -236,9 +235,10 @@ spec: ), ( ServiceSpec( - service_type='rgw' + service_type='rgw', + service_id='foo', ), - RGWSpec(), + RGWSpec(service_id='foo'), True ), ]) -- 2.39.5