From 34525ba3af31012d7874920883374d4f23ad90a1 Mon Sep 17 00:00:00 2001 From: Volker Theile Date: Fri, 28 Sep 2018 14:55:32 +0200 Subject: [PATCH] Relocate cluster_log(). Only active modules can use it. Signed-off-by: Volker Theile --- qa/tasks/mgr/test_module_selftest.py | 18 ++++++++++++++---- src/mgr/ActivePyModule.h | 4 ++-- src/mgr/ActivePyModules.cc | 14 +++++++++++++- src/mgr/ActivePyModules.h | 3 +++ src/mgr/BaseMgrModule.cc | 15 ++++++++++++--- src/mgr/MgrStandby.cc | 2 +- src/mgr/PyModuleRegistry.cc | 2 +- src/mgr/PyModuleRegistry.h | 6 +++--- src/mgr/PyModuleRunner.cc | 10 ---------- src/mgr/PyModuleRunner.h | 8 ++------ src/mgr/StandbyPyModules.cc | 6 ++---- src/mgr/StandbyPyModules.h | 8 +++----- 12 files changed, 56 insertions(+), 40 deletions(-) diff --git a/qa/tasks/mgr/test_module_selftest.py b/qa/tasks/mgr/test_module_selftest.py index 44c38b04deddb..780f475123940 100644 --- a/qa/tasks/mgr/test_module_selftest.py +++ b/qa/tasks/mgr/test_module_selftest.py @@ -296,10 +296,10 @@ class TestModuleSelftest(MgrTestCase): Use the selftest module to test the cluster/audit log interface. """ priority_map = { - 'info': 'INF', - 'security': 'SEC', - 'warning': 'WRN', - 'error': 'ERR' + "info": "INF", + "security": "SEC", + "warning": "WRN", + "error": "ERR" } self._load_module("selftest") for priority in priority_map.keys(): @@ -318,3 +318,13 @@ class TestModuleSelftest(MgrTestCase): self.mgr_cluster.mon_manager.raw_cluster_cmd( "mgr", "self-test", "cluster-log", "audit", priority, message) + + def test_selftest_cluster_log_unknown_channel(self): + """ + Use the selftest module to test the cluster/audit log interface. + """ + with self.assertRaises(CommandFailedError) as exc_raised: + self.mgr_cluster.mon_manager.raw_cluster_cmd( + "mgr", "self-test", "cluster-log", "xyz", + "ERR", "The channel does not exist") + self.assertEqual(exc_raised.exception.exitstatus, errno.EOPNOTSUPP) diff --git a/src/mgr/ActivePyModule.h b/src/mgr/ActivePyModule.h index 6b382bdc863db..149d297faf13e 100644 --- a/src/mgr/ActivePyModule.h +++ b/src/mgr/ActivePyModule.h @@ -44,8 +44,8 @@ private: public: ActivePyModule(const PyModuleRef &py_module_, - LogChannelRef clog_, LogChannelRef audit_clog_) - : PyModuleRunner(py_module_, clog_, audit_clog_) + LogChannelRef clog_) + : PyModuleRunner(py_module_, clog_) {} int load(ActivePyModules *py_modules); diff --git a/src/mgr/ActivePyModules.cc b/src/mgr/ActivePyModules.cc index 7e22775bac7ce..cb635ab12217c 100644 --- a/src/mgr/ActivePyModules.cc +++ b/src/mgr/ActivePyModules.cc @@ -377,7 +377,7 @@ int ActivePyModules::start_one(PyModuleRef py_module) ceph_assert(modules.count(py_module->get_name()) == 0); - modules[py_module->get_name()].reset(new ActivePyModule(py_module, clog, audit_clog)); + modules[py_module->get_name()].reset(new ActivePyModule(py_module, clog)); auto active_module = modules.at(py_module->get_name()).get(); int r = active_module->load(this); @@ -904,3 +904,15 @@ void ActivePyModules::remove_osd_perf_query(OSDPerfMetricQueryID query_id) << cpp_strerror(r) << dendl; } } + +void ActivePyModules::cluster_log(const std::string &channel, clog_type prio, + const std::string &message) +{ + Mutex::Locker l(lock); + + if (channel == "audit") { + audit_clog->do_log(prio, message); + } else { + clog->do_log(prio, message); + } +} diff --git a/src/mgr/ActivePyModules.h b/src/mgr/ActivePyModules.h index 48302d63ac5b7..16f781e03a758 100644 --- a/src/mgr/ActivePyModules.h +++ b/src/mgr/ActivePyModules.h @@ -157,5 +157,8 @@ public: void dump_server(const std::string &hostname, const DaemonStateCollection &dmc, Formatter *f); + + void cluster_log(const std::string &channel, clog_type prio, + const std::string &message); }; diff --git a/src/mgr/BaseMgrModule.cc b/src/mgr/BaseMgrModule.cc index 91687b6c8a108..5c587b91cbc32 100644 --- a/src/mgr/BaseMgrModule.cc +++ b/src/mgr/BaseMgrModule.cc @@ -484,13 +484,22 @@ ceph_cluster_log(BaseMgrModule *self, PyObject *args) int prio = 0; char *channel = nullptr; char *message = nullptr; + std::vector channels = { "audit", "cluster" }; + if (!PyArg_ParseTuple(args, "sis:ceph_cluster_log", &channel, &prio, &message)) { return nullptr; } - ceph_assert(self->this_module); + if (std::find(channels.begin(), channels.end(), std::string(channel)) == channels.end()) { + std::string msg("Unknown channel: "); + msg.append(channel); + PyErr_SetString(PyExc_ValueError, msg.c_str()); + return nullptr; + } - self->this_module->cluster_log(channel, (clog_type)prio, message); + PyThreadState *tstate = PyEval_SaveThread(); + self->py_modules->cluster_log(channel, (clog_type)prio, message); + PyEval_RestoreThread(tstate); Py_RETURN_NONE; } @@ -709,7 +718,7 @@ PyMethodDef BaseMgrModule_methods[] = { "Emit a (local) log message"}, {"_ceph_cluster_log", (PyCFunction)ceph_cluster_log, METH_VARARGS, - "Emit an cluster log message"}, + "Emit a cluster log message"}, {"_ceph_get_version", (PyCFunction)ceph_get_version, METH_VARARGS, "Get the ceph version of this process"}, diff --git a/src/mgr/MgrStandby.cc b/src/mgr/MgrStandby.cc index ae4b853d15928..d90b799f57772 100644 --- a/src/mgr/MgrStandby.cc +++ b/src/mgr/MgrStandby.cc @@ -54,7 +54,7 @@ MgrStandby::MgrStandby(int argc, const char **argv) : audit_clog(log_client.create_channel(CLOG_CHANNEL_AUDIT)), lock("MgrStandby::lock"), timer(g_ceph_context, lock), - py_module_registry(clog, audit_clog), + py_module_registry(clog), active_mgr(nullptr), orig_argc(argc), orig_argv(argv), diff --git a/src/mgr/PyModuleRegistry.cc b/src/mgr/PyModuleRegistry.cc index 0f48d6bbd0e7d..6966bd1aefa1c 100644 --- a/src/mgr/PyModuleRegistry.cc +++ b/src/mgr/PyModuleRegistry.cc @@ -141,7 +141,7 @@ void PyModuleRegistry::standby_start(MonClient &mc) dout(4) << "Starting modules in standby mode" << dendl; standby_modules.reset(new StandbyPyModules( - mgr_map, module_config, clog, audit_clog, mc)); + mgr_map, module_config, clog, mc)); std::set failed_modules; for (const auto &i : modules) { diff --git a/src/mgr/PyModuleRegistry.h b/src/mgr/PyModuleRegistry.h index 06e49b2e2ad34..1657d67b38f67 100644 --- a/src/mgr/PyModuleRegistry.h +++ b/src/mgr/PyModuleRegistry.h @@ -38,7 +38,7 @@ class PyModuleRegistry { private: mutable Mutex lock{"PyModuleRegistry::lock"}; - LogChannelRef clog, audit_clog; + LogChannelRef clog; std::map modules; @@ -76,8 +76,8 @@ public: return modules_out; } - explicit PyModuleRegistry(LogChannelRef clog_, LogChannelRef audit_clog_) - : clog(clog_), audit_clog(audit_clog_) + explicit PyModuleRegistry(LogChannelRef clog_) + : clog(clog_) {} /** diff --git a/src/mgr/PyModuleRunner.cc b/src/mgr/PyModuleRunner.cc index b6f7f07853fec..403c8a9f183f3 100644 --- a/src/mgr/PyModuleRunner.cc +++ b/src/mgr/PyModuleRunner.cc @@ -99,16 +99,6 @@ void PyModuleRunner::log(int level, const std::string &record) #define dout_prefix *_dout << "mgr " << __func__ << " " } -void PyModuleRunner::cluster_log(const std::string &channel, clog_type prio, - const std::string &message) -{ - if (std::string(channel) == "audit") { - audit_clog->do_log(prio, message); - } else { - clog->do_log(prio, message); - } -} - void* PyModuleRunner::PyModuleRunnerThread::entry() { // No need to acquire the GIL here; the module does it. diff --git a/src/mgr/PyModuleRunner.h b/src/mgr/PyModuleRunner.h index 550f544d196eb..fb58bf0c2323f 100644 --- a/src/mgr/PyModuleRunner.h +++ b/src/mgr/PyModuleRunner.h @@ -33,7 +33,7 @@ protected: // Populated by descendent class PyObject *pClassInstance = nullptr; - LogChannelRef clog, audit_clog; + LogChannelRef clog; class PyModuleRunnerThread : public Thread { @@ -52,8 +52,6 @@ public: int serve(); void shutdown(); void log(int level, const std::string &record); - void cluster_log(const std::string &channel, clog_type prio, - const std::string &message); const char *get_thread_name() const { @@ -62,12 +60,10 @@ public: PyModuleRunner( const PyModuleRef &py_module_, - LogChannelRef clog_, - LogChannelRef audit_clog_) + LogChannelRef clog_) : py_module(py_module_), clog(clog_), - audit_clog(audit_clog_), thread(this) { // Shortened name for use as thread name, because thread names diff --git a/src/mgr/StandbyPyModules.cc b/src/mgr/StandbyPyModules.cc index 259d70fe0b12c..f2d1eeb45b8d4 100644 --- a/src/mgr/StandbyPyModules.cc +++ b/src/mgr/StandbyPyModules.cc @@ -36,11 +36,9 @@ StandbyPyModules::StandbyPyModules( const MgrMap &mgr_map_, PyModuleConfig &module_config, LogChannelRef clog_, - LogChannelRef audit_clog_, MonClient &monc_) : state(module_config, monc_), - clog(clog_), - audit_clog(audit_clog_) + clog(clog_) { state.set_mgr_map(mgr_map_); } @@ -83,7 +81,7 @@ int StandbyPyModules::start_one(PyModuleRef py_module) modules[module_name].reset(new StandbyPyModule( state, - py_module, clog, audit_clog)); + py_module, clog)); int r = modules[module_name]->load(); if (r != 0) { diff --git a/src/mgr/StandbyPyModules.h b/src/mgr/StandbyPyModules.h index 28abc2593ac88..f443ba67e7947 100644 --- a/src/mgr/StandbyPyModules.h +++ b/src/mgr/StandbyPyModules.h @@ -81,10 +81,9 @@ class StandbyPyModule : public PyModuleRunner StandbyPyModule( StandbyPyModuleState &state_, const PyModuleRef &py_module_, - LogChannelRef clog_, - LogChannelRef audit_clog_) + LogChannelRef clog_) : - PyModuleRunner(py_module_, clog_, audit_clog_), + PyModuleRunner(py_module_, clog_), state(state_) { } @@ -104,7 +103,7 @@ private: StandbyPyModuleState state; - LogChannelRef clog, audit_clog; + LogChannelRef clog; public: @@ -112,7 +111,6 @@ public: const MgrMap &mgr_map_, PyModuleConfig &module_config, LogChannelRef clog_, - LogChannelRef audit_clog_, MonClient &monc); int start_one(PyModuleRef py_module); -- 2.39.5