From 05e2ade13292e7d924e6385ea234f58c13eb2d51 Mon Sep 17 00:00:00 2001 From: Ali Masarwa Date: Sun, 29 Sep 2024 15:00:13 +0300 Subject: [PATCH] RGW|Bucket notification: fix for v2 topics rgw-admin list operation Signed-off-by: Ali Masarwa (cherry picked from commit 575a19d2a81e959d21f64d1a9ed39a5db3b92957) --- src/rgw/rgw_admin.cc | 50 ++++++++++++--------- src/rgw/rgw_pubsub.cc | 21 ++++----- src/rgw/rgw_pubsub.h | 11 +++-- src/rgw/rgw_rest_pubsub.cc | 9 +++- src/test/rgw/bucket_notification/test_bn.py | 11 ++--- 5 files changed, 58 insertions(+), 44 deletions(-) diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc index 0ce00d993e962..7bdd0544899ac 100644 --- a/src/rgw/rgw_admin.cc +++ b/src/rgw/rgw_admin.cc @@ -11082,22 +11082,22 @@ next: } 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 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 subscribed_buckets; ret = driver->get_bucket_topic_mapping(topic, subscribed_buckets, null_yield, dpp()); if (ret < 0) { @@ -11105,15 +11105,21 @@ next: << 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()) { diff --git a/src/rgw/rgw_pubsub.cc b/src/rgw/rgw_pubsub.cc index 7eca57e430866..20ee4f8c17dfd 100644 --- a/src/rgw/rgw_pubsub.cc +++ b/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, diff --git a/src/rgw/rgw_pubsub.h b/src/rgw/rgw_pubsub.h index b7ce443af037e..8a6b290cb8568 100644 --- a/src/rgw/rgw_pubsub.h +++ b/src/rgw/rgw_pubsub.h @@ -643,9 +643,14 @@ public: // 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" diff --git a/src/rgw/rgw_rest_pubsub.cc b/src/rgw/rgw_rest_pubsub.cc index 7a93c7114e2ca..01fdcceb54510 100644 --- a/src/rgw/rgw_rest_pubsub.cc +++ b/src/rgw/rgw_rest_pubsub.cc @@ -473,8 +473,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) { diff --git a/src/test/rgw/bucket_notification/test_bn.py b/src/test/rgw/bucket_notification/test_bn.py index f03be32d01b4c..8f93dd5d2f3a5 100644 --- a/src/test/rgw/bucket_notification/test_bn.py +++ b/src/test/rgw/bucket_notification/test_bn.py @@ -611,19 +611,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') -- 2.39.5