From: Kefu Chai Date: Wed, 25 Nov 2020 10:44:04 +0000 (+0800) Subject: mgr: do not migrate conf from config-key store to new-style conf X-Git-Tag: v16.1.0~441^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=70e99e76b5f63265f6b2700f8af77ece4f1dd11b;p=ceph.git mgr: do not migrate conf from config-key store to new-style conf since all module options are using the new-style config framework. the migration is offered for the use case of upgrade from luminous to mimic, since pacific can only be upgraded from octopus. the mimic monitors are alreay able to populate the configurations to mgr, not to mention the octopus monitors, so there is no need to migrate the options stored in config-key store anymore. Signed-off-by: Kefu Chai --- diff --git a/qa/tasks/mgr/test_module_selftest.py b/qa/tasks/mgr/test_module_selftest.py index 59f91d43c149..8abfe51abe8f 100644 --- a/qa/tasks/mgr/test_module_selftest.py +++ b/qa/tasks/mgr/test_module_selftest.py @@ -113,88 +113,6 @@ class TestModuleSelftest(MgrTestCase): "bar") self.wait_until_equal(get_localized_value, "bar", timeout=10) - def test_selftest_config_upgrade(self): - """ - That pre-mimic config-key config settings are migrated into - mimic-style config settings and visible from mgr modules. - """ - self._load_module("selftest") - - def get_value(): - return self.mgr_cluster.mon_manager.raw_cluster_cmd( - "mgr", "self-test", "config", "get", "testkey").strip() - - def get_config(): - lines = self.mgr_cluster.mon_manager.raw_cluster_cmd( - "config", "dump")\ - .strip().split("\n") - result = [] - for line in lines[1:]: - tokens = line.strip().split() - log.info("tokens: {0}".format(tokens)) - subsys, key, value = tokens[0], tokens[2], tokens[3] - result.append((subsys, key, value)) - - return result - - # Stop ceph-mgr while we synthetically create a pre-mimic - # configuration scenario - for mgr_id in self.mgr_cluster.mgr_daemons.keys(): - self.mgr_cluster.mgr_stop(mgr_id) - self.mgr_cluster.mgr_fail(mgr_id) - - # Blow away any modern-style mgr module config options - # (the ceph-mgr implementation may only do the upgrade if - # it doesn't see new style options) - stash = [] - for subsys, key, value in get_config(): - if subsys == "mgr" and key.startswith("mgr/"): - log.info("Removing config key {0} ahead of upgrade".format( - key)) - self.mgr_cluster.mon_manager.raw_cluster_cmd( - "config", "rm", subsys, key) - stash.append((subsys, key, value)) - - # Inject an old-style configuration setting in config-key - self.mgr_cluster.mon_manager.raw_cluster_cmd( - "config-key", "set", "mgr/selftest/testkey", "testvalue") - - # Inject configuration settings that looks data-ish and should - # not be migrated to a config key - self.mgr_cluster.mon_manager.raw_cluster_cmd( - "config-key", "set", "mgr/selftest/testnewline", "foo\nbar") - - # Inject configuration setting that does not appear in the - # module's config schema - self.mgr_cluster.mon_manager.raw_cluster_cmd( - "config-key", "set", "mgr/selftest/kvitem", "foo\nbar") - - # Bring mgr daemons back online, the one that goes active - # should be doing the upgrade. - for mgr_id in self.mgr_cluster.mgr_daemons.keys(): - self.mgr_cluster.mgr_restart(mgr_id) - - # Wait for a new active - self.wait_until_true( - lambda: self.mgr_cluster.get_active_id() != "", timeout=30) - - # Check that the selftest module sees the upgraded value - self.assertEqual(get_value(), "testvalue") - - # Check that the upgraded value is visible in the configuration - seen_keys = [k for s,k,v in get_config()] - self.assertIn("mgr/selftest/testkey", seen_keys) - - # ...and that the non-config-looking one isn't - self.assertNotIn("mgr/selftest/testnewline", seen_keys) - - # ...and that the not-in-schema one isn't - self.assertNotIn("mgr/selftest/kvitem", seen_keys) - - # Restore previous configuration - for subsys, key, value in stash: - self.mgr_cluster.mon_manager.raw_cluster_cmd( - "config", "set", subsys, key, value) def test_selftest_command_spam(self): # Use the selftest module to stress the mgr daemon diff --git a/src/mgr/Mgr.cc b/src/mgr/Mgr.cc index 781017a26890..1e9675944f5c 100644 --- a/src/mgr/Mgr.cc +++ b/src/mgr/Mgr.cc @@ -308,12 +308,6 @@ void Mgr::init() // Load module KV store auto kv_store = load_store(); - // Migrate config from KV store on luminous->mimic - // drop lock because we do blocking config sets to mon - lock.unlock(); - py_module_registry->upgrade_config(monc, kv_store); - lock.lock(); - // assume finisher already initialized in background_init dout(4) << "starting python modules..." << dendl; py_module_registry->active_start(daemon_state, cluster_state, diff --git a/src/mgr/PyModuleRegistry.cc b/src/mgr/PyModuleRegistry.cc index 2aa556cc9d51..ad461aa47bc8 100644 --- a/src/mgr/PyModuleRegistry.cc +++ b/src/mgr/PyModuleRegistry.cc @@ -445,83 +445,3 @@ void PyModuleRegistry::handle_config_notify() active_modules->config_notify(); } } - -void PyModuleRegistry::upgrade_config( - MonClient *monc, - const std::map &old_config) -{ - // Only bother doing anything if we didn't already have - // some new-style config. - if (module_config.config.empty()) { - dout(1) << "Upgrading module configuration for Mimic" << dendl; - // Upgrade luminous->mimic: migrate config-key configuration - // into main configuration store - for (auto &i : old_config) { - auto last_slash = i.first.rfind('/'); - const std::string module_name = i.first.substr(4, i.first.substr(4).find('/')); - const std::string key = i.first.substr(last_slash + 1); - - const auto &value = i.second; - - // Heuristic to skip things that look more like stores - // than configs. - bool is_config = true; - for (const auto &c : value) { - if (c == '\n' || c == '\r' || c < 0x20) { - is_config = false; - break; - } - } - - if (value.size() > 256) { - is_config = false; - } - - if (!is_config) { - dout(1) << "Not migrating config module:key " - << module_name << " : " << key << dendl; - continue; - } - - // 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 " - << module_config.config.size() << " keys" << dendl; - } -} -