From: Volker Theile Date: Thu, 3 Jan 2019 16:02:52 +0000 (+0100) Subject: mgr: Allow modules to get/set other module options X-Git-Tag: v14.1.0~355^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=030e85bb9c6c3571fec514f457010efea8f49296;p=ceph.git mgr: Allow modules to get/set other module options Add the _ceph_get_module_option_ex and _ceph_set_module_option_ex methods to the BaseMgrModule. This allows a module to get/set options from other modules, e.g. allow the Dashboard to set the telemetry module settings, either the other module is not enabled. Fixes: https://tracker.ceph.com/issues/37722 Signed-off-by: Volker Theile --- diff --git a/src/mgr/ActivePyModules.cc b/src/mgr/ActivePyModules.cc index 8e35353f778..4069b100b67 100644 --- a/src/mgr/ActivePyModules.cc +++ b/src/mgr/ActivePyModules.cc @@ -26,6 +26,7 @@ // For ::config_prefix #include "PyModule.h" +#include "PyModuleRegistry.h" #include "ActivePyModules.h" #include "DaemonServer.h" @@ -40,11 +41,12 @@ ActivePyModules::ActivePyModules(PyModuleConfig &module_config_, DaemonStateIndex &ds, ClusterState &cs, MonClient &mc, LogChannelRef clog_, LogChannelRef audit_clog_, Objecter &objecter_, - Client &client_, Finisher &f, DaemonServer &server) + Client &client_, Finisher &f, DaemonServer &server, + PyModuleRegistry &pmr) : module_config(module_config_), daemon_state(ds), cluster_state(cs), monc(mc), clog(clog_), audit_clog(audit_clog_), objecter(objecter_), client(client_), finisher(f), - server(server), lock("ActivePyModules") + server(server), py_module_registry(pmr), lock("ActivePyModules") { store_cache = std::move(store_data); } @@ -519,6 +521,27 @@ bool ActivePyModules::get_config(const std::string &module_name, } } +PyObject *ActivePyModules::get_typed_config( + const std::string &module_name, + const std::string &key) const +{ + if (!py_module_registry.module_exists(module_name)) { + derr << "Module '" << module_name << "' is not available" << dendl; + Py_RETURN_NONE; + } + + std::string value; + bool found = get_config(module_name, key, &value); + if (found) { + PyModuleRef module = py_module_registry.get_module(module_name); + dout(10) << __func__ << " " << key << " found: " << value << dendl; + return module->get_typed_option_value(key, value); + } + + dout(4) << __func__ << " " << key << " not found " << dendl; + Py_RETURN_NONE; +} + PyObject *ActivePyModules::get_store_prefix(const std::string &module_name, const std::string &prefix) const { diff --git a/src/mgr/ActivePyModules.h b/src/mgr/ActivePyModules.h index b0d0eade8e1..df1477f8f35 100644 --- a/src/mgr/ActivePyModules.h +++ b/src/mgr/ActivePyModules.h @@ -32,6 +32,7 @@ class health_check_map_t; class DaemonServer; +class PyModuleRegistry; class ActivePyModules { @@ -46,6 +47,7 @@ class ActivePyModules Client &client; Finisher &finisher; DaemonServer &server; + PyModuleRegistry &py_module_registry; mutable Mutex lock{"ActivePyModules::lock"}; @@ -55,7 +57,7 @@ public: std::map store_data, DaemonStateIndex &ds, ClusterState &cs, MonClient &mc, LogChannelRef clog_, LogChannelRef audit_clog_, Objecter &objecter_, Client &client_, - Finisher &f, DaemonServer &server); + Finisher &f, DaemonServer &server, PyModuleRegistry &pmr); ~ActivePyModules(); @@ -110,6 +112,9 @@ public: void set_config(const std::string &module_name, const std::string &key, const boost::optional &val); + PyObject *get_typed_config(const std::string &module_name, + const std::string &key) const; + void set_health_checks(const std::string& module_name, health_check_map_t&& checks); void get_health_checks(health_check_map_t *checks); @@ -165,4 +170,3 @@ public: 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 31a22665bf6..3a1881f9ab8 100644 --- a/src/mgr/BaseMgrModule.cc +++ b/src/mgr/BaseMgrModule.cc @@ -386,28 +386,36 @@ ceph_option_get(BaseMgrModule *self, PyObject *args) } static PyObject* -ceph_get_module_option(BaseMgrModule *self, PyObject *args) +ceph_get_module_option_ex(BaseMgrModule *self, PyObject *args) { + char *module = nullptr; char *what = nullptr; - if (!PyArg_ParseTuple(args, "s:ceph_get_module_option", &what)) { + if (!PyArg_ParseTuple(args, "ss:ceph_get_module_option_ex", &module, &what)) { derr << "Invalid args!" << dendl; return nullptr; } - PyThreadState *tstate = PyEval_SaveThread(); - std::string value; - bool found = self->py_modules->get_config(self->this_module->get_name(), - what, &value); - + auto pResult = self->py_modules->get_typed_config(module, what); PyEval_RestoreThread(tstate); + return pResult; +} - if (found) { - dout(10) << __func__ << " " << what << " found: " << value.c_str() << dendl; - return self->this_module->py_module->get_typed_option_value(what, value); - } else { - dout(4) << __func__ << " " << what << " not found " << dendl; - Py_RETURN_NONE; +static PyObject* +ceph_get_module_option(BaseMgrModule *self, PyObject *args) +{ + char *key = nullptr; + if (!PyArg_ParseTuple(args, "s:ceph_get_module_option", &key)) { + derr << "Invalid args!" << dendl; + return nullptr; } + auto pKey = PyString_FromString(key); + auto pModule = PyString_FromString(self->this_module->get_name().c_str()); + auto pArgs = PyTuple_Pack(2, pModule, pKey); + Py_DECREF(pKey); + Py_DECREF(pModule); + auto pResult = ceph_get_module_option_ex(self, pArgs); + Py_DECREF(pArgs); + return pResult; } static PyObject* @@ -424,22 +432,45 @@ ceph_store_get_prefix(BaseMgrModule *self, PyObject *args) } static PyObject* -ceph_set_module_option(BaseMgrModule *self, PyObject *args) +ceph_set_module_option_ex(BaseMgrModule *self, PyObject *args) { + char *module = nullptr; char *key = nullptr; char *value = nullptr; - if (!PyArg_ParseTuple(args, "sz:ceph_set_module_option", &key, &value)) { + if (!PyArg_ParseTuple(args, "ssz:ceph_set_module_option_ex", + &module, &key, &value)) { + derr << "Invalid args!" << dendl; return nullptr; } boost::optional val; if (value) { val = value; } - self->py_modules->set_config(self->this_module->get_name(), key, val); + self->py_modules->set_config(module, key, val); Py_RETURN_NONE; } +static PyObject* +ceph_set_module_option(BaseMgrModule *self, PyObject *args) +{ + char *key = nullptr; + char *value = nullptr; + if (!PyArg_ParseTuple(args, "sz:ceph_set_module_option", &key, &value)) { + derr << "Invalid args!" << dendl; + return nullptr; + } + auto pModule = PyString_FromString(self->this_module->get_name().c_str()); + auto pKey = PyString_FromString(key); + auto pValue = PyString_FromString(value); + auto pArgs = PyTuple_Pack(3, pModule, pKey, pValue); + Py_DECREF(pValue); + Py_DECREF(pKey); + Py_DECREF(pModule); + auto pResult = ceph_set_module_option_ex(self, pArgs); + Py_DECREF(pArgs); + return pResult; +} static PyObject* ceph_store_get(BaseMgrModule *self, PyObject *args) @@ -974,12 +1005,18 @@ PyMethodDef BaseMgrModule_methods[] = { {"_ceph_get_module_option", (PyCFunction)ceph_get_module_option, METH_VARARGS, "Get a module configuration option value"}, + {"_ceph_get_module_option_ex", (PyCFunction)ceph_get_module_option_ex, METH_VARARGS, + "Get a module configuration option value from the specified module"}, + {"_ceph_get_store_prefix", (PyCFunction)ceph_store_get_prefix, METH_VARARGS, "Get all KV store values with a given prefix"}, {"_ceph_set_module_option", (PyCFunction)ceph_set_module_option, METH_VARARGS, "Set a module configuration option value"}, + {"_ceph_set_module_option_ex", (PyCFunction)ceph_set_module_option_ex, METH_VARARGS, + "Set a module configuration option value for the specified module"}, + {"_ceph_get_store", (PyCFunction)ceph_store_get, METH_VARARGS, "Get a stored field"}, diff --git a/src/mgr/PyModuleRegistry.cc b/src/mgr/PyModuleRegistry.cc index 5446e06681f..5ef411a5d55 100644 --- a/src/mgr/PyModuleRegistry.cc +++ b/src/mgr/PyModuleRegistry.cc @@ -199,7 +199,8 @@ void PyModuleRegistry::active_start( active_modules.reset(new ActivePyModules( module_config, kv_store, ds, cs, mc, - clog_, audit_clog_, objecter_, client_, f, server)); + clog_, audit_clog_, objecter_, client_, f, server, + *this)); for (const auto &i : modules) { // Anything we're skipping because of !can_run will be flagged diff --git a/src/mgr/PyModuleRegistry.h b/src/mgr/PyModuleRegistry.h index 8c46ab2923d..6868aec65bc 100644 --- a/src/mgr/PyModuleRegistry.h +++ b/src/mgr/PyModuleRegistry.h @@ -122,6 +122,13 @@ public: return modules.at(module_name); } + bool module_exists(const std::string &module_name) const + { + std::lock_guard l(lock); + auto mod_iter = modules.find(module_name); + return mod_iter != modules.end(); + } + /** * Pass through command to the named module for execution. * diff --git a/src/pybind/mgr/mgr_module.py b/src/pybind/mgr/mgr_module.py index 7ad331b0242..4e8fe84f2c0 100644 --- a/src/pybind/mgr/mgr_module.py +++ b/src/pybind/mgr/mgr_module.py @@ -823,6 +823,23 @@ class MgrModule(ceph_module.BaseMgrModule): self._validate_module_option(key) return self._get_module_option(key, default) + def get_module_option_ex(self, module, key, default=None): + """ + Retrieve the value of a persistent configuration setting + for the specified module. + + :param str module: The name of the module, e.g. 'dashboard' + or 'telemetry'. + :param str key: The configuration key, e.g. 'server_addr'. + :param str,None default: The default value to use when the + returned value is ``None``. Defaults to ``None``. + :return: str,int,bool,float,None + """ + if module == self.module_name: + self._validate_module_option(key) + r = self._ceph_get_module_option_ex(module, key) + return default if r is None else r + def get_store_prefix(self, key_prefix): """ Retrieve a dict of KV store keys to values, where the keys @@ -866,6 +883,19 @@ class MgrModule(ceph_module.BaseMgrModule): self._validate_module_option(key) return self._set_module_option(key, val) + def set_module_option_ex(self, module, key, val): + """ + Set the value of a persistent configuration setting + for the specified module. + + :param str module: + :param str key: + :param str val: + """ + if module == self.module_name: + self._validate_module_option(key) + return self._ceph_set_module_option_ex(module, key, str(val)) + def set_localized_module_option(self, key, val): """ Set localized configuration for this ceph-mgr instance diff --git a/src/pybind/mgr/selftest/module.py b/src/pybind/mgr/selftest/module.py index 0a5b867d706..23a3c861025 100644 --- a/src/pybind/mgr/selftest/module.py +++ b/src/pybind/mgr/selftest/module.py @@ -29,9 +29,16 @@ class Module(MgrModule): # The test code in qa/ relies on these options existing -- they # are of course not really used for anything in the module MODULE_OPTIONS = [ - {'name': 'testkey'}, - {'name': 'testlkey'}, - {'name': 'testnewline'} + {'name': 'testkey'}, + {'name': 'testlkey'}, + {'name': 'testnewline'}, + {'name': 'roption1'}, + {'name': 'roption2', 'type': 'str', 'default': 'xyz'}, + {'name': 'rwoption1'}, + {'name': 'rwoption2', 'type': 'int'}, + {'name': 'rwoption3', 'type': 'float'}, + {'name': 'rwoption4', 'type': 'str'}, + {'name': 'rwoption5', 'type': 'bool'} ] COMMANDS = [ @@ -260,6 +267,73 @@ class Module(MgrModule): self.set_localized_module_option("testkey", "testvalue") assert self.get_localized_module_option("testkey") == "testvalue" + # Use default value. + assert self.get_module_option("roption1") is None + assert self.get_module_option("roption1", "foobar") == "foobar" + assert self.get_module_option("roption2") == "xyz" + assert self.get_module_option("roption2", "foobar") == "xyz" + + # Option type is not defined => return as string. + self.set_module_option("rwoption1", 8080) + value = self.get_module_option("rwoption1") + assert isinstance(value, str) + assert value == "8080" + + # Option type is defined => return as integer. + self.set_module_option("rwoption2", 10) + value = self.get_module_option("rwoption2") + assert isinstance(value, int) + assert value == 10 + + # Option type is defined => return as float. + self.set_module_option("rwoption3", 1.5) + value = self.get_module_option("rwoption3") + assert isinstance(value, float) + assert value == 1.5 + + # Option type is defined => return as string. + self.set_module_option("rwoption4", "foo") + value = self.get_module_option("rwoption4") + assert isinstance(value, str) + assert value == "foo" + + # Option type is defined => return as bool. + self.set_module_option("rwoption5", False) + value = self.get_module_option("rwoption5") + assert isinstance(value, bool) + assert value is False + + # Specified module does not exist => return None. + assert self.get_module_option_ex("foo", "bar") is None + + # Specified key does not exist => return None. + assert self.get_module_option_ex("dashboard", "bar") is None + + self.set_module_option_ex("telemetry", "contact", "test@test.com") + assert self.get_module_option_ex("telemetry", "contact") == "test@test.com" + + # No option default value, so use the specified one. + assert self.get_module_option_ex("dashboard", "password") is None + assert self.get_module_option_ex("dashboard", "password", "foobar") == "foobar" + + # Option type is not defined => return as string. + self.set_module_option_ex("dashboard", "server_port", 8080) + value = self.get_module_option_ex("dashboard", "server_port") + assert isinstance(value, str) + assert value == "8080" + + # Option type is defined => return as integer. + self.set_module_option_ex("telemetry", "interval", 60) + value = self.get_module_option_ex("telemetry", "interval") + assert isinstance(value, int) + assert value == 60 + + # Option type is defined => return as bool. + self.set_module_option_ex("telemetry", "leaderboard", True) + value = self.get_module_option_ex("telemetry", "leaderboard") + assert isinstance(value, bool) + assert value is True + def _self_test_store(self): existing_keys = set(self.get_store_prefix("test").keys()) self.set_store("testkey", "testvalue")