From: Sebastian Wagner <sewagner@redhat.com>
Date: Wed, 1 Sep 2021 13:46:12 +0000 (+0200)
Subject: python-common: DriveGroupSpec: move pacement validation to validate()
X-Git-Tag: v16.2.8~251^2~3
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=cf884ec9fc399fa548229488778570fccb5d0110;p=ceph.git

python-common: DriveGroupSpec: move pacement validation to validate()

Signed-off-by: Sebastian Wagner <sewagner@redhat.com>
(cherry picked from commit 311860412e840e6b31e04b80a9de5e9ae05e7fb7)
---

diff --git a/src/python-common/ceph/deployment/drive_group.py b/src/python-common/ceph/deployment/drive_group.py
index 823959eea9d0..c0b343ef82a0 100644
--- a/src/python-common/ceph/deployment/drive_group.py
+++ b/src/python-common/ceph/deployment/drive_group.py
@@ -234,29 +234,21 @@ class DriveGroupSpec(ServiceSpec):
         :param json_drive_group: A valid json string with a Drive Group
                specification
         """
-        args: Dict[str, Any] = {}
+        args: Dict[str, Any] = json_drive_group.copy()
         # legacy json (pre Octopus)
-        if 'host_pattern' in json_drive_group and 'placement' not in json_drive_group:
-            json_drive_group['placement'] = {'host_pattern': json_drive_group['host_pattern']}
-            del json_drive_group['host_pattern']
-
-        args['service_type'] = json_drive_group.pop('service_type', 'osd')
+        if 'host_pattern' in args and 'placement' not in args:
+            args['placement'] = {'host_pattern': args['host_pattern']}
+            del args['host_pattern']
 
         s_id = args.get('service_id', '<unnamed>')
-        try:
-            args['placement'] = PlacementSpec.from_json(json_drive_group.pop('placement'))
-        except KeyError:
-            args['placement'] = PlacementSpec()
 
         # spec: was not mandatory in octopus
-        if 'spec' in json_drive_group:
-            args.update(cls._drive_group_spec_from_json(s_id, json_drive_group.pop('spec')))
+        if 'spec' in args:
+            args['spec'].update(cls._drive_group_spec_from_json(s_id, args['spec']))
         else:
-            args.update(cls._drive_group_spec_from_json(s_id, json_drive_group))
-
-        args['unmanaged'] = json_drive_group.pop('unmanaged', False)
+            args.update(cls._drive_group_spec_from_json(s_id, args))
 
-        return cls(**args)
+        return super(DriveGroupSpec, cls)._from_json_impl(args)
 
     @classmethod
     def _drive_group_spec_from_json(cls, name: str, json_drive_group: dict) -> dict:
@@ -287,9 +279,8 @@ class DriveGroupSpec(ServiceSpec):
         # type: () -> None
         super(DriveGroupSpec, self).validate()
 
-        if not isinstance(self.placement.host_pattern, str) and \
-                self.placement.host_pattern is not None:
-            raise DriveGroupValidationError(self.service_id, 'host_pattern must be of type string')
+        if self.placement.is_empty():
+            raise DriveGroupValidationError(self.service_id, '`placement` required')
 
         if self.data_devices is None:
             raise DriveGroupValidationError(self.service_id, "`data_devices` element is required.")
diff --git a/src/python-common/ceph/deployment/service_spec.py b/src/python-common/ceph/deployment/service_spec.py
index e1935f0e9bf6..ffa54c9fbf48 100644
--- a/src/python-common/ceph/deployment/service_spec.py
+++ b/src/python-common/ceph/deployment/service_spec.py
@@ -311,8 +311,12 @@ class PlacementSpec(object):
             raise SpecValidationError(
                 "count-per-host cannot be combined explicit placement with names or networks"
             )
-        if self.host_pattern and self.hosts:
-            raise SpecValidationError('cannot combine host patterns and hosts')
+        if self.host_pattern:
+            if not isinstance(self.host_pattern, str):
+                raise SpecValidationError('host_pattern must be of type string')
+            if self.hosts:
+                raise SpecValidationError('cannot combine host patterns and hosts')
+
         for h in self.hosts:
             h.validate()
 
diff --git a/src/python-common/ceph/tests/test_drive_group.py b/src/python-common/ceph/tests/test_drive_group.py
index dfc9bed35c05..49df20ca9f37 100644
--- a/src/python-common/ceph/tests/test_drive_group.py
+++ b/src/python-common/ceph/tests/test_drive_group.py
@@ -14,20 +14,28 @@ from ceph.deployment.drive_group import DriveGroupSpec, DeviceSelection, \
 
 @pytest.mark.parametrize("test_input",
 [
+    (  # new style json
+        """service_type: osd
+service_id: testing_drivegroup
+placement:
+  host_pattern: hostname
+data_devices:
+  paths:
+  - /dev/sda 
+"""
+    ),
     (
-        [  # new style json
-            {
-                'service_type': 'osd',
-                'service_id': 'testing_drivegroup',
-                'placement': {'host_pattern': 'hostname'},
-                'data_devices': {'paths': ['/dev/sda']}
-            }
-        ]
+        """service_type: osd
+service_id: testing_drivegroup
+placement:
+  host_pattern: hostname
+data_devices:
+  paths:
+  - /dev/sda"""
     ),
 ])
 def test_DriveGroup(test_input):
-    dg = [DriveGroupSpec.from_json(inp) for inp in test_input][0]
-    assert dg.placement.filter_matching_hostspecs([HostSpec('hostname')]) == ['hostname']
+    dg = DriveGroupSpec.from_json(yaml.safe_load(test_input))
     assert dg.service_id == 'testing_drivegroup'
     assert all([isinstance(x, Device) for x in dg.data_devices.paths])
     assert dg.data_devices.paths[0].path == '/dev/sda'
@@ -39,7 +47,7 @@ def test_DriveGroup(test_input):
         ''
     ),
     (
-        'Failed to validate OSD spec "": `placement` key required',
+        'Failed to validate OSD spec "<unnamed>": `placement` required',
         """data_devices:
   all: True
 """