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 <john.spray@redhat.com>

diff --git a/qa/tasks/mgr/test_module_selftest.py b/qa/tasks/mgr/test_module_selftest.py
index 5248fa41a2bc3989ae830ea8f37c80292ece63fe..fa609f7348c281b4fcc9249a74bafeba80492bf2 100644 (file)
--- 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 7b5b7544213452f463905496e0417ba5adaa004a..a9f3222a03e996287417ef72c706ab687e8b1442 100644 (file)
--- 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 4c2ce2df0f2e6c20a1f9582652c2a1b9d61a9796..c4a9a356e362900fc4ce265093c0f91cedcb13dd 100644 (file)
--- 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 e077b04ffc4d966356c50b4bab55b73eab4fd760..14c6af3ff5b50d0fbceb5ff21fac245becc5e190 100644 (file)
--- 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 9133fafae2e706f2facab148ef2a77fe7e54068a..f5c3179ded1cbd8463e7e96d60f39c5b0efe77a1 100644 (file)
--- 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 501938e29d16bf6d1e1ebdded38d3164a9534534..4bae68be5d74b7053781439c711ab15cd481d67e 100644 (file)
--- 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 ed7a337dcdda515abe5703cded5819ee48c89bdd..f2c8d738f6cac669b56e92cc3dd015580b3fdf64 100644 (file)
--- 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 68edd5b30d45eeae87e305dd406af9f3d9a597d6..1b519a368d9d82d77ddaf36b7a5d616bcf75884a 100644 (file)
--- 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 cfe87781c89973a03f87e6bbd32e421314365f37..06b71ed942fbe731e6c8b689a88b7a2e885b8345 100644 (file)
--- 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 c3e24f150e04754ad9d3038cb7b6f2c384116659..466226fbe60a3ea91ea007959d09afcae22c0344 100644 (file)
--- 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 a9162656732f649c50a676a0b013ee82566c1bce..27f7a5c3296b8dd15345921124850a9ef59ad3e9 100644 (file)
--- 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']))