From 0bcd29a247f4a01ba69c6d43db668ef3dcc1610d Mon Sep 17 00:00:00 2001
From: Sebastian Wagner <sebastian.wagner@suse.com>
Date: Mon, 14 Sep 2020 15:15:12 +0200
Subject: [PATCH] mgr/cephadm: fix RemoveUtil.load_from_store()

* Run `json.loads` only once
* `nodename` -> `hostname`
* Remove network calls from `repr(OSD(...)`

Fixes: https://tracker.ceph.com/issues/47438

Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
---
 src/pybind/mgr/cephadm/services/osd.py        | 15 ++++++-----
 .../mgr/cephadm/tests/test_osd_removal.py     | 25 ++++++++++++++++++-
 src/pybind/mgr/orchestrator/module.py         |  2 +-
 3 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/src/pybind/mgr/cephadm/services/osd.py b/src/pybind/mgr/cephadm/services/osd.py
index 9f350e82f5856..2d83c2f363b81 100644
--- a/src/pybind/mgr/cephadm/services/osd.py
+++ b/src/pybind/mgr/cephadm/services/osd.py
@@ -338,8 +338,8 @@ class RemoveUtil(object):
 
             if not osd.exists:
                 continue
-            self.mgr._remove_daemon(osd.fullname, osd.nodename)
-            logger.info(f"Successfully removed OSD <{osd.osd_id}> on {osd.nodename}")
+            self.mgr._remove_daemon(osd.fullname, osd.hostname)
+            logger.info(f"Successfully removed OSD <{osd.osd_id}> on {osd.hostname}")
             logger.debug(f"Removing {osd.osd_id} from the queue.")
 
         # self.mgr.to_remove_osds could change while this is processing (osds get added from the CLI)
@@ -464,7 +464,7 @@ class RemoveUtil(object):
         for k, v in self.mgr.get_store_prefix('osd_remove_queue').items():
             for osd in json.loads(v):
                 logger.debug(f"Loading osd ->{osd} from store")
-                osd_obj = OSD.from_json(json.loads(osd), ctx=self)
+                osd_obj = OSD.from_json(osd, ctx=self)
                 self.mgr.to_remove_osds.add(osd_obj)
 
 
@@ -518,7 +518,7 @@ class OSD:
         # If we wait for the osd to be drained
         self.force = force
         # The name of the node
-        self.nodename = hostname
+        self.hostname = hostname
         # The full name of the osd
         self.fullname = fullname
 
@@ -615,7 +615,7 @@ class OSD:
         out['stopped'] = self.stopped
         out['replace'] = self.replace
         out['force'] = self.force
-        out['nodename'] = self.nodename  # type: ignore
+        out['hostname'] = self.hostname  # type: ignore
 
         for k in ['drain_started_at', 'drain_stopped_at', 'drain_done_at', 'process_started_at']:
             if getattr(self, k):
@@ -632,6 +632,9 @@ class OSD:
             if inp.get(date_field):
                 inp.update({date_field: datetime.strptime(inp.get(date_field, ''), DATEFMT)})
         inp.update({'remove_util': ctx})
+        if 'nodename' in inp:
+            hostname = inp.pop('nodename')
+            inp['hostname'] = hostname
         return cls(**inp)
 
     def __hash__(self):
@@ -643,7 +646,7 @@ class OSD:
         return self.osd_id == other.osd_id
 
     def __repr__(self) -> str:
-        return f"<OSD>(osd_id={self.osd_id}, is_draining={self.is_draining})"
+        return f"<OSD>(osd_id={self.osd_id}, draining={self.draining})"
 
 
 class OSDQueue(Set):
diff --git a/src/pybind/mgr/cephadm/tests/test_osd_removal.py b/src/pybind/mgr/cephadm/tests/test_osd_removal.py
index 55179da363b1c..c9cfe464f2308 100644
--- a/src/pybind/mgr/cephadm/tests/test_osd_removal.py
+++ b/src/pybind/mgr/cephadm/tests/test_osd_removal.py
@@ -1,6 +1,8 @@
+import json
+
 from cephadm.services.osd import RemoveUtil, OSDQueue, OSD
 import pytest
-from .fixtures import rm_util, osd_obj
+from .fixtures import rm_util, osd_obj, cephadm_module
 from tests import mock
 from datetime import datetime
 
@@ -71,6 +73,27 @@ class TestOSDRemoval:
         rm_util._run_mon_cmd.assert_called_with(
             {'prefix': 'osd purge-actual', 'id': 1, 'yes_i_really_mean_it': True})
 
+    def test_load(self, cephadm_module):
+        data = json.dumps([
+            {
+                "osd_id": 35,
+                "started": True,
+                "draining": True,
+                "stopped": False,
+                "replace": False,
+                "force": False,
+                "nodename": "node2",
+                "drain_started_at": "2020-09-14T11:41:53.960463",
+                "drain_stopped_at": None,
+                "drain_done_at": None,
+                "process_started_at": "2020-09-14T11:41:52.245832"
+            }
+        ])
+        cephadm_module.set_store('osd_remove_queue', data)
+        cephadm_module.rm_util.load_from_store()
+
+        assert repr(cephadm_module.to_remove_osds) == 'OSDQueue({<OSD>(osd_id=35, draining=True)})'
+
 
 class TestOSD:
 
diff --git a/src/pybind/mgr/orchestrator/module.py b/src/pybind/mgr/orchestrator/module.py
index a21b2a588d22c..f61d02ff3de32 100644
--- a/src/pybind/mgr/orchestrator/module.py
+++ b/src/pybind/mgr/orchestrator/module.py
@@ -807,7 +807,7 @@ Usage:
             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.nodename, osd.drain_status_human(),
+                table.add_row([osd.osd_id, osd.hostname, osd.drain_status_human(),
                                osd.get_pg_count(), osd.replace, osd.replace, osd.drain_started_at])
             out = table.get_string()
 
-- 
2.39.5