From bc9643657af9be29be54467ad4eb38bfc7e7e7a3 Mon Sep 17 00:00:00 2001 From: Volker Theile Date: Mon, 4 Mar 2019 09:55:40 +0100 Subject: [PATCH] mgr: Fix broken get_localized_module_option function Fixes: https://tracker.ceph.com/issues/38560 Signed-off-by: Volker Theile --- qa/tasks/mgr/test_module_selftest.py | 27 +++++++++++---------------- src/pybind/mgr/mgr_module.py | 26 ++++++++++++++++---------- src/pybind/mgr/selftest/module.py | 12 +++++++++--- 3 files changed, 36 insertions(+), 29 deletions(-) diff --git a/qa/tasks/mgr/test_module_selftest.py b/qa/tasks/mgr/test_module_selftest.py index 26673971b64ee..a5190b13804b5 100644 --- a/qa/tasks/mgr/test_module_selftest.py +++ b/qa/tasks/mgr/test_module_selftest.py @@ -68,7 +68,6 @@ class TestModuleSelftest(MgrTestCase): "osd", "pool", "delete", pool_name, pool_name, "--yes-i-really-really-mean-it") - def test_selftest_run(self): self._load_module("selftest") self.mgr_cluster.mon_manager.raw_cluster_cmd("mgr", "self-test", "run") @@ -87,27 +86,23 @@ class TestModuleSelftest(MgrTestCase): def get_value(): return self.mgr_cluster.mon_manager.raw_cluster_cmd( - "mgr", "self-test", "config", "get", "testkey").strip() + "mgr", "self-test", "config", "get", "testkey").strip() self.assertEqual(get_value(), "None") - - self.mgr_cluster.mon_manager.raw_cluster_cmd("config", "set", - "mgr", "mgr/selftest/testkey", "testvalue") - - self.wait_until_equal(get_value, "testvalue",timeout=10) - - active_id = self.mgr_cluster.get_active_id() + self.mgr_cluster.mon_manager.raw_cluster_cmd( + "config", "set", "mgr", "mgr/selftest/testkey", "foo") + self.wait_until_equal(get_value, "foo", timeout=10) def get_localized_value(): return self.mgr_cluster.mon_manager.raw_cluster_cmd( - "mgr", "self-test", "config", "get_localized", "testlkey").strip() + "mgr", "self-test", "config", "get_localized", "testkey").strip() - self.mgr_cluster.mon_manager.raw_cluster_cmd("config", "set", - "mgr", "mgr/selftest/{0}/testlkey".format(active_id), - "test localized value") - - self.wait_until_equal(get_localized_value, "test localized value", - timeout=10) + self.assertEqual(get_localized_value(), "None") + self.mgr_cluster.mon_manager.raw_cluster_cmd( + "config", "set", "mgr", "mgr/selftest/{}/testkey".format( + self.mgr_cluster.get_active_id()), + "bar") + self.wait_until_equal(get_localized_value, "bar", timeout=10) def test_selftest_config_upgrade(self): """ diff --git a/src/pybind/mgr/mgr_module.py b/src/pybind/mgr/mgr_module.py index 916e65c946d0f..9540420ec2a70 100644 --- a/src/pybind/mgr/mgr_module.py +++ b/src/pybind/mgr/mgr_module.py @@ -1,4 +1,3 @@ - import ceph_module # noqa import logging @@ -352,6 +351,7 @@ def CLIReadCommand(prefix, args="", desc=""): def CLIWriteCommand(prefix, args="", desc=""): return CLICommand(prefix, args, desc, "w") + class Option(dict): """ Helper class to declare options for MODULE_OPTIONS list. @@ -361,6 +361,7 @@ class Option(dict): TODO: type validation. """ + def __init__( self, name, default=None, @@ -950,13 +951,6 @@ class MgrModule(ceph_module.BaseMgrModule): """ return self._ceph_get_store_prefix(key_prefix) - def _get_localized(self, key, default, getter): - r = getter(self.get_mgr_id() + '/' + key, None) - if r is None: - r = getter(key, default) - - return r - def _set_localized(self, key, val, setter): return setter(self.get_mgr_id() + '/' + key, val) @@ -968,7 +962,14 @@ class MgrModule(ceph_module.BaseMgrModule): :return: str """ self._validate_module_option(key) - return self._get_localized(key, default, self._get_module_option) + 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 def _set_module_option(self, key, val): return self._ceph_set_module_option(self.module_name, key, str(val)) @@ -1027,7 +1028,12 @@ class MgrModule(ceph_module.BaseMgrModule): return r def get_localized_store(self, key, default=None): - return self._get_localized(key, default, self.get_store) + r = self._ceph_get_store('{}/{}'.format(self.get_mgr_id(), key)) + if r is None: + r = self._ceph_get_store(key) + if r is None: + r = default + return r def set_localized_store(self, key, val): return self._set_localized(key, val, self.set_store) diff --git a/src/pybind/mgr/selftest/module.py b/src/pybind/mgr/selftest/module.py index 42acfd081135e..71913d31d7bf2 100644 --- a/src/pybind/mgr/selftest/module.py +++ b/src/pybind/mgr/selftest/module.py @@ -38,7 +38,8 @@ class Module(MgrModule): {'name': 'rwoption2', 'type': 'int'}, {'name': 'rwoption3', 'type': 'float'}, {'name': 'rwoption4', 'type': 'str'}, - {'name': 'rwoption5', 'type': 'bool'} + {'name': 'rwoption5', 'type': 'bool'}, + {'name': 'rwoption6', 'type': 'bool', 'default': True} ] COMMANDS = [ @@ -264,8 +265,13 @@ class Module(MgrModule): self.set_module_option("testkey", "testvalue") assert self.get_module_option("testkey") == "testvalue" - self.set_localized_module_option("testkey", "testvalue") - assert self.get_localized_module_option("testkey") == "testvalue" + self.set_localized_module_option("testkey", "foo") + assert self.get_localized_module_option("testkey") == "foo" + + # Must return the default value defined in MODULE_OPTIONS. + value = self.get_localized_module_option("rwoption6") + assert isinstance(value, bool) + assert value is True # Use default value. assert self.get_module_option("roption1") is None -- 2.39.5