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 <sewagner@redhat.com>
(cherry picked from commit 3f38583b7189d99be360d8475fe6ef8cd53dee7c)

Conflicts:
	src/pybind/mgr/orchestrator/module.py

diff --git a/src/pybind/mgr/orchestrator/module.py b/src/pybind/mgr/orchestrator/module.py
index 47c47f82dfede1cd85c62ff924d015c761999d99..10381db9c14a7bcf53b09129dd82cb83021b2b15 100644 (file)
--- 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 3afddd69b689a0178b3ef8f2bff4f385a9601c93..7fe32e559be4a608741cde0bd3361e92b16a989c 100644 (file)
--- 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 9984623d62627cf6a94aba7870c956bef42a181c..8cd264cc77b7dbbfb64b77000f1dfef3bbf6bce2 100644 (file)
--- 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: