From 818c242c3cb7f1dfd0c100b32b155018ec0bbe40 Mon Sep 17 00:00:00 2001 From: Sebastian Wagner Date: Wed, 10 Nov 2021 15:54:42 +0100 Subject: [PATCH] python-common: Don't valiate ServiceSpec.from_json() in `orch ls` unfortunately `ceph orch ls` may return invalid OSD specs for OSDs not associated to and specs. Signed-off-by: Sebastian Wagner (cherry picked from commit 3f38583b7189d99be360d8475fe6ef8cd53dee7c) Conflicts: src/pybind/mgr/orchestrator/module.py --- src/pybind/mgr/orchestrator/module.py | 13 ++++++----- .../orchestrator/tests/test_orchestrator.py | 19 +++++++++++++++ .../ceph/deployment/service_spec.py | 23 ++++++++++++++++--- 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/src/pybind/mgr/orchestrator/module.py b/src/pybind/mgr/orchestrator/module.py index 47c47f82dfede..10381db9c14a7 100644 --- a/src/pybind/mgr/orchestrator/module.py +++ b/src/pybind/mgr/orchestrator/module.py @@ -10,7 +10,7 @@ from prettytable import PrettyTable from ceph.deployment.inventory import Device from ceph.deployment.drive_group import DriveGroupSpec, DeviceSelection -from ceph.deployment.service_spec import PlacementSpec, ServiceSpec, \ +from ceph.deployment.service_spec import PlacementSpec, ServiceSpec, service_spec_allow_invalid_from_json, \ SNMPGatewaySpec from ceph.deployment.hostspec import SpecValidationError from ceph.utils import datetime_now @@ -584,11 +584,12 @@ class OrchestratorCli(OrchestratorClientMixin, MgrModule, if len(services) == 0: return HandleCommandResult(stdout="No services reported") elif format != Format.plain: - if export: - data = [s.spec for s in services if s.deleted is None] - return HandleCommandResult(stdout=to_format(data, format, many=True, cls=ServiceSpec)) - else: - return HandleCommandResult(stdout=to_format(services, format, many=True, cls=ServiceDescription)) + with service_spec_allow_invalid_from_json(): + if export: + data = [s.spec for s in services if s.deleted is None] + return HandleCommandResult(stdout=to_format(data, format, many=True, cls=ServiceSpec)) + else: + return HandleCommandResult(stdout=to_format(services, format, many=True, cls=ServiceDescription)) else: now = datetime_now() table = PrettyTable( diff --git a/src/pybind/mgr/orchestrator/tests/test_orchestrator.py b/src/pybind/mgr/orchestrator/tests/test_orchestrator.py index 3afddd69b689a..7fe32e559be4a 100644 --- a/src/pybind/mgr/orchestrator/tests/test_orchestrator.py +++ b/src/pybind/mgr/orchestrator/tests/test_orchestrator.py @@ -1,6 +1,7 @@ from __future__ import absolute_import import json +import textwrap import pytest import yaml @@ -157,6 +158,24 @@ def test_orch_ls(_describe_service): 'osd 123 - - ' assert r == HandleCommandResult(retval=0, stdout=out, stderr='') + cmd = { + 'prefix': 'orch ls', + 'format': 'yaml', + } + m = OrchestratorCli('orchestrator', 0, 0) + r = m._handle_command(None, cmd) + out = textwrap.dedent(""" + service_type: osd + service_name: osd + spec: + filter_logic: AND + objectstore: bluestore + status: + running: 123 + size: 0 + """).lstrip() + assert r == HandleCommandResult(retval=0, stdout=out, stderr='') + def test_preview_table_osd_smoke(): data = [ diff --git a/src/python-common/ceph/deployment/service_spec.py b/src/python-common/ceph/deployment/service_spec.py index 9984623d62627..8cd264cc77b7d 100644 --- a/src/python-common/ceph/deployment/service_spec.py +++ b/src/python-common/ceph/deployment/service_spec.py @@ -2,10 +2,11 @@ import fnmatch import re import enum from collections import OrderedDict +from contextlib import contextmanager from functools import wraps from ipaddress import ip_network, ip_address from typing import Optional, Dict, Any, List, Union, Callable, Iterable, Type, TypeVar, cast, \ - NamedTuple, Mapping + NamedTuple, Mapping, Iterator import yaml @@ -410,6 +411,22 @@ tPlacementSpec(hostname='host2', network='', name='')]) return ps +_service_spec_from_json_validate = True + + +@contextmanager +def service_spec_allow_invalid_from_json() -> Iterator[None]: + """ + I know this is evil, but unfortunately `ceph orch ls` + may return invalid OSD specs for OSDs not associated to + and specs. If you have a better idea, please! + """ + global _service_spec_from_json_validate + _service_spec_from_json_validate = False + yield + _service_spec_from_json_validate = True + + class ServiceSpec(object): """ Details of service creation. @@ -552,7 +569,6 @@ class ServiceSpec(object): :meta private: """ - if not isinstance(json_spec, dict): raise SpecValidationError( f'Service Spec is not an (JSON or YAML) object. got "{str(json_spec)}"') @@ -603,7 +619,8 @@ class ServiceSpec(object): continue args.update({k: v}) _cls = cls(**args) - _cls.validate() + if _service_spec_from_json_validate: + _cls.validate() return _cls def service_name(self) -> str: -- 2.39.5