From 494728c5dc729de2dcc9c40d92e154c2bd5d8008 Mon Sep 17 00:00:00 2001
From: Joshua Schmid <jschmid@suse.de>
Date: Thu, 12 Mar 2020 22:52:14 +0100
Subject: [PATCH] python-common: python-common: fix /hosts/ parsing in
 servicespecs

Signed-off-by: Joshua Schmid <jschmid@suse.de>
---
 .../ceph/deployment/service_spec.py           | 14 ++++--
 .../ceph/tests/test_service_spec.py           | 50 ++++++++++++-------
 2 files changed, 44 insertions(+), 20 deletions(-)

diff --git a/src/python-common/ceph/deployment/service_spec.py b/src/python-common/ceph/deployment/service_spec.py
index 13bc2dd5426..bfcbdd73ffb 100644
--- a/src/python-common/ceph/deployment/service_spec.py
+++ b/src/python-common/ceph/deployment/service_spec.py
@@ -110,7 +110,9 @@ class HostPlacementSpec(namedtuple('HostPlacementSpec', ['hostname', 'network',
         if ',' in network:
             networks = [x for x in network.split(',')]
         else:
-            networks.append(network)
+            if network != '':
+                networks.append(network)
+
         for network in networks:
             # only if we have versioned network configs
             if network.startswith('v') or network.startswith('[v'):
@@ -207,7 +209,11 @@ class PlacementSpec(object):
         c = data.copy()
         hosts = c.get('hosts', [])
         if hosts:
-            c['hosts'] = [HostPlacementSpec.from_json(host) for host in hosts]
+            c['hosts'] = []
+            for host in hosts:
+                c['hosts'].append(HostPlacementSpec.parse(host) if
+                                  isinstance(host, str) else
+                                  HostPlacementSpec.from_json(host))
         _cls = cls(**c)
         _cls.validate()
         return _cls
@@ -426,7 +432,9 @@ class ServiceSpec(object):
                 args.update(v)
                 continue
             args.update({k: v})
-        return cls(**args)
+        _cls = cls(**args)
+        _cls.validate()
+        return _cls
 
     def service_name(self):
         n = self.service_type
diff --git a/src/python-common/ceph/tests/test_service_spec.py b/src/python-common/ceph/tests/test_service_spec.py
index 4b2c85c4b1f..12a8be84bfd 100644
--- a/src/python-common/ceph/tests/test_service_spec.py
+++ b/src/python-common/ceph/tests/test_service_spec.py
@@ -1,10 +1,12 @@
 # flake8: noqa
 import json
+import yaml
 
 import pytest
 
-from ceph.deployment.service_spec import HostPlacementSpec, PlacementSpec, RGWSpec, \
-    servicespec_validate_add, ServiceSpecValidationError
+from ceph.deployment.service_spec import HostPlacementSpec, PlacementSpec, RGWSpec, NFSServiceSpec, \
+    servicespec_validate_add, ServiceSpec, ServiceSpecValidationError
+from ceph.deployment.drive_group import DriveGroupSpec
 
 
 @pytest.mark.parametrize("test_input,expected, require_network",
@@ -24,6 +26,7 @@ def test_parse_host_placement_specs(test_input, expected, require_network):
     assert ret == expected
     assert str(ret) == test_input
 
+
 @pytest.mark.parametrize(
     "test_input,expected",
     [
@@ -66,23 +69,36 @@ def test_parse_placement_specs_raises(test_input):
                          [("myhost:1.1.1.1/24"),
                           # wrong ip format
                           ("myhost:1"),
-                          # empty string
-                          ("myhost=1"),
                           ])
-def test_parse_host_placement_specs_raises(test_input):
+def test_parse_host_placement_specs_raises_wrong_format(test_input):
     with pytest.raises(ValueError):
-        ret = HostPlacementSpec.parse(test_input)
+        HostPlacementSpec.parse(test_input)
 
 
-def test_rgwspec():
-    """
-    {
-        "rgw_zone": "zonename",
-        "service_type": "rgw",
-        "rgw_frontend_port": 8080,
-        "rgw_realm": "realm"
+@pytest.mark.parametrize(
+    "s_type,o_spec,s_id",
+    [
+        ("mgr", ServiceSpec, 'test'),
+        ("mon", ServiceSpec, 'test'),
+        ("mds", ServiceSpec, 'test'),
+        ("rgw", RGWSpec, 'realm.zone'),
+        ("nfs", NFSServiceSpec, 'test'),
+        ("osd", DriveGroupSpec, 'test'),
+    ])
+def test_servicespec_map_test(s_type, o_spec, s_id):
+    dict_spec = {
+        "service_id": s_id,
+        "service_type": s_type,
+        "placement":
+            dict(hosts=["host1:1.1.1.1"])
     }
-    """
-    example = json.loads(test_rgwspec.__doc__.strip())
-    spec = RGWSpec.from_json(example)
-    assert servicespec_validate_add(spec) is None
\ No newline at end of file
+    spec = ServiceSpec.from_json(dict_spec)
+    assert isinstance(spec, o_spec)
+    assert isinstance(spec.placement, PlacementSpec)
+    assert isinstance(spec.placement.hosts[0], HostPlacementSpec)
+    assert spec.placement.hosts[0].hostname == 'host1'
+    assert spec.placement.hosts[0].network == '1.1.1.1'
+    assert spec.placement.hosts[0].name == ''
+    assert servicespec_validate_add(spec) is None
+    ServiceSpec.from_json(spec.to_json())
+
-- 
2.39.5