python-common: Joined ServiceSpec and DriveGroupSpec from_json()

Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>

diff --git a/src/pybind/mgr/test_orchestrator/module.py b/src/pybind/mgr/test_orchestrator/module.py
index 55f43d56a320c01566def5cab4a063625f8c5acf..c82b4b82189243c4f9d54eb98772bddadb559cc5 100644 (file)
--- a/src/pybind/mgr/test_orchestrator/module.py
+++ b/src/pybind/mgr/test_orchestrator/module.py
@@ -255,7 +255,7 @@ class TestOrchestrator(MgrModule, orchestrator.Orchestrator):
 
         def run(all_hosts):
             # type: (List[orchestrator.HostSpec]) -> None
-            drive_group.validate([h.hostname for h in all_hosts])
+            drive_group.validate()
         return self.get_hosts().then(run).then(
             on_complete=orchestrator.ProgressReference(
                 message='create_osds',

diff --git a/src/python-common/ceph/deployment/drive_group.py b/src/python-common/ceph/deployment/drive_group.py
index aecae06cff4bc741e718600f8bdde4e8211d3ae6..25019dd59e941f567600b718c290319252f93e28 100644 (file)
--- a/src/python-common/ceph/deployment/drive_group.py
+++ b/src/python-common/ceph/deployment/drive_group.py
@@ -132,7 +132,7 @@ class DriveGroupSpec(ServiceSpec):
 
     _supported_features = [
         "encrypted", "block_wal_size", "osds_per_device",
-        "db_slots", "wal_slots", "block_db_size", "placement", "service_id",
+        "db_slots", "wal_slots", "block_db_size", "placement", "service_id", "service_type",
         "data_devices", "db_devices", "wal_devices", "journal_devices",
         "data_directories", "osds_per_device", "objectstore", "osd_id_claims",
         "journal_size"
@@ -155,8 +155,9 @@ class DriveGroupSpec(ServiceSpec):
                  block_db_size=None,  # type: Optional[int]
                  block_wal_size=None,  # type: Optional[int]
                  journal_size=None,  # type: Optional[int]
+                 service_type=None,  # type: Optional[str]
                  ):
-
+        assert service_type is None or service_type == 'osd'
         super(DriveGroupSpec, self).__init__('osd', service_id=service_id, placement=placement)
 
         #: A :class:`ceph.deployment.drive_group.DeviceSelection`
@@ -205,7 +206,7 @@ class DriveGroupSpec(ServiceSpec):
         self.osd_id_claims = osd_id_claims
 
     @classmethod
-    def from_json(cls, json_drive_group):
+    def _from_json_impl(cls, json_drive_group):
         # type: (dict) -> DriveGroupSpec
         """
         Initialize 'Drive group' structure
@@ -236,8 +237,10 @@ class DriveGroupSpec(ServiceSpec):
         except (KeyError, TypeError) as e:
             raise DriveGroupValidationError(str(e))
 
-    def validate(self, all_hosts):
-        # type: (List[str]) -> None
+    def validate(self):
+        # type: () -> None
+        super(DriveGroupSpec, self).validate()
+
         if not isinstance(self.placement.host_pattern, six.string_types):
             raise DriveGroupValidationError('host_pattern must be of type string')
 
@@ -250,9 +253,6 @@ class DriveGroupSpec(ServiceSpec):
 
         if self.objectstore not in ('filestore', 'bluestore'):
             raise DriveGroupValidationError("objectstore not in ('filestore', 'bluestore')")
-        if not self.placement.pattern_matches_hosts(all_hosts):
-            raise DriveGroupValidationError(
-                "host_pattern '{}' does not match any hosts".format(self.placement.host_pattern))
 
         if self.block_wal_size is not None and type(self.block_wal_size) != int:
             raise DriveGroupValidationError('block_wal_size must be of type int')

diff --git a/src/python-common/ceph/deployment/service_spec.py b/src/python-common/ceph/deployment/service_spec.py
index b924cea3a8fac9adc2a09f9a75cdccf21fdb18b2..566e041fd91f74dea15532252b11ca5666d67fee 100644 (file)
--- a/src/python-common/ceph/deployment/service_spec.py
+++ b/src/python-common/ceph/deployment/service_spec.py
@@ -287,21 +287,29 @@ class ServiceSpec(object):
         self.service_type = service_type
         self.service_id = service_id
 
-    @classmethod
-    def from_json(cls, json_spec: dict) -> "ServiceSpec":
+    @staticmethod
+    def from_json(json_spec):
+        # type: (dict) -> "ServiceSpec"
         """
         Initialize 'ServiceSpec' object data from a json structure
         :param json_spec: A valid dict with ServiceSpec
         """
-        args = {}  # type: Dict[str, Dict[Any, Any]]
-        service_type = json_spec.get('service_type', '')
+        from ceph.deployment.drive_group import DriveGroupSpec
+
+        if 'service_type' not in json_spec:
+            raise ServiceSpecValidationError('Spec needs a "service_type" key.')
+        service_type = json_spec.get('service_type')
         assert service_type
-        if service_type == 'rgw':
-            _cls = RGWSpec  # type: ignore
-        elif service_type == 'nfs':
-            _cls = NFSServiceSpec  # type: ignore
-        else:
-            _cls = ServiceSpec  # type: ignore
+        _cls = {
+            'rgw': RGWSpec,
+            'nfs': NFSServiceSpec,
+            'osd': DriveGroupSpec
+        }.get(service_type, ServiceSpec)
+        return _cls._from_json_impl(json_spec)  # type: ignore
+
+    @classmethod
+    def _from_json_impl(cls, json_spec):
+        args = {}  # type: Dict[str, Dict[Any, Any]]
         for k, v in json_spec.items():
             if k == 'placement':
                 v = PlacementSpec.from_json(v)
@@ -309,7 +317,7 @@ class ServiceSpec(object):
                 args.update(v)
                 continue
             args.update({k: v})
-        return _cls(**args)  # type: ignore
+        return cls(**args)
 
     def service_name(self):
         n = self.service_type
@@ -324,6 +332,13 @@ class ServiceSpec(object):
             c['placement'] = self.placement.to_json()
         return c
 
+    def validate(self):
+        if not self.service_type:
+            raise ServiceSpecValidationError('Cannot add Service: type required')
+
+        if self.placement is not None:
+            self.placement.validate()
+
     def __repr__(self):
         return "{}({!r})".format(self.__class__.__name__, self.__dict__)
 
@@ -331,8 +346,7 @@ class ServiceSpec(object):
 def servicespec_validate_add(self: ServiceSpec):
     # This must not be a method of ServiceSpec, otherwise you'll hunt
     # sub-interpreter affinity bugs.
-    if not self.service_type:
-        raise ServiceSpecValidationError('Cannot add Service: type required')
+    ServiceSpec.validate(self)
     if self.service_type in ['mds', 'rgw', 'nfs'] and not self.service_id:
         raise ServiceSpecValidationError('Cannot add Service: id required')
 

diff --git a/src/python-common/ceph/tests/test_disk_selector.py b/src/python-common/ceph/tests/test_disk_selector.py
index 3e0dba1d150ec4a4b412f2b202bbda435790b0c0..d6e9665dc73f10a3c0e3580fa5f5fff358f5ef15 100644 (file)
--- a/src/python-common/ceph/tests/test_disk_selector.py
+++ b/src/python-common/ceph/tests/test_disk_selector.py
@@ -275,6 +275,7 @@ class TestDriveGroup(object):
                              osds_per_device='',
                              disk_format='bluestore'):
             raw_sample_bluestore = {
+                'service_type': 'osd',
                 'placement': {'host_pattern': 'data*'},
                 'data_devices': {
                     'size': '30G:50G',
@@ -299,6 +300,7 @@ class TestDriveGroup(object):
                 'encrypted': True,
             }
             raw_sample_filestore = {
+                'service_type': 'osd',
                 'placement': {'host_pattern': 'data*'},
                 'objectstore': 'filestore',
                 'data_devices': {
@@ -320,7 +322,10 @@ class TestDriveGroup(object):
                 raw_sample = raw_sample_bluestore
 
             if empty:
-                raw_sample = {'placement': {'host_pattern': 'data*'}}
+                raw_sample = {
+                    'service_type': 'osd',
+                    'placement': {'host_pattern': 'data*'}
+                }
 
             dgo = DriveGroupSpec.from_json(raw_sample)
             return dgo

diff --git a/src/python-common/ceph/tests/test_drive_group.py b/src/python-common/ceph/tests/test_drive_group.py
index 158fbd710982c92e6c5cc01b06e34464de9f10bf..98deaf926cbf1d2a431260e04df10314bc3ecffa 100644 (file)
--- a/src/python-common/ceph/tests/test_drive_group.py
+++ b/src/python-common/ceph/tests/test_drive_group.py
@@ -2,7 +2,7 @@ import pytest
 
 from ceph.deployment import drive_selection, translate
 from ceph.deployment.inventory import Device
-from ceph.deployment.service_spec import PlacementSpec
+from ceph.deployment.service_spec import PlacementSpec, ServiceSpecValidationError
 from ceph.tests.utils import _mk_inventory, _mk_device
 from ceph.deployment.drive_group import DriveGroupSpec, DriveGroupSpecs, \
                                         DeviceSelection, DriveGroupValidationError
@@ -11,6 +11,7 @@ from ceph.deployment.drive_group import DriveGroupSpec, DriveGroupSpecs, \
 def test_DriveGroup():
     dg_json = [
         {
+            'service_type': 'osd',
             'service_id': 'testing_drivegroup',
             'placement': {'host_pattern': 'hostname'},
             'data_devices': {'paths': ['/dev/sda']}
@@ -26,7 +27,7 @@ def test_DriveGroup():
 
 
 def test_DriveGroup_fail():
-    with pytest.raises(DriveGroupValidationError):
+    with pytest.raises(ServiceSpecValidationError):
         DriveGroupSpec.from_json({})