mgr/cephadm: serialize OSD class before returning for OSD rm status

Fixes: https://tracker.ceph.com/issues/74862
Signed-off-by: Adam King <adking@redhat.com>
(cherry picked from commit 179e9bd3296a973f0889e611d8269d06fb7faf38)

diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py
index 07ae2c7f6fc336509f5033c7abe24b290cc3ab06..171e3e1846f16493464a9262e1210d25d1afc955 100644 (file)
--- a/src/pybind/mgr/cephadm/module.py
+++ b/src/pybind/mgr/cephadm/module.py
@@ -4206,11 +4206,11 @@ Then run the following:
         return "Stopped OSD(s) removal"
 
     @handle_orch_error
-    def remove_osds_status(self) -> List[OSD]:
+    def remove_osds_status(self) -> List[Dict[str, Any]]:
         """
         The CLI call to retrieve an osd removal report
         """
-        return self.to_remove_osds.all_osds()
+        return self.to_remove_osds.all_osds_status_json()
 
     @handle_orch_error
     def set_osd_spec(self, service_name: str, osd_ids: List[str]) -> str:

diff --git a/src/pybind/mgr/cephadm/services/osd.py b/src/pybind/mgr/cephadm/services/osd.py
index 42a7e1ce9cf86c3e56f57aed096a99d8d39bf5ac..db300a5238f3c4e8920913d2e660c5722d4f34df 100644 (file)
--- a/src/pybind/mgr/cephadm/services/osd.py
+++ b/src/pybind/mgr/cephadm/services/osd.py
@@ -646,6 +646,13 @@ class NotFoundError(Exception):
 
 class OSD:
 
+    # fields we may add when converting to json so the orchestrator module
+    # can display them, but should not be passed back into the init function
+    display_only_fields = [
+        'pg_count',
+        'drain_status'
+    ]
+
     def __init__(self,
                  osd_id: int,
                  remove_util: RemoveUtil,
@@ -805,6 +812,21 @@ class OSD:
     def pg_count_str(self) -> str:
         return 'n/a' if self.get_pg_count() < 0 else str(self.get_pg_count())
 
+    def _get_display_only_fields(self) -> Dict[str, Any]:
+        _display_only_fields = {
+            'pg_count': self.pg_count_str(),
+            'drain_status': self.drain_status_human(),
+        }
+        # verify we're setting the expected set of fields here. This should cause
+        # failures in some of our teuthology tests if what we set here and what we have
+        # explicitly listed as being a display only field in the class attr differ
+        if sorted(list(_display_only_fields.keys())) != sorted(self.display_only_fields):
+            raise OrchestratorError(
+                f'Expected display specific fields {self.display_only_fields} '
+                f'to be set but instead got {list(_display_only_fields.keys())}'
+            )
+        return _display_only_fields
+
     def to_json(self) -> dict:
         out: Dict[str, Any] = dict()
         out['osd_id'] = self.osd_id
@@ -819,6 +841,7 @@ class OSD:
         out['zap'] = self.zap
         out['hostname'] = self.hostname  # type: ignore
         out['original_weight'] = self.original_weight
+        out.update(self._get_display_only_fields())
 
         for k in ['drain_started_at', 'drain_stopped_at', 'drain_done_at', 'process_started_at']:
             if getattr(self, k):
@@ -835,6 +858,11 @@ class OSD:
             if inp.get(date_field):
                 inp.update({date_field: str_to_datetime(inp.get(date_field, ''))})
         inp.update({'remove_util': rm_util})
+
+        if getattr(cls, 'display_only_fields', None):
+            for attr in cls.display_only_fields:
+                inp.pop(attr, None)
+
         if 'nodename' in inp:
             hostname = inp.pop('nodename')
             inp['hostname'] = hostname
@@ -858,14 +886,6 @@ class OSD:
     def __repr__(self) -> str:
         return f"osd.{self.osd_id}{' (draining)' if self.draining else ''}"
 
-    def __getstate__(self) -> Dict[str, Any]:
-        # the rm_util field of this class cannot be pickled
-        # and we should not need it in any case where this class
-        # has been serialized and deserialized. The from_json function also
-        # requires an instance of the class to explicitly be passed back in
-        self.__dict__.update({'remove_util': None})
-        return self.__dict__
-
 
 class OSDRemovalQueue(object):
 
@@ -1026,6 +1046,10 @@ class OSDRemovalQueue(object):
         with self.lock:
             return [osd for osd in self.osds]
 
+    def all_osds_status_json(self) -> List[Dict[str, Any]]:
+        with self.lock:
+            return [osd.to_json() for osd in self.osds]
+
     def _not_in_cluster(self) -> List["OSD"]:
         return [osd for osd in self.osds if not osd.exists]
 

diff --git a/src/pybind/mgr/orchestrator/module.py b/src/pybind/mgr/orchestrator/module.py
index 7502407a1203a69765dc0dffffc28c3728962676..bd0c0080791dc9f531655a3072ffb09a309ce312 100644 (file)
--- a/src/pybind/mgr/orchestrator/module.py
+++ b/src/pybind/mgr/orchestrator/module.py
@@ -1622,10 +1622,19 @@ Usage:
             table._align['PGS'] = 'r'
             table.left_padding_width = 0
             table.right_padding_width = 2
-            for osd in sorted(report, key=lambda o: o.osd_id):
-                table.add_row([osd.osd_id, osd.hostname, osd.drain_status_human(),
-                               osd.get_pg_count(), osd.replace, osd.force, osd.zap,
-                               osd.drain_started_at or ''])
+            for osd in sorted(report, key=lambda o: o.get('osd_id')):
+                table.add_row(
+                    [
+                        osd.get('osd_id'),
+                        osd.get('hostname'),
+                        osd.get('drain_status'),
+                        osd.get('pg_count'),
+                        osd.get('replace'),
+                        osd.get('force'),
+                        osd.get('zap'),
+                        osd.get('drain_started_at') or ''
+                    ]
+                )
             out = table.get_string()
 
         return HandleCommandResult(stdout=out)