From 642a6f13130f5ae154679188233c1e03fdca5e38 Mon Sep 17 00:00:00 2001 From: Sebastian Wagner Date: Mon, 31 Aug 2020 11:12:47 +0200 Subject: [PATCH] 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 --- src/pybind/mgr/cephadm/tests/test_cephadm.py | 4 +-- src/pybind/mgr/cephadm/tests/test_spec.py | 12 ++++++++- .../ceph/deployment/service_spec.py | 25 ++++++++++--------- .../ceph/tests/test_service_spec.py | 20 ++++++++++++--- 4 files changed, 43 insertions(+), 18 deletions(-) diff --git a/src/pybind/mgr/cephadm/tests/test_cephadm.py b/src/pybind/mgr/cephadm/tests/test_cephadm.py index ebddee226cb4..64ba0acbf31f 100644 --- 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 eb02f95e086b..55001f036d86 100644 --- 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 7238cc7c93c9..8659a28a10fd 100644 --- 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 2d923c6c7d24..1d4bbf1e95d4 100644 --- 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 -- 2.47.3