Merge pull request ceph#60045 from AliMasarweh/wip-alimasa-notif-list-bucket

RGW|Bucket notification: fix for v2 topics rgw-admin list operation

Reviewed-by: yuvalif<[email protected]>

Author: AliMasarweh

src/rgw/rgw_admin.cc

@@ -11187,38 +11187,44 @@ int main(int argc, const char **argv)
     }
 
     formatter->open_object_section("result");
-    formatter->open_array_section("topics");
-    do {
-      rgw_pubsub_topics result;
-      int ret = ps.get_topics(dpp(), next_token, max_entries,
-                              result, next_token, null_yield);
-      if (ret < 0 && ret != -ENOENT) {
-        cerr << "ERROR: could not get topics: " << cpp_strerror(-ret) << std::endl;
-        return -ret;
-      }
-      for (const auto& [_, topic] : result.topics) {
-        if (owner && *owner != topic.owner) {
-          continue;
+    rgw_pubsub_topics result;
+    if (rgw::all_zonegroups_support(*site, rgw::zone_features::notification_v2) &&
+        driver->stat_topics_v1(tenant, null_yield, dpp()) == -ENOENT) {
+      formatter->open_array_section("topics");
+      do {
+        int ret = ps.get_topics_v2(dpp(), next_token, max_entries,
+                                   result, next_token, null_yield);
+        if (ret < 0 && ret != -ENOENT) {
+          cerr << "ERROR: could not get topics: " << cpp_strerror(-ret) << std::endl;
+          return -ret;
         }
-        std::set<std::string> subscribed_buckets;
-        if (rgw::all_zonegroups_support(*site, rgw::zone_features::notification_v2) &&
-            driver->stat_topics_v1(tenant, null_yield, dpp()) == -ENOENT) {
+        for (const auto& [_, topic] : result.topics) {
+          if (owner && *owner != topic.owner) {
+            continue;
+          }
+          std::set<std::string> subscribed_buckets;
           ret = driver->get_bucket_topic_mapping(topic, subscribed_buckets,
                                                  null_yield, dpp());
           if (ret < 0) {
             cerr << "failed to fetch bucket topic mapping info for topic: "
                  << topic.name << ", ret=" << ret << std::endl;
           }
           show_topics_info_v2(topic, subscribed_buckets, formatter.get());
-        } else {
-          encode_json("result", result, formatter.get());
-        }
-        if (max_entries_specified) {
-          --max_entries;
+          if (max_entries_specified) {
+            --max_entries;
+          }
         }
+        result.topics.clear();
+      } while (!next_token.empty() && max_entries > 0);
+      formatter->close_section(); // topics
+    } else { // v1, list all topics
+      int ret = ps.get_topics_v1(dpp(), result, null_yield);
+      if (ret < 0 && ret != -ENOENT) {
+        cerr << "ERROR: could not get topics: " << cpp_strerror(-ret) << std::endl;
+        return -ret;
       }
-    } while (!next_token.empty() && max_entries > 0);
-    formatter->close_section(); // topics
+      encode_json("result", result, formatter.get());
+    }
     if (max_entries_specified) {
       encode_json("truncated", !next_token.empty(), formatter.get());
       if (!next_token.empty()) {

src/rgw/rgw_pubsub.cc

@@ -570,22 +570,16 @@ RGWPubSub::RGWPubSub(rgw::sal::Driver* _driver,
 {
 }
 
-int RGWPubSub::get_topics(const DoutPrefixProvider* dpp,
-                          const std::string& start_marker, int max_items,
-                          rgw_pubsub_topics& result, std::string& next_marker,
-                          optional_yield y) const
+int RGWPubSub::get_topics_v2(const DoutPrefixProvider* dpp,
+                             const std::string& start_marker, int max_items,
+                             rgw_pubsub_topics& result, std::string& next_marker,
+                             optional_yield y) const
 {
   if (rgw::account::validate_id(tenant)) {
     // if our tenant is an account, return the account listing
     return list_account_topics(dpp, start_marker, max_items,
                                result, next_marker, y);
   }
-
-  if (!use_notification_v2 || driver->stat_topics_v1(tenant, y, dpp) != -ENOENT) {
-    // in case of v1 or during migration we use v1 topics
-    // v1 returns all topics, ignoring marker/max_items
-    return read_topics_v1(dpp, result, nullptr, y);
-  }
 
   // TODO: prefix filter on 'tenant:'
   void* handle = NULL;
@@ -629,6 +623,13 @@ int RGWPubSub::get_topics(const DoutPrefixProvider* dpp,
   return ret;
 }
 
+int RGWPubSub::get_topics_v1(const DoutPrefixProvider* dpp,
+                             rgw_pubsub_topics& result,
+                             optional_yield y) const
+{
+  return read_topics_v1(dpp, result, nullptr, y);
+}
+
 int RGWPubSub::list_account_topics(const DoutPrefixProvider* dpp,
                                    const std::string& start_marker,
                                    int max_items, rgw_pubsub_topics& result,

src/rgw/rgw_pubsub.h

@@ -643,9 +643,14 @@ class RGWPubSub
 
   // get a paginated list of topics
   // return 0 on success, error code otherwise
-  int get_topics(const DoutPrefixProvider* dpp,
-                 const std::string& start_marker, int max_items,
-                 rgw_pubsub_topics& result, std::string& next_marker,
+  int get_topics_v2(const DoutPrefixProvider* dpp,
+                    const std::string& start_marker, int max_items,
+                    rgw_pubsub_topics& result, std::string& next_marker,
+                    optional_yield y) const;
+
+  // return 0 on success, error code otherwise
+  int get_topics_v1(const DoutPrefixProvider* dpp,
+                 rgw_pubsub_topics& result,
                  optional_yield y) const;
 
   // get a topic with by its name and populate it into "result"

src/rgw/rgw_rest_pubsub.cc

@@ -493,8 +493,13 @@ void RGWPSListTopicsOp::execute(optional_yield y) {
   const std::string start_token = s->info.args.get("NextToken");
 
   const RGWPubSub ps(driver, get_account_or_tenant(s->owner.id), *s->penv.site);
-  constexpr int max_items = 100;
-  op_ret = ps.get_topics(this, start_token, max_items, result, next_token, y);
+  if (rgw::all_zonegroups_support(*s->penv.site, rgw::zone_features::notification_v2) &&
+      driver->stat_topics_v1(s->bucket->get_tenant(), null_yield, this) == -ENOENT) {
+    op_ret = ps.get_topics_v1(this, result, y);
+  } else {
+    constexpr int max_items = 100;
+    op_ret = ps.get_topics_v2(this, start_token, max_items, result, next_token, y);
+  }
   // if there are no topics it is not considered an error
   op_ret = op_ret == -ENOENT ? 0 : op_ret;
   if (op_ret < 0) {

src/test/rgw/bucket_notification/test_bn.py

@@ -711,19 +711,16 @@ def test_ps_s3_topic_on_master():
     assert_equal(status, 404)
 
     # get the remaining 2 topics
-    result, status = topic_conf1.get_list()
-    assert_equal(status, 200)
-    assert_equal(len(result['ListTopicsResponse']['ListTopicsResult']['Topics']['member']), 2)
+    list_topics(2, tenant)
 
     # delete topics
-    result = topic_conf2.del_config()
+    status = topic_conf2.del_config()
     assert_equal(status, 200)
-    result = topic_conf3.del_config()
+    status = topic_conf3.del_config()
     assert_equal(status, 200)
 
     # get topic list, make sure it is empty
-    result, status = topic_conf1.get_list()
-    assert_equal(result['ListTopicsResponse']['ListTopicsResult']['Topics'], None)
+    list_topics(0, tenant)
 
 
 @attr('basic_test')