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 db765bd)

diff --git a/src/pybind/mgr/mds_autoscaler/module.py b/src/pybind/mgr/mds_autoscaler/module.py
index c9fdcd8922fb117cd9f918bf1bd02d16c98cf69a..a6bbadfd57bbd3de8b7acb480d2b2cf500e0b5cb 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 Optional, List, Set
 from mgr_module import MgrModule, NotifyType
-from ceph.deployment.service_spec import ServiceSpec
+from orchestrator._interface import MDSSpec, ServiceSpec
 import orchestrator
 import copy
 
@@ -31,12 +31,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 0783cb2303370efbfe1630c6657041b24c55bf3f..b99a380b00de2e71f4a8c6a14d0388998a537531 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
@@ -609,7 +609,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 968c5b0031b15274f0d407fb8f64502e9f014fe5..56612dd8dc6c8437cdba81a7f5572e51aea5a089 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
-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:
@@ -1066,12 +1065,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')
@@ -1109,6 +1111,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')
@@ -1133,6 +1137,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')
@@ -1162,6 +1168,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 d56cd17f7ae6c62fabcadb44e22f04683cd0c8b1..76132e61ded4ef18732a9ddc7426bedad437e576 100644 (file)
--- 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')
 
@@ -1313,3 +1314,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'),