From 90b6b709e9f4002376b656b155d00d85382f1828 Mon Sep 17 00:00:00 2001

From: Pavel Moravec <pmoravec@redhat.com>

Date: Mon, 29 Mar 2021 16:23:01 +0200

Subject: [PATCH] [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>

---

 man/en/sos-report.1            | 18 +++++++++-

 sos/collector/__init__.py      |  3 ++

 sos/collector/sosnode.py       |  5 +++

 sos/options.py                 |  3 +-

 sos/report/__init__.py         |  5 ++-

 sos/report/plugins/__init__.py | 63 ++++++++++++++++++++++++----------

 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.

-- 

2.26.3