From: John Spray <john.spray@redhat.com>
Date: Tue, 10 Jul 2018 16:40:49 +0000 (-0400)
Subject: mgr: generic self test command
X-Git-Tag: v14.0.1~798^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=d24e6cb32acebf6f3fe6cb96d60ae323437d99fd;p=ceph.git

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 5248fa41a2bc3..fa609f7348c28 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 7b5b754421345..a9f3222a03e99 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 4c2ce2df0f2e6..c4a9a356e3629 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 e077b04ffc4d9..14c6af3ff5b50 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 9133fafae2e70..f5c3179ded1cb 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 501938e29d16b..4bae68be5d74b 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 ed7a337dcdda5..f2c8d738f6cac 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 68edd5b30d45e..1b519a368d9d8 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 cfe87781c8997..06b71ed942fbe 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 c3e24f150e047..466226fbe60a3 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 a9162656732f6..27f7a5c3296b8 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']))