rgw/notifications: delete persistent queue only if topic is deleted

Signed-off-by: Yuval Lifshitz <ylifshit@redhat.com>

diff --git a/src/rgw/driver/rados/rgw_notify.cc b/src/rgw/driver/rados/rgw_notify.cc
index 19d4c0cbb6e3b1152decf55167756d9404f33091..3fe441eec14ee6337e036eb358c80f71c0a3a4a0 100644 (file)
--- a/src/rgw/driver/rados/rgw_notify.cc
+++ b/src/rgw/driver/rados/rgw_notify.cc
@@ -1065,7 +1065,7 @@ static inline bool notification_match(reservation_t& res,
       ldpp_dout(res.dpp, 1)
           << "INFO: failed to load topic: " << topic_cfg.name
           << ". error: " << ret
-          << " while resrving persistent notification event" << dendl;
+          << " while reserving persistent notification event" << dendl;
       if (ret == -ENOENT) {
         // either the topic is deleted but the corresponding notification still
         // exist or in v2 mode the notification could have synced first but

diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc
index 71f7abda805841f68e4ec0a3d9fd6ec0e4a50294..0125451d9a2391e7edbdef9e364aaa815dca36ef 100644 (file)
--- a/src/rgw/rgw_admin.cc
+++ b/src/rgw/rgw_admin.cc
@@ -10771,12 +10771,6 @@ next:
       cerr << "ERROR: Run 'topic rm' from master zone " << std::endl;
       return -EINVAL;
     }
-    ret = rgw::notify::remove_persistent_topic(
-        dpp(), static_cast<rgw::sal::RadosStore*>(driver)->getRados()->get_notif_pool_ctx(), topic_name, null_yield);
-    if (ret < 0) {
-      cerr << "ERROR: could not remove persistent topic: " << cpp_strerror(-ret) << std::endl;
-      return -ret;
-    }
 
     RGWPubSub ps(driver, tenant, *site);
 
@@ -10785,6 +10779,13 @@ next:
       cerr << "ERROR: could not remove topic: " << cpp_strerror(-ret) << std::endl;
       return -ret;
     }
+    
+    ret = rgw::notify::remove_persistent_topic(
+        dpp(), static_cast<rgw::sal::RadosStore*>(driver)->getRados()->get_notif_pool_ctx(), topic_name, null_yield);
+    if (ret < 0 && ret != -ENOENT) {
+      cerr << "ERROR: could not remove persistent topic: " << cpp_strerror(-ret) << std::endl;
+      return -ret;
+    }
   }
 
   if (opt_cmd == OPT::PUBSUB_NOTIFICATION_RM) {

diff --git a/src/rgw/rgw_rest_pubsub.cc b/src/rgw/rgw_rest_pubsub.cc
index 138150b002750287a31b17cc392ce9fc1bf8e5dd..585eb68caf9bda57ce5a9ba422301d845353b0e2 100644 (file)
--- a/src/rgw/rgw_rest_pubsub.cc
+++ b/src/rgw/rgw_rest_pubsub.cc
@@ -814,35 +814,35 @@ void RGWPSDeleteTopicOp::execute(optional_yield y) {
     op_ret = verify_topic_owner_or_policy(
         s, result, driver->get_zone()->get_zonegroup().get_name(),
         rgw::IAM::snsDeleteTopic);
-    if (op_ret != 0) {
+    if (op_ret < 0) {
       ldpp_dout(this, 1) << "no permission to remove topic '" << topic_name
                          << "'" << dendl;
       return;
     }
-  } else {
+    op_ret = ps.remove_topic(this, topic_name, y);
+    if (op_ret < 0 && op_ret != -ENOENT) {
+      ldpp_dout(this, 1) << "failed to remove topic '" << topic_name << ", ret=" << op_ret << dendl;
+      return;
+    }
+    ldpp_dout(this, 1) << "successfully removed topic '" << topic_name << "'" << dendl;
+  } else if (op_ret != -ENOENT) {
     ldpp_dout(this, 1) << "failed to fetch topic '" << topic_name
                        << "' with error: " << op_ret << dendl;
-    if (op_ret == -ENOENT) {
-      // its not an error if no topics exist, just a no-op
-      op_ret = 0;
-    }
     return;
   }
+  if (op_ret == -ENOENT) {
+    // its not an error if no topics exist, just a no-op
+    op_ret = 0;
+  }
   // upon deletion it is not known if topic is persistent or not
   // will try to delete the persistent topic anyway
-  op_ret = rgw::notify::remove_persistent_topic(topic_name, s->yield);
-  if (op_ret != -ENOENT && op_ret < 0) {
+  // doing this regardless of the topic being previously deleted
+  // to allow for cleanup if only the queue deletion failed
+  if (const auto ret = rgw::notify::remove_persistent_topic(topic_name, s->yield); ret < 0 && ret != -ENOENT) {
     ldpp_dout(this, 1) << "DeleteTopic Action failed to remove queue for "
                           "persistent topics. error:"
-                       << op_ret << dendl;
-    return;
-  }
-  op_ret = ps.remove_topic(this, topic_name, y);
-  if (op_ret < 0) {
-    ldpp_dout(this, 1) << "failed to remove topic '" << topic_name << ", ret=" << op_ret << dendl;
-    return;
+                       << ret << dendl;
   }
-  ldpp_dout(this, 1) << "successfully removed topic '" << topic_name << "'" << dendl;
 }
 
 using op_generator = RGWOp*(*)(bufferlist);