rbd-mirror: remove mirror image at shut_down when there is no images

Some cases makes the ImageReplayer to be eternally restarted if there is
no local and remote images.

If both images are absent and that the local image id exists, the
ImageReplayer shutdown will request a mirror image removal.

Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@cern.ch>
(cherry picked from commit 0c1c7fb886fcaaff5f00937cf62cf69feb8d4deb)

 Conflicts:
	src/tools/rbd_mirror/image_deleter/TrashMoveRequest.cc

Added a condition to handle the case where m_image_ctx is null on
close_image and handle_close_image in the TrashMoveRequest. This fix is
not needed in newer versions of Ceph as ImageCtx no longer needs to be
destroyed explicitely with a destroy method after Octopus.

diff --git a/src/test/rbd_mirror/test_mock_ImageReplayer.cc b/src/test/rbd_mirror/test_mock_ImageReplayer.cc
index 57c2639a00ced2cda881268b60e699e5a1ed2749..e75fa0ac4907ccc79369deb59766e2440deec8b1 100644 (file)
--- a/src/test/rbd_mirror/test_mock_ImageReplayer.cc
+++ b/src/test/rbd_mirror/test_mock_ImageReplayer.cc
@@ -283,10 +283,11 @@ public:
   void expect_send(MockBootstrapRequest& mock_bootstrap_request,
                    MockStateBuilder& mock_state_builder,
                    librbd::MockTestImageCtx& mock_local_image_ctx,
-                   bool do_resync, int r) {
+                   bool do_resync, bool set_local_image, int r) {
     EXPECT_CALL(mock_bootstrap_request, send())
       .WillOnce(Invoke([this, &mock_bootstrap_request, &mock_state_builder,
-                        &mock_local_image_ctx, do_resync, r]() {
+                        &mock_local_image_ctx, set_local_image, do_resync,
+                        r]() {
             if (r == 0 || r == -ENOLINK) {
               mock_state_builder.local_image_id = mock_local_image_ctx.id;
               mock_state_builder.remote_image_id = m_remote_image_ctx->id;
@@ -296,9 +297,15 @@ public:
               mock_state_builder.local_image_ctx = &mock_local_image_ctx;
               *mock_bootstrap_request.do_resync = do_resync;
             }
-            if (r < 0) {
+            if (r < 0 && r != -ENOENT) {
               mock_state_builder.remote_image_id = "";
             }
+            if (r == -ENOENT) {
+              *mock_bootstrap_request.state_builder = &mock_state_builder;
+            }
+            if (set_local_image) {
+              mock_state_builder.local_image_id = mock_local_image_ctx.id;
+            }
             mock_bootstrap_request.on_finish->complete(r);
           }));
   }
@@ -405,7 +412,7 @@ TEST_F(TestMockImageReplayer, StartStop) {
   MockBootstrapRequest mock_bootstrap_request;
   MockStateBuilder mock_state_builder;
   expect_send(mock_bootstrap_request, mock_state_builder, mock_local_image_ctx,
-              false, 0);
+              false, false, 0);
 
   expect_create_replayer(mock_state_builder, mock_replayer);
   expect_init(mock_replayer, 0);
@@ -447,8 +454,42 @@ TEST_F(TestMockImageReplayer, LocalImagePrimary) {
 
   MockStateBuilder mock_state_builder;
   expect_send(mock_bootstrap_request, mock_state_builder, mock_local_image_ctx,
-              false, -ENOMSG);
+              false, false, -ENOMSG);
+
+  expect_mirror_image_status_exists(false);
+
+  create_image_replayer(mock_threads);
+
+  C_SaferCond start_ctx;
+  m_image_replayer->start(&start_ctx);
+  ASSERT_EQ(0, start_ctx.wait());
+}
+
+TEST_F(TestMockImageReplayer, MetadataCleanup) {
+  // START
+
+  create_local_image();
+  librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx);
+
+  MockThreads mock_threads(m_threads);
+  expect_work_queue_repeatedly(mock_threads);
+  expect_add_event_after_repeatedly(mock_threads);
+
+  MockImageDeleter mock_image_deleter;
+  MockBootstrapRequest mock_bootstrap_request;
+  MockReplayer mock_replayer;
+
+  expect_get_replay_status(mock_replayer);
+  expect_set_mirror_image_status_repeatedly();
+
+  InSequence seq;
+
+  MockStateBuilder mock_state_builder;
+  expect_send(mock_bootstrap_request, mock_state_builder, mock_local_image_ctx,
+              false, true, -ENOLINK);
 
+  expect_close(mock_state_builder, 0);
+  expect_trash_move(mock_image_deleter, "global image id", false, 0);
   expect_mirror_image_status_exists(false);
 
   create_image_replayer(mock_threads);
@@ -475,7 +516,7 @@ TEST_F(TestMockImageReplayer, BootstrapRemoteDeleted) {
   MockBootstrapRequest mock_bootstrap_request;
   MockStateBuilder mock_state_builder;
   expect_send(mock_bootstrap_request, mock_state_builder, mock_local_image_ctx,
-              false, -ENOLINK);
+              false, false, -ENOLINK);
 
   expect_close(mock_state_builder, 0);
 
@@ -506,7 +547,7 @@ TEST_F(TestMockImageReplayer, BootstrapResyncRequested) {
   MockBootstrapRequest mock_bootstrap_request;
   MockStateBuilder mock_state_builder;
   expect_send(mock_bootstrap_request, mock_state_builder, mock_local_image_ctx,
-              true, 0);
+              true, false, 0);
 
   expect_close(mock_state_builder, 0);
 
@@ -536,7 +577,7 @@ TEST_F(TestMockImageReplayer, BootstrapError) {
   InSequence seq;
   MockStateBuilder mock_state_builder;
   expect_send(mock_bootstrap_request, mock_state_builder, mock_local_image_ctx,
-              false, -EINVAL);
+              false, false, -EINVAL);
 
   expect_mirror_image_status_exists(false);
 
@@ -599,7 +640,7 @@ TEST_F(TestMockImageReplayer, StopError) {
   InSequence seq;
   MockStateBuilder mock_state_builder;
   expect_send(mock_bootstrap_request, mock_state_builder, mock_local_image_ctx,
-              false, 0);
+              false, false, 0);
 
   expect_create_replayer(mock_state_builder, mock_replayer);
   expect_init(mock_replayer, 0);
@@ -638,7 +679,7 @@ TEST_F(TestMockImageReplayer, ReplayerError) {
   InSequence seq;
   MockStateBuilder mock_state_builder;
   expect_send(mock_bootstrap_request, mock_state_builder, mock_local_image_ctx,
-              false, 0);
+              false, false, 0);
 
   expect_create_replayer(mock_state_builder, mock_replayer);
   expect_init(mock_replayer, -EINVAL);
@@ -675,7 +716,7 @@ TEST_F(TestMockImageReplayer, ReplayerResync) {
   InSequence seq;
   MockStateBuilder mock_state_builder;
   expect_send(mock_bootstrap_request, mock_state_builder, mock_local_image_ctx,
-              false, 0);
+              false, false, 0);
 
   expect_create_replayer(mock_state_builder, mock_replayer);
   expect_init(mock_replayer, 0);
@@ -718,7 +759,7 @@ TEST_F(TestMockImageReplayer, ReplayerInterrupted) {
   InSequence seq;
   MockStateBuilder mock_state_builder;
   expect_send(mock_bootstrap_request, mock_state_builder, mock_local_image_ctx,
-              false, 0);
+              false, false, 0);
 
   expect_create_replayer(mock_state_builder, mock_replayer);
   expect_init(mock_replayer, 0);
@@ -766,7 +807,7 @@ TEST_F(TestMockImageReplayer, ReplayerRenamed) {
   InSequence seq;
   MockStateBuilder mock_state_builder;
   expect_send(mock_bootstrap_request, mock_state_builder, mock_local_image_ctx,
-              false, 0);
+              false, false, 0);
 
   expect_create_replayer(mock_state_builder, mock_replayer);
   expect_init(mock_replayer, 0);

diff --git a/src/tools/rbd_mirror/image_deleter/TrashMoveRequest.cc b/src/tools/rbd_mirror/image_deleter/TrashMoveRequest.cc
index 5a2dc9c7243a90d9e46526dafafe3739ea55f6a3..4372e5ebe6942d77c38eddfd3683d976a883339a 100644 (file)
--- a/src/tools/rbd_mirror/image_deleter/TrashMoveRequest.cc
+++ b/src/tools/rbd_mirror/image_deleter/TrashMoveRequest.cc
@@ -180,6 +180,13 @@ template <typename I>
 void TrashMoveRequest<I>::handle_open_image(int r) {
   dout(10) << "r=" << r << dendl;
 
+  if (r == -ENOENT) {
+    dout(5) << "mirror image does not exist, removing orphaned metadata" << dendl;
+    m_image_ctx = nullptr;
+    remove_mirror_image();
+    return;
+  }
+
   if (r < 0) {
     derr << "failed to open image: " << cpp_strerror(r) << dendl;
     m_image_ctx->destroy();
@@ -337,6 +344,10 @@ template <typename I>
 void TrashMoveRequest<I>::close_image() {
   dout(10) << dendl;
 
+  if (m_image_ctx == nullptr) {
+    handle_close_image(0);
+    return;
+  }
   Context *ctx = create_context_callback<
     TrashMoveRequest<I>, &TrashMoveRequest<I>::handle_close_image>(this);
   m_image_ctx->state->close(ctx);
@@ -346,8 +357,10 @@ template <typename I>
 void TrashMoveRequest<I>::handle_close_image(int r) {
   dout(10) << "r=" << r << dendl;
 
-  m_image_ctx->destroy();
-  m_image_ctx = nullptr;
+  if (m_image_ctx != nullptr) {
+    m_image_ctx->destroy();
+    m_image_ctx = nullptr;
+  }
 
   if (r < 0) {
     derr << "failed to close image: " << cpp_strerror(r) << dendl;

diff --git a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc
index 7241618be2b9a81c4462ba39acd168a689f7f1a1..ca77d87f8fa64b9b7a737bce099bbe467d776239 100644 (file)
--- a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc
+++ b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc
@@ -194,10 +194,10 @@ void BootstrapRequest<I>::handle_prepare_remote_image(int r) {
     // TODO need to support multiple remote images
     if (state_builder != nullptr &&
         state_builder->remote_image_id.empty() &&
-        !state_builder->local_image_id.empty() &&
-        state_builder->is_linked()) {
-      // local image exists and is non-primary and linked to the missing
-      // remote image
+        (state_builder->local_image_id.empty() ||
+         state_builder->is_linked())) {
+      // both images doesn't exist or local image exists and is non-primary
+      // and linked to the missing remote image
       finish(-ENOLINK);
     } else {
       finish(-ENOENT);

diff --git a/src/tools/rbd_mirror/image_replayer/StateBuilder.h b/src/tools/rbd_mirror/image_replayer/StateBuilder.h
index d055c84e02068fcad5aca354314a9e1aacc4efd2..807fe7f9162e4e01f37c92a86b4717e82c49cc3f 100644 (file)
--- a/src/tools/rbd_mirror/image_replayer/StateBuilder.h
+++ b/src/tools/rbd_mirror/image_replayer/StateBuilder.h
@@ -81,13 +81,13 @@ public:
 
   std::string global_image_id;
 
-  std::string local_image_id;
+  std::string local_image_id{};
   librbd::mirror::PromotionState local_promotion_state =
     librbd::mirror::PROMOTION_STATE_PRIMARY;
   ImageCtxT* local_image_ctx = nullptr;
 
   std::string remote_mirror_uuid;
-  std::string remote_image_id;
+  std::string remote_image_id{};
   librbd::mirror::PromotionState remote_promotion_state =
     librbd::mirror::PROMOTION_STATE_NON_PRIMARY;
   ImageCtxT* remote_image_ctx = nullptr;