From 3f38583b7189d99be360d8475fe6ef8cd53dee7c Mon Sep 17 00:00:00 2001
From: Sebastian Wagner <sewagner@redhat.com>
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 <sewagner@redhat.com>
---
 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 c67685a0472..353d0d78fcf 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, OSDMethod
-from ceph.deployment.service_spec import PlacementSpec, ServiceSpec
+from ceph.deployment.service_spec import PlacementSpec, ServiceSpec, service_spec_allow_invalid_from_json
 from ceph.deployment.hostspec import SpecValidationError
 from ceph.utils import datetime_now
 
@@ -563,11 +563,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 5e3f2883b8a..6c847b86881 100644
--- a/src/pybind/mgr/orchestrator/tests/test_orchestrator.py
+++ b/src/pybind/mgr/orchestrator/tests/test_orchestrator.py
@@ -1,5 +1,6 @@
 
 import json
+import textwrap
 
 import pytest
 import yaml
@@ -156,6 +157,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 0735bb6e30c..683e5b70d4b 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 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
 
@@ -394,6 +395,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.
@@ -535,7 +552,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)}"')
@@ -586,7 +602,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