[sosreport] Allow user-controllable plugin timeouts

Allows users to specify a timeout for each plugin using the '-k
plugin.timeout=value' syntax by adding the 'timeout' option to every
plugin.

Additionally, adds the --plugin-timeout option to set a timeout for
_all_ plugins. If --plugin-timeout and a specific -k timeout option is
provided, the -k timeout option will be applied for those specific
plugins, with --plugin-timeout being applied to all others.

In either case, specifying a timeout of 0 seconds results in no timeout
being applied. In the event that an invalid timeout is set, the timeout
will be set to the default value for the plugin.

Resolves: #1499

Signed-off-by: Jake Hunsaker <jhunsake@redhat.com>
Signed-off-by: Bryn M. Reeves <bmr@redhat.com>

4 files changed, 57 insertions, 6 deletions

diff --git a/man/en/sosreport.1 b/man/en/sosreport.1
index b6051edc..014d01a3 100644
--- a/man/en/sosreport.1
+++ b/man/en/sosreport.1
@@ -13,6 +13,7 @@ sosreport \- Collect and package diagnostic and support data
           [--batch] [--build] [--debug]\fR
           [--label label] [--case-id id] [--ticket-number nr]
           [--threads threads]
+          [--plugin-timeout TIMEOUT]\fR
           [-s|--sysroot SYSROOT]\fR
           [-c|--chroot {auto|always|never}\fR
           [--tmp-dir directory]\fR
@@ -163,6 +164,18 @@ alphanumeric characters.
 .B \--threads THREADS
 Specify the number of threads sosreport will use for concurrency. Defaults to 4.
 .TP
+.B \--plugin-timeout TIMEOUT
+Specify a timeout in seconds to allow each plugin to run for. A value of 0
+means no timeout will be set.
+
+Note that this options sets the timeout for all plugins. If you want to set
+a timeout for a specific plugin, use the 'timeout' plugin option available to
+all plugins - e.g. '-k logs.timeout=600'.
+
+The plugin-specific timeout option will override this option. For example, using
+\'--plugin-timeout=60 -k logs.timeout=600\' will set a timeout of 600 seconds for
+the logs plugin and 60 seconds for all other enabled plugins.
+.TP
 .B \--case-id NUMBER
 Specify a case identifier to associate with the archive.
 Identifiers may include alphanumeric characters, commas and periods ('.').
diff --git a/sos/__init__.py b/sos/__init__.py
index cd9779bd..cc795ab2 100644
--- a/sos/__init__.py
+++ b/sos/__init__.py
@@ -47,8 +47,8 @@ _arg_names = [
     'chroot', 'compression_type', 'config_file', 'desc', 'debug', 'del_preset',
     'enableplugins', 'encrypt_key', 'encrypt_pass', 'experimental', 'label',
     'list_plugins', 'list_presets', 'list_profiles', 'log_size', 'noplugins',
-    'noreport', 'note', 'onlyplugins', 'plugopts', 'preset', 'profiles',
-    'quiet', 'sysroot', 'threads', 'tmp_dir', 'verbosity', 'verify'
+    'noreport', 'note', 'onlyplugins', 'plugin_timeout', 'plugopts', 'preset',
+    'profiles', 'quiet', 'sysroot', 'threads', 'tmp_dir', 'verbosity', 'verify'
 ]
 
 #: Arguments with non-zero default values
@@ -96,6 +96,7 @@ class SoSOptions(object):
     noreport = False
     note = ""
     onlyplugins = []
+    plugin_timeout = None
     plugopts = []
     preset = ""
     profiles = []
diff --git a/sos/plugins/__init__.py b/sos/plugins/__init__.py
index 030e7a30..d8582670 100644
--- a/sos/plugins/__init__.py
+++ b/sos/plugins/__init__.py
@@ -127,7 +127,7 @@ class Plugin(object):
     archive = None
     profiles = ()
     sysroot = '/'
-    timeout = 300
+    plugin_timeout = 300
 
     def __init__(self, commons):
         if not getattr(self, "option_list", False):
@@ -150,12 +150,40 @@ class Plugin(object):
         self.soslog = self.commons['soslog'] if 'soslog' in self.commons \
             else logging.getLogger('sos')
 
+        # add the 'timeout' plugin option automatically
+        self.option_list.append(('timeout', 'timeout in seconds for plugin',
+                                 'fast', -1))
+
         # get the option list into a dictionary
         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]})
 
+    @property
+    def timeout(self):
+        '''Returns either the default plugin timeout value, the value as
+        provided on the commandline via -k plugin.timeout=value, or the value
+        of the global --plugin-timeout option.
+        '''
+        _timeout = None
+        try:
+            opt_timeout = self.get_option('plugin_timeout')
+            own_timeout = int(self.get_option('timeout'))
+            if opt_timeout is None:
+                _timeout = own_timeout
+            elif opt_timeout is not None and own_timeout == -1:
+                _timeout = int(opt_timeout)
+            elif opt_timeout is not None and own_timeout > -1:
+                _timeout = own_timeout
+            else:
+                return None
+        except ValueError:
+            return self.plugin_timeout  # Default to known safe value
+        if _timeout is not None and _timeout > -1:
+            return _timeout
+        return self.plugin_timeout
+
     @classmethod
     def name(cls):
         """Returns the plugin's name as a string. This should return a
@@ -530,7 +558,7 @@ class Plugin(object):
         matches any of the option names is returned.
         """
 
-        global_options = ('verify', 'all_logs', 'log_size')
+        global_options = ('verify', 'all_logs', 'log_size', 'plugin_timeout')
 
         if optionname in global_options:
             return getattr(self.commons['cmdlineopts'], optionname)
diff --git a/sos/sosreport.py b/sos/sosreport.py
index b4a6b84a..a57b4252 100644
--- a/sos/sosreport.py
+++ b/sos/sosreport.py
@@ -280,6 +280,8 @@ def _parse_args(args):
                         help="enable these plugins only", default=[])
     parser.add_argument("--preset", action="store", type=str,
                         help="A preset identifier", default="auto")
+    parser.add_argument("--plugin-timeout", default=None,
+                        help="set a timeout for all plugins")
     parser.add_argument("-p", "--profile", action="extend",
                         dest="profiles", type=str, default=[],
                         help="enable plugins used by the given profiles")
@@ -831,8 +833,12 @@ class SoSReport(object):
 
         if self.all_options:
             self.ui_log.info(_("The following plugin options are available:"))
-            self.ui_log.info("")
+            self.ui_log.info(_("\n Option 'timeout' available to all plugins -"
+                               " time in seconds to allow plugin to run, use 0"
+                               " for no timeout\n"))
             for (plug, plugname, optname, optparm) in self.all_options:
+                if optname == 'timeout':
+                    continue
                 # format option value based on its type (int or bool)
                 if type(optparm["enabled"]) == bool:
                     if optparm["enabled"] is True:
@@ -1089,7 +1095,10 @@ class SoSReport(object):
         with ThreadPoolExecutor(1) as pool:
             try:
                 t = pool.submit(self.collect_plugin, plugin)
-                t.result(timeout=self.loaded_plugins[plugin[0]-1][1].timeout)
+                # Re-type int 0 to NoneType, as otherwise result() will treat
+                # it as a literal 0-second timeout
+                timeout = self.loaded_plugins[plugin[0]-1][1].timeout or None
+                t.result(timeout=timeout)
                 return True
             except TimeoutError:
                 self.ui_log.error("\n Plugin %s timed out\n" % plugin[1])