From: Zack Cerza <zack@redhat.com>
Date: Tue, 18 Oct 2016 19:25:33 +0000 (-0600)
Subject: Use psutil.Popen instead of subprocess.Popen
X-Git-Tag: 1.1.0~514^2~1
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=c37fe08e398c1be6d8fcecaa57c5a73c4bd87342;p=teuthology.git

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 cd7a47171e..ce3f63c9af 100644
--- 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 f308e5a837..c038ab354a 100644
--- 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)