From f2986a4400bb73d5ed3a80f626011609976c375a Mon Sep 17 00:00:00 2001
From: Patrick Donnelly <pdonnell@redhat.com>
Date: Thu, 21 Nov 2019 12:31:16 -0800
Subject: [PATCH] mon/MgrMonitor: blacklist previous instance

This wasn't realized as necessary early on in the ceph-mgr development
because the mgr didn't interact with RADOS. However, now it is becoming
common for plugins to store data there. It's important that the previous
instance can no longer interact with RADOS while the new mgr takes over.

In particular, this means that the mgr's client sessions with the MDS
are automatically evicted once the MDS receives the new OSDMap. This
avoids a pesky "unresponsive client" warning in the cluster logs.

Fixes: https://tracker.ceph.com/issues/42939
Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
---
 src/common/options.cc |  8 ++++++++
 src/mgr/Mgr.cc        | 41 ++++++++++++++++++++++++-----------------
 src/mon/MgrMap.h      |  9 ++++++++-
 src/mon/MgrMonitor.cc | 29 ++++++++++++++++++++++++++++-
 src/osdc/Objecter.cc  |  6 +++---
 src/osdc/Objecter.h   |  2 +-
 6 files changed, 72 insertions(+), 23 deletions(-)

diff --git a/src/common/options.cc b/src/common/options.cc
index 4a250a77409..705bca131bc 100644
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -2089,6 +2089,14 @@ std::vector<Option> get_global_options() {
                      "daemons remain in the OSD map")
     .set_flag(Option::FLAG_RUNTIME),
 
+    Option("mon_mgr_blacklist_interval", Option::TYPE_FLOAT, Option::LEVEL_DEV)
+    .set_default(1_day)
+    .set_min(1_hr)
+    .add_service("mon")
+    .set_description("Duration in seconds that blacklist entries for mgr "
+                     "daemons remain in the OSD map")
+    .set_flag(Option::FLAG_RUNTIME),
+
     Option("mon_osd_crush_smoke_test", Option::TYPE_BOOL, Option::LEVEL_ADVANCED)
     .set_default(true)
     .add_service("mon")
diff --git a/src/mgr/Mgr.cc b/src/mgr/Mgr.cc
index 8a8b7e9f28a..c216d604a33 100644
--- a/src/mgr/Mgr.cc
+++ b/src/mgr/Mgr.cc
@@ -248,22 +248,6 @@ void Mgr::init()
   register_async_signal_handler_oneshot(SIGINT, handle_mgr_signal);
   register_async_signal_handler_oneshot(SIGTERM, handle_mgr_signal);
 
-  // Start communicating with daemons to learn statistics etc
-  int r = server.init(monc->get_global_id(), client_messenger->get_myaddrs());
-  if (r < 0) {
-    derr << "Initialize server fail: " << cpp_strerror(r) << dendl;
-    // This is typically due to a bind() failure, so let's let
-    // systemd restart us.
-    exit(1);
-  }
-  dout(4) << "Initialized server at " << server.get_myaddrs() << dendl;
-
-  // Preload all daemon metadata (will subsequently keep this
-  // up to date by watching maps, so do the initial load before
-  // we subscribe to any maps)
-  dout(4) << "Loading daemon metadata..." << dendl;
-  load_all_metadata();
-
   // subscribe to all the maps
   monc->sub_want("log-info", 0, 0);
   monc->sub_want("mgrdigest", 0, 0);
@@ -281,9 +265,32 @@ void Mgr::init()
 
   // Start Objecter and wait for OSD map
   lock.unlock();  // Drop lock because OSDMap dispatch calls into my ms_dispatch
-  objecter->wait_for_osd_map();
+  epoch_t e;
+  cluster_state.with_mgrmap([&e](const MgrMap& m) {
+    e = m.last_failure_osd_epoch;
+  });
+  /* wait for any blacklists to be applied to previous mgr instance */
+  dout(4) << "Waiting for new OSDMap (e=" << e
+          << ") that may blacklist prior active." << dendl;
+  objecter->wait_for_osd_map(e);
   lock.lock();
 
+  // Start communicating with daemons to learn statistics etc
+  int r = server.init(monc->get_global_id(), client_messenger->get_myaddrs());
+  if (r < 0) {
+    derr << "Initialize server fail: " << cpp_strerror(r) << dendl;
+    // This is typically due to a bind() failure, so let's let
+    // systemd restart us.
+    exit(1);
+  }
+  dout(4) << "Initialized server at " << server.get_myaddrs() << dendl;
+
+  // Preload all daemon metadata (will subsequently keep this
+  // up to date by watching maps, so do the initial load before
+  // we subscribe to any maps)
+  dout(4) << "Loading daemon metadata..." << dendl;
+  load_all_metadata();
+
   // Populate PGs in ClusterState
   cluster_state.with_osdmap_and_pgmap([this](const OSDMap &osd_map,
 					     const PGMap& pg_map) {
diff --git a/src/mon/MgrMap.h b/src/mon/MgrMap.h
index ab82b2d63e4..410ebd2f611 100644
--- a/src/mon/MgrMap.h
+++ b/src/mon/MgrMap.h
@@ -223,6 +223,7 @@ public:
   };
 
   epoch_t epoch = 0;
+  epoch_t last_failure_osd_epoch = 0;
 
   /// global_id of the ceph-mgr instance selected as a leader
   uint64_t active_gid = 0;
@@ -255,6 +256,7 @@ public:
   std::map<std::string, std::string> services;
 
   epoch_t get_epoch() const { return epoch; }
+  epoch_t get_last_failure_osd_epoch() const { return last_failure_osd_epoch; }
   entity_addrvec_t get_active_addrs() const { return active_addrs; }
   uint64_t get_active_gid() const { return active_gid; }
   bool get_available() const { return available; }
@@ -379,7 +381,7 @@ public:
       ENCODE_FINISH(bl);
       return;
     }
-    ENCODE_START(9, 6, bl);
+    ENCODE_START(10, 6, bl);
     encode(epoch, bl);
     encode(active_addrs, bl, features);
     encode(active_gid, bl);
@@ -392,6 +394,7 @@ public:
     encode(active_change, bl);
     encode(always_on_modules, bl);
     encode(active_mgr_features, bl);
+    encode(last_failure_osd_epoch, bl);
     ENCODE_FINISH(bl);
     return;
   }
@@ -440,6 +443,9 @@ public:
     if (struct_v >= 9) {
       decode(active_mgr_features, p);
     }
+    if (struct_v >= 10) {
+      decode(last_failure_osd_epoch, p);
+    }
     DECODE_FINISH(p);
   }
 
@@ -491,6 +497,7 @@ public:
       }
       f->close_section();
     }
+    f->dump_int("last_failure_osd_epoch", last_failure_osd_epoch);
     f->close_section();
   }
 
diff --git a/src/mon/MgrMonitor.cc b/src/mon/MgrMonitor.cc
index c9cf10fc2d8..a361978fcdc 100644
--- a/src/mon/MgrMonitor.cc
+++ b/src/mon/MgrMonitor.cc
@@ -454,6 +454,12 @@ bool MgrMonitor::prepare_beacon(MonOpRequestRef op)
     dout(4) << "Active daemon restart (mgr." << m->get_name() << ")" << dendl;
     mon->clog->info() << "Active manager daemon " << m->get_name()
                       << " restarted";
+    if (!mon->osdmon()->is_writeable()) {
+      dout(1) << __func__ << ":  waiting for osdmon writeable to"
+                 " blacklist old instance." << dendl;
+      mon->osdmon()->wait_for_writeable(op, new C_RetryMessage(this, op));
+      return false;
+    }
     drop_active();
   }
 
@@ -744,7 +750,8 @@ void MgrMonitor::tick()
   }
 
   if (pending_map.active_gid != 0
-      && last_beacon.at(pending_map.active_gid) < cutoff) {
+      && last_beacon.at(pending_map.active_gid) < cutoff
+      && mon->osdmon()->is_writeable()) {
     const std::string old_active_name = pending_map.active_name;
     drop_active();
     propose = true;
@@ -814,10 +821,21 @@ bool MgrMonitor::promote_standby()
 
 void MgrMonitor::drop_active()
 {
+  ceph_assert(mon->osdmon()->is_writeable());
+
   if (last_beacon.count(pending_map.active_gid) > 0) {
     last_beacon.erase(pending_map.active_gid);
   }
 
+  ceph_assert(pending_map.active_gid > 0);
+  auto until = ceph_clock_now();
+  until += g_conf().get_val<double>("mon_mgr_blacklist_interval");
+  dout(5) << "blacklisting previous mgr." << pending_map.active_name << "."
+          << pending_map.active_gid << " ("
+          << pending_map.active_addrs << ")" << dendl;
+  auto blacklist_epoch = mon->osdmon()->blacklist(pending_map.active_addrs, until);
+  request_proposal(mon->osdmon());
+
   pending_metadata_rm.insert(pending_map.active_name);
   pending_metadata.erase(pending_map.active_name);
   pending_map.active_name = "";
@@ -827,6 +845,7 @@ void MgrMonitor::drop_active()
   pending_map.available = false;
   pending_map.active_addrs = entity_addrvec_t();
   pending_map.services.clear();
+  pending_map.last_failure_osd_epoch = blacklist_epoch;
 
   // So that when new active mgr subscribes to mgrdigest, it will
   // get an immediate response instead of waiting for next timer
@@ -1023,6 +1042,10 @@ bool MgrMonitor::prepare_command(MonOpRequestRef op)
     if (!err.empty()) {
       // Does not parse as a gid, treat it as a name
       if (pending_map.active_name == who) {
+        if (!mon->osdmon()->is_writeable()) {
+          mon->osdmon()->wait_for_writeable(op, new C_RetryMessage(this, op));
+          return false;
+        }
         drop_active();
         changed = true;
       } else {
@@ -1042,6 +1065,10 @@ bool MgrMonitor::prepare_command(MonOpRequestRef op)
       }
     } else {
       if (pending_map.active_gid == gid) {
+        if (!mon->osdmon()->is_writeable()) {
+          mon->osdmon()->wait_for_writeable(op, new C_RetryMessage(this, op));
+          return false;
+        }
         drop_active();
         changed = true;
       } else if (pending_map.standbys.count(gid) > 0) {
diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc
index 37dc01d0144..6665957ddb3 100644
--- a/src/osdc/Objecter.cc
+++ b/src/osdc/Objecter.cc
@@ -1911,10 +1911,10 @@ void Objecter::close_session(OSDSession *s)
   logger->set(l_osdc_osd_sessions, osd_sessions.size());
 }
 
-void Objecter::wait_for_osd_map()
+void Objecter::wait_for_osd_map(epoch_t e)
 {
   unique_lock l(rwlock);
-  if (osdmap->get_epoch()) {
+  if (osdmap->get_epoch() >= e) {
     l.unlock();
     return;
   }
@@ -1925,7 +1925,7 @@ void Objecter::wait_for_osd_map()
   bool done;
   std::unique_lock mlock{lock};
   C_SafeCond *context = new C_SafeCond(lock, cond, &done, NULL);
-  waiting_for_map[0].push_back(pair<Context*, int>(context, 0));
+  waiting_for_map[e].push_back(pair<Context*, int>(context, 0));
   l.unlock();
   cond.wait(mlock, [&done] { return done; });
 }
diff --git a/src/osdc/Objecter.h b/src/osdc/Objecter.h
index f669ce31c5f..c9413e1f354 100644
--- a/src/osdc/Objecter.h
+++ b/src/osdc/Objecter.h
@@ -2185,7 +2185,7 @@ private:
   void handle_osd_backoff(class MOSDBackoff *m);
   void handle_watch_notify(class MWatchNotify *m);
   void handle_osd_map(class MOSDMap *m);
-  void wait_for_osd_map();
+  void wait_for_osd_map(epoch_t e=0);
 
   /**
    * Get std::list of entities blacklisted since this was last called,
-- 
2.39.5