From b6a932ac8525282afad8dc22ebc1589a2a446442 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Tue, 10 Mar 2020 15:50:38 +0800 Subject: [PATCH] crimson/osd: add tell command support * change the function signature of `call()` to accept a bufferlist, as MCommand is supposed to carry a bufferlist which can be consumed by the hook. * drop the wrapper of `tell()`, instead always use `call()`. simpler this way. * promote `tell_result_t` to top level of `crimson::admin` as it is used as part of the interface between hook and `AdminSocket` * replace `maybe_parsed_t` with `std::variant`, as the latter is able to convey more detailed error information if something goes wrong. * osd now handles `MCommand`. Signed-off-by: Kefu Chai --- src/crimson/admin/admin_socket.cc | 169 ++++++++++++++++-------------- src/crimson/admin/admin_socket.h | 46 +++++--- src/crimson/admin/osd_admin.cc | 111 ++++++++------------ src/crimson/osd/osd.cc | 9 ++ src/crimson/osd/osd.h | 3 + 5 files changed, 178 insertions(+), 160 deletions(-) diff --git a/src/crimson/admin/admin_socket.cc b/src/crimson/admin/admin_socket.cc index afbffa3d6da..cdb8e31d155 100644 --- a/src/crimson/admin/admin_socket.cc +++ b/src/crimson/admin/admin_socket.cc @@ -12,6 +12,8 @@ #include #include "common/version.h" +#include "messages/MCommand.h" +#include "messages/MCommandReply.h" #include "crimson/common/log.h" #include "crimson/net/Socket.h" @@ -32,6 +34,19 @@ seastar::logger& logger() namespace crimson::admin { +tell_result_t::tell_result_t(int ret, std::string&& err) + : ret{ret}, err(std::move(err)) +{} + +tell_result_t::tell_result_t(int ret, std::string&& err, ceph::bufferlist&& out) + : ret{ret}, err(std::move(err)), out(std::move(out)) +{} + +tell_result_t::tell_result_t(Formatter* formatter) +{ + formatter->flush(out); +} + seastar::future<> AdminSocket::register_command(std::unique_ptr&& hook) { @@ -51,14 +66,15 @@ AdminSocket::register_command(std::unique_ptr&& hook) /* * Note: parse_cmd() is executed with servers_tbl_rwlock held as shared */ -AdminSocket::maybe_parsed_t -AdminSocket::parse_cmd(const std::vector& cmd, ceph::bufferlist& out) +auto AdminSocket::parse_cmd(const std::vector& cmd) + -> std::variant { // preliminaries: // - create the formatter specified by the cmd parameters // - locate the "op-code" string (the 'prefix' segment) // - prepare for command parameters extraction via cmdmap_t cmdmap_t cmdmap; + ceph::bufferlist out; try { stringstream errss; @@ -67,12 +83,12 @@ AdminSocket::parse_cmd(const std::vector& cmd, ceph::bufferlist& ou logger().error("{}: incoming command error: {}", __func__, errss.str()); out.append("error:"s); out.append(errss.str()); - return maybe_parsed_t{ std::nullopt }; + return tell_result_t{-EINVAL, "invalid json", std::move(out)}; } - } catch (std::runtime_error& e) { + } catch (const std::runtime_error& e) { logger().error("{}: incoming command syntax: {}", __func__, cmd); - out.append("error: command syntax"s); - return maybe_parsed_t{ std::nullopt }; + out.append(string{e.what()}); + return tell_result_t{-EINVAL, "invalid json", std::move(out)}; } string format; @@ -82,43 +98,17 @@ AdminSocket::parse_cmd(const std::vector& cmd, ceph::bufferlist& ou cmd_getval(cmdmap, "prefix", prefix); } catch (const bad_cmd_get& e) { logger().error("{}: invalid syntax: {}", __func__, cmd); - out.append("error: command syntax: missing 'prefix'"s); - return maybe_parsed_t{ std::nullopt }; - } - - if (prefix.empty()) { - // no command identified - out.append("error: no command identified"s); - return maybe_parsed_t{ std::nullopt }; + out.append(string{e.what()}); + return tell_result_t{-EINVAL, "invalid json", std::move(out)}; } // match the incoming op-code to one of the registered APIs if (auto found = hooks.find(prefix); found != hooks.end()) { return parsed_command_t{ cmdmap, format, *found->second }; } else { - return maybe_parsed_t{ std::nullopt }; - } -} - -/* - * Note: validate_command() is executed with servers_tbl_rwlock held as shared - */ -bool AdminSocket::validate_command(const parsed_command_t& parsed, - const std::string& command_text, - ceph::bufferlist& out) const -{ - logger().info("{}: validating {} against:{}", __func__, command_text, - parsed.hook.desc); - - stringstream os; // for possible validation error messages - if (validate_cmd(nullptr, std::string{parsed.hook.desc}, parsed.parameters, os)) { - return true; - } else { - os << "error: command validation failure "; - logger().error("{}: validation failure (incoming:{}) {}", __func__, - command_text, os.str()); - out.append(os); - return false; + return tell_result_t{-EINVAL, + fmt::format("unknown command '{}'", prefix), + std::move(out)}; } } @@ -136,25 +126,56 @@ seastar::future<> AdminSocket::finalize_response( .then([&out, outbuf_cont] { return out.write(outbuf_cont.c_str()); }); } + +seastar::future<> AdminSocket::handle_command(crimson::net::Connection* conn, + boost::intrusive_ptr m) +{ + return execute_command(m->cmd, std::move(m->get_data())).then( + [conn, tid=m->get_tid(), this](auto result) { + auto [ret, err, out] = std::move(result); + auto reply = make_message(ret, err); + reply->set_tid(tid); + reply->set_data(out); + return conn->send(reply); + }); +} + seastar::future<> AdminSocket::execute_line(std::string cmdline, seastar::output_stream& out) +{ + return execute_command({cmdline}, {}).then([&out, this](auto result) { + auto [ret, stderr, stdout] = std::move(result); + if (ret < 0) { + stdout.append(fmt::format("ERROR: {}\n", cpp_strerror(ret))); + stdout.append(stderr); + } + return finalize_response(out, std::move(stdout)); + }); +} + +auto AdminSocket::execute_command(const std::vector& cmd, + ceph::bufferlist&& buf) + -> seastar::future { return seastar::with_shared(servers_tbl_rwlock, - [this, cmdline, &out]() mutable { - ceph::bufferlist err; - auto parsed = parse_cmd({cmdline}, err); - if (!parsed.has_value() || - !validate_command(*parsed, cmdline, err)) { - return finalize_response(out, std::move(err)); + [cmd, buf=std::move(buf), this]() mutable { + auto maybe_parsed = parse_cmd(cmd); + if (auto parsed = std::get_if(&maybe_parsed); parsed) { + stringstream os; + string desc{parsed->hook.desc}; + if (!validate_cmd(nullptr, desc, parsed->params, os)) { + logger().error("AdminSocket::execute_command: " + "failed to validate '{}': {}", cmd, os.str()); + ceph::bufferlist out; + out.append(os); + return seastar::make_ready_future( + tell_result_t{-EINVAL, "invalid command json", std::move(out)}); + } + return parsed->hook.call(parsed->params, parsed->format, std::move(buf)); + } else { + auto& result = std::get(maybe_parsed); + return seastar::make_ready_future(std::move(result)); } - return parsed->hook.call(parsed->hook.prefix, - parsed->format, - parsed->parameters).then( - [this, &out](auto result) { - // add 'failed' to the contents of out_buf? not what - // happens in the old code - return finalize_response(out, std::move(result)); - }); }); } @@ -268,9 +289,9 @@ class VersionHook final : public AdminSocketHook { VersionHook() : AdminSocketHook{"version", "version", "get ceph version"} {} - seastar::future call(std::string_view, - std::string_view format, - const cmdmap_t&) const final + seastar::future call(const cmdmap_t&, + std::string_view format, + ceph::bufferlist&&) const final { unique_ptr f{Formatter::create(format, "json-pretty", "json-pretty")}; f->open_object_section("version"); @@ -278,9 +299,7 @@ class VersionHook final : public AdminSocketHook { f->dump_string("release", ceph_release_to_str()); f->dump_string("release_type", ceph_release_type()); f->close_section(); - bufferlist out; - f->flush(out); - return seastar::make_ready_future(std::move(out)); + return seastar::make_ready_future(f.get()); } }; @@ -293,17 +312,15 @@ class GitVersionHook final : public AdminSocketHook { GitVersionHook() : AdminSocketHook{"git_version", "git_version", "get git sha1"} {} - seastar::future call(std::string_view command, - std::string_view format, - const cmdmap_t&) const final + seastar::future call(const cmdmap_t&, + std::string_view format, + ceph::bufferlist&&) const final { unique_ptr f{Formatter::create(format, "json-pretty", "json-pretty")}; f->open_object_section("version"); f->dump_string("git_version", git_version_to_str()); f->close_section(); - ceph::bufferlist out; - f->flush(out); - return seastar::make_ready_future(std::move(out)); + return seastar::make_ready_future(f.get()); } }; @@ -316,12 +333,12 @@ class HelpHook final : public AdminSocketHook { m_as{as} {} - seastar::future call(std::string_view command, - std::string_view format, - const cmdmap_t& cmdmap) const final + seastar::future call(const cmdmap_t&, + std::string_view format, + ceph::bufferlist&&) const final { return seastar::with_shared(m_as.servers_tbl_rwlock, - [this, format] { + [format, this] { unique_ptr f{Formatter::create(format, "json-pretty", "json-pretty")}; f->open_object_section("help"); for (const auto& [prefix, hook] : m_as) { @@ -330,9 +347,7 @@ class HelpHook final : public AdminSocketHook { } } f->close_section(); - ceph::bufferlist out; - f->flush(out); - return seastar::make_ready_future(std::move(out)); + return seastar::make_ready_future(f.get()); }); } }; @@ -347,12 +362,12 @@ class GetdescsHook final : public AdminSocketHook { "list available commands"}, m_as{ as } {} - seastar::future call(std::string_view command, - std::string_view format, - const cmdmap_t& cmdmap) const final + seastar::future call(const cmdmap_t& cmdmap, + std::string_view format, + ceph::bufferlist&&) const final { - unique_ptr f{Formatter::create(format, "json-pretty", "json-pretty")}; - return seastar::with_shared(m_as.servers_tbl_rwlock, [this, f=move(f)] { + return seastar::with_shared(m_as.servers_tbl_rwlock, [format, this] { + unique_ptr f{Formatter::create(format, "json-pretty", "json-pretty")}; int cmdnum = 0; f->open_object_section("command_descriptions"); for (const auto& [prefix, hook] : m_as) { @@ -363,9 +378,7 @@ class GetdescsHook final : public AdminSocketHook { cmdnum++; } f->close_section(); - ceph::bufferlist out; - f->flush(out); - return seastar::make_ready_future(std::move(out)); + return seastar::make_ready_future(f.get()); }); } }; diff --git a/src/crimson/admin/admin_socket.h b/src/crimson/admin/admin_socket.h index a6358d4acec..088f693ac67 100644 --- a/src/crimson/admin/admin_socket.h +++ b/src/crimson/admin/admin_socket.h @@ -19,13 +19,27 @@ #include #include "common/cmdparse.h" +#include "include/buffer.h" +#include "crimson/net/Fwd.h" using namespace std::literals; +class MCommand; + namespace crimson::admin { class AdminSocket; +struct tell_result_t { + int ret = 0; + std::string err; + ceph::bufferlist out; + tell_result_t() = default; + tell_result_t(int ret, std::string&& err); + tell_result_t(int ret, std::string&& err, ceph::bufferlist&& out); + tell_result_t(Formatter* formatter); +}; + /** * A specific hook must implement exactly one of the two interfaces: * (1) call(command, cmdmap, format, out) @@ -46,13 +60,9 @@ class AdminSocketHook { std::string_view help) : prefix{prefix}, desc{desc}, help{help} {} - /** - * \retval 'false' for hook execution errors - */ - virtual seastar::future - call(std::string_view command, - std::string_view format, - const cmdmap_t& cmdmap) const = 0; + virtual seastar::future call(const cmdmap_t& cmdmap, + std::string_view format, + ceph::bufferlist&& input) const = 0; virtual ~AdminSocketHook() {} const std::string_view prefix; const std::string_view desc; @@ -110,20 +120,26 @@ class AdminSocket : public seastar::enable_lw_shared_from_this { * Registering the APIs that are served directly by the admin_socket server. */ seastar::future<> register_admin_commands(); + /** + * handle a command message by replying an MCommandReply with the same tid + * + * \param conn connection over which the incoming command message is received + * \param m message carrying the command vector and optional input buffer + */ + seastar::future<> handle_command(crimson::net::Connection* conn, + boost::intrusive_ptr m); - private: +private: /** * the result of analyzing an incoming command, and locating it in * the registered APIs collection. */ struct parsed_command_t { - cmdmap_t parameters; + cmdmap_t params; std::string format; const AdminSocketHook& hook; }; // and the shorthand: - using maybe_parsed_t = std::optional; - seastar::future<> handle_client(seastar::input_stream& inp, seastar::output_stream& out); @@ -133,9 +149,8 @@ class AdminSocket : public seastar::enable_lw_shared_from_this { seastar::future<> finalize_response(seastar::output_stream& out, ceph::bufferlist&& msgs); - bool validate_command(const parsed_command_t& parsed, - const std::string& command_text, - ceph::bufferlist& out) const; + seastar::future execute_command(const std::vector& cmd, + ceph::bufferlist&& buf); std::optional server_sock; std::optional connected_sock; @@ -150,7 +165,8 @@ class AdminSocket : public seastar::enable_lw_shared_from_this { * the API, and into its arguments. Locate the command string in the * registered blocks. */ - maybe_parsed_t parse_cmd(const std::vector& cmd, bufferlist& out); + std::variant + parse_cmd(const std::vector& cmd); /** * The servers table is protected by a rw-lock, to be acquired exclusively diff --git a/src/crimson/admin/osd_admin.cc b/src/crimson/admin/osd_admin.cc index 13f096e3ec6..6d6f38519ed 100644 --- a/src/crimson/admin/osd_admin.cc +++ b/src/crimson/admin/osd_admin.cc @@ -36,57 +36,24 @@ std::unique_ptr make_asok_hook(Args&&... args) return std::make_unique(std::forward(args)...); } -class OsdAdminHookBase : public AdminSocketHook { -protected: - OsdAdminHookBase(std::string_view prefix, - std::string_view desc, - std::string_view help) - : AdminSocketHook(prefix, desc, help) {} - struct tell_result_t { - int ret = 0; - std::string err; - }; - /// the specific command implementation - virtual seastar::future tell(const cmdmap_t& cmdmap, - Formatter* f) const = 0; - seastar::future call(std::string_view prefix, - std::string_view format, - const cmdmap_t& cmdmap) const final - { - unique_ptr f{Formatter::create(format, "json-pretty", "json-pretty")}; - auto ret = seastar::do_with(std::move(f), [cmdmap, this] (auto& f) { - Formatter* formatter = f.get(); - return tell(cmdmap, formatter).then([formatter](auto result) { - bufferlist out; - if (auto& [ret, err] = result; ret < 0) { - out.append(fmt::format("ERROR: {}\n", cpp_strerror(ret))); - out.append(err); - } else { - formatter->flush(out); - } - return seastar::make_ready_future(std::move(out)); - }); - }); - return ret; - } -}; - /** * An OSD admin hook: OSD status */ -class OsdStatusHook : public OsdAdminHookBase { +class OsdStatusHook : public AdminSocketHook { public: explicit OsdStatusHook(crimson::osd::OSD& osd) : - OsdAdminHookBase("status", "status", "OSD status"), + AdminSocketHook("status", "status", "OSD status"), osd(osd) {} - seastar::future tell(const cmdmap_t&, - Formatter* f) const final + seastar::future call(const cmdmap_t&, + std::string_view format, + ceph::bufferlist&& input) const final { + unique_ptr f{Formatter::create(format, "json-pretty", "json-pretty")}; f->open_object_section("status"); - osd.dump_status(f); + osd.dump_status(f.get()); f->close_section(); - return seastar::make_ready_future(); + return seastar::make_ready_future(f.get()); } private: crimson::osd::OSD& osd; @@ -97,15 +64,17 @@ make_asok_hook(crimson::osd::OSD& osd); /** * An OSD admin hook: send beacon */ -class SendBeaconHook : public OsdAdminHookBase { +class SendBeaconHook : public AdminSocketHook { public: explicit SendBeaconHook(crimson::osd::OSD& osd) : - OsdAdminHookBase("send_beacon", - "send_beacon", - "send OSD beacon to mon immediately"), - osd(osd) + AdminSocketHook("send_beacon", + "send_beacon", + "send OSD beacon to mon immediately"), + osd(osd) {} - seastar::future tell(const cmdmap_t&, Formatter*) const final + seastar::future call(const cmdmap_t&, + std::string_view format, + ceph::bufferlist&& input) const final { return osd.send_beacon().then([] { return seastar::make_ready_future(); @@ -120,19 +89,22 @@ make_asok_hook(crimson::osd::OSD& osd); /** * A CephContext admin hook: listing the configuration values */ -class ConfigShowHook : public OsdAdminHookBase { +class ConfigShowHook : public AdminSocketHook { public: explicit ConfigShowHook() : - OsdAdminHookBase("config show", + AdminSocketHook("config show", "config show", "dump current config settings") {} - seastar::future tell(const cmdmap_t&, Formatter* f) const final + seastar::future call(const cmdmap_t&, + std::string_view format, + ceph::bufferlist&& input) const final { + unique_ptr f{Formatter::create(format, "json-pretty", "json-pretty")}; f->open_object_section("config_show"); - local_conf().show_config(f); + local_conf().show_config(f.get()); f->close_section(); - return seastar::make_ready_future(); + return seastar::make_ready_future(f.get()); } }; template std::unique_ptr make_asok_hook(); @@ -141,15 +113,16 @@ template std::unique_ptr make_asok_hook(); * A CephContext admin hook: fetching the value of a specific * configuration item */ -class ConfigGetHook : public OsdAdminHookBase { +class ConfigGetHook : public AdminSocketHook { public: ConfigGetHook() : - OsdAdminHookBase("config get", + AdminSocketHook("config get", "config get name=var,type=CephString", "config get : get the config value") {} - seastar::future tell(const cmdmap_t& cmdmap, - ceph::Formatter* f) const final + seastar::future call(const cmdmap_t& cmdmap, + std::string_view format, + ceph::bufferlist&& input) const final { std::string var; if (!cmd_getval(cmdmap, "var", var)) { @@ -158,12 +131,13 @@ public: tell_result_t{-EINVAL, "syntax error: 'config get '"}); } try { + unique_ptr f{Formatter::create(format, "json-pretty", "json-pretty")}; f->open_object_section("config_get"); std::string conf_val = local_conf().get_val(var); f->dump_string(var.c_str(), conf_val.c_str()); f->close_section(); - return seastar::make_ready_future(); + return seastar::make_ready_future(f.get()); } catch (const boost::bad_get&) { return seastar::make_ready_future( tell_result_t{-EINVAL, fmt::format("unrecognized option {}", var)}); @@ -177,15 +151,16 @@ template std::unique_ptr make_asok_hook(); * item (a real example: {"prefix": "config set", "var":"debug_osd", "val": * ["30/20"]} ) */ -class ConfigSetHook : public OsdAdminHookBase { +class ConfigSetHook : public AdminSocketHook { public: ConfigSetHook() : - OsdAdminHookBase("config set", + AdminSocketHook("config set", "config set name=var,type=CephString name=val,type=CephString,n=N", "config set [ ...]: set a config variable") {} - seastar::future tell(const cmdmap_t& cmdmap, - ceph::Formatter* f) const final + seastar::future call(const cmdmap_t& cmdmap, + std::string_view format, + ceph::bufferlist&&) const final { std::string var; std::vector new_val; @@ -196,11 +171,12 @@ public: } // val may be multiple words string valstr = str_join(new_val, " "); - return local_conf().set_val(var, valstr).then([f] { + return local_conf().set_val(var, valstr).then([format] { + unique_ptr f{Formatter::create(format, "json-pretty", "json-pretty")}; f->open_object_section("config_set"); f->dump_string("success", ""); f->close_section(); - return seastar::make_ready_future(); + return seastar::make_ready_future(f.get()); }).handle_exception_type([](std::invalid_argument& e) { return seastar::make_ready_future( tell_result_t{-EINVAL, e.what()}); @@ -213,15 +189,16 @@ template std::unique_ptr make_asok_hook(); * A CephContext admin hook: calling assert (if allowed by * 'debug_asok_assert_abort') */ -class AssertAlwaysHook : public OsdAdminHookBase { +class AssertAlwaysHook : public AdminSocketHook { public: AssertAlwaysHook() : - OsdAdminHookBase("assert", + AdminSocketHook("assert", "assert", "asserts") {} - seastar::future tell(const cmdmap_t&, - ceph::Formatter*) const final + seastar::future call(const cmdmap_t&, + std::string_view format, + ceph::bufferlist&& input) const final { if (local_conf().get_val("debug_asok_assert_abort")) { ceph_assert_always(0); diff --git a/src/crimson/osd/osd.cc b/src/crimson/osd/osd.cc index f0c1724f30e..9ce90100535 100644 --- a/src/crimson/osd/osd.cc +++ b/src/crimson/osd/osd.cc @@ -14,6 +14,7 @@ #include "common/pick_address.h" #include "include/util.h" +#include "messages/MCommand.h" #include "messages/MOSDAlive.h" #include "messages/MOSDBeacon.h" #include "messages/MOSDBoot.h" @@ -411,6 +412,12 @@ seastar::future<> OSD::_send_alive() } } +seastar::future<> OSD::handle_command(crimson::net::Connection* conn, + Ref m) +{ + return asok->handle_command(conn, std::move(m)); +} + /* The OSD's Admin Socket object created here has two servers (i.e. - blocks of commands to handle) registered to it: @@ -574,6 +581,8 @@ seastar::future<> OSD::ms_dispatch(crimson::net::Connection* conn, MessageRef m) conn->get_shared(), m); return seastar::now(); + case MSG_COMMAND: + return handle_command(conn, boost::static_pointer_cast(m)); case MSG_OSD_PG_LEASE: [[fallthrough]]; case MSG_OSD_PG_LEASE_ACK: diff --git a/src/crimson/osd/osd.h b/src/crimson/osd/osd.h index 64f19aac1f7..baf90c8d37f 100644 --- a/src/crimson/osd/osd.h +++ b/src/crimson/osd/osd.h @@ -31,6 +31,7 @@ #include "osd/osd_perf_counters.h" #include "osd/PGPeeringEvent.h" +class MCommand; class MOSDMap; class MOSDOp; class MOSDRepOpReply; @@ -186,6 +187,8 @@ private: void check_osdmap_features(); + seastar::future<> handle_command(crimson::net::Connection* conn, + Ref m); seastar::future<> start_asok_admin(); public: -- 2.39.5