From 4ddb111dd1491a4456bdc0379b6fbc5d641f4291 Mon Sep 17 00:00:00 2001 From: Sebastian Wagner Date: Thu, 11 Jun 2020 11:54:45 +0200 Subject: [PATCH] python-common: make YAML representaition of ServiceSpec readable * Add test for new yaml representation Signed-off-by: Sebastian Wagner --- src/pybind/mgr/cephadm/tests/test_cephadm.py | 8 ++- src/pybind/mgr/cephadm/tests/test_spec.py | 71 +++++++++++++------ .../ceph/deployment/service_spec.py | 42 +++++++++-- .../ceph/tests/test_service_spec.py | 47 ++++++++++++ 4 files changed, 136 insertions(+), 32 deletions(-) diff --git a/src/pybind/mgr/cephadm/tests/test_cephadm.py b/src/pybind/mgr/cephadm/tests/test_cephadm.py index e80728c3871..9311a7c3b68 100644 --- a/src/pybind/mgr/cephadm/tests/test_cephadm.py +++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py @@ -111,7 +111,7 @@ class TestCephadm(object): assert wait(cephadm_module, c) == 'Scheduled rgw.r.z update...' c = cephadm_module.describe_service() - out = [o.to_json() for o in wait(cephadm_module, c)] + out = [dict(o.to_json()) for o in wait(cephadm_module, c)] expected = [ { 'placement': {'hosts': [{'hostname': 'test', 'name': '', 'network': ''}]}, @@ -126,8 +126,10 @@ class TestCephadm(object): 'count': 1, 'hosts': [{'hostname': 'test', 'name': '', 'network': ''}] }, - 'rgw_realm': 'r', - 'rgw_zone': 'z', + 'spec': { + 'rgw_realm': 'r', + 'rgw_zone': 'z', + }, 'service_id': 'r.z', 'service_name': 'rgw.r.z', 'service_type': 'rgw', diff --git a/src/pybind/mgr/cephadm/tests/test_spec.py b/src/pybind/mgr/cephadm/tests/test_spec.py index ea4e8c8f734..1a048d82d6f 100644 --- a/src/pybind/mgr/cephadm/tests/test_spec.py +++ b/src/pybind/mgr/cephadm/tests/test_spec.py @@ -8,11 +8,9 @@ from ceph.deployment.service_spec import ServiceSpec, NFSServiceSpec, RGWSpec, \ from orchestrator import DaemonDescription, OrchestratorError -def test_spec_octopus(): - # https://tracker.ceph.com/issues/44934 - # Those are real user data from early octopus. - # Please do not modify those JSON values. - specs_text = """[ +@pytest.mark.parametrize( + "spec_json", + json.loads("""[ { "placement": { "count": 1 @@ -70,10 +68,47 @@ def test_spec_octopus(): "rgw_realm": "default-rgw-realm", "rgw_zone": "eu-central-1", "subcluster": "1" +}, +{ + "service_type": "osd", + "service_id": "osd_spec_default", + "placement": { + "host_pattern": "*" + }, + "data_devices": { + "model": "MC-55-44-XZ" + }, + "db_devices": { + "model": "SSD-123-foo" + }, + "wal_devices": { + "model": "NVME-QQQQ-987" + } } ] -""" - dds_text = """[ +""") +) +def test_spec_octopus(spec_json): + # https://tracker.ceph.com/issues/44934 + # Those are real user data from early octopus. + # Please do not modify those JSON values. + + spec = ServiceSpec.from_json(spec_json) + # just some verification that we can sill read old octopus specs + def convert_to_old_style_json(j): + j_c = dict(j.copy()) + j_c.pop('service_name', None) + if 'spec' in j_c: + spec = j_c.pop('spec') + j_c.update(spec) + j_c.pop('objectstore', None) + return j_c + assert spec_json == convert_to_old_style_json(spec.to_json()) + + +@pytest.mark.parametrize( + "dd_json", + json.loads("""[ { "hostname": "ceph-001", "container_id": "d94d7969094d", @@ -207,21 +242,13 @@ def test_spec_octopus(): "status": 1, "status_desc": "starting" } -]""" - specs_json = json.loads(specs_text) - dds_json = json.loads(dds_text) - specs = [ServiceSpec.from_json(j) for j in specs_json] - dds = [DaemonDescription.from_json(j) for j in dds_json] - - # just some verification that we can sill read old octopus specs - def remove_service_name(j): - if 'service_name' in j: - j_c = j.copy() - del j_c['service_name'] - return j_c - return j - assert specs_json == [remove_service_name(s.to_json()) for s in specs] - assert dds_json == [d.to_json() for d in dds] +]""") +) +def test_dd_octopus(dd_json): + # https://tracker.ceph.com/issues/44934 + # Those are real user data from early octopus. + # Please do not modify those JSON values. + assert dd_json == DaemonDescription.from_json(dd_json).to_json() @pytest.mark.parametrize("spec,dd,valid", diff --git a/src/python-common/ceph/deployment/service_spec.py b/src/python-common/ceph/deployment/service_spec.py index 4d86d6fc167..d53a2832e36 100644 --- a/src/python-common/ceph/deployment/service_spec.py +++ b/src/python-common/ceph/deployment/service_spec.py @@ -1,10 +1,11 @@ import fnmatch import re -from collections import namedtuple +from collections import namedtuple, OrderedDict from functools import wraps from typing import Optional, Dict, Any, List, Union, Callable, Iterator import six +import yaml from ceph.deployment.hostspec import HostSpec @@ -38,7 +39,7 @@ def handle_type_error(method): return method(cls, *args, **kwargs) except (TypeError, AttributeError) as e: error_msg = '{}: {}'.format(cls.__name__, e) - raise ServiceSpecValidationError(error_msg) + raise ServiceSpecValidationError(error_msg) return inner @@ -470,16 +471,27 @@ class ServiceSpec(object): return n def to_json(self): - # type: () -> Dict[str, Any] + # type: () -> OrderedDict[str, Any] + ret: OrderedDict[str, Any] = OrderedDict() + ret['service_type'] = self.service_type + if self.service_id: + ret['service_id'] = self.service_id + ret['service_name'] = self.service_name() + ret['placement'] = self.placement.to_json() + if self.unmanaged: + ret['unmanaged'] = self.unmanaged + c = {} - for key, val in self.__dict__.items(): + for key, val in sorted(self.__dict__.items(), key=lambda tpl: tpl[0]): + if key in ret: + continue if hasattr(val, 'to_json'): val = val.to_json() if val: c[key] = val - - c['service_name'] = self.service_name() - return c + if c: + ret['spec'] = c + return ret def validate(self): if not self.service_type: @@ -497,6 +509,13 @@ class ServiceSpec(object): def one_line_str(self): return '<{} for service_name={}>'.format(self.__class__.__name__, self.service_name()) + @staticmethod + def yaml_representer(dumper: 'yaml.SafeDumper', data: 'ServiceSpec'): + return dumper.represent_dict(data.to_json().items()) + + +yaml.add_representer(ServiceSpec, ServiceSpec.yaml_representer) + class NFSServiceSpec(ServiceSpec): def __init__(self, @@ -539,6 +558,9 @@ class NFSServiceSpec(ServiceSpec): return url +yaml.add_representer(NFSServiceSpec, ServiceSpec.yaml_representer) + + class RGWSpec(ServiceSpec): """ Settings to configure a (multisite) Ceph RGW @@ -600,6 +622,9 @@ class RGWSpec(ServiceSpec): return f'beast {" ".join(ports)}' +yaml.add_representer(RGWSpec, ServiceSpec.yaml_representer) + + class IscsiServiceSpec(ServiceSpec): def __init__(self, service_type: str = 'iscsi', @@ -644,3 +669,6 @@ class IscsiServiceSpec(ServiceSpec): if not self.api_password: raise ServiceSpecValidationError( 'Cannot add ISCSI: No Api password specified') + + +yaml.add_representer(IscsiServiceSpec, ServiceSpec.yaml_representer) diff --git a/src/python-common/ceph/tests/test_service_spec.py b/src/python-common/ceph/tests/test_service_spec.py index e72a37fbc63..6aa26adc423 100644 --- a/src/python-common/ceph/tests/test_service_spec.py +++ b/src/python-common/ceph/tests/test_service_spec.py @@ -109,3 +109,50 @@ def test_servicespec_map_test(s_type, o_spec, s_id): assert spec.validate() is None ServiceSpec.from_json(spec.to_json()) + +def test_yaml(): + y = """service_type: crash +service_name: crash +placement: + host_pattern: '*' +--- +service_type: crash +service_name: crash +placement: + host_pattern: '*' +unmanaged: true +--- +service_type: rgw +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: '' +spec: + rgw_realm: default-rgw-realm + rgw_zone: eu-central-1 + subcluster: '1' +--- +service_type: osd +service_id: osd_spec_default +service_name: osd.osd_spec_default +placement: + host_pattern: '*' +spec: + data_devices: + model: MC-55-44-XZ + db_devices: + model: SSD-123-foo + objectstore: bluestore + wal_devices: + model: NVME-QQQQ-987 +""" + + for y in y.split('---\n'): + data = yaml.safe_load(y) + object = ServiceSpec.from_json(data) + + assert yaml.dump(object) == y + assert yaml.dump(ServiceSpec.from_json(object.to_json())) == y -- 2.39.5