diff options
| -rw-r--r-- | sos/report/__init__.py | 61 | ||||
| -rw-r--r-- | sos/report/plugins/__init__.py | 157 | ||||
| -rw-r--r-- | tests/unittests/conformance_tests.py | 5 | ||||
| -rw-r--r-- | tests/unittests/option_tests.py | 20 | ||||
| -rw-r--r-- | tests/unittests/plugin_tests.py | 37 |
5 files changed, 163 insertions, 117 deletions
diff --git a/sos/report/__init__.py b/sos/report/__init__.py index 16213c0a..b0159e5b 100644 --- a/sos/report/__init__.py +++ b/sos/report/__init__.py @@ -619,21 +619,24 @@ class SoSReport(SoSComponent): def _set_all_options(self): if self.opts.alloptions: for plugname, plug in self.loaded_plugins: - for name, parms in zip(plug.opt_names, plug.opt_parms): - if type(parms["enabled"]) == bool: - parms["enabled"] = True + for opt in plug.options: + if bool in opt.val_type: + opt.value = True def _set_tunables(self): if self.opts.plugopts: opts = {} for opt in self.opts.plugopts: - # split up "general.syslogsize=5" try: opt, val = opt.split("=") except ValueError: val = True - else: - if val.lower() in ["off", "disable", "disabled", "false"]: + + if isinstance(val, str): + val = val.lower() + if val in ["on", "enable", "enabled", "true", "yes"]: + val = True + elif val in ["off", "disable", "disabled", "false", "no"]: val = False else: # try to convert string "val" to int() @@ -642,7 +645,6 @@ class SoSReport(SoSComponent): except ValueError: pass - # split up "general.syslogsize" try: plug, opt = opt.split(".") except ValueError: @@ -652,16 +654,25 @@ class SoSReport(SoSComponent): try: opts[plug] except KeyError: - opts[plug] = [] - opts[plug].append((opt, val)) + opts[plug] = {} + opts[plug][opt] = val for plugname, plug in self.loaded_plugins: if plugname in opts: - for opt, val in opts[plugname]: - if not plug.set_option(opt, val): + for opt in opts[plugname]: + if opt not in plug.options: self.soslog.error('no such option "%s" for plugin ' '(%s)' % (opt, plugname)) self._exit(1) + try: + plug.options[opt].set_value(opts[plugname][opt]) + self.soslog.debug( + "Set %s plugin option to %s" + % (plugname, plug.options[opt]) + ) + except Exception as err: + self.soslog.error(err) + self._exit(1) del opts[plugname] for plugname in opts.keys(): self.soslog.error('WARNING: unable to set option for disabled ' @@ -688,10 +699,8 @@ class SoSReport(SoSComponent): def _set_plugin_options(self): for plugin_name, plugin in self.loaded_plugins: - names, parms = plugin.get_all_options() - for optname, optparm in zip(names, parms): - self.all_options.append((plugin, plugin_name, optname, - optparm)) + for opt in plugin.options: + self.all_options.append(plugin.options[opt]) def _report_profiles_and_plugins(self): self.ui_log.info("") @@ -732,31 +741,33 @@ class SoSReport(SoSComponent): if self.all_options: self.ui_log.info(_("The following options are available for ALL " "plugins:")) - for opt in self.all_options[0][0]._default_plug_opts: - val = opt[3] - if val == -1: + _defaults = self.loaded_plugins[0][1]._default_plug_opts + for _opt in _defaults: + opt = _defaults[_opt] + val = opt.default + if opt.default == -1: val = TIMEOUT_DEFAULT - self.ui_log.info(" %-25s %-15s %s" % (opt[0], val, opt[1])) + self.ui_log.info(" %-25s %-15s %s" % (opt.name, val, opt.desc)) self.ui_log.info("") self.ui_log.info(_("The following plugin options are available:")) - for (plug, plugname, optname, optparm) in self.all_options: - if optname in ('timeout', 'postproc', 'cmd-timeout'): + for opt in self.all_options: + if opt.name in ('timeout', 'postproc', 'cmd-timeout'): continue # format option value based on its type (int or bool) - if type(optparm["enabled"]) == bool: - if optparm["enabled"] is True: + if isinstance(opt.default, bool): + if opt.default is True: tmpopt = "on" else: tmpopt = "off" else: - tmpopt = optparm["enabled"] + tmpopt = opt.default if tmpopt is None: tmpopt = 0 self.ui_log.info(" %-25s %-15s %s" % ( - plugname + "." + optname, tmpopt, optparm["desc"])) + opt.plugin + "." + opt.name, tmpopt, opt.desc)) else: self.ui_log.info(_("No plugin options available.")) diff --git a/sos/report/plugins/__init__.py b/sos/report/plugins/__init__.py index 549a70ef..6c32c665 100644 --- a/sos/report/plugins/__init__.py +++ b/sos/report/plugins/__init__.py @@ -406,7 +406,86 @@ class SoSCommand(object): sorted(self.__dict__.items())) -class Plugin(object): +class PluginOpt(): + """This is used to define options available to plugins. Plugins will need + to define options alongside their distro-specific classes in order to add + support for user-controlled changes in Plugin behavior. + + :param name: The name of the plugin option + :type name: ``str`` + + :param default: The default value of the option + :type default: Any + + :param desc: A short description of the effect of the option + :type desc: ``str`` + + :param long_desc: A detailed description of the option. Will be used by + `sos info` + :type long_desc: ``str`` + + :param val_type: The type of object the option accepts for values. If + not provided, auto-detect from the type of ``default`` + :type val_type: A single type or a ``list`` of types + """ + + name = '' + default = None + enabled = False + desc = '' + long_desc = '' + value = None + val_type = [None] + plugin = '' + + def __init__(self, name='undefined', default=None, desc='', long_desc='', + val_type=None): + self.name = name + self.default = default + self.desc = desc + self.long_desc = long_desc + self.value = self.default + if val_type is not None: + if not isinstance(val_type, list): + val_type = [val_type] + else: + val_type = [default.__class__] + self.val_type = val_type + + def __str__(self): + items = [ + 'name=%s' % self.name, + 'desc=\'%s\'' % self.desc, + 'value=%s' % self.value, + 'default=%s' % self.default + ] + return '(' + ', '.join(items) + ')' + + def __repr__(self): + return self.__str__() + + def set_value(self, val): + if not any([type(val) == _t for _t in self.val_type]): + valid = [] + for t in self.val_type: + if t is None: + continue + if t.__name__ == 'bool': + valid.append("boolean true/false (on/off, etc)") + elif t.__name__ == 'str': + valid.append("string (no spaces)") + elif t.__name__ == 'int': + valid.append("integer values") + raise Exception( + "Plugin option '%s.%s' takes %s, not %s" % ( + self.plugin, self.name, ', '.join(valid), + type(val).__name__ + ) + ) + self.value = val + + +class Plugin(): """This is the base class for sosreport plugins. Plugins should subclass this and set the class variables where applicable. @@ -474,12 +553,14 @@ class Plugin(object): # Default predicates predicate = None cmd_predicate = None - _default_plug_opts = [ - ('timeout', 'Timeout in seconds for plugin to finish', 'fast', -1), - ('cmd-timeout', 'Timeout in seconds for a command', 'fast', -1), - ('postproc', 'Enable post-processing collected plugin data', 'fast', - True) - ] + _default_plug_opts = { + 'timeout': PluginOpt('timeout', default=-1, val_type=int, + desc='Timeout in seconds for plugin to finish'), + 'cmd-timeout': PluginOpt('cmd-timeout', default=-1, val_type=int, + desc='Timeout in seconds for cmds to finish'), + 'postproc': PluginOpt('postproc', default=True, val_type=bool, + desc='Enable post-processing of collected data') + } def __init__(self, commons): @@ -488,13 +569,12 @@ class Plugin(object): self._env_vars = set() self.alerts = [] self.custom_text = "" - self.opt_names = [] - self.opt_parms = [] self.commons = commons self.forbidden_paths = [] self.copy_paths = set() self.copy_strings = [] self.collect_cmds = [] + self.options = {} self.sysroot = commons['sysroot'] self.policy = commons['policy'] self.devices = commons['devices'] @@ -506,13 +586,12 @@ class Plugin(object): else logging.getLogger('sos') # add the default plugin opts - self.option_list.extend(self._default_plug_opts) - - # get the option list into a dictionary + self.options.update(self._default_plug_opts) + for popt in self.options: + self.options[popt].plugin = self.name() for opt in self.option_list: - self.opt_names.append(opt[0]) - self.opt_parms.append({'desc': opt[1], 'speed': opt[2], - 'enabled': opt[3]}) + opt.plugin = self.name() + self.options[opt.name] = opt # Initialise the default --dry-run predicate self.set_predicate(SoSPredicate(self)) @@ -1211,10 +1290,6 @@ class Plugin(object): for path in glob.glob(forbid, recursive=recursive): self.forbidden_paths.append(path) - def get_all_options(self): - """return a list of all options selected""" - return (self.opt_names, self.opt_parms) - def set_option(self, optionname, value): """Set the named option to value. Ensure the original type of the option value is preserved @@ -1227,18 +1302,13 @@ class Plugin(object): :returns: ``True`` if the option is successfully set, else ``False`` :rtype: ``bool`` """ - for name, parms in zip(self.opt_names, self.opt_parms): - if name == optionname: - # FIXME: ensure that the resulting type of the set option - # matches that of the default value. This prevents a string - # option from being coerced to int simply because it holds - # a numeric value (e.g. a password). - # See PR #1526 and Issue #1597 - defaulttype = type(parms['enabled']) - if defaulttype != type(value) and defaulttype != type(None): - value = (defaulttype)(value) - parms['enabled'] = value + if optionname in self.options: + try: + self.options[optionname].set_value(value) return True + except Exception as err: + self._log_error(err) + raise return False def get_option(self, optionname, default=0): @@ -1266,29 +1336,12 @@ class Plugin(object): if optionname in global_options: return getattr(self.commons['cmdlineopts'], optionname) - for name, parms in zip(self.opt_names, self.opt_parms): - if name == optionname: - val = parms['enabled'] - if val is not None: - return val - else: - # if the value is `None`, use any non-zero default here, - # but still return `None` if no default is given since - # optionname did exist and had a `None` value - return default or val - - return default - - def get_option_as_list(self, optionname, delimiter=",", default=None): - """Will try to return the option as a list separated by the - delimiter. - """ - option = self.get_option(optionname) - try: - opt_list = [opt.strip() for opt in option.split(delimiter)] - return list(filter(None, opt_list)) - except Exception: + if optionname in self.options: + opt = self.options[optionname] + if not default or opt.value is not None: + return opt.value return default + return default def _add_copy_paths(self, copy_paths): self.copy_paths.update(copy_paths) diff --git a/tests/unittests/conformance_tests.py b/tests/unittests/conformance_tests.py index 3a3f550f..5419f9d3 100644 --- a/tests/unittests/conformance_tests.py +++ b/tests/unittests/conformance_tests.py @@ -8,7 +8,7 @@ import unittest -from sos.report.plugins import import_plugin +from sos.report.plugins import import_plugin, PluginOpt from sos.utilities import ImporterHelper @@ -50,7 +50,8 @@ class PluginConformance(unittest.TestCase): for plug in self.plug_classes: self.assertIsInstance(plug.option_list, list) for opt in plug.option_list: - self.assertIsInstance(opt, tuple) + self.assertIsInstance(opt, PluginOpt) + self.assertFalse(opt.name == 'undefined') def test_plugin_architectures_set_correctly(self): for plug in self.plug_classes: diff --git a/tests/unittests/option_tests.py b/tests/unittests/option_tests.py index 58f54e94..4a9271ad 100644 --- a/tests/unittests/option_tests.py +++ b/tests/unittests/option_tests.py @@ -7,7 +7,7 @@ # See the LICENSE file in the source distribution for further information. import unittest -from sos.report.plugins import Plugin +from sos.report.plugins import Plugin, PluginOpt from sos.policies.distros import LinuxPolicy from sos.policies.init_systems import InitSystem @@ -21,6 +21,18 @@ class MockOptions(object): skip_files = [] +class MockPlugin(Plugin): + + option_list = [ + PluginOpt('baz', default=False), + PluginOpt('empty', default=None), + PluginOpt('test_option', default='foobar') + ] + + def __init__(self, commons): + super(MockPlugin, self).__init__(commons=commons) + + class GlobalOptionTest(unittest.TestCase): def setUp(self): @@ -30,11 +42,7 @@ class GlobalOptionTest(unittest.TestCase): 'cmdlineopts': MockOptions(), 'devices': {} } - self.plugin = Plugin(self.commons) - self.plugin.opt_names = ['baz', 'empty', 'test_option'] - self.plugin.opt_parms = [ - {'enabled': False}, {'enabled': None}, {'enabled': 'foobar'} - ] + self.plugin = MockPlugin(self.commons) def test_simple_lookup(self): self.assertEquals(self.plugin.get_option('test_option'), 'foobar') diff --git a/tests/unittests/plugin_tests.py b/tests/unittests/plugin_tests.py index 8dc038cb..fcd24143 100644 --- a/tests/unittests/plugin_tests.py +++ b/tests/unittests/plugin_tests.py @@ -12,7 +12,7 @@ import shutil from io import StringIO -from sos.report.plugins import Plugin, regex_findall, _mangle_command +from sos.report.plugins import Plugin, regex_findall, _mangle_command, PluginOpt from sos.archive import TarFileArchive from sos.policies.distros import LinuxPolicy from sos.policies.init_systems import InitSystem @@ -64,8 +64,10 @@ class MockArchive(TarFileArchive): class MockPlugin(Plugin): - option_list = [("opt", 'an option', 'fast', None), - ("opt2", 'another option', 'fast', False)] + option_list = [ + PluginOpt("opt", default=None, desc='an option', val_type=str), + PluginOpt("opt2", default=False, desc='another option') + ] def setup(self): pass @@ -271,35 +273,6 @@ class PluginTests(unittest.TestCase): }) self.assertEquals(p.get_option("opt2", True), False) - def test_get_option_as_list_plugin_option(self): - p = MockPlugin({ - 'sysroot': self.sysroot, - 'policy': LinuxPolicy(init=InitSystem(), probe_runtime=False), - 'cmdlineopts': MockOptions(), - 'devices': {} - }) - p.set_option("opt", "one,two,three") - self.assertEquals(p.get_option_as_list("opt"), ['one', 'two', 'three']) - - def test_get_option_as_list_plugin_option_default(self): - p = MockPlugin({ - 'sysroot': self.sysroot, - 'policy': LinuxPolicy(init=InitSystem(), probe_runtime=False), - 'cmdlineopts': MockOptions(), - 'devices': {} - }) - self.assertEquals(p.get_option_as_list("opt", default=[]), []) - - def test_get_option_as_list_plugin_option_not_list(self): - p = MockPlugin({ - 'sysroot': self.sysroot, - 'policy': LinuxPolicy(init=InitSystem(), probe_runtime=False), - 'cmdlineopts': MockOptions(), - 'devices': {} - }) - p.set_option("opt", "testing") - self.assertEquals(p.get_option_as_list("opt"), ['testing']) - def test_copy_dir(self): self.mp._do_copy_path("tests") self.assertEquals( |