rbd-mirror: prevent blacklisting of local instance after failover/back

If the leader role is manually released, upon failback the instance
will have removed its local instance object, preventing RPC
messaging.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
(cherry picked from commit ce974305893c4262b62547ce184d514578358218)

diff --git a/src/test/rbd_mirror/test_Instances.cc b/src/test/rbd_mirror/test_Instances.cc
index f6a80e572b7aea55aa03e8305c9d6f11716639b0..b9085352ff2dfe3d9c377d79491647dab441d4c2 100644 (file)
--- a/src/test/rbd_mirror/test_Instances.cc
+++ b/src/test/rbd_mirror/test_Instances.cc
@@ -58,15 +58,18 @@ public:
     TestFixture::SetUp();
     m_local_io_ctx.remove(RBD_MIRROR_LEADER);
     EXPECT_EQ(0, m_local_io_ctx.create(RBD_MIRROR_LEADER, true));
+
+    m_instance_id = stringify(m_local_io_ctx.get_instance_id());
   }
 
   Listener m_listener;
+  std::string m_instance_id;
 };
 
 TEST_F(TestInstances, InitShutdown)
 {
   m_listener.add.count = 1;
-  Instances<> instances(m_threads, m_local_io_ctx, m_listener);
+  Instances<> instances(m_threads, m_local_io_ctx, m_instance_id, m_listener);
 
   std::string instance_id = "instance_id";
   ASSERT_EQ(0, librbd::cls_client::mirror_instances_add(&m_local_io_ctx,
@@ -89,7 +92,7 @@ TEST_F(TestInstances, InitShutdown)
 
 TEST_F(TestInstances, InitEnoent)
 {
-  Instances<> instances(m_threads, m_local_io_ctx, m_listener);
+  Instances<> instances(m_threads, m_local_io_ctx, m_instance_id, m_listener);
 
   m_local_io_ctx.remove(RBD_MIRROR_LEADER);
 
@@ -113,7 +116,7 @@ TEST_F(TestInstances, NotifyRemove)
 
   m_listener.add.count = 2;
   m_listener.remove.count = 1;
-  Instances<> instances(m_threads, m_local_io_ctx, m_listener);
+  Instances<> instances(m_threads, m_local_io_ctx, m_instance_id, m_listener);
 
   std::string instance_id1 = "instance_id1";
   std::string instance_id2 = "instance_id2";

diff --git a/src/test/rbd_mirror/test_mock_LeaderWatcher.cc b/src/test/rbd_mirror/test_mock_LeaderWatcher.cc
index f4eae19b9e3898a674f4bc828b7b62c061269f87..85c9c3c785c0dbded57c90922f3c1100bb1a51dc 100644 (file)
--- a/src/test/rbd_mirror/test_mock_LeaderWatcher.cc
+++ b/src/test/rbd_mirror/test_mock_LeaderWatcher.cc
@@ -212,7 +212,9 @@ struct Instances<librbd::MockTestImageCtx> {
   static Instances* s_instance;
 
   static Instances *create(Threads<librbd::MockTestImageCtx> *threads,
-                           librados::IoCtx &ioctx, instances::Listener&) {
+                           librados::IoCtx &ioctx,
+                           const std::string& instance_id,
+                           instances::Listener&) {
     assert(s_instance != nullptr);
     return s_instance;
   }
@@ -453,6 +455,10 @@ public:
     EXPECT_CALL(mock_instances, unblock_listener());
   }
 
+  void expect_instances_acked(MockInstances& mock_instances) {
+    EXPECT_CALL(mock_instances, acked(_));
+  }
+
   MockThreads *m_mock_threads;
 };
 
@@ -479,6 +485,7 @@ TEST_F(TestMockLeaderWatcher, InitShutdown) {
   expect_acquire_notify(mock_managed_lock, listener, 0);
   expect_unblock_listener(mock_instances);
   expect_notify_heartbeat(mock_managed_lock, &on_heartbeat_finish);
+  expect_instances_acked(mock_instances);
 
   ASSERT_EQ(0, leader_watcher.init());
   ASSERT_EQ(0, on_heartbeat_finish.wait());
@@ -517,6 +524,7 @@ TEST_F(TestMockLeaderWatcher, InitReleaseShutdown) {
   expect_acquire_notify(mock_managed_lock, listener, 0);
   expect_unblock_listener(mock_instances);
   expect_notify_heartbeat(mock_managed_lock, &on_heartbeat_finish);
+  expect_instances_acked(mock_instances);
 
   ASSERT_EQ(0, leader_watcher.init());
   ASSERT_EQ(0, on_heartbeat_finish.wait());
@@ -592,6 +600,7 @@ TEST_F(TestMockLeaderWatcher, AcquireError) {
   expect_acquire_notify(mock_managed_lock, listener, 0);
   expect_unblock_listener(mock_instances);
   expect_notify_heartbeat(mock_managed_lock, &on_heartbeat_finish);
+  expect_instances_acked(mock_instances);
 
   ASSERT_EQ(0, leader_watcher.init());
   ASSERT_EQ(0, on_heartbeat_finish.wait());
@@ -647,6 +656,7 @@ TEST_F(TestMockLeaderWatcher, Break) {
   expect_acquire_notify(mock_managed_lock, listener, 0);
   expect_unblock_listener(mock_instances);
   expect_notify_heartbeat(mock_managed_lock, &on_heartbeat_finish);
+  expect_instances_acked(mock_instances);
 
   ASSERT_EQ(0, leader_watcher.init());
   ASSERT_EQ(0, on_heartbeat_finish.wait());

diff --git a/src/tools/rbd_mirror/Instances.cc b/src/tools/rbd_mirror/Instances.cc
index 7f4033570b66f649bcac5dd0fbb87263b08a6885..17d81bfdf6ef5366db3ae6536dc3261ce360aa56 100644 (file)
--- a/src/tools/rbd_mirror/Instances.cc
+++ b/src/tools/rbd_mirror/Instances.cc
@@ -26,9 +26,10 @@ using librbd::util::create_rados_callback;
 
 template <typename I>
 Instances<I>::Instances(Threads<I> *threads, librados::IoCtx &ioctx,
+                        const std::string& instance_id,
                         instances::Listener& listener) :
-  m_threads(threads), m_ioctx(ioctx), m_listener(listener),
-  m_cct(reinterpret_cast<CephContext *>(ioctx.cct())),
+  m_threads(threads), m_ioctx(ioctx), m_instance_id(instance_id),
+  m_listener(listener), m_cct(reinterpret_cast<CephContext *>(ioctx.cct())),
   m_lock("rbd::mirror::Instances " + ioctx.get_pool_name()) {
 }
 
@@ -243,6 +244,9 @@ void Instances<I>::remove_instances(const utime_t& time) {
 
   InstanceIds instance_ids;
   for (auto& instance_pair : m_instances) {
+    if (instance_pair.first == m_instance_id) {
+      continue;
+    }
     auto& instance = instance_pair.second;
     if (instance.state != INSTANCE_STATE_REMOVING &&
         instance.acked_time <= time) {
@@ -317,6 +321,9 @@ void Instances<I>::schedule_remove_task(const utime_t& time) {
   bool schedule = false;
   utime_t oldest_time = time;
   for (auto& instance : m_instances) {
+    if (instance.first == m_instance_id) {
+      continue;
+    }
     if (instance.second.state == INSTANCE_STATE_REMOVING) {
       // removal is already in-flight
       continue;

diff --git a/src/tools/rbd_mirror/Instances.h b/src/tools/rbd_mirror/Instances.h
index 3a4f5f3e8445c097da25ac24c18de1ff040c9940..f36cc2df9d7029bc1e350809e76fad45a857cc5c 100644 (file)
--- a/src/tools/rbd_mirror/Instances.h
+++ b/src/tools/rbd_mirror/Instances.h
@@ -28,15 +28,16 @@ public:
 
   static Instances *create(Threads<ImageCtxT> *threads,
                            librados::IoCtx &ioctx,
+                           const std::string& instance_id,
                            instances::Listener& listener) {
-    return new Instances(threads, ioctx, listener);
+    return new Instances(threads, ioctx, instance_id, listener);
   }
   void destroy() {
     delete this;
   }
 
   Instances(Threads<ImageCtxT> *threads, librados::IoCtx &ioctx,
-            instances::Listener& listener);
+            const std::string& instance_id, instances::Listener& listener);
   virtual ~Instances();
 
   void init(Context *on_finish);
@@ -129,6 +130,7 @@ private:
 
   Threads<ImageCtxT> *m_threads;
   librados::IoCtx &m_ioctx;
+  std::string m_instance_id;
   instances::Listener& m_listener;
   CephContext *m_cct;
 

diff --git a/src/tools/rbd_mirror/LeaderWatcher.cc b/src/tools/rbd_mirror/LeaderWatcher.cc
index f9edf021b7742cde5a26695a1c195b4a9ad87241..8f0c2c9654c7cac03b2a3e305daa34f42f160c5a 100644 (file)
--- a/src/tools/rbd_mirror/LeaderWatcher.cc
+++ b/src/tools/rbd_mirror/LeaderWatcher.cc
@@ -32,6 +32,7 @@ LeaderWatcher<I>::LeaderWatcher(Threads<I> *threads, librados::IoCtx &io_ctx,
     m_threads(threads), m_listener(listener), m_instances_listener(this),
     m_lock("rbd::mirror::LeaderWatcher " + io_ctx.get_pool_name()),
     m_notifier_id(librados::Rados(io_ctx).get_instance_id()),
+    m_instance_id(stringify(m_notifier_id)),
     m_leader_lock(new LeaderLock(m_ioctx, m_work_queue, m_oid, this, true,
                                  m_cct->_conf->get_val<int64_t>(
                                    "rbd_blacklist_expire_seconds"))) {
@@ -48,7 +49,7 @@ LeaderWatcher<I>::~LeaderWatcher() {
 
 template <typename I>
 std::string LeaderWatcher<I>::get_instance_id() {
-  return stringify(m_notifier_id);
+  return m_instance_id;
 }
 
 template <typename I>
@@ -284,7 +285,7 @@ bool LeaderWatcher<I>::get_leader_instance_id(std::string *instance_id) const {
   Mutex::Locker locker(m_lock);
 
   if (is_leader(m_lock) || is_releasing_leader(m_lock)) {
-    *instance_id = stringify(m_notifier_id);
+    *instance_id = m_instance_id;
     return true;
   }
 
@@ -765,7 +766,8 @@ void LeaderWatcher<I>::init_instances() {
   assert(m_lock.is_locked());
   assert(m_instances == nullptr);
 
-  m_instances = Instances<I>::create(m_threads, m_ioctx, m_instances_listener);
+  m_instances = Instances<I>::create(m_threads, m_ioctx, m_instance_id,
+                                     m_instances_listener);
 
   Context *ctx = create_context_callback<
     LeaderWatcher<I>, &LeaderWatcher<I>::handle_init_instances>(this);
@@ -992,10 +994,6 @@ void LeaderWatcher<I>::handle_notify_heartbeat(int r) {
   std::vector<std::string> instance_ids;
   for (auto &it: m_heartbeat_response.acks) {
     uint64_t notifier_id = it.first.gid;
-    if (notifier_id == m_notifier_id) {
-      continue;
-    }
-
     instance_ids.push_back(stringify(notifier_id));
   }
   if (!instance_ids.empty()) {

diff --git a/src/tools/rbd_mirror/LeaderWatcher.h b/src/tools/rbd_mirror/LeaderWatcher.h
index b21a781697736bd2e6f85f0984751be0301af9b8..14571357c093d9c494a5d3ba8d8122d5cc2856e5 100644 (file)
--- a/src/tools/rbd_mirror/LeaderWatcher.h
+++ b/src/tools/rbd_mirror/LeaderWatcher.h
@@ -210,6 +210,7 @@ private:
   InstancesListener m_instances_listener;
   mutable Mutex m_lock;
   uint64_t m_notifier_id;
+  std::string m_instance_id;
   LeaderLock *m_leader_lock;
   Context *m_on_finish = nullptr;
   Context *m_on_shut_down_finish = nullptr;