From: Sage Weil Date: Fri, 8 Mar 2019 14:48:39 +0000 (-0600) Subject: mgr: push localized config handling into BaseMgr*Module X-Git-Tag: v14.2.0~65^2 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=4ca5596a45007805e6977cf8619e65bcff220250;p=ceph-ci.git mgr: push localized config handling into BaseMgr*Module We need both (1) the raw key name (in order to look up the correct option name and transform the result into the correct type) and (2) the optional prefix (in order to look up both possible keys). This simplifies the mgr_module implementation. There is a weird mismatch between the way that BaseMgrModule and BaseMgrStandbyModule implement this method. I suspect they could be unified to work the same, but I'll leave that for another day. Signed-off-by: Sage Weil --- diff --git a/src/mgr/ActivePyModules.cc b/src/mgr/ActivePyModules.cc index e90d877b852..9e32dce58ca 100644 --- a/src/mgr/ActivePyModules.cc +++ b/src/mgr/ActivePyModules.cc @@ -527,11 +527,21 @@ bool ActivePyModules::get_config(const std::string &module_name, PyObject *ActivePyModules::get_typed_config( const std::string &module_name, - const std::string &key) const + const std::string &key, + const std::string &prefix) const { PyThreadState *tstate = PyEval_SaveThread(); std::string value; - bool found = get_config(module_name, key, &value); + std::string final_key; + bool found = false; + if (prefix.size()) { + final_key = prefix + "/" + key; + found = get_config(module_name, final_key, &value); + } + if (!found) { + final_key = key; + found = get_config(module_name, final_key, &value); + } if (found) { PyModuleRef module = py_module_registry.get_module(module_name); PyEval_RestoreThread(tstate); @@ -539,11 +549,16 @@ PyObject *ActivePyModules::get_typed_config( derr << "Module '" << module_name << "' is not available" << dendl; Py_RETURN_NONE; } - dout(10) << __func__ << " " << key << " found: " << value << dendl; + dout(10) << __func__ << " " << final_key << " found: " << value << dendl; return module->get_typed_option_value(key, value); } PyEval_RestoreThread(tstate); - dout(4) << __func__ << " " << key << " not found " << dendl; + if (prefix.size()) { + dout(4) << __func__ << " [" << prefix << "/]" << key << " not found " + << dendl; + } else { + dout(4) << __func__ << " " << key << " not found " << dendl; + } Py_RETURN_NONE; } diff --git a/src/mgr/ActivePyModules.h b/src/mgr/ActivePyModules.h index bd3aee07d3c..cf6106c5d4a 100644 --- a/src/mgr/ActivePyModules.h +++ b/src/mgr/ActivePyModules.h @@ -115,7 +115,8 @@ public: const std::string &key, const boost::optional &val); PyObject *get_typed_config(const std::string &module_name, - const std::string &key) const; + const std::string &key, + const std::string &prefix) const; void set_health_checks(const std::string& module_name, health_check_map_t&& checks); diff --git a/src/mgr/BaseMgrModule.cc b/src/mgr/BaseMgrModule.cc index f7aff7fe07b..0e33e0f4b6d 100644 --- a/src/mgr/BaseMgrModule.cc +++ b/src/mgr/BaseMgrModule.cc @@ -400,11 +400,18 @@ ceph_get_module_option(BaseMgrModule *self, PyObject *args) { char *module = nullptr; char *key = nullptr; - if (!PyArg_ParseTuple(args, "ss:ceph_get_module_option", &module, &key)) { + char *prefix = nullptr; + if (!PyArg_ParseTuple(args, "sss:ceph_get_module_option", &module, &key, + &prefix)) { derr << "Invalid args!" << dendl; return nullptr; } - auto pResult = self->py_modules->get_typed_config(module, key); + std::string str_prefix; + if (prefix) { + str_prefix = prefix; + } + assert(self->this_module->py_module); + auto pResult = self->py_modules->get_typed_config(module, key, str_prefix); return pResult; } diff --git a/src/mgr/BaseMgrStandbyModule.cc b/src/mgr/BaseMgrStandbyModule.cc index ee923552f40..6a20b114c74 100644 --- a/src/mgr/BaseMgrStandbyModule.cc +++ b/src/mgr/BaseMgrStandbyModule.cc @@ -64,18 +64,33 @@ static PyObject* ceph_get_module_option(BaseMgrStandbyModule *self, PyObject *args) { char *what = nullptr; - if (!PyArg_ParseTuple(args, "s:ceph_get_module_option", &what)) { + char *prefix = nullptr; + if (!PyArg_ParseTuple(args, "ss:ceph_get_module_option", &what)) { derr << "Invalid args!" << dendl; return nullptr; } - + std::string final_key; std::string value; - bool found = self->this_module->get_config(what, &value); + bool found = false; + if (prefix) { + final_key = std::string(prefix) + "/" + what; + found = self->this_module->get_config(final_key, &value); + } + if (!found) { + final_key = what; + found = self->this_module->get_config(final_key, &value); + } if (found) { - dout(10) << __func__ << " " << what << " found: " << value.c_str() << dendl; + dout(10) << __func__ << " " << final_key << " 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; + if (prefix) { + dout(4) << __func__ << " [" << prefix << "/]" << what << " not found " + << dendl; + } else { + dout(4) << __func__ << " " << what << " not found " << dendl; + } Py_RETURN_NONE; } } diff --git a/src/pybind/mgr/mgr_module.py b/src/pybind/mgr/mgr_module.py index 9540420ec2a..6e82ffef4ef 100644 --- a/src/pybind/mgr/mgr_module.py +++ b/src/pybind/mgr/mgr_module.py @@ -428,7 +428,7 @@ class MgrStandbyModule(ceph_module.BaseMgrStandbyModule): :param default: the default value of the config if it is not found :return: str """ - r = self._ceph_get_module_option(key) + r = self._ceph_get_module_option(key, "") if r is None: final_key = key.split('/')[-1] return self.MODULE_OPTION_DEFAULTS.get(final_key, default) @@ -451,13 +451,12 @@ class MgrStandbyModule(ceph_module.BaseMgrStandbyModule): return self._ceph_get_active_uri() def get_localized_module_option(self, key, default=None): - r = self.get_module_option(self.get_mgr_id() + '/' + key) + r = self._ceph_get_module_option(key, self.get_mgr_id()) if r is None: - r = self.get_module_option(key) - - if r is None: - r = default - return r + final_key = key.split('/')[-1] + return self.MODULE_OPTION_DEFAULTS.get(final_key, default) + else: + return r class MgrModule(ceph_module.BaseMgrModule): @@ -905,8 +904,9 @@ class MgrModule(ceph_module.BaseMgrModule): raise RuntimeError("Config option '{0}' is not in {1}.MODULE_OPTIONS". format(key, self.__class__.__name__)) - def _get_module_option(self, key, default): - r = self._ceph_get_module_option(self.module_name, key) + def _get_module_option(self, key, default, localized_prefix=""): + r = self._ceph_get_module_option(self.module_name, key, + localized_prefix) if r is None: final_key = key.split('/')[-1] return self.MODULE_OPTION_DEFAULTS.get(final_key, default) @@ -938,7 +938,7 @@ class MgrModule(ceph_module.BaseMgrModule): """ if module == self.module_name: self._validate_module_option(key) - r = self._ceph_get_module_option(module, key) + r = self._ceph_get_module_option(module, key, "") return default if r is None else r def get_store_prefix(self, key_prefix): @@ -962,14 +962,7 @@ class MgrModule(ceph_module.BaseMgrModule): :return: str """ self._validate_module_option(key) - r = self._ceph_get_module_option( - self.module_name, '{}/{}'.format(self.get_mgr_id(), key)) - if r is None: - r = self._ceph_get_module_option(self.module_name, key) - if r is None: - final_key = key.split('/')[-1] - r = self.MODULE_OPTION_DEFAULTS.get(final_key, default) - return r + return self._get_module_option(key, default, self.get_mgr_id()) def _set_module_option(self, key, val): return self._ceph_set_module_option(self.module_name, key, str(val))