[report] add --cmd-timeout option

Add --cmd-timeout option to configure command timeout. Plugin-specific
option of the same name (i.e. -k logs.cmd-timeout=60) can control the
timeout per plugin.

Option defaults and global/plugin-specific option preference follows the
--plugin-timeout rules.

Resolves: #2466

Signed-off-by: Pavel Moravec <pmoravec@redhat.com>
Signed-off-by: Jake Hunsaker <jhunsake@redhat.com>

6 files changed, 76 insertions, 21 deletions

diff --git a/man/en/sos-report.1 b/man/en/sos-report.1
index 81005959..51cf3436 100644
--- a/man/en/sos-report.1
+++ b/man/en/sos-report.1
@@ -17,6 +17,7 @@ sosreport \- Collect and package diagnostic and support data
           [--label label] [--case-id id]\fR
           [--threads threads]\fR
           [--plugin-timeout TIMEOUT]\fR
+          [--cmd-timeout TIMEOUT]\fR
           [-s|--sysroot SYSROOT]\fR
           [-c|--chroot {auto|always|never}\fR
           [--tmp-dir directory]\fR
@@ -247,7 +248,7 @@ Specify a timeout in seconds to allow each plugin to run for. A value of 0
 means no timeout will be set. A value of -1 is used to indicate the default
 timeout of 300 seconds.
 
-Note that this options sets the timeout for all plugins. If you want to set
+Note that this option 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'.
 
@@ -255,6 +256,21 @@ 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 \--cmd-timeout TIMEOUT
+Specify a timeout limit in seconds for a command execution. Same defaults logic
+from --plugin-timeout applies here.
+
+This option sets the command timeout for all plugins. If you want to set a cmd
+timeout for a specific plugin, use the 'cmd-timeout' plugin option available to
+all plugins - e.g. '-k logs.cmd-timeout=600'.
+
+Again, the same plugin/global precedence logic as for --plugin-timeout applies
+here.
+
+Note that setting --cmd-timeout (or -k logs.cmd-timeout) high should be followed
+by increasing the --plugin-timeout equivalent, otherwise the plugin can easily
+timeout on slow commands execution.
+.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/collector/__init__.py b/sos/collector/__init__.py
index 406c8f35..1ae73508 100644
--- a/sos/collector/__init__.py
+++ b/sos/collector/__init__.py
@@ -82,6 +82,7 @@ class SoSCollector(SoSComponent):
         'password_per_node': False,
         'plugin_options': [],
         'plugin_timeout': None,
+        'cmd_timeout': None,
         'preset': '',
         'save_group': '',
         'since': '',
@@ -276,6 +277,8 @@ class SoSCollector(SoSComponent):
                              help='Do not collect env vars in sosreports')
         sos_grp.add_argument('--plugin-timeout', type=int, default=None,
                              help='Set the global plugin timeout value')
+        sos_grp.add_argument('--cmd-timeout', type=int, default=None,
+                             help='Set the global command timeout value')
         sos_grp.add_argument('--since', default=None,
                              help=('Escapes archived files older than date. '
                                    'This will also affect --all-logs. '
diff --git a/sos/collector/sosnode.py b/sos/collector/sosnode.py
index a1679655..dbbee12e 100644
--- a/sos/collector/sosnode.py
+++ b/sos/collector/sosnode.py
@@ -664,6 +664,11 @@ class SosNode():
                     '--skip-files=%s' % (quote(self.opts.skip_files))
                 )
 
+        if self.check_sos_version('4.2'):
+            if self.opts.cmd_timeout:
+                sos_opts.append('--cmd-timeout=%s'
+                                % quote(str(self.opts.cmd_timeout)))
+
         sos_cmd = sos_cmd.replace(
             'sosreport',
             os.path.join(self.host.sos_bin_path, self.sos_bin)
diff --git a/sos/options.py b/sos/options.py
index b82a7d36..1eda55d6 100644
--- a/sos/options.py
+++ b/sos/options.py
@@ -283,7 +283,8 @@ class SoSOptions():
             if name in ("add_preset", "del_preset", "desc", "note"):
                 return False
             # Exception list for options that still need to be reported when 0
-            if name in ['log_size', 'plugin_timeout'] and value == 0:
+            if name in ['log_size', 'plugin_timeout', 'cmd_timeout'] \
+               and value == 0:
                 return True
             return has_value(name, value)
 
diff --git a/sos/report/__init__.py b/sos/report/__init__.py
index 25478ba7..945d0fc1 100644
--- a/sos/report/__init__.py
+++ b/sos/report/__init__.py
@@ -107,6 +107,7 @@ class SoSReport(SoSComponent):
         'only_plugins': [],
         'preset': 'auto',
         'plugin_timeout': 300,
+        'cmd_timeout': 300,
         'profiles': [],
         'since': None,
         'verify': False,
@@ -266,6 +267,8 @@ class SoSReport(SoSComponent):
                                 help="A preset identifier", default="auto")
         report_grp.add_argument("--plugin-timeout", default=None,
                                 help="set a timeout for all plugins")
+        report_grp.add_argument("--cmd-timeout", default=None,
+                                help="set a command timeout for all plugins")
         report_grp.add_argument("-p", "--profile", "--profiles",
                                 action="extend", dest="profiles", type=str,
                                 default=[],
@@ -709,7 +712,7 @@ class SoSReport(SoSComponent):
 
             self.ui_log.info(_("The following plugin options are available:"))
             for (plug, plugname, optname, optparm) in self.all_options:
-                if optname in ('timeout', 'postproc'):
+                if optname in ('timeout', 'postproc', 'cmd-timeout'):
                     continue
                 # format option value based on its type (int or bool)
                 if type(optparm["enabled"]) == bool:
diff --git a/sos/report/plugins/__init__.py b/sos/report/plugins/__init__.py
index 02625eb1..779119af 100644
--- a/sos/report/plugins/__init__.py
+++ b/sos/report/plugins/__init__.py
@@ -472,6 +472,9 @@ class Plugin(object):
     _default_plug_opts = [
         ('timeout', 'Timeout in seconds for plugin. The default value (-1) ' +
             'defers to the general plugin timeout, 300 seconds', 'fast', -1),
+        ('cmd-timeout', 'Timeout in seconds for a command execution. The ' +
+            'default value (-1) defers to the general cmd timeout, 300 ' +
+            'seconds', 'fast', -1),
         ('postproc', 'Enable post-processing collected plugin data', 'fast',
          True)
     ]
@@ -532,16 +535,15 @@ class Plugin(object):
         self.manifest.add_list('commands', [])
         self.manifest.add_list('files', [])
 
-    @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.
+    def timeout_from_options(self, optname, plugoptname, default_timeout):
+        """Returns either the default [plugin|cmd] timeout value, the value as
+        provided on the commandline via -k plugin.[|cmd-]timeout=value, or the
+        value of the global --[plugin|cmd]-timeout option.
         """
         _timeout = None
         try:
-            opt_timeout = self.get_option('plugin_timeout')
-            own_timeout = int(self.get_option('timeout'))
+            opt_timeout = self.get_option(optname)
+            own_timeout = int(self.get_option(plugoptname))
             if opt_timeout is None:
                 _timeout = own_timeout
             elif opt_timeout is not None and own_timeout == -1:
@@ -551,10 +553,30 @@ class Plugin(object):
             else:
                 return None
         except ValueError:
-            return self.plugin_timeout  # Default to known safe value
+            return default_timeout  # Default to known safe value
         if _timeout is not None and _timeout > -1:
             return _timeout
-        return self.plugin_timeout
+        return default_timeout
+
+    @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 = self.timeout_from_options('plugin_timeout', 'timeout',
+                                             self.plugin_timeout)
+        return _timeout
+
+    @property
+    def cmdtimeout(self):
+        """Returns either the default command timeout value, the value as
+        provided on the commandline via -k plugin.cmd-timeout=value, or the
+        value of the global --cmd-timeout option.
+        """
+        _cmdtimeout = self.timeout_from_options('cmd_timeout', 'cmd-timeout',
+                                                self.cmd_timeout)
+        return _cmdtimeout
 
     def set_timeout_hit(self):
         self._timeout_hit = True
@@ -1235,8 +1257,8 @@ class Plugin(object):
         """
 
         global_options = (
-            'all_logs', 'allow_system_changes', 'log_size', 'plugin_timeout',
-            'since', 'verify'
+            'all_logs', 'allow_system_changes', 'cmd_timeout', 'log_size',
+            'plugin_timeout', 'since', 'verify'
         )
 
         if optionname in global_options:
@@ -1505,7 +1527,7 @@ class Plugin(object):
                     'tags': _spec_tags
                 })
 
-    def add_blockdev_cmd(self, cmds, devices='block', timeout=300,
+    def add_blockdev_cmd(self, cmds, devices='block', timeout=None,
                          sizelimit=None, chroot=True, runat=None, env=None,
                          binary=False, prepend_path=None, whitelist=[],
                          blacklist=[], tags=[]):
@@ -1569,7 +1591,7 @@ class Plugin(object):
                              whitelist=whitelist, blacklist=blacklist,
                              tags=_dev_tags)
 
-    def _add_device_cmd(self, cmds, devices, timeout=300, sizelimit=None,
+    def _add_device_cmd(self, cmds, devices, timeout=None, sizelimit=None,
                         chroot=True, runat=None, env=None, binary=False,
                         prepend_path=None, whitelist=[], blacklist=[],
                         tags=[]):
@@ -1627,7 +1649,7 @@ class Plugin(object):
                                  changes=soscmd.changes)
 
     def add_cmd_output(self, cmds, suggest_filename=None,
-                       root_symlink=None, timeout=cmd_timeout, stderr=True,
+                       root_symlink=None, timeout=None, stderr=True,
                        chroot=True, runat=None, env=None, binary=False,
                        sizelimit=None, pred=None, subdir=None,
                        changes=False, foreground=False, tags=[]):
@@ -1849,7 +1871,7 @@ class Plugin(object):
         self._log_debug("added string ...'%s' as '%s'" % (summary, filename))
 
     def _collect_cmd_output(self, cmd, suggest_filename=None,
-                            root_symlink=False, timeout=cmd_timeout,
+                            root_symlink=False, timeout=None,
                             stderr=True, chroot=True, runat=None, env=None,
                             binary=False, sizelimit=None, subdir=None,
                             changes=False, foreground=False, tags=[]):
@@ -1883,6 +1905,8 @@ class Plugin(object):
         if self._timeout_hit:
             return
 
+        if timeout is None:
+            timeout = self.cmdtimeout
         _tags = []
 
         if isinstance(tags, str):
@@ -1975,7 +1999,7 @@ class Plugin(object):
         return result
 
     def collect_cmd_output(self, cmd, suggest_filename=None,
-                           root_symlink=False, timeout=cmd_timeout,
+                           root_symlink=False, timeout=None,
                            stderr=True, chroot=True, runat=None, env=None,
                            binary=False, sizelimit=None, pred=None,
                            subdir=None, tags=[]):
@@ -2044,7 +2068,7 @@ class Plugin(object):
             tags=tags
         )
 
-    def exec_cmd(self, cmd, timeout=cmd_timeout, stderr=True, chroot=True,
+    def exec_cmd(self, cmd, timeout=None, stderr=True, chroot=True,
                  runat=None, env=None, binary=False, pred=None,
                  foreground=False, container=False, quotecmd=False):
         """Execute a command right now and return the output and status, but
@@ -2095,6 +2119,9 @@ class Plugin(object):
         if not self.test_predicate(cmd=True, pred=pred):
             return _default
 
+        if timeout is None:
+            timeout = self.cmdtimeout
+
         if chroot or self.commons['cmdlineopts'].chroot == 'always':
             root = self.sysroot
         else:
@@ -2331,7 +2358,7 @@ class Plugin(object):
 
     def add_journal(self, units=None, boot=None, since=None, until=None,
                     lines=None, allfields=False, output=None,
-                    timeout=cmd_timeout, identifier=None, catalog=None,
+                    timeout=None, identifier=None, catalog=None,
                     sizelimit=None, pred=None, tags=[]):
         """Collect journald logs from one of more units.