mgr/cephadm: using MDSSPec instead of ServiceSpec

Fixes:  https://tracker.ceph.com/issues/54184
Signed-off-by: Redouane Kachach <rkachach@redhat.com>
(cherry picked from commit db765bd80608b7c6930a5111eb006b5d12f73de2)

diff --git a/src/pybind/mgr/mds_autoscaler/module.py b/src/pybind/mgr/mds_autoscaler/module.py
index 4d9165e478ccfa2e595925d64065901517762e5a..2f780059c0b7d8b461f46f605ea2bd8ee39444bd 100644 (file)
--- 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 21f354c4d83b1869c85e26e31927e1ae5eb69580..ff2493795e384c961715c6492047d8b9b56d7c1d 100644 (file)
--- 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
@@ -610,7 +610,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 ed183bb55a82bf00d2e4d8f2685add67131899a3..8a0181fe70e1272266d6c011ba5f8985ec2497b5 100644 (file)
--- 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:
@@ -1071,12 +1070,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')
@@ -1115,6 +1117,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')
@@ -1139,6 +1143,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')
@@ -1168,6 +1174,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 be3f9deebeeee8332fa013ce5af5b4c598e69836..f1d00edab0dee21eb70142f2e4a50f12ba66cff5 100644 (file)
--- a/src/python-common/ceph/deployment/service_spec.py
+++ b/src/python-common/ceph/deployment/service_spec.py
@@ -459,6 +459,7 @@ class ServiceSpec(object):
             'rgw': RGWSpec,
             'nfs': NFSServiceSpec,
             'osd': DriveGroupSpec,
+            'mds': MDSSpec,
             'iscsi': IscsiServiceSpec,
             'alertmanager': AlertManagerSpec,
             'ingress': IngressSpec,
@@ -686,7 +687,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')
 
@@ -1305,3 +1306,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 080e57732170231ae0d516992b7522096227edab..d3fb4329668bb74d193a10669969f888eced5703 100644 (file)
--- 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'),