From db765bd80608b7c6930a5111eb006b5d12f73de2 Mon Sep 17 00:00:00 2001 From: Redouane Kachach Date: Mon, 7 Feb 2022 19:17:55 +0100 Subject: [PATCH] mgr/cephadm: using MDSSPec instead of ServiceSpec Fixes: https://tracker.ceph.com/issues/54184 Signed-off-by: Redouane Kachach --- src/pybind/mgr/mds_autoscaler/module.py | 10 +++---- src/pybind/mgr/orchestrator/_interface.py | 4 +-- src/pybind/mgr/orchestrator/module.py | 16 ++++++++--- .../ceph/deployment/service_spec.py | 27 ++++++++++++++++++- .../ceph/tests/test_service_spec.py | 4 ++- 5 files changed, 48 insertions(+), 13 deletions(-) diff --git a/src/pybind/mgr/mds_autoscaler/module.py b/src/pybind/mgr/mds_autoscaler/module.py index 4d9165e478c..2f780059c0b 100644 --- a/src/pybind/mgr/mds_autoscaler/module.py +++ b/src/pybind/mgr/mds_autoscaler/module.py @@ -5,7 +5,7 @@ Automatically scale MDSs based on status of the file-system using the FSMap import logging from typing import Any, Optional from mgr_module import MgrModule, NotifyType -from ceph.deployment.service_spec import ServiceSpec +from orchestrator._interface import MDSSpec, ServiceSpec import orchestrator import copy @@ -32,12 +32,12 @@ class MDSAutoscaler(orchestrator.OrchestratorClientMixin, MgrModule): return completion.result[0] return None - def update_daemon_count(self, spec: ServiceSpec, fs_name: str, abscount: int) -> ServiceSpec: + def update_daemon_count(self, spec: ServiceSpec, fs_name: str, abscount: int) -> MDSSpec: ps = copy.deepcopy(spec.placement) ps.count = abscount - newspec = ServiceSpec(service_type=spec.service_type, - service_id=spec.service_id, - placement=ps) + newspec = MDSSpec(service_type=spec.service_type, + service_id=spec.service_id, + placement=ps) return newspec def get_required_standby_count(self, fs_map: dict, fs_name: str) -> int: diff --git a/src/pybind/mgr/orchestrator/_interface.py b/src/pybind/mgr/orchestrator/_interface.py index 8fb45273631..344dea13854 100644 --- a/src/pybind/mgr/orchestrator/_interface.py +++ b/src/pybind/mgr/orchestrator/_interface.py @@ -31,7 +31,7 @@ import yaml from ceph.deployment import inventory from ceph.deployment.service_spec import ServiceSpec, NFSServiceSpec, RGWSpec, \ - IscsiServiceSpec, IngressSpec, SNMPGatewaySpec + IscsiServiceSpec, IngressSpec, SNMPGatewaySpec, MDSSpec from ceph.deployment.drive_group import DriveGroupSpec from ceph.deployment.hostspec import HostSpec, SpecValidationError from ceph.utils import datetime_to_str, str_to_datetime @@ -608,7 +608,7 @@ class Orchestrator(object): """Update mgr cluster""" raise NotImplementedError() - def apply_mds(self, spec: ServiceSpec) -> OrchResult[str]: + def apply_mds(self, spec: MDSSpec) -> OrchResult[str]: """Update MDS cluster""" raise NotImplementedError() diff --git a/src/pybind/mgr/orchestrator/module.py b/src/pybind/mgr/orchestrator/module.py index 2d73361fc6b..2ac249e075a 100644 --- a/src/pybind/mgr/orchestrator/module.py +++ b/src/pybind/mgr/orchestrator/module.py @@ -10,8 +10,7 @@ from prettytable import PrettyTable from ceph.deployment.inventory import Device from ceph.deployment.drive_group import DriveGroupSpec, DeviceSelection, OSDMethod -from ceph.deployment.service_spec import PlacementSpec, ServiceSpec, service_spec_allow_invalid_from_json, \ - SNMPGatewaySpec +from ceph.deployment.service_spec import PlacementSpec, ServiceSpec, service_spec_allow_invalid_from_json from ceph.deployment.hostspec import SpecValidationError from ceph.utils import datetime_now @@ -23,7 +22,7 @@ from ._interface import OrchestratorClientMixin, DeviceLightLoc, _cli_read_comma NoOrchestrator, OrchestratorValidationError, NFSServiceSpec, \ RGWSpec, InventoryFilter, InventoryHost, HostSpec, CLICommandMeta, \ ServiceDescription, DaemonDescription, IscsiServiceSpec, json_to_generic_spec, \ - GenericSpec, DaemonDescriptionStatus + GenericSpec, DaemonDescriptionStatus, SNMPGatewaySpec, MDSSpec def nice_delta(now: datetime.datetime, t: Optional[datetime.datetime], suffix: str = '') -> str: @@ -1068,12 +1067,15 @@ Usage: if inbuf: raise OrchestratorValidationError('unrecognized command -i; -h or --help for usage') - spec = ServiceSpec( + spec = MDSSpec( service_type='mds', service_id=fs_name, placement=PlacementSpec.from_string(placement), unmanaged=unmanaged, preview_only=dry_run) + + spec.validate() # force any validation exceptions to be caught correctly + return self._apply_misc([spec], dry_run, format, no_overwrite) @_cli_write_command('orch apply rgw') @@ -1112,6 +1114,8 @@ Usage: preview_only=dry_run ) + spec.validate() # force any validation exceptions to be caught correctly + return self._apply_misc([spec], dry_run, format, no_overwrite) @_cli_write_command('orch apply nfs') @@ -1136,6 +1140,8 @@ Usage: preview_only=dry_run ) + spec.validate() # force any validation exceptions to be caught correctly + return self._apply_misc([spec], dry_run, format, no_overwrite) @_cli_write_command('orch apply iscsi') @@ -1165,6 +1171,8 @@ Usage: preview_only=dry_run ) + spec.validate() # force any validation exceptions to be caught correctly + return self._apply_misc([spec], dry_run, format, no_overwrite) @_cli_write_command('orch apply snmp-gateway') diff --git a/src/python-common/ceph/deployment/service_spec.py b/src/python-common/ceph/deployment/service_spec.py index 5bf47d39c79..6b9d108fc17 100644 --- a/src/python-common/ceph/deployment/service_spec.py +++ b/src/python-common/ceph/deployment/service_spec.py @@ -453,6 +453,7 @@ class ServiceSpec(object): 'rgw': RGWSpec, 'nfs': NFSServiceSpec, 'osd': DriveGroupSpec, + 'mds': MDSSpec, 'iscsi': IscsiServiceSpec, 'alertmanager': AlertManagerSpec, 'ingress': IngressSpec, @@ -680,7 +681,7 @@ class ServiceSpec(object): f'Service of type \'{self.service_type}\' should not contain a service id') if self.service_id: - if not re.match('^[a-zA-Z0-9_.-]+$', self.service_id): + if not re.match('^[a-zA-Z0-9_.-]+$', str(self.service_id)): raise SpecValidationError('Service id contains invalid characters, ' 'only [a-zA-Z0-9_.-] allowed') @@ -1299,3 +1300,27 @@ class SNMPGatewaySpec(ServiceSpec): yaml.add_representer(SNMPGatewaySpec, ServiceSpec.yaml_representer) + + +class MDSSpec(ServiceSpec): + def __init__(self, + service_type: str = 'mds', + service_id: Optional[str] = None, + placement: Optional[PlacementSpec] = None, + unmanaged: bool = False, + preview_only: bool = False, + ): + assert service_type == 'mds' + super(MDSSpec, self).__init__('mds', service_id=service_id, + placement=placement, + unmanaged=unmanaged, + preview_only=preview_only) + + def validate(self) -> None: + super(MDSSpec, self).validate() + + if str(self.service_id)[0].isdigit(): + raise SpecValidationError('MDS service id cannot start with a numeric digit') + + +yaml.add_representer(MDSSpec, 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 080e5773217..d3fb4329668 100644 --- a/src/python-common/ceph/tests/test_service_spec.py +++ b/src/python-common/ceph/tests/test_service_spec.py @@ -475,7 +475,9 @@ def test_service_name(s_type, s_id, s_name): @pytest.mark.parametrize( 's_type,s_id', [ - ('mds', 's:id'), + ('mds', 's:id'), # MDS service_id cannot contain an invalid char ':' + ('mds', '1abc'), # MDS service_id cannot start with a numeric digit + ('mds', ''), # MDS service_id cannot be empty ('rgw', '*s_id'), ('nfs', 's/id'), ('iscsi', 's@id'), -- 2.39.5