From 4c50ad69c37110d42f1f68f6e567cdf5ac506a32 Mon Sep 17 00:00:00 2001
From: Casey Bodley <cbodley@redhat.com>
Date: Wed, 20 Mar 2024 14:14:29 -0400
Subject: [PATCH] rgw/pubsub: RGWPubSub::remove_topic() removes persistent
 queue

move the persistent queue removal into remove_topic() where we have
access to the topic metadata. avoid trying to remove the queue if it
isn't enabled

Signed-off-by: Casey Bodley <cbodley@redhat.com>
---
 src/rgw/rgw_admin.cc       |  7 -------
 src/rgw/rgw_pubsub.cc      | 28 ++++++++++++++++++++++++++--
 src/rgw/rgw_rest_pubsub.cc |  9 ---------
 3 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc
index 58117e96ea3cc..dc147f7f132f1 100644
--- a/src/rgw/rgw_admin.cc
+++ b/src/rgw/rgw_admin.cc
@@ -11194,13 +11194,6 @@ 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_pubsub.cc b/src/rgw/rgw_pubsub.cc
index ce84b38724a3e..0f292ab3758f2 100644
--- a/src/rgw/rgw_pubsub.cc
+++ b/src/rgw/rgw_pubsub.cc
@@ -12,6 +12,7 @@
 #include "rgw_arn.h"
 #include "rgw_pubsub_push.h"
 #include "rgw_bucket.h"
+#include "driver/rados/rgw_notify.h"
 #include "common/errno.h"
 #include "include/function2.hpp"
 #include <regex>
@@ -1086,7 +1087,17 @@ int RGWPubSub::remove_topic_v2(const DoutPrefixProvider* dpp,
                       << dendl;
     return ret;
   }
-  return ret;
+
+  const rgw_pubsub_dest& dest = topic.dest;
+  if (!dest.push_endpoint.empty() && dest.persistent &&
+      !dest.persistent_queue.empty()) {
+    ret = rgw::notify::remove_persistent_topic(topic.name, y);
+    if (ret < 0 && ret != -ENOENT) {
+      ldpp_dout(dpp, 1) << "WARNING: failed to remove queue for "
+          "persistent topic: " << cpp_strerror(ret) << dendl;
+    } // not fatal
+  }
+  return 0;
 }
 
 int RGWPubSub::remove_topic(const DoutPrefixProvider *dpp, const std::string& name, optional_yield y) const
@@ -1112,7 +1123,12 @@ int RGWPubSub::remove_topic(const DoutPrefixProvider *dpp, const std::string& na
       return 0;
   }
 
-  topics.topics.erase(name);
+  auto t = topics.topics.find(name);
+  if (t == topics.topics.end()) {
+    return -ENOENT;
+  }
+  const rgw_pubsub_dest dest = std::move(t->second.dest);
+  topics.topics.erase(t);
 
   ret = write_topics_v1(dpp, topics, &objv_tracker, y);
   if (ret < 0) {
@@ -1120,5 +1136,13 @@ int RGWPubSub::remove_topic(const DoutPrefixProvider *dpp, const std::string& na
     return ret;
   }
 
+  if (!dest.push_endpoint.empty() && dest.persistent &&
+      !dest.persistent_queue.empty()) {
+    ret = rgw::notify::remove_persistent_topic(name, y);
+    if (ret < 0 && ret != -ENOENT) {
+      ldpp_dout(dpp, 1) << "WARNING: failed to remove queue for "
+          "persistent topic: " << cpp_strerror(ret) << dendl;
+    } // not fatal
+  }
   return 0;
 }
diff --git a/src/rgw/rgw_rest_pubsub.cc b/src/rgw/rgw_rest_pubsub.cc
index 7e1b0491a1939..ff5c824a6be6d 100644
--- a/src/rgw/rgw_rest_pubsub.cc
+++ b/src/rgw/rgw_rest_pubsub.cc
@@ -992,15 +992,6 @@ void RGWPSDeleteTopicOp::execute(optional_yield y) {
     // 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
-  // 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:"
-                       << ret << dendl;
-  }
 }
 
 using op_generator = RGWOp*(*)(bufferlist);
-- 
2.39.5