From 36fb964f0c683a0fec478d2c1288f6d675d604f6 Mon Sep 17 00:00:00 2001
From: Davanum Srinivas <davanum@gmail.com>
Date: Wed, 24 Jun 2015 14:24:50 -0400
Subject: [PATCH] Allow preexec_fn method for processutils.execute

If the user specifies preexec_fn, we should call that in
our existing _subprocess_setup. On windows, we silently
drop this preexec_fn as subprocess.Popen raises a ValueError
if we do pass it in.

Change-Id: I0176c66fa2de001aa14f0d928d06fd894de55511
---
 oslo_concurrency/processutils.py              | 20 +++++++++++++++----
 .../tests/unit/test_processutils.py           | 17 ++++++++++++++++
 2 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/oslo_concurrency/processutils.py b/oslo_concurrency/processutils.py
index 01c02db..36ac9b0 100644
--- a/oslo_concurrency/processutils.py
+++ b/oslo_concurrency/processutils.py
@@ -17,6 +17,7 @@
 System-level utilities and helper functions.
 """
 
+import functools
 import logging
 import multiprocessing
 import os
@@ -87,10 +88,12 @@ class NoRootWrapSpecified(Exception):
         super(NoRootWrapSpecified, self).__init__(message)
 
 
-def _subprocess_setup():
+def _subprocess_setup(on_preexec_fn):
     # Python installs a SIGPIPE handler by default. This is usually not what
     # non-Python subprocesses expect.
     signal.signal(signal.SIGPIPE, signal.SIG_DFL)
+    if on_preexec_fn:
+        on_preexec_fn()
 
 
 LOG_ALL_ERRORS = 1
@@ -159,6 +162,13 @@ def execute(*cmd, **kwargs):
                             `processutils.execute` to track process completion
                             asynchronously.
     :type on_completion:    function(:class:`subprocess.Popen`)
+    :param preexec_fn:      This function will be called
+                            in the child process just before the child
+                            is executed. WARNING: On windows, we silently
+                            drop this preexec_fn as it is not supported by
+                            subprocess.Popen on windows (throws a
+                            ValueError)
+    :type preexec_fn:       function()
     :returns:               (stdout, stderr) from process execution
     :raises:                :class:`UnknownArgumentError` on
                             receiving unknown arguments
@@ -181,6 +191,7 @@ def execute(*cmd, **kwargs):
     binary = kwargs.pop('binary', False)
     on_execute = kwargs.pop('on_execute', None)
     on_completion = kwargs.pop('on_completion', None)
+    preexec_fn = kwargs.pop('preexec_fn', None)
 
     if isinstance(check_exit_code, bool):
         ignore_exit_code = not check_exit_code
@@ -220,10 +231,11 @@ def execute(*cmd, **kwargs):
             _PIPE = subprocess.PIPE  # pylint: disable=E1101
 
             if os.name == 'nt':
-                preexec_fn = None
+                on_preexec_fn = None
                 close_fds = False
             else:
-                preexec_fn = _subprocess_setup
+                on_preexec_fn = functools.partial(_subprocess_setup,
+                                                  preexec_fn)
                 close_fds = True
 
             obj = subprocess.Popen(cmd,
@@ -231,7 +243,7 @@ def execute(*cmd, **kwargs):
                                    stdout=_PIPE,
                                    stderr=_PIPE,
                                    close_fds=close_fds,
-                                   preexec_fn=preexec_fn,
+                                   preexec_fn=on_preexec_fn,
                                    shell=shell,
                                    cwd=cwd,
                                    env=env_variables)
diff --git a/oslo_concurrency/tests/unit/test_processutils.py b/oslo_concurrency/tests/unit/test_processutils.py
index 771d966..5ec5a9b 100644
--- a/oslo_concurrency/tests/unit/test_processutils.py
+++ b/oslo_concurrency/tests/unit/test_processutils.py
@@ -97,6 +97,23 @@ class UtilsTest(test_base.BaseTestCase):
         self.assertEqual(1, on_execute_callback.call_count)
         self.assertEqual(1, on_completion_callback.call_count)
 
+    def test_execute_with_preexec_fn(self):
+        # NOTE(dims): preexec_fn is set to a callable object, this object
+        # will be called in the child process just before the child is
+        # executed. So we cannot pass share variables etc, simplest is to
+        # check if a specific exception is thrown which can be caught here.
+        def preexec_fn():
+            raise processutils.InvalidArgumentError()
+
+        processutils.execute("/bin/true")
+
+        expected_exception = (processutils.InvalidArgumentError if six.PY2
+                              else subprocess.SubprocessError)
+        self.assertRaises(expected_exception,
+                          processutils.execute,
+                          "/bin/true",
+                          preexec_fn=preexec_fn)
+
 
 class ProcessExecutionErrorTest(test_base.BaseTestCase):