python-common: Make HostPlacementSpec.to_json() more friendly

We will now generate

```yaml
placement:
  hosts:
  - ceph-001
```

instead of

```yaml
placement:
  hosts:
  - hostname: ceph-001
    name: ''
    network: ''
```

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

diff --git a/src/pybind/mgr/cephadm/tests/test_cephadm.py b/src/pybind/mgr/cephadm/tests/test_cephadm.py
index ebddee226cb4095e8168149f1863d28afb2df6b8..64ba0acbf31f79518f69b7c14e046ebb3a80bbd0 100644 (file)
--- a/src/pybind/mgr/cephadm/tests/test_cephadm.py
+++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py
@@ -124,7 +124,7 @@ class TestCephadm(object):
                     out = [dict(o.to_json()) for o in wait(cephadm_module, c)]
                     expected = [
                         {
-                            'placement': {'hosts': [{'hostname': 'test', 'name': '', 'network': ''}]},
+                            'placement': {'hosts': ['test']},
                             'service_id': 'name',
                             'service_name': 'mds.name',
                             'service_type': 'mds',
@@ -134,7 +134,7 @@ class TestCephadm(object):
                         {
                             'placement': {
                                 'count': 1,
-                                'hosts': [{'hostname': 'test', 'name': '', 'network': ''}]
+                                'hosts': ["test"]
                             },
                             'spec': {
                                 'rgw_realm': 'r',

diff --git a/src/pybind/mgr/cephadm/tests/test_spec.py b/src/pybind/mgr/cephadm/tests/test_spec.py
index eb02f95e086bbb8e2ae42b9c30bf1f639fb3b587..55001f036d86710010d3dd4fb1c574608f47b307 100644 (file)
--- a/src/pybind/mgr/cephadm/tests/test_spec.py
+++ b/src/pybind/mgr/cephadm/tests/test_spec.py
@@ -7,7 +7,7 @@ import json
 import pytest
 
 from ceph.deployment.service_spec import ServiceSpec, NFSServiceSpec, RGWSpec, \
-    IscsiServiceSpec, AlertManagerSpec
+    IscsiServiceSpec, AlertManagerSpec, HostPlacementSpec
 
 from orchestrator import DaemonDescription, OrchestratorError
 
@@ -105,6 +105,16 @@ def test_spec_octopus(spec_json):
         if 'spec' in j_c:
             spec = j_c.pop('spec')
             j_c.update(spec)
+        if 'placement' in j_c:
+            if 'hosts' in j_c['placement']:
+                j_c['placement']['hosts'] = [
+                    {
+                        'hostname': HostPlacementSpec.parse(h).hostname,
+                        'network': HostPlacementSpec.parse(h).network,
+                        'name': HostPlacementSpec.parse(h).name
+                    }
+                    for h in j_c['placement']['hosts']
+                ]
         j_c.pop('objectstore', None)
         j_c.pop('filter_logic', None)
         return j_c

diff --git a/src/python-common/ceph/deployment/service_spec.py b/src/python-common/ceph/deployment/service_spec.py
index 7238cc7c93c943e1dca496c7183ce6ad2690fcc9..8659a28a10fdc165ba14257849ba4a0bc32b2084 100644 (file)
--- a/src/python-common/ceph/deployment/service_spec.py
+++ b/src/python-common/ceph/deployment/service_spec.py
@@ -58,14 +58,12 @@ class HostPlacementSpec(namedtuple('HostPlacementSpec', ['hostname', 'network',
     @classmethod
     @handle_type_error
     def from_json(cls, data):
+        if isinstance(data, str):
+            return cls.parse(data)
         return cls(**data)
 
-    def to_json(self):
-        return {
-            'hostname': self.hostname,
-            'network': self.network,
-            'name': self.name
-        }
+    def to_json(self) -> str:
+        return str(self)
 
     @classmethod
     def parse(cls, host, require_network=True):
@@ -209,16 +207,21 @@ class PlacementSpec(object):
         return len(self.filter_matching_hostspecs(hostspecs))
 
     def pretty_str(self):
+        """
+        >>> #doctest: +SKIP
+        ... ps = PlacementSpec(...)  # For all placement specs:
+        ... PlacementSpec.from_string(ps.pretty_str()) == ps
+        """
         kv = []
+        if self.hosts:
+            kv.append(';'.join([str(h) for h in self.hosts]))
         if self.count:
             kv.append('count:%d' % self.count)
         if self.label:
             kv.append('label:%s' % self.label)
-        if self.hosts:
-            kv.append('%s' % ','.join([str(h) for h in self.hosts]))
         if self.host_pattern:
             kv.append(self.host_pattern)
-        return ' '.join(kv)
+        return ';'.join(kv)
 
     def __repr__(self):
         kv = []
@@ -240,9 +243,7 @@ class PlacementSpec(object):
         if hosts:
             c['hosts'] = []
             for host in hosts:
-                c['hosts'].append(HostPlacementSpec.parse(host) if
-                                  isinstance(host, str) else
-                                  HostPlacementSpec.from_json(host))
+                c['hosts'].append(HostPlacementSpec.from_json(host))
         _cls = cls(**c)
         _cls.validate()
         return _cls

diff --git a/src/python-common/ceph/tests/test_service_spec.py b/src/python-common/ceph/tests/test_service_spec.py
index 2d923c6c7d24135ac653f35fbb28dafe0d6ddb93..1d4bbf1e95d4387579d123f8997395d85619b176 100644 (file)
--- a/src/python-common/ceph/tests/test_service_spec.py
+++ b/src/python-common/ceph/tests/test_service_spec.py
@@ -26,6 +26,21 @@ def test_parse_host_placement_specs(test_input, expected, require_network):
     assert ret == expected
     assert str(ret) == test_input
 
+    ps = PlacementSpec.from_string(test_input)
+    assert ps.pretty_str() == test_input
+    assert ps == PlacementSpec.from_string(ps.pretty_str())
+
+    # Testing the old verbose way of generating json. Don't remove:
+    assert ret == HostPlacementSpec.from_json({
+            'hostname': ret.hostname,
+            'network': ret.network,
+            'name': ret.name
+        })
+
+    assert ret == HostPlacementSpec.from_json(ret.to_json())
+
+
+
 
 @pytest.mark.parametrize(
     "test_input,expected",
@@ -51,6 +66,7 @@ def test_parse_host_placement_specs(test_input, expected, require_network):
 def test_parse_placement_specs(test_input, expected):
     ret = PlacementSpec.from_string(test_input)
     assert str(ret) == expected
+    assert PlacementSpec.from_string(ret.pretty_str()) == ret, f'"{ret.pretty_str()}" != "{test_input}"'
 
 @pytest.mark.parametrize(
     "test_input",
@@ -141,9 +157,7 @@ service_id: default-rgw-realm.eu-central-1.1
 service_name: rgw.default-rgw-realm.eu-central-1.1
 placement:
   hosts:
-  - hostname: ceph-001
-    name: ''
-    network: ''
+  - ceph-001
 spec:
   rgw_realm: default-rgw-realm
   rgw_zone: eu-central-1