From d24e6cb32acebf6f3fe6cb96d60ae323437d99fd Mon Sep 17 00:00:00 2001 From: John Spray Date: Tue, 10 Jul 2018 12:40:49 -0400 Subject: [PATCH] mgr: generic self test command Avoid need for each module to expose a self-test command: they can just implement the method, and then get it called via the selftest module. As well as fewer LOC, this means that the self test commands are not cluttering the interface for end users, as they've invisible until the selftest module is loaded. Signed-off-by: John Spray --- qa/tasks/mgr/test_module_selftest.py | 6 ++++-- src/pybind/mgr/crash/module.py | 12 ++--------- src/pybind/mgr/devicehealth/module.py | 8 ------- src/pybind/mgr/influx/module.py | 30 ++++++++++++--------------- src/pybind/mgr/iostat/module.py | 25 ++++++++-------------- src/pybind/mgr/mgr_module.py | 8 ++++++- src/pybind/mgr/prometheus/module.py | 15 +++++--------- src/pybind/mgr/selftest/module.py | 12 +++++++++++ src/pybind/mgr/telegraf/module.py | 8 ------- src/pybind/mgr/telemetry/module.py | 8 ------- src/pybind/mgr/zabbix/module.py | 8 ------- 11 files changed, 52 insertions(+), 88 deletions(-) diff --git a/qa/tasks/mgr/test_module_selftest.py b/qa/tasks/mgr/test_module_selftest.py index 5248fa41a2b..fa609f7348c 100644 --- a/qa/tasks/mgr/test_module_selftest.py +++ b/qa/tasks/mgr/test_module_selftest.py @@ -26,10 +26,12 @@ class TestModuleSelftest(MgrTestCase): self.setup_mgrs() def _selftest_plugin(self, module_name): + self._load_module("selftest") self._load_module(module_name) - # Execute the module's self-test routine - self.mgr_cluster.mon_manager.raw_cluster_cmd(module_name, "self-test") + # Execute the module's self_test() method + self.mgr_cluster.mon_manager.raw_cluster_cmd( + "mgr", "self-test", "module", module_name) def test_zabbix(self): # Set these mandatory config fields so that the zabbix module diff --git a/src/pybind/mgr/crash/module.py b/src/pybind/mgr/crash/module.py index 7b5b7544213..a9f3222a03e 100644 --- a/src/pybind/mgr/crash/module.py +++ b/src/pybind/mgr/crash/module.py @@ -172,14 +172,12 @@ class Module(MgrModule): return 0, '', json.dumps(report) - def do_self_test(self, cmd, inbuf): + def self_test(self): # test time conversion timestr = '2018-06-22 20:35:38.058818Z' dt = self.time_from_string(timestr) if dt != datetime.datetime(2018, 6, 22, 20, 35, 38, 58818): - return errno.EINVAL, '', 'time_from_string() failed' - - return 0, '', 'self-test succeeded' + raise RuntimeError('time_from_string() failed') COMMANDS = [ { @@ -212,12 +210,6 @@ class Module(MgrModule): 'perm': 'rw', 'handler': do_rm, }, - { - 'cmd': 'crash self-test', - 'desc': 'Run a self test of the crash module', - 'perm': 'r', - 'handler': do_self_test, - }, { 'cmd': 'crash stat', 'desc': 'Summarize recorded crashes', diff --git a/src/pybind/mgr/devicehealth/module.py b/src/pybind/mgr/devicehealth/module.py index 4c2ce2df0f2..c4a9a356e36 100644 --- a/src/pybind/mgr/devicehealth/module.py +++ b/src/pybind/mgr/devicehealth/module.py @@ -50,11 +50,6 @@ class Module(MgrModule): "desc": "Show stored device metrics for the device", "perm": "r" }, - { - "cmd": "devicehealth self-test", - "desc": "Run a self-test on the devicehealth module", - "perm": "rw", - }, ] def __init__(self, *args, **kwargs): @@ -97,8 +92,6 @@ class Module(MgrModule): return self.scrape_all(); elif cmd['prefix'] == 'device show-health-metrics': return self.show_device_metrics(cmd['devid'], cmd.get('sample')) - elif cmd['prefix'] == 'devicehealth self-test': - return self.self_test() else: # mgr should respect our self.COMMANDS and not call us for # any prefix we don't advertise @@ -119,7 +112,6 @@ class Module(MgrModule): (r, after, err) = self.show_device_metrics(devid, '') assert r == 0 assert before != after - return (0, '', '') def refresh_config(self): self.enable_monitoring = self.get_config('enable_monitoring', '') is not '' or 'false' diff --git a/src/pybind/mgr/influx/module.py b/src/pybind/mgr/influx/module.py index e077b04ffc4..14c6af3ff5b 100644 --- a/src/pybind/mgr/influx/module.py +++ b/src/pybind/mgr/influx/module.py @@ -72,12 +72,7 @@ class Module(MgrModule): "cmd": "influx send", "desc": "Force sending data to Influx", "perm": "rw" - }, - { - "cmd": "influx self-test", - "desc": "debug the module", - "perm": "rw" - }, + } ] def __init__(self, *args, **kwargs): @@ -327,6 +322,18 @@ class Module(MgrModule): self.run = False self.event.set() + def self_test(self): + daemon_stats = self.get_daemon_stats() + assert len(daemon_stats) + df_stats, pools = self.get_df_stats() + + result = { + 'daemon_stats': daemon_stats, + 'df_stats': df_stats + } + + return json.dumps(result, indent=2) + def handle_command(self, inbuf, cmd): if cmd['prefix'] == 'influx config-show': return 0, json.dumps(self.config), '' @@ -343,17 +350,6 @@ class Module(MgrModule): elif cmd['prefix'] == 'influx send': self.send_to_influx() return 0, 'Sending data to Influx', '' - if cmd['prefix'] == 'influx self-test': - daemon_stats = self.get_daemon_stats() - assert len(daemon_stats) - df_stats, pools = self.get_df_stats() - - result = { - 'daemon_stats': daemon_stats, - 'df_stats': df_stats - } - - return 0, json.dumps(result, indent=2), 'Self-test OK' return (-errno.EINVAL, '', "Command not found '{0}'".format(cmd['prefix'])) diff --git a/src/pybind/mgr/iostat/module.py b/src/pybind/mgr/iostat/module.py index 9133fafae2e..f5c3179ded1 100644 --- a/src/pybind/mgr/iostat/module.py +++ b/src/pybind/mgr/iostat/module.py @@ -10,17 +10,21 @@ class Module(MgrModule): "perm": "r", "poll": "true" }, - { - "cmd": "iostat self-test", - "desc": "Run a self test the iostat module", - "perm": "r" - } ] def __init__(self, *args, **kwargs): super(Module, self).__init__(*args, **kwargs) + def self_test(self): + r = self.get('io_rate') + assert('pg_stats_delta' in r) + assert('stamp_delta' in r['pg_stats_delta']) + assert('stat_sum' in r['pg_stats_delta']) + assert('num_read_kb' in r['pg_stats_delta']['stat_sum']) + assert('num_write_kb' in r['pg_stats_delta']['stat_sum']) + assert('num_write' in r['pg_stats_delta']['stat_sum']) + assert('num_read' in r['pg_stats_delta']['stat_sum']) def handle_command(self, inbuf, command): rd = 0 @@ -66,15 +70,4 @@ class Module(MgrModule): ] ret += self.get_pretty_row(elems, width) - elif command['prefix'] == 'iostat self-test': - r = self.get('io_rate') - assert('pg_stats_delta' in r) - assert('stamp_delta' in r['pg_stats_delta']) - assert('stat_sum' in r['pg_stats_delta']) - assert('num_read_kb' in r['pg_stats_delta']['stat_sum']) - assert('num_write_kb' in r['pg_stats_delta']['stat_sum']) - assert('num_write' in r['pg_stats_delta']['stat_sum']) - assert('num_read' in r['pg_stats_delta']['stat_sum']) - ret = 'iostat self-test OK' - return 0, '', ret diff --git a/src/pybind/mgr/mgr_module.py b/src/pybind/mgr/mgr_module.py index 501938e29d1..4bae68be5d7 100644 --- a/src/pybind/mgr/mgr_module.py +++ b/src/pybind/mgr/mgr_module.py @@ -687,7 +687,13 @@ class MgrModule(ceph_module.BaseMgrModule): """ Run a self-test on the module. Override this function and implement a best as possible self-test for (automated) testing of the module - :return: bool + + Indicate any failures by raising an exception. This does not have + to be pretty, it's mainly for picking up regressions during + development, rather than use in the field. + + :return: None, or an advisory string for developer interest, such + as a json dump of some state. """ pass diff --git a/src/pybind/mgr/prometheus/module.py b/src/pybind/mgr/prometheus/module.py index ed7a337dcdd..f2c8d738f6c 100644 --- a/src/pybind/mgr/prometheus/module.py +++ b/src/pybind/mgr/prometheus/module.py @@ -145,11 +145,6 @@ class Metric(object): class Module(MgrModule): COMMANDS = [ - { - "cmd": "prometheus self-test", - "desc": "Run a self test on the prometheus module", - "perm": "rw" - }, { "cmd": "prometheus file_sd_config", "desc": "Return file_sd compatible prometheus config for mgr cluster", @@ -596,12 +591,12 @@ class Module(MgrModule): ] return 0, json.dumps(ret), "" + def self_test(self): + self.collect() + self.get_file_sd_config() + def handle_command(self, inbuf, cmd): - if cmd['prefix'] == 'prometheus self-test': - self.collect() - self.get_file_sd_config() - return 0, '', 'Self-test OK' - elif cmd['prefix'] == 'prometheus file_sd_config': + if cmd['prefix'] == 'prometheus file_sd_config': return self.get_file_sd_config() else: return (-errno.EINVAL, '', diff --git a/src/pybind/mgr/selftest/module.py b/src/pybind/mgr/selftest/module.py index 68edd5b30d4..1b519a368d9 100644 --- a/src/pybind/mgr/selftest/module.py +++ b/src/pybind/mgr/selftest/module.py @@ -65,6 +65,11 @@ class Module(MgrModule): "desc": "Test inter-module calls", "perm": "r" }, + { + "cmd": "mgr self-test module name=module,type=CephString", + "desc": "Run another module's self_test() method", + "perm": "r" + }, ] def __init__(self, *args, **kwargs): @@ -101,6 +106,13 @@ class Module(MgrModule): elif command['prefix'] == 'mgr self-test remote': self._test_remote_calls() return 0, '', 'Successfully called' + elif command['prefix'] == 'mgr self-test module': + try: + r = self.remote(command['module'], "self_test") + except RuntimeError as e: + return -1, '', "Test failed: {0}".format(e.message) + else: + return 0, str(r), "Self-test OK" else: return (-errno.EINVAL, '', "Command not found '{0}'".format(command['prefix'])) diff --git a/src/pybind/mgr/telegraf/module.py b/src/pybind/mgr/telegraf/module.py index cfe87781c89..06b71ed942f 100644 --- a/src/pybind/mgr/telegraf/module.py +++ b/src/pybind/mgr/telegraf/module.py @@ -34,11 +34,6 @@ class Module(MgrModule): "desc": "Force sending data to Telegraf", "perm": "rw" }, - { - "cmd": "telegraf self-test", - "desc": "debug the module", - "perm": "rw" - }, ] OPTIONS = [ @@ -275,9 +270,6 @@ class Module(MgrModule): elif cmd['prefix'] == 'telegraf send': self.send_to_telegraf() return 0, 'Sending data to Telegraf', '' - if cmd['prefix'] == 'telegraf self-test': - self.self_test() - return 0, '', 'Self-test OK' return (-errno.EINVAL, '', "Command not found '{0}'".format(cmd['prefix'])) diff --git a/src/pybind/mgr/telemetry/module.py b/src/pybind/mgr/telemetry/module.py index c3e24f150e0..466226fbe60 100644 --- a/src/pybind/mgr/telemetry/module.py +++ b/src/pybind/mgr/telemetry/module.py @@ -88,11 +88,6 @@ class Module(MgrModule): "desc": "Show last report or report to be sent", "perm": "r" }, - { - "cmd": "telemetry self-test", - "desc": "Perform a self-test", - "perm": "r" - } ] @property @@ -309,9 +304,6 @@ class Module(MgrModule): if not report: report = self.compile_report() return 0, json.dumps(report), '' - elif command['prefix'] == 'telemetry self-test': - self.self_test() - return 0, 'Self-test succeeded', '' else: return (-errno.EINVAL, '', "Command not found '{0}'".format(command['prefix'])) diff --git a/src/pybind/mgr/zabbix/module.py b/src/pybind/mgr/zabbix/module.py index a9162656732..27f7a5c3296 100644 --- a/src/pybind/mgr/zabbix/module.py +++ b/src/pybind/mgr/zabbix/module.py @@ -97,11 +97,6 @@ class Module(MgrModule): "desc": "Force sending data to Zabbix", "perm": "rw" }, - { - "cmd": "zabbix self-test", - "desc": "Run a self-test on the Zabbix module", - "perm": "r" - } ] def __init__(self, *args, **kwargs): @@ -320,9 +315,6 @@ class Module(MgrModule): return 0, 'Sending data to Zabbix', '' return 1, 'Failed to send data to Zabbix', '' - elif command['prefix'] == 'zabbix self-test': - self.self_test() - return 0, 'Self-test succeeded', '' else: return (-errno.EINVAL, '', "Command not found '{0}'".format(command['prefix'])) -- 2.39.5