From fa5398d31f9467b58a8b4eda55d1b5e2bbdb6500 Mon Sep 17 00:00:00 2001 From: Jesse Williamson Date: Fri, 8 Dec 2017 15:50:01 -0800 Subject: [PATCH] rgw: normalize delete/remove in admin console Normalizes "rm", deprecates any other (eg. "delete", del", or "remove"). Provides backward compatibility with existing commands, deprecates non-"rm". http://tracker.ceph.com/issues/14363 Cleanup: http://tracker.ceph.com/issues/22444 Signed-off-by: Jesse Williamson --- PendingReleaseNotes | 5 + doc/man/8/radosgw-admin.rst | 28 +++--- src/include/util.h | 11 +++ src/mon/ConfigKeyService.cc | 1 + src/mon/MgrStatMonitor.cc | 2 + src/mon/OSDMonitor.cc | 148 +++++++++++++++++++++++++----- src/mon/OSDMonitor.h | 2 + src/rgw/rgw_admin.cc | 28 +++--- src/test/cli/radosgw-admin/help.t | 16 ++-- 9 files changed, 182 insertions(+), 59 deletions(-) diff --git a/PendingReleaseNotes b/PendingReleaseNotes index 935a5e2f244..938a15e4d4c 100644 --- a/PendingReleaseNotes +++ b/PendingReleaseNotes @@ -1,6 +1,11 @@ 13.0.1 ------ +* *RGW, MON*: + Commands variously marked as "del", "delete", "remove" etc. should now all be + normalized as "rm". Commands already supporting alternatives to "rm" remain + backward-compatible. + * *CephFS*: * Several "ceph mds" commands have been obsoleted and replaced diff --git a/doc/man/8/radosgw-admin.rst b/doc/man/8/radosgw-admin.rst index d5db7fffae5..b4c66150961 100644 --- a/doc/man/8/radosgw-admin.rst +++ b/doc/man/8/radosgw-admin.rst @@ -134,8 +134,8 @@ which are as follows: :command:`objects expire` Run expired objects cleanup. -:command:`period delete` - Delete a period. +:command:`period rm` + Remove a period. :command:`period get` Get the period info. @@ -182,8 +182,8 @@ which are as follows: :command:`realm create` Create a new realm. -:command:`realm delete` - Delete a realm. +:command:`realm rm` + Remove a realm. :command:`realm get` Show the realm info. @@ -218,8 +218,8 @@ which are as follows: :command:`zonegroup default` Set the default zone group. -:command:`zonegroup delete` - Delete a zone group info. +:command:`zonegroup rm` + Remove a zone group info. :command:`zonegroup get` Show the zone group info. @@ -257,8 +257,8 @@ which are as follows: :command:`zone create` Create a new zone. -:command:`zone delete` - Delete a zone. +:command:`zone rm` + Remove a zone. :command:`zone get` Show zone cluster params. @@ -406,8 +406,8 @@ which are as follows: :command:`replicalog update` Update replica metadata log entry. -:command:`replicalog delete` - Delete replica metadata log entry. +:command:`replicalog rm` + Remove replica metadata log entry. :command:`orphans find` Init and run search for leaked rados objects @@ -421,8 +421,8 @@ which are as follows: :command:`role create` create a new AWS role for use with STS. -:command:`role delete` - Delete a role. +:command:`role rm` + Remove a role. :command:`role get` Get a role. @@ -442,8 +442,8 @@ which are as follows: :command:`role-policy get` Get the specified inline policy document embedded with the given role. -:command:`role-policy delete` - Delete the policy attached to a role +:command:`role-policy rm` + Remove the policy attached to a role :command:`reshard add` Schedule a resharding of a bucket diff --git a/src/include/util.h b/src/include/util.h index 4b27cc1f3e3..b3fee365657 100644 --- a/src/include/util.h +++ b/src/include/util.h @@ -86,4 +86,15 @@ void dump_services(Formatter* f, const map >& services, const string cleanbin(bufferlist &bl, bool &b64); string cleanbin(string &str); + +namespace ceph::util { + +// Returns true if s matches any parameters: +template +bool match_str(const std::string& s, const XS& ...xs) +{ + return ((s == xs) || ...); +} + +} // namespace ceph::util #endif /* CEPH_UTIL_H */ diff --git a/src/mon/ConfigKeyService.cc b/src/mon/ConfigKeyService.cc index 43181718480..dd774ae4347 100644 --- a/src/mon/ConfigKeyService.cc +++ b/src/mon/ConfigKeyService.cc @@ -23,6 +23,7 @@ #include "common/errno.h" #include "include/stringify.h" +#include "include/assert.h" // re-clobber assert() #define dout_subsys ceph_subsys_mon #undef dout_prefix #define dout_prefix _prefix(_dout, mon, this) diff --git a/src/mon/MgrStatMonitor.cc b/src/mon/MgrStatMonitor.cc index efc16671c43..8c55628975f 100644 --- a/src/mon/MgrStatMonitor.cc +++ b/src/mon/MgrStatMonitor.cc @@ -11,6 +11,8 @@ #include "messages/MStatfsReply.h" #include "messages/MServiceMap.h" +#include "include/assert.h" // re-clobber assert + #define dout_subsys ceph_subsys_mon #undef dout_prefix #define dout_prefix _prefix(_dout, mon) diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc index 57adb3f29fd..4eef4b1d6be 100644 --- a/src/mon/OSDMonitor.cc +++ b/src/mon/OSDMonitor.cc @@ -16,10 +16,13 @@ * */ -#include -#include +#include #include #include +#include + +#include +#include #include "mon/OSDMonitor.h" #include "mon/Monitor.h" @@ -92,6 +95,108 @@ const uint32_t MAX_POOL_APPLICATION_LENGTH = 128; } // anonymous namespace +// Helpers for parsing commands: +namespace { + +inline std::string make_ws_trimmed(const std::string& input) +{ + return input.substr( + input.find_first_not_of(" \t\n\r"), + input.find_last_not_of(" \t\n\r")); +} + +// A shame we don't have std::string_view, but this should not be /too/ high-overhead: +inline bool cmd_match(const string cmd_string, const string cmd_prefix, bool matching_fn(const string)) +try +{ + if(std::string::npos == cmd_string.find(cmd_prefix)) + return false; + + return matching_fn(make_ws_trimmed( + cmd_string.substr(cmd_prefix.size(), cmd_string.size()))); +} +catch(std::out_of_range&) +{ + return false; +} + +inline bool cmd_match(const string cmd_string, const string cmd_prefix, const string match_str) +try +{ + if(std::string::npos == cmd_string.find(cmd_prefix)) + return false; + + return match_str == make_ws_trimmed( + cmd_string.substr(cmd_prefix.size(), cmd_string.size())); +} +catch(std::out_of_range&) +{ + return false; +} + +// Matches the tail of a command, starting after cmd_prefix: +inline bool cmd_match_any(const string, const string) +{ + return false; +} + +template +inline bool cmd_match_any(const string cmd_string, const string cmd_prefix, MatchT match_with, MatchTS ...match_ts) +{ + return cmd_match(cmd_string, cmd_prefix, match_with) || cmd_match_any(cmd_string, cmd_prefix, match_ts...); +} + +// Note that ValueT is not defined in terms of value_type: +template +inline bool equal_to_any_in(const ValueT v, const std::vector&& xs) +{ + using namespace std; + + return any_of(begin(xs), end(xs), + [&v](const ValueT cand) { return v == cand; }); +} + +template +inline bool ends_with_any_in(const ValueT v, const std::vector&& xs) +{ + using namespace std; + + return any_of(begin(xs), end(xs), [&v](const ValueT cand) { + return boost::algorithm::ends_with(v, cand); + }); +} + +bool match_cmd_delete_remove(const std::string cmd) +{ + return equal_to_any_in(cmd, { "rm", "remove", "del", "delete" }); +} + +constexpr auto cmd_delete_remove_strings { "(rm|remove|del|delete)" }; + +// Pretty half-baked, but should make life easier in several situations: +// Helps handle the form "osd del-foo": +auto match_remove_cmd_in(const string prefix, const string alternatives) +{ + // Synthesize a regular expression such as: + // osd\s+(rm|remove|del|delete)+\-(noup|nodown|noin|noout)($|(\s+.*)) + string regex_pattern = prefix + R"(\s+)" + + cmd_delete_remove_strings + '+' + + R"(\-)" + + '(' + alternatives + ')' + + R"(($|(\s+.*)))" + ; + + return regex { regex_pattern }; +} + +bool matches_cmd_delete_remove(const std::string input, const std::string prefix, const std::string alternatives) +{ + return regex_match(input, + match_remove_cmd_in(prefix, "noup|nodown|noin|noout")); +} + +} // anonymous namespace + void LastEpochClean::Lec::report(ps_t ps, epoch_t last_epoch_clean) { if (epoch_by_pg.size() <= ps) { @@ -6519,7 +6624,7 @@ int OSDMonitor::prepare_command_pool_application(const string &prefix, p.application_metadata[app][key] = value; ss << "set application '" << app << "' key '" << key << "' to '" << value << "' on pool '" << pool_name << "'"; - } else if (boost::algorithm::ends_with(prefix, "rm")) { + } else if (ends_with_any_in(prefix, { "rm", "remove", "del", "delete" })) { if (!app_exists) { ss << "application '" << app << "' is not enabled on pool '" << pool_name << "'"; @@ -7753,12 +7858,11 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op, newcrush.encode(pending_inc.crush, mon->get_quorum_con_features()); goto update; - } else if (prefix == "osd crush weight-set rm" || - prefix == "osd crush weight-set rm-compat") { + } else if (cmd_match_any(prefix, "osd crush weight-set", &match_cmd_delete_remove, "rm-compat")) { CrushWrapper newcrush; _get_pending_crush(newcrush); int64_t pool; - if (prefix == "osd crush weight-set rm") { + if (cmd_match(prefix, "osd crush weight-set", &match_cmd_delete_remove)) { // eg. not "rm-compat" string poolname; cmd_getval(cct, cmdmap, "pool", poolname); pool = osdmap.lookup_pg_pool_name(poolname.c_str()); @@ -8099,9 +8203,7 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op, wait_for_finished_proposal(op, new Monitor::C_Command(mon, op, err, ss.str(), get_last_committed() + 1)); return true; - } else if (prefix == "osd crush rm" || - prefix == "osd crush remove" || - prefix == "osd crush unlink") { + } else if (cmd_match(prefix, "osd crush", "rm|remove|del|delete|unlink")) { do { // osd crush rm [ancestor] CrushWrapper newcrush; @@ -8407,7 +8509,7 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op, get_last_committed() + 1)); return true; - } else if (prefix == "osd erasure-code-profile rm") { + } else if (cmd_match_any(prefix, "osd erasure-code-profile", &match_cmd_delete_remove)) { string name; cmd_getval(cct, cmdmap, "name", name); @@ -8563,7 +8665,7 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op, get_last_committed() + 1)); return true; - } else if (prefix == "osd crush rule rm") { + } else if (cmd_match_any(prefix, "osd crush rule", &match_cmd_delete_remove)) { string name; cmd_getval(cct, cmdmap, "name", name); @@ -8973,7 +9075,7 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op, } else if (prefix == "osd down" || prefix == "osd out" || prefix == "osd in" || - prefix == "osd rm") { + cmd_match_any(prefix, "osd", &match_cmd_delete_remove)) { bool any = false; bool stop = false; @@ -9063,7 +9165,7 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op, ss << "marked in osd." << osd << ". "; any = true; } - } else if (prefix == "osd rm") { + } else if (cmd_match_any(prefix, "osd", &match_cmd_delete_remove)) { err = prepare_command_osd_remove(osd); if (err == -EBUSY) { @@ -9222,10 +9324,7 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op, get_last_committed() + 1)); return true; } - } else if (prefix == "osd rm-noup" || - prefix == "osd rm-nodown" || - prefix == "osd rm-noin" || - prefix == "osd rm-noout") { + } else if (matches_cmd_delete_remove(prefix, "osd", "noup|nodown|noin|noout")) { enum { OP_NOUP, @@ -10095,7 +10194,7 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op, wait_for_finished_proposal(op, new Monitor::C_Command(mon, op, 0, rs, get_last_committed() + 1)); return true; - } else if (blacklistop == "rm") { + } else if (match_cmd_delete_remove(blacklistop)) { if (osdmap.is_blacklisted(addr) || pending_inc.new_blacklist.count(addr)) { if (osdmap.is_blacklisted(addr)) @@ -10349,8 +10448,7 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op, get_last_committed() + 1)); return true; - } else if (prefix == "osd pool delete" || - prefix == "osd pool rm") { + } else if (cmd_match_any(prefix, "osd pool", &match_cmd_delete_remove)) { // osd pool delete/rm --yes-i-really-really-mean-it string poolstr, poolstr2, sure; cmd_getval(cct, cmdmap, "pool", poolstr); @@ -10501,8 +10599,7 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op, wait_for_finished_proposal(op, new Monitor::C_Command(mon, op, 0, ss.str(), get_last_committed() + 1)); return true; - } else if (prefix == "osd tier remove" || - prefix == "osd tier rm") { + } else if (cmd_match_any(prefix, "osd tier", &match_cmd_delete_remove)) { string poolstr; cmd_getval(cct, cmdmap, "pool", poolstr); int64_t pool_id = osdmap.lookup_pg_pool_name(poolstr); @@ -10618,8 +10715,9 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op, wait_for_finished_proposal(op, new Monitor::C_Command(mon, op, 0, ss.str(), get_last_committed() + 1)); return true; - } else if (prefix == "osd tier remove-overlay" || - prefix == "osd tier rm-overlay") { + } else if (cmd_match_any(prefix, "osd tier", + "remove-overlay", "rm-overlay", + "delete-overlay", "del-overlay")) { string poolstr; cmd_getval(cct, cmdmap, "pool", poolstr); int64_t pool_id = osdmap.lookup_pg_pool_name(poolstr); @@ -10950,7 +11048,7 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op, } else if (prefix == "osd pool application enable" || prefix == "osd pool application disable" || prefix == "osd pool application set" || - prefix == "osd pool application rm") { + cmd_match_any(prefix, "osd pool application", &match_cmd_delete_remove)) { err = prepare_command_pool_application(prefix, cmdmap, ss); if (err == -EAGAIN) goto wait; diff --git a/src/mon/OSDMonitor.h b/src/mon/OSDMonitor.h index 4b52cf54010..e445b8b2daf 100644 --- a/src/mon/OSDMonitor.h +++ b/src/mon/OSDMonitor.h @@ -24,6 +24,8 @@ #include #include +#include + #include "include/types.h" #include "common/simple_cache.hpp" #include "msg/Messenger.h" diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc index d7c43cc0662..13855f4e416 100644 --- a/src/rgw/rgw_admin.cc +++ b/src/rgw/rgw_admin.cc @@ -19,6 +19,8 @@ #include "common/errno.h" #include "common/safe_io.h" +#include "include/util.h" + #include "cls/rgw/cls_rgw_client.h" #include "global/global_init.h" @@ -94,7 +96,7 @@ void usage() cout << " object unlink unlink object from bucket index\n"; cout << " object rewrite rewrite the specified object\n"; cout << " objects expire run expired objects cleanup\n"; - cout << " period delete delete a period\n"; + cout << " period rm remove a period\n"; cout << " period get get period info\n"; cout << " period get-current get current period info\n"; cout << " period pull pull a period\n"; @@ -110,7 +112,7 @@ void usage() cout << " global quota enable enable a global quota\n"; cout << " global quota disable disable a global quota\n"; cout << " realm create create a new realm\n"; - cout << " realm delete delete a realm\n"; + cout << " realm rm remove a realm\n"; cout << " realm get show realm info\n"; cout << " realm get-default get default realm name\n"; cout << " realm list list realms\n"; @@ -122,11 +124,11 @@ void usage() cout << " zonegroup add add a zone to a zonegroup\n"; cout << " zonegroup create create a new zone group info\n"; cout << " zonegroup default set default zone group\n"; - cout << " zonegroup delete delete a zone group info\n"; + cout << " zonegroup rm remove a zone group info\n"; cout << " zonegroup get show zone group info\n"; cout << " zonegroup modify modify an existing zonegroup\n"; cout << " zonegroup set set zone group info (requires infile)\n"; - cout << " zonegroup remove remove a zone from a zonegroup\n"; + cout << " zonegroup rm remove a zone from a zonegroup\n"; cout << " zonegroup rename rename a zone group\n"; cout << " zonegroup list list all zone groups set on this cluster\n"; cout << " zonegroup placement list list zonegroup's placement targets\n"; @@ -135,7 +137,7 @@ void usage() cout << " zonegroup placement rm remove a placement target from a zonegroup\n"; cout << " zonegroup placement default set a zonegroup's default placement target\n"; cout << " zone create create a new zone\n"; - cout << " zone delete delete a zone\n"; + cout << " zone rm remove a zone\n"; cout << " zone get show zone cluster params\n"; cout << " zone modify modify an existing zone\n"; cout << " zone set set zone cluster params (requires infile)\n"; @@ -190,19 +192,19 @@ void usage() cout << " opstate rm remove entry (use client_id, op_id, object)\n"; cout << " replicalog get get replica metadata log entry\n"; cout << " replicalog update update replica metadata log entry\n"; - cout << " replicalog delete delete replica metadata log entry\n"; + cout << " replicalog rm remove replica metadata log entry\n"; cout << " orphans find init and run search for leaked rados objects (use job-id, pool)\n"; cout << " orphans finish clean up search for leaked rados objects\n"; cout << " orphans list-jobs list the current job-ids for orphans search\n"; cout << " role create create a AWS role for use with STS\n"; - cout << " role delete delete a role\n"; + cout << " role rm remove a role\n"; cout << " role get get a role\n"; cout << " role list list roles with specified path prefix\n"; cout << " role modify modify the assume role policy of an existing role\n"; cout << " role-policy put add/update permission policy to role\n"; cout << " role-policy list list policies attached to a role\n"; cout << " role-policy get get the specified inline policy document embedded with the given role\n"; - cout << " role-policy delete delete policy attached to a role\n"; + cout << " role-policy rm remove policy attached to a role\n"; cout << " reshard add schedule a resharding of a bucket\n"; cout << " reshard list list all bucket resharding or scheduled to be resharded\n"; cout << " reshard status read bucket resharding status\n"; @@ -504,6 +506,8 @@ enum { static int get_cmd(const char *cmd, const char *prev_cmd, const char *prev_prev_cmd, bool *need_more) { + using ceph::util::match_str; + *need_more = false; // NOTE: please keep the checks in alphabetical order !!! if (strcmp(cmd, "bi") == 0 || @@ -706,7 +710,7 @@ static int get_cmd(const char *cmd, const char *prev_cmd, const char *prev_prev_ if (strcmp(cmd, "disable") == 0) return OPT_GLOBAL_QUOTA_DISABLE; } else if (strcmp(prev_cmd, "period") == 0) { - if (strcmp(cmd, "delete") == 0) + if (match_str(cmd, "rm", "delete")) return OPT_PERIOD_DELETE; if (strcmp(cmd, "get") == 0) return OPT_PERIOD_GET; @@ -725,7 +729,7 @@ static int get_cmd(const char *cmd, const char *prev_cmd, const char *prev_prev_ } else if (strcmp(prev_cmd, "realm") == 0) { if (strcmp(cmd, "create") == 0) return OPT_REALM_CREATE; - if (strcmp(cmd, "delete") == 0) + if (match_str(cmd, "rm", "delete")) return OPT_REALM_DELETE; if (strcmp(cmd, "get") == 0) return OPT_REALM_GET; @@ -772,7 +776,7 @@ static int get_cmd(const char *cmd, const char *prev_cmd, const char *prev_prev_ return OPT_ZONEGROUP_LIST; if (strcmp(cmd, "set") == 0) return OPT_ZONEGROUP_SET; - if (strcmp(cmd, "remove") == 0) + if (match_str(cmd, "rm", "remove")) return OPT_ZONEGROUP_REMOVE; if (strcmp(cmd, "rename") == 0) return OPT_ZONEGROUP_RENAME; @@ -937,7 +941,7 @@ static int get_cmd(const char *cmd, const char *prev_cmd, const char *prev_prev_ return OPT_ROLE_POLICY_LIST; if (strcmp(cmd, "get") == 0) return OPT_ROLE_POLICY_GET; - if (strcmp(cmd, "delete") == 0) + if (match_str(cmd, "rm", "delete")) return OPT_ROLE_POLICY_DELETE; } else if (strcmp(prev_cmd, "reshard") == 0) { if (strcmp(cmd, "bucket") == 0) diff --git a/src/test/cli/radosgw-admin/help.t b/src/test/cli/radosgw-admin/help.t index 521b02849eb..3f11d396f18 100644 --- a/src/test/cli/radosgw-admin/help.t +++ b/src/test/cli/radosgw-admin/help.t @@ -37,7 +37,7 @@ object unlink unlink object from bucket index object rewrite rewrite the specified object objects expire run expired objects cleanup - period delete delete a period + period rm remove a period period get get period info period get-current get current period info period pull pull a period @@ -53,7 +53,7 @@ global quota enable enable a global quota global quota disable disable a global quota realm create create a new realm - realm delete delete a realm + realm rm remove a realm realm get show realm info realm get-default get default realm name realm list list realms @@ -65,11 +65,11 @@ zonegroup add add a zone to a zonegroup zonegroup create create a new zone group info zonegroup default set default zone group - zonegroup delete delete a zone group info + zonegroup rm remove a zone group info zonegroup get show zone group info zonegroup modify modify an existing zonegroup zonegroup set set zone group info (requires infile) - zonegroup remove remove a zone from a zonegroup + zonegroup rm remove a zone from a zonegroup zonegroup rename rename a zone group zonegroup list list all zone groups set on this cluster zonegroup placement list list zonegroup's placement targets @@ -78,7 +78,7 @@ zonegroup placement rm remove a placement target from a zonegroup zonegroup placement default set a zonegroup's default placement target zone create create a new zone - zone delete delete a zone + zone rm remove a zone zone get show zone cluster params zone modify modify an existing zone zone set set zone cluster params (requires infile) @@ -133,19 +133,19 @@ opstate rm remove entry (use client_id, op_id, object) replicalog get get replica metadata log entry replicalog update update replica metadata log entry - replicalog delete delete replica metadata log entry + replicalog rm remove replica metadata log entry orphans find init and run search for leaked rados objects (use job-id, pool) orphans finish clean up search for leaked rados objects orphans list-jobs list the current job-ids for orphans search role create create a AWS role for use with STS - role delete delete a role + role rm remove a role role get get a role role list list roles with specified path prefix role modify modify the assume role policy of an existing role role-policy put add/update permission policy to role role-policy list list policies attached to a role role-policy get get the specified inline policy document embedded with the given role - role-policy delete delete policy attached to a role + role-policy rm remove policy attached to a role reshard add schedule a resharding of a bucket reshard list list all bucket resharding or scheduled to be resharded reshard status read bucket resharding status -- 2.39.5