From 179e9bd3296a973f0889e611d8269d06fb7faf38 Mon Sep 17 00:00:00 2001 From: Adam King Date: Tue, 17 Mar 2026 14:30:51 -0400 Subject: [PATCH] mgr/cephadm: serialize OSD class before returning for OSD rm status Fixes: https://tracker.ceph.com/issues/74862 Signed-off-by: Adam King --- src/pybind/mgr/cephadm/module.py | 4 +-- src/pybind/mgr/cephadm/services/osd.py | 40 ++++++++++++++++++++------ src/pybind/mgr/orchestrator/module.py | 17 ++++++++--- 3 files changed, 47 insertions(+), 14 deletions(-) diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py index 720e88343f1f..d6765d652c94 100644 --- a/src/pybind/mgr/cephadm/module.py +++ b/src/pybind/mgr/cephadm/module.py @@ -4394,11 +4394,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 4195f31ad72b..28a097c82ddd 100644 --- a/src/pybind/mgr/cephadm/services/osd.py +++ b/src/pybind/mgr/cephadm/services/osd.py @@ -648,6 +648,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, @@ -807,6 +814,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 @@ -821,6 +843,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): @@ -837,6 +860,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 @@ -860,14 +888,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): @@ -1030,6 +1050,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 39acef29cbe7..2d1d11005325 100644 --- a/src/pybind/mgr/orchestrator/module.py +++ b/src/pybind/mgr/orchestrator/module.py @@ -1645,10 +1645,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) -- 2.47.3