osd/OSDMonitor: check svc is writeable before changing pending

Fixes: https://tracker.ceph.com/issues/59813
Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

diff --git a/src/mon/AuthMonitor.cc b/src/mon/AuthMonitor.cc
index 8cb6789394b36dd08cc8aa99f250f14f47cbeb1f..9fe7f409f4798c9c51b52c80a6ae43d0b79b6931 100644 (file)
--- a/src/mon/AuthMonitor.cc
+++ b/src/mon/AuthMonitor.cc
@@ -1123,11 +1123,12 @@ int AuthMonitor::validate_osd_destroy(
   return 0;
 }
 
-int AuthMonitor::do_osd_destroy(
-    const EntityName& cephx_entity,
-    const EntityName& lockbox_entity)
+void AuthMonitor::do_osd_destroy(
+     const EntityName& cephx_entity,
+     const EntityName& lockbox_entity)
 {
   ceph_assert(paxos.is_plugged());
+  ceph_assert(is_writeable());
 
   dout(10) << __func__ << " cephx " << cephx_entity
                        << " lockbox " << lockbox_entity << dendl;
@@ -1150,14 +1151,13 @@ int AuthMonitor::do_osd_destroy(
 
   if (!removed) {
     dout(10) << __func__ << " entities do not exist -- no-op." << dendl;
-    return 0;
+    return;
   }
 
   // given we have paxos plugged, this will not result in a proposal
   // being triggered, but it will still be needed so that we get our
   // pending state encoded into the paxos' pending transaction.
   propose_pending();
-  return 0;
 }
 
 int _create_auth(

diff --git a/src/mon/AuthMonitor.h b/src/mon/AuthMonitor.h
index 51fba9949774082131a6e2012d6776c322b5c4b7..b0c7dbe8bb4e0207132f47bc5d50bcaa52eb41b1 100644 (file)
--- a/src/mon/AuthMonitor.h
+++ b/src/mon/AuthMonitor.h
@@ -231,9 +231,9 @@ private:
       EntityName& cephx_entity,
       EntityName& lockbox_entity,
       std::stringstream& ss);
-  int do_osd_destroy(
-      const EntityName& cephx_entity,
-      const EntityName& lockbox_entity);
+  void do_osd_destroy(
+       const EntityName& cephx_entity,
+       const EntityName& lockbox_entity);
 
   int do_osd_new(
       const auth_entity_t& cephx_entity,

diff --git a/src/mon/KVMonitor.cc b/src/mon/KVMonitor.cc
index 37a81a8048d4eb9151b53de100af5c2bd88cac00..0e9511c7c96754e6177aa0d4cfaad55518187065 100644 (file)
--- a/src/mon/KVMonitor.cc
+++ b/src/mon/KVMonitor.cc
@@ -368,6 +368,8 @@ int KVMonitor::validate_osd_destroy(
 
 void KVMonitor::do_osd_destroy(int32_t id, uuid_d& uuid)
 {
+  ceph_assert(is_writeable());
+
   string dmcrypt_prefix = _get_dmcrypt_prefix(uuid, "");
   string daemon_prefix =
     "daemon-private/osd." + stringify(id) + "/";
@@ -416,6 +418,7 @@ void KVMonitor::do_osd_new(
   const string& dmcrypt_key)
 {
   ceph_assert(paxos.is_plugged());
+  ceph_assert(is_writeable());
 
   string dmcrypt_key_prefix = _get_dmcrypt_prefix(uuid, "luks");
   bufferlist dmcrypt_key_value;

diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index 4ad44d0309d5c960e0912b2d09b19fa2f837b368..f3eacea69b6b55b94afbd45b2aa628225d057686 100644 (file)
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -9783,6 +9783,7 @@ static int parse_reweights(CephContext *cct,
 }
 
 int OSDMonitor::prepare_command_osd_destroy(
+    MonOpRequestRef op,
     int32_t id,
     stringstream& ss)
 {
@@ -9814,10 +9815,11 @@ int OSDMonitor::prepare_command_osd_destroy(
   EntityName cephx_entity, lockbox_entity;
   bool idempotent_auth = false, idempotent_cks = false;
 
-  int err = mon.authmon()->validate_osd_destroy(id, uuid,
-                                                 cephx_entity,
-                                                 lockbox_entity,
-                                                 ss);
+  auto&& authmon = mon.authmon();
+  int err = authmon->validate_osd_destroy(id, uuid,
+                                          cephx_entity,
+                                          lockbox_entity,
+                                          ss);
   if (err < 0) {
     if (err == -ENOENT) {
       idempotent_auth = true;
@@ -9826,21 +9828,28 @@ int OSDMonitor::prepare_command_osd_destroy(
     }
   }
 
-  auto svc = mon.kvmon();
-  err = svc->validate_osd_destroy(id, uuid);
+  auto&& kvmon = mon.kvmon();
+  err = kvmon->validate_osd_destroy(id, uuid);
   if (err < 0) {
     ceph_assert(err == -ENOENT);
     err = 0;
     idempotent_cks = true;
   }
 
-  if (!idempotent_auth) {
-    err = mon.authmon()->do_osd_destroy(cephx_entity, lockbox_entity);
-    ceph_assert(0 == err);
+  if (!idempotent_auth && !authmon->is_writeable()) {
+    authmon->wait_for_writeable(op, new C_RetryMessage(this, op));
+    return -EAGAIN;
+  }
+  if (!idempotent_cks && !kvmon->is_writeable()) {
+    kvmon->wait_for_writeable(op, new C_RetryMessage(this, op));
+    return -EAGAIN;
   }
 
+  if (!idempotent_auth) {
+    authmon->do_osd_destroy(cephx_entity, lockbox_entity);
+  }
   if (!idempotent_cks) {
-    svc->do_osd_destroy(id, uuid);
+    kvmon->do_osd_destroy(id, uuid);
   }
 
   pending_inc.new_state[id] = CEPH_OSD_DESTROYED;
@@ -9855,6 +9864,7 @@ int OSDMonitor::prepare_command_osd_destroy(
 }
 
 int OSDMonitor::prepare_command_osd_purge(
+    MonOpRequestRef op,
     int32_t id,
     stringstream& ss)
 {
@@ -9895,7 +9905,11 @@ int OSDMonitor::prepare_command_osd_purge(
 
   // no point destroying the osd again if it has already been marked destroyed
   if (!osdmap.is_destroyed(id)) {
-    err = prepare_command_osd_destroy(id, ss);
+    /* N.B.: up to this point, we've not changed pending at all.
+     * ::prepare_command_osd_destroy may return -EAGAIN if the kvmon/authmon is
+     * not writeable without changing pending. It will queue `op` if we should wait.
+     */
+    err = prepare_command_osd_destroy(op, id, ss);
     if (err < 0) {
       if (err == -ENOENT) {
         err = 0;
@@ -12669,11 +12683,17 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op,
 
     paxos.plug();
     if (is_destroy) {
-      err = prepare_command_osd_destroy(id, ss);
+      err = prepare_command_osd_destroy(op, id, ss);
+      if (err == EAGAIN) {
+        return false;
+      }
       // we checked above that it should exist.
       ceph_assert(err != -ENOENT);
     } else {
-      err = prepare_command_osd_purge(id, ss);
+      err = prepare_command_osd_purge(op, id, ss);
+      if (err == EAGAIN) {
+        return false;
+      }
       if (err == -ENOENT) {
         err = 0;
         ss << "osd." << id << " does not exist.";

diff --git a/src/mon/OSDMonitor.h b/src/mon/OSDMonitor.h
index 921a9d98b3f1da417b50e6f070371a5bc97e723c..3f6226c057a19e67fb78892ab9465be29204f5f3 100644 (file)
--- a/src/mon/OSDMonitor.h
+++ b/src/mon/OSDMonitor.h
@@ -699,8 +699,8 @@ public:
   void do_osd_create(const int32_t id, const uuid_d& uuid,
                     const std::string& device_class,
                     int32_t* new_id);
-  int prepare_command_osd_purge(int32_t id, std::stringstream& ss);
-  int prepare_command_osd_destroy(int32_t id, std::stringstream& ss);
+  int prepare_command_osd_purge(MonOpRequestRef op, int32_t id, std::stringstream& ss);
+  int prepare_command_osd_destroy(MonOpRequestRef op, int32_t id, std::stringstream& ss);
   int _prepare_command_osd_crush_remove(
       CrushWrapper &newcrush,
       int32_t id,