Merge "Add timeout default to execute command"

os_brick/privileged/rootwrap.py

@@ -130,7 +130,11 @@ def custom_execute(*cmd, **kwargs):
         interval = kwargs.pop('interval', 1)
         backoff_rate = kwargs.pop('backoff_rate', 2)
 
-    timeout = kwargs.pop('timeout', None)
+    # Operations performed by OS-Brick should be relatively quick.  The longest
+    # default timeout is probably the iSCSI ones, with 120 seconds.  Since CLI
+    # tools may get stuck we don't want to leave the timeout to infinite, so we
+    # set the more than reasonable timeout of 10 minutes (600 seconds).
+    timeout = kwargs.pop('timeout', 600)
     sig_end = kwargs.pop('signal', signal.SIGTERM)
     default_raise_timeout = kwargs.get('check_exit_code', True)
     raise_timeout = kwargs.pop('raise_timeout', default_raise_timeout)

os_brick/tests/privileged/test_rootwrap.py

@@ -73,7 +73,15 @@ class PrivRootwrapTestCase(base.TestCase):
                                           run_as_root=False,
                                           keyword_arg=mock.sentinel.kwarg)
 
-    def test_custom_execute(self):
+    @mock.patch('threading.Timer')
+    def test_custom_execute_default_timeout(self, mock_timer):
+        """Confirm timeout defaults to 600 and the thread timer is started."""
+        priv_rootwrap.custom_execute('echo', 'hola')
+        mock_timer.assert_called_once_with(600, mock.ANY, mock.ANY)
+        mock_timer.return_value.start.assert_called_once_with()
+
+    def test_custom_execute_callbacks(self):
+        """Confirm execute callbacks are called on execute."""
         on_execute = mock.Mock()
         on_completion = mock.Mock()
         msg = 'hola'

releasenotes/notes/default-timeout-26c838af8b7af9fc.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Add a 10 minutes default timeout to shell commands executed through
+    subprocess to prevent the possibility of them getting stuck forever.