Use psutil.Popen instead of subprocess.Popen

psutil.Popen is identical other than offering nice features like making
sure a PID hasn't been reused before it sends signals to it.

Signed-off-by: Zack Cerza <zack@redhat.com>

diff --git a/teuthology/orchestra/console.py b/teuthology/orchestra/console.py
index cd7a47171e7ac28cf1e3f3800635977ecce445df..ce3f63c9aff6f6d5478c1c964900ee8051386752 100644 (file)
--- a/teuthology/orchestra/console.py
+++ b/teuthology/orchestra/console.py
@@ -1,6 +1,7 @@
 import logging
 import os
 import pexpect
+import psutil
 import subprocess
 import time
 
@@ -37,7 +38,7 @@ class PhysicalConsole():
         )
         self.conserver_master = config.conserver_master
         self.conserver_port = config.conserver_port
-        conserver_client_found = subprocess.Popen(
+        conserver_client_found = psutil.Popen(
             'which console',
             shell=True,
             stdout=subprocess.PIPE,
@@ -273,7 +274,7 @@ class PhysicalConsole():
         Using the subprocess module, spawn an ipmitool process using 'sol
         activate' and redirect its output to a file.
 
-        :returns: a subprocess.Popen object
+        :returns: a psutil.Popen object
         """
         pexpect_templ = \
             "import pexpect; " \
@@ -288,7 +289,7 @@ class PhysicalConsole():
                     log=dest_path,
                 ),
             ]
-            return subprocess.Popen(
+            return psutil.Popen(
                 python_cmd,
                 env=os.environ,
             )

diff --git a/teuthology/orchestra/test/test_console.py b/teuthology/orchestra/test/test_console.py
index f308e5a83703b52952b70791ef0410fa94df54e3..c038ab354a345bd35afc2abb397152327f730d61 100644 (file)
--- a/teuthology/orchestra/test/test_console.py
+++ b/teuthology/orchestra/test/test_console.py
@@ -88,15 +88,16 @@ class TestPhysicalConsole(TestConsole):
 
     def test_spawn_log_conserver(self):
         with patch(
-            'teuthology.orchestra.console.subprocess.Popen',
+            'teuthology.orchestra.console.psutil.subprocess.Popen',
             autospec=True,
         ) as m_popen:
+            m_popen.return_value.pid = 42
+            m_popen.return_value.returncode = 0
             m_popen.return_value.wait.return_value = 0
             cons = self.klass(self.hostname)
             assert cons.has_conserver is True
             m_popen.reset_mock()
             m_popen.return_value.poll.return_value = None
-            m_popen.poll.return_value = None
             cons.spawn_sol_log('/fake/path')
             assert m_popen.call_count == 1
             call_args = m_popen.call_args_list[0][0][0]
@@ -106,9 +107,11 @@ class TestPhysicalConsole(TestConsole):
 
     def test_spawn_log_ipmi(self):
         with patch(
-            'teuthology.orchestra.console.subprocess.Popen',
+            'teuthology.orchestra.console.psutil.subprocess.Popen',
             autospec=True,
         ) as m_popen:
+            m_popen.return_value.pid = 42
+            m_popen.return_value.returncode = 1
             m_popen.return_value.wait.return_value = 1
             cons = self.klass(self.hostname)
             assert cons.has_conserver is False
@@ -123,9 +126,11 @@ class TestPhysicalConsole(TestConsole):
 
     def test_spawn_log_fallback(self):
         with patch(
-            'teuthology.orchestra.console.subprocess.Popen',
+            'teuthology.orchestra.console.psutil.subprocess.Popen',
             autospec=True,
         ) as m_popen:
+            m_popen.return_value.pid = 42
+            m_popen.return_value.returncode = 0
             m_popen.return_value.wait.return_value = 0
             cons = self.klass(self.hostname)
             assert cons.has_conserver is True
@@ -141,9 +146,11 @@ class TestPhysicalConsole(TestConsole):
 
     def test_get_console_conserver(self):
         with patch(
-            'teuthology.orchestra.console.subprocess.Popen',
+            'teuthology.orchestra.console.psutil.subprocess.Popen',
             autospec=True,
         ) as m_popen:
+            m_popen.return_value.pid = 42
+            m_popen.return_value.returncode = 0
             m_popen.return_value.wait.return_value = 0
             cons = self.klass(self.hostname)
         assert cons.has_conserver is True
@@ -158,9 +165,11 @@ class TestPhysicalConsole(TestConsole):
 
     def test_get_console_ipmitool(self):
         with patch(
-            'teuthology.orchestra.console.subprocess.Popen',
+            'teuthology.orchestra.console.psutil.subprocess.Popen',
             autospec=True,
         ) as m_popen:
+            m_popen.return_value.pid = 42
+            m_popen.return_value.returncode = 0
             m_popen.return_value.wait.return_value = 0
             cons = self.klass(self.hostname)
         assert cons.has_conserver is True
@@ -175,9 +184,11 @@ class TestPhysicalConsole(TestConsole):
 
     def test_get_console_fallback(self):
         with patch(
-            'teuthology.orchestra.console.subprocess.Popen',
+            'teuthology.orchestra.console.psutil.subprocess.Popen',
             autospec=True,
         ) as m_popen:
+            m_popen.return_value.pid = 42
+            m_popen.return_value.returncode = 0
             m_popen.return_value.wait.return_value = 0
             cons = self.klass(self.hostname)
         assert cons.has_conserver is True
@@ -195,9 +206,11 @@ class TestPhysicalConsole(TestConsole):
 
     def test_disable_conserver(self):
         with patch(
-            'teuthology.orchestra.console.subprocess.Popen',
+            'teuthology.orchestra.console.psutil.subprocess.Popen',
             autospec=True,
         ) as m_popen:
+            m_popen.return_value.pid = 42
+            m_popen.return_value.returncode = 0
             m_popen.return_value.wait.return_value = 0
             teuth_config.use_conserver = False
             cons = self.klass(self.hostname)