diff --git a/src/mgr/PyModule.cc b/src/mgr/PyModule.cc index 89927a72c64..65656d0a60a 100644 --- a/src/mgr/PyModule.cc +++ b/src/mgr/PyModule.cc @@ -363,8 +363,17 @@ int PyModule::load(PyThreadState *pMainThreadState) r = load_commands(); if (r != 0) { - derr << "Missing COMMAND attr in module '" << module_name << "'" << dendl; - error_string = "Missing COMMAND attribute"; + derr << "Missing or invalid COMMANDS attribute in module '" + << module_name << "'" << dendl; + error_string = "Missing or invalid COMMANDS attribute"; + return r; + } + + r = load_options(); + if (r != 0) { + derr << "Missing or invalid OPTIONS attribute in module '" + << module_name << "'" << dendl; + error_string = "Missing or invalid OPTIONS attribute"; return r; } @@ -421,63 +430,111 @@ int PyModule::load(PyThreadState *pMainThreadState) return 0; } -int PyModule::load_commands() +int PyModule::walk_dict_list( + const std::string &attr_name, + std::function fn) { - // Don't need a Gil here -- this is called from load(), - // which already has one. - PyObject *command_list = PyObject_GetAttrString(pClass, "COMMANDS"); + PyObject *command_list = PyObject_GetAttrString(pClass, attr_name.c_str()); if (command_list == nullptr) { - // Even modules that don't define command should still have the COMMANDS - // from the MgrModule definition. Something is wrong! - derr << "Module " << get_name() << " has missing COMMANDS member" << dendl; + derr << "Module " << get_name() << " has missing " << attr_name + << " member" << dendl; return -EINVAL; } if (!PyObject_TypeCheck(command_list, &PyList_Type)) { // Relatively easy mistake for human to make, e.g. defining COMMANDS // as a {} instead of a [] - derr << "Module " << get_name() << " has COMMANDS member of wrong type (" - "should be a list)" << dendl; + derr << "Module " << get_name() << " has " << attr_name + << " member of wrong type (should be a list)" << dendl; return -EINVAL; } + + // Invoke fn on each item in the list + int r = 0; const size_t list_size = PyList_Size(command_list); for (size_t i = 0; i < list_size; ++i) { PyObject *command = PyList_GetItem(command_list, i); assert(command != nullptr); - ModuleCommand item; - - PyObject *pCmd = PyDict_GetItemString(command, "cmd"); - assert(pCmd != nullptr); - item.cmdstring = PyString_AsString(pCmd); - - dout(20) << "loaded command " << item.cmdstring << dendl; - - PyObject *pDesc = PyDict_GetItemString(command, "desc"); - assert(pDesc != nullptr); - item.helpstring = PyString_AsString(pDesc); - - PyObject *pPerm = PyDict_GetItemString(command, "perm"); - assert(pPerm != nullptr); - item.perm = PyString_AsString(pPerm); - - item.polling = false; - PyObject *pPoll = PyDict_GetItemString(command, "poll"); - if (pPoll) { - std::string polling = PyString_AsString(pPoll); - if (boost::iequals(polling, "true")) { - item.polling = true; - } + if (!PyDict_Check(command)) { + derr << "Module " << get_name() << " has non-dict entry " + << "in " << attr_name << " list" << dendl; + return -EINVAL; } - item.module_name = module_name; - - commands.push_back(item); + r = fn(command); + if (r != 0) { + break; + } } Py_DECREF(command_list); + return r; +} + +int PyModule::load_commands() +{ + int r = walk_dict_list("COMMANDS", [this](PyObject *pCommand) -> int { + ModuleCommand command; + + PyObject *pCmd = PyDict_GetItemString(pCommand, "cmd"); + assert(pCmd != nullptr); + command.cmdstring = PyString_AsString(pCmd); + + dout(20) << "loaded command " << command.cmdstring << dendl; + + PyObject *pDesc = PyDict_GetItemString(pCommand, "desc"); + assert(pDesc != nullptr); + command.helpstring = PyString_AsString(pDesc); + + PyObject *pPerm = PyDict_GetItemString(pCommand, "perm"); + assert(pPerm != nullptr); + command.perm = PyString_AsString(pPerm); + + command.polling = false; + PyObject *pPoll = PyDict_GetItemString(pCommand, "poll"); + if (pPoll) { + std::string polling = PyString_AsString(pPoll); + if (boost::iequals(polling, "true")) { + command.polling = true; + } + } + + command.module_name = module_name; + + commands.push_back(std::move(command)); + + return 0; + }); + dout(10) << "loaded " << commands.size() << " commands" << dendl; - return 0; + return r; +} + +int PyModule::load_options() +{ + int r = walk_dict_list("OPTIONS", [this](PyObject *pOption) -> int { + PyObject *pName = PyDict_GetItemString(pOption, "name"); + assert(pName != nullptr); + + ModuleOption option; + option.name = PyString_AsString(pName); + dout(20) << "loaded option " << option.name << dendl; + + options[option.name] = std::move(option); + + return 0; + }); + + dout(10) << "loaded " << options.size() << " options" << dendl; + + return r; +} + +bool PyModule::is_option(const std::string &option_name) +{ + Mutex::Locker l(lock); + return options.count(option_name) > 0; } int PyModule::load_subclass_of(const char* base_class, PyObject** py_class) diff --git a/src/mgr/PyModule.h b/src/mgr/PyModule.h index 512313d60da..d9ce1695609 100644 --- a/src/mgr/PyModule.h +++ b/src/mgr/PyModule.h @@ -41,6 +41,15 @@ public: std::string module_name; }; + +/** + * An option declared by the python module in its configuration schema + */ +class ModuleOption { + public: + std::string name; +}; + class PyModule { mutable Mutex lock{"PyModule::lock"}; @@ -67,9 +76,17 @@ private: // Populated if loaded, can_run or failed indicates a problem std::string error_string; + // Helper for loading OPTIONS and COMMANDS members + int walk_dict_list( + const std::string &attr_name, + std::function fn); + int load_commands(); std::vector commands; + int load_options(); + std::map options; + public: static std::string config_prefix; @@ -84,6 +101,8 @@ public: ~PyModule(); + bool is_option(const std::string &option_name); + int load(PyThreadState *pMainThreadState); #if PY_MAJOR_VERSION >= 3 static PyObject* init_ceph_logger(); diff --git a/src/mgr/PyModuleRegistry.cc b/src/mgr/PyModuleRegistry.cc index 940d65f6054..2364b1cfb8b 100644 --- a/src/mgr/PyModuleRegistry.cc +++ b/src/mgr/PyModuleRegistry.cc @@ -433,9 +433,41 @@ void PyModuleRegistry::upgrade_config( continue; } - module_config.set_config(monc, module_name, key, i.second); - dout(4) << "Rewrote configuration module:key " - << module_name << ":" << key << dendl; + // Check that the named module exists + auto module_iter = modules.find(module_name); + if (module_iter == modules.end()) { + dout(1) << "KV store contains data for unknown module '" + << module_name << "'" << dendl; + continue; + } + PyModuleRef module = module_iter->second; + + // Parse option name out of key + std::string option_name; + auto slash_loc = key.find("/"); + if (slash_loc != std::string::npos) { + if (key.size() > slash_loc + 1) { + // Localized option + option_name = key.substr(slash_loc + 1); + } else { + // Trailing slash: garbage. + derr << "Invalid mgr store key: '" << key << "'" << dendl; + continue; + } + } else { + option_name = key; + } + + // Consult module schema to see if this is really + // a configuration value + if (!option_name.empty() && module->is_option(option_name)) { + module_config.set_config(monc, module_name, key, i.second); + dout(4) << "Rewrote configuration module:key " + << module_name << ":" << key << dendl; + } else { + dout(4) << "Leaving store module:key " << module_name + << ":" << key << " in store, not config" << dendl; + } } } else { dout(10) << "Module configuration contains "