From 6f35d2835268eade059535b62378d6d407ef9e87 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Fri, 27 Sep 2019 16:01:44 -0500 Subject: [PATCH] mgr,mgr/MgrClient: use fsid to signal mon-mgr vs cli MCommands We can't use the feature bit for the MCommand connection to tell whether it is a tell or CLI command because new clients may have to send CLI commands via MCommand for old clusters, and they don't always know whether this mgr is new or old yet. Prior to octopus, MCommand contained a mon/mgr CLI command, and did not have the fsid field set. Start populating the fsid field, and use this to signal whether a client is a new MgrClient that knows MCommand vs MMgrCommand. If we get an MCommand with the fsid set, that means it is a tell command; otherwise, it's an old client sending a CLI command. Signed-off-by: Sage Weil --- src/librados/RadosClient.cc | 2 +- src/mds/MDSDaemon.cc | 2 +- src/messages/MMonCommand.h | 4 ++++ src/mgr/DaemonServer.cc | 4 +++- src/mgr/MgrClient.cc | 20 ++++++++++++-------- src/mgr/MgrClient.h | 4 +++- src/mgr/MgrStandby.cc | 2 +- src/mon/Monitor.cc | 2 +- src/osd/OSD.cc | 2 +- 9 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/librados/RadosClient.cc b/src/librados/RadosClient.cc index 2a441a9f1a1..372419816d8 100644 --- a/src/librados/RadosClient.cc +++ b/src/librados/RadosClient.cc @@ -62,7 +62,7 @@ librados::RadosClient::RadosClient(CephContext *cct_) conf(cct_->_conf), state(DISCONNECTED), monclient(cct_), - mgrclient(cct_, nullptr), + mgrclient(cct_, nullptr, &monclient.monmap), messenger(NULL), instance_id(0), objecter(NULL), diff --git a/src/mds/MDSDaemon.cc b/src/mds/MDSDaemon.cc index 75d80d54e6b..5595a01a42e 100644 --- a/src/mds/MDSDaemon.cc +++ b/src/mds/MDSDaemon.cc @@ -71,7 +71,7 @@ MDSDaemon::MDSDaemon(std::string_view n, Messenger *m, MonClient *mc) : name(n), messenger(m), monc(mc), - mgrc(m->cct, m), + mgrc(m->cct, m, &mc->monmap), log_client(m->cct, messenger, &mc->monmap, LogClient::NO_FLAGS), mds_rank(NULL), asok_hook(NULL), diff --git a/src/messages/MMonCommand.h b/src/messages/MMonCommand.h index 8b4c1af9e43..583b9a89c93 100644 --- a/src/messages/MMonCommand.h +++ b/src/messages/MMonCommand.h @@ -22,6 +22,10 @@ class MMonCommand : public PaxosServiceMessage { public: + // weird note: prior to octopus, MgrClient would leave fsid blank when + // sending commands to the mgr. Starting with octopus, this is either + // populated with a valid fsid (tell command) or an MMgrCommand is sent + // instead. uuid_d fsid; std::vector cmd; diff --git a/src/mgr/DaemonServer.cc b/src/mgr/DaemonServer.cc index de894ad0b85..b5ad5e9323a 100644 --- a/src/mgr/DaemonServer.cc +++ b/src/mgr/DaemonServer.cc @@ -791,7 +791,9 @@ public: bool DaemonServer::handle_command(const ref_t& m) { std::lock_guard l(lock); - if (HAVE_FEATURE(m->get_connection()->get_features(), SERVER_OCTOPUS)) { + // a blank fsid in MCommand signals a legacy client sending a "mon-mgr" CLI + // command. + if (m->fsid != uuid_d()) { cct->get_admin_socket()->queue_tell_command(m); return true; } else { diff --git a/src/mgr/MgrClient.cc b/src/mgr/MgrClient.cc index 87a301c72c3..94ff99c390f 100644 --- a/src/mgr/MgrClient.cc +++ b/src/mgr/MgrClient.cc @@ -15,6 +15,7 @@ #include "MgrClient.h" #include "mgr/MgrContext.h" +#include "mon/MonMap.h" #include "msg/Messenger.h" #include "messages/MMgrMap.h" @@ -37,9 +38,12 @@ using ceph::bufferlist; #undef dout_prefix #define dout_prefix *_dout << "mgrc " << __func__ << " " -MgrClient::MgrClient(CephContext *cct_, Messenger *msgr_) - : Dispatcher(cct_), cct(cct_), msgr(msgr_), - timer(cct_, lock) +MgrClient::MgrClient(CephContext *cct_, Messenger *msgr_, MonMap *monmap_) + : Dispatcher(cct_), + cct(cct_), + msgr(msgr_), + monmap(monmap_), + timer(cct_, lock) { ceph_assert(cct != nullptr); } @@ -472,7 +476,8 @@ int MgrClient::start_command(const vector& cmd, const bufferlist& inbl, op.on_finish = onfinish; if (session && session->con) { - // Leaving fsid argument null because it isn't used. + // Leaving fsid argument null because it isn't used historically, and + // we can use it as a signal that we are sending a non-tell command. auto m = op.get_message( {}, HAVE_FEATURE(map.active_mgr_features, SERVER_OCTOPUS)); @@ -508,10 +513,9 @@ int MgrClient::start_tell_command( op.on_finish = onfinish; if (session && session->con && (name.size() == 0 || map.active_name == name)) { - // Leaving fsid argument null because it isn't used. - // Note: this simply won't work we pre-octopus mgrs because they route - // MCommand to the cluster command handler. - auto m = op.get_message({}, false); + // Set fsid argument to signal that this is really a tell message (and + // we are not a legacy client sending a non-tell command via MCommand). + auto m = op.get_message(monmap->fsid, false); session->con->send_message2(std::move(m)); } else { ldout(cct, 5) << "no mgr session (no running mgr daemon?), or " diff --git a/src/mgr/MgrClient.h b/src/mgr/MgrClient.h index 9d5251d9d20..7e5272dbedb 100644 --- a/src/mgr/MgrClient.h +++ b/src/mgr/MgrClient.h @@ -32,6 +32,7 @@ class MMgrClose; class Messenger; class MCommandReply; class MPGStats; +class MonMap; class MgrSessionState { @@ -59,6 +60,7 @@ protected: CephContext *cct; MgrMap map; Messenger *msgr; + MonMap *monmap; std::unique_ptr session; @@ -105,7 +107,7 @@ protected: bool mgr_optional = false; public: - MgrClient(CephContext *cct_, Messenger *msgr_); + MgrClient(CephContext *cct_, Messenger *msgr_, MonMap *monmap); void set_messenger(Messenger *msgr_) { msgr = msgr_; } diff --git a/src/mgr/MgrStandby.cc b/src/mgr/MgrStandby.cc index 465af9299ec..7d193b6575b 100644 --- a/src/mgr/MgrStandby.cc +++ b/src/mgr/MgrStandby.cc @@ -48,7 +48,7 @@ MgrStandby::MgrStandby(int argc, const char **argv) : 0)), objecter{g_ceph_context, client_messenger.get(), &monc, NULL, 0, 0}, client{client_messenger.get(), &monc, &objecter}, - mgrc(g_ceph_context, client_messenger.get()), + mgrc(g_ceph_context, client_messenger.get(), &monc.monmap), log_client(g_ceph_context, client_messenger.get(), &monc.monmap, LogClient::NO_FLAGS), clog(log_client.create_channel(CLOG_CHANNEL_CLUSTER)), audit_clog(log_client.create_channel(CLOG_CHANNEL_AUDIT)), diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc index 796dcfba1b4..42c977891b8 100644 --- a/src/mon/Monitor.cc +++ b/src/mon/Monitor.cc @@ -146,7 +146,7 @@ Monitor::Monitor(CephContext* cct_, string nm, MonitorDBStore *s, cct->_conf->auth_supported.empty() ? cct->_conf->auth_service_required : cct->_conf->auth_supported), mgr_messenger(mgr_m), - mgr_client(cct_, mgr_m), + mgr_client(cct_, mgr_m, monmap), gss_ktfile_client(cct->_conf.get_val("gss_ktab_client_file")), store(s), diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc index ab63822f072..3d7fb104177 100644 --- a/src/osd/OSD.cc +++ b/src/osd/OSD.cc @@ -2117,7 +2117,7 @@ OSD::OSD(CephContext *cct_, ObjectStore *store_, client_messenger(external_messenger), objecter_messenger(osdc_messenger), monc(mc), - mgrc(cct_, client_messenger), + mgrc(cct_, client_messenger, &mc->monmap), logger(NULL), recoverystate_perf(NULL), store(store_),