From 9310d56436d043ab604b417da9265e8b9fab79c1 Mon Sep 17 00:00:00 2001
From: Sebastian Wagner <sebastian.wagner@suse.com>
Date: Mon, 9 Mar 2020 17:39:38 +0100
Subject: [PATCH] python-common: Add support for legacy serialization format
 for Drive Groups

Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
---
 .../ceph/deployment/drive_group.py            | 19 ++++++++--
 .../ceph/deployment/service_spec.py           | 20 ++++++----
 .../ceph/tests/test_drive_group.py            | 38 +++++++++++++------
 3 files changed, 53 insertions(+), 24 deletions(-)

diff --git a/src/python-common/ceph/deployment/drive_group.py b/src/python-common/ceph/deployment/drive_group.py
index 25019dd59e9..340c2ca89ff 100644
--- a/src/python-common/ceph/deployment/drive_group.py
+++ b/src/python-common/ceph/deployment/drive_group.py
@@ -114,11 +114,17 @@ class DriveGroupSpecs(object):
     def __init__(self, drive_group_json):
         # type: (list) -> None
         self.drive_group_json = drive_group_json
-        self.drive_groups = list()  # type: List[DriveGroupSpec]
-        self.build_drive_groups()
 
-    def build_drive_groups(self):
-        self.drive_groups = list(map(DriveGroupSpec.from_json, self.drive_group_json))
+        if isinstance(self.drive_group_json, dict):
+            # from legacy representation (till Octopus)
+            self.drive_group_json = [
+                dict(service_id=name, service_type='osd', **dg)
+                for name, dg in self.drive_group_json.items()
+            ]
+        if not isinstance(self.drive_group_json, list):
+            raise ServiceSpecValidationError('Specs needs to be a list of specs')
+        dgs = list(map(DriveGroupSpec.from_json, self.drive_group_json))  # type: ignore
+        self.drive_groups = dgs  # type: List[DriveGroupSpec]
 
     def __repr__(self):
         return ", ".join([repr(x) for x in self.drive_groups])
@@ -214,6 +220,11 @@ class DriveGroupSpec(ServiceSpec):
         :param json_drive_group: A valid json string with a Drive Group
                specification
         """
+        # 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']
+
         for applied_filter in list(json_drive_group.keys()):
             if applied_filter not in cls._supported_features:
                 raise DriveGroupValidationError(
diff --git a/src/python-common/ceph/deployment/service_spec.py b/src/python-common/ceph/deployment/service_spec.py
index 35d01facc4b..5bf357ed3a3 100644
--- a/src/python-common/ceph/deployment/service_spec.py
+++ b/src/python-common/ceph/deployment/service_spec.py
@@ -317,24 +317,28 @@ class ServiceSpec(object):
         self.service_type = service_type
         self.service_id = service_id
 
-    @staticmethod
-    def from_json(json_spec):
-        # type: (dict) -> "ServiceSpec"
+    @classmethod
+    def from_json(cls, json_spec):
+        # type: (dict) -> Any
+        # Python 3:
+        # >>> ServiceSpecs = TypeVar('Base', bound=ServiceSpec)
+        # then, the real type is: (dict) -> ServiceSpecs
         """
         Initialize 'ServiceSpec' object data from a json structure
         :param json_spec: A valid dict with ServiceSpec
         """
         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
+        service_type = json_spec.get('service_type', '')
         _cls = {
             'rgw': RGWSpec,
             'nfs': NFSServiceSpec,
             'osd': DriveGroupSpec
-        }.get(service_type, ServiceSpec)
+        }.get(service_type, cls)
+
+        if _cls == ServiceSpec and not service_type:
+            raise ServiceSpecValidationError('Spec needs a "service_type" key.')
+
         return _cls._from_json_impl(json_spec)  # type: ignore
 
     @classmethod
diff --git a/src/python-common/ceph/tests/test_drive_group.py b/src/python-common/ceph/tests/test_drive_group.py
index 98deaf926cb..a12a36589da 100644
--- a/src/python-common/ceph/tests/test_drive_group.py
+++ b/src/python-common/ceph/tests/test_drive_group.py
@@ -7,18 +7,32 @@ from ceph.tests.utils import _mk_inventory, _mk_device
 from ceph.deployment.drive_group import DriveGroupSpec, DriveGroupSpecs, \
                                         DeviceSelection, DriveGroupValidationError
 
-
-def test_DriveGroup():
-    dg_json = [
-        {
-            'service_type': 'osd',
-            'service_id': 'testing_drivegroup',
-            'placement': {'host_pattern': 'hostname'},
-            'data_devices': {'paths': ['/dev/sda']}
-        }
-    ]
-
-    dgs = DriveGroupSpecs(dg_json)
+@pytest.mark.parametrize("test_input",
+[
+    (
+        [  # new style json
+            {
+                'service_type': 'osd',
+                'service_id': 'testing_drivegroup',
+                'placement': {'host_pattern': 'hostname'},
+                'data_devices': {'paths': ['/dev/sda']}
+            }
+        ]
+    ),
+    (
+        {  # old style json
+            'testing_drivegroup':
+                {
+                    'host_pattern': 'hostname',
+                    'data_devices': {'paths': ['/dev/sda']}
+                }
+       }
+    )
+
+])
+
+def test_DriveGroup(test_input):
+    dgs = DriveGroupSpecs(test_input)
     for dg in dgs.drive_groups:
         assert dg.placement.pattern_matches_hosts(['hostname']) == ['hostname']
         assert dg.service_id == 'testing_drivegroup'
-- 
2.39.5