mgr/cephadm: remove daemon before osd destroy/purge

Otherwise it doesn't work!

Drop the fullname property: it is always "osd.{self.osd_id}".

Signed-off-by: Sage Weil <sage@newdream.net>
(cherry picked from commit b5eab0ddfa0bb8ae7b1a6aec4ea2e4257a01a045)

diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py
index 4085f6542d934dfed27ce6c641994c40d1ba998a..ede8a1c3456f0160f8dd8cc7cda2e4bdf44f1ed9 100644 (file)
--- a/src/pybind/mgr/cephadm/module.py
+++ b/src/pybind/mgr/cephadm/module.py
@@ -2281,7 +2281,6 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule,
                                                 replace=replace,
                                                 force=force,
                                                 hostname=daemon.hostname,
-                                                fullname=daemon.name(),
                                                 process_started_at=datetime_now(),
                                                 remove_util=self.to_remove_osds.rm_util))
             except NotFoundError:

diff --git a/src/pybind/mgr/cephadm/services/osd.py b/src/pybind/mgr/cephadm/services/osd.py
index 7da145134618a27fd80bd9b64565941dad00902f..46f2a5ff67a76c0c8dc4f8edbff002b96abd2956 100644 (file)
--- a/src/pybind/mgr/cephadm/services/osd.py
+++ b/src/pybind/mgr/cephadm/services/osd.py
@@ -481,7 +481,6 @@ class OSD:
                  replace: bool = False,
                  force: bool = False,
                  hostname: Optional[str] = None,
-                 fullname: Optional[str] = None,
                  ):
         # the ID of the OSD
         self.osd_id = osd_id
@@ -513,8 +512,6 @@ class OSD:
         self.force = force
         # The name of the node
         self.hostname = hostname
-        # The full name of the osd
-        self.fullname = fullname
 
         # mgr obj to make mgr/mon calls
         self.rm_util: RemoveUtil = remove_util
@@ -709,21 +706,24 @@ class OSDRemovalQueue(object):
                 raise orchestrator.OrchestratorError(
                     f"Could not set OSD <{osd.osd_id}> to 'down'")
 
+            # stop and remove daemon
+            assert osd.hostname is not None
+            CephadmServe(self.mgr)._remove_daemon(f'osd.{osd.osd_id}', osd.hostname)
+            logger.info(f"Successfully removed {osd} on {osd.hostname}")
+
             if osd.replace:
+                # mark destroyed in osdmap
                 if not osd.destroy():
                     raise orchestrator.OrchestratorError(
-                        f"Could not destroy OSD <{osd.osd_id}>")
+                        f"Could not destroy {osd}")
+                logger.info(f"Successfully destroyed old {osd} on {osd.hostname}; ready for replacement")
             else:
+                # purge from osdmap
                 if not osd.purge():
-                    raise orchestrator.OrchestratorError(f"Could not purge OSD <{osd.osd_id}>")
+                    raise orchestrator.OrchestratorError(f"Could not purge {osd}")
+                logger.info(f"Successfully purged {osd} on {osd.hostname}")
 
-            if not osd.exists:
-                continue
-            assert osd.fullname is not None
-            assert osd.hostname is not None
-            CephadmServe(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.")
+            logger.debug(f"Removing {osd} from the queue.")
 
         # self could change while this is processing (osds get added from the CLI)
         # The new set is: 'an intersection of all osds that are still not empty/removed (new_queue) and

diff --git a/src/pybind/mgr/cephadm/tests/test_cephadm.py b/src/pybind/mgr/cephadm/tests/test_cephadm.py
index 310c763827542dfe07ab3ae81f0be7ac7cccfa2c..375fe63c22e75a0109780e00be420cf3defcb102 100644 (file)
--- a/src/pybind/mgr/cephadm/tests/test_cephadm.py
+++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py
@@ -515,7 +515,6 @@ class TestCephadm(object):
                                                       replace=False,
                                                       force=False,
                                                       hostname='test',
-                                                      fullname='osd.0',
                                                       process_started_at=datetime_now(),
                                                       remove_util=cephadm_module.to_remove_osds.rm_util
                                                       ))

diff --git a/src/pybind/mgr/cephadm/tests/test_osd_removal.py b/src/pybind/mgr/cephadm/tests/test_osd_removal.py
index c2bf95b439403b86268f1c89309d9a00967022c4..cffdcaf03fbcbed85095218b8deca730f9476505 100644 (file)
--- a/src/pybind/mgr/cephadm/tests/test_osd_removal.py
+++ b/src/pybind/mgr/cephadm/tests/test_osd_removal.py
@@ -104,13 +104,25 @@ class TestOSD:
         assert osd_obj.started is True
         assert osd_obj.stopped is False
 
-    def test_start_draining(self, osd_obj):
+    def test_start_draining_purge(self, osd_obj):
         assert osd_obj.draining is False
         assert osd_obj.drain_started_at is None
         ret = osd_obj.start_draining()
+        osd_obj.rm_util.reweight_osd.assert_called_with(osd_obj, 0.0)
+        assert isinstance(osd_obj.drain_started_at, datetime)
+        assert osd_obj.draining is True
+        assert osd_obj.replace is False
+        assert ret is True
+
+    def test_start_draining_replace(self, osd_obj):
+        assert osd_obj.draining is False
+        assert osd_obj.drain_started_at is None
+        osd_obj.replace = True
+        ret = osd_obj.start_draining()
         osd_obj.rm_util.set_osd_flag.assert_called_with([osd_obj], 'out')
         assert isinstance(osd_obj.drain_started_at, datetime)
         assert osd_obj.draining is True
+        assert osd_obj.replace is True
         assert ret is True
 
     def test_start_draining_stopped(self, osd_obj):
@@ -120,13 +132,22 @@ class TestOSD:
         assert ret is False
         assert osd_obj.draining is False
 
-    def test_stop_draining(self, osd_obj):
+    def test_stop_draining_replace(self, osd_obj):
+        osd_obj.replace = True
         ret = osd_obj.stop_draining()
         osd_obj.rm_util.set_osd_flag.assert_called_with([osd_obj], 'in')
         assert isinstance(osd_obj.drain_stopped_at, datetime)
         assert osd_obj.draining is False
         assert ret is True
 
+    def test_stop_draining_purge(self, osd_obj):
+        osd_obj.original_weight = 1.0
+        ret = osd_obj.stop_draining()
+        osd_obj.rm_util.reweight_osd.assert_called_with(osd_obj, 1.0)
+        assert isinstance(osd_obj.drain_stopped_at, datetime)
+        assert osd_obj.draining is False
+        assert ret is True
+
     @mock.patch('cephadm.services.osd.OSD.stop_draining')
     def test_stop(self, stop_draining_mock, osd_obj):
         osd_obj.stop()