diff --git a/neutron/agent/linux/async_process.py b/neutron/agent/linux/async_process.py index 25b311dfbd..8a5d928562 100644 --- a/neutron/agent/linux/async_process.py +++ b/neutron/agent/linux/async_process.py @@ -18,7 +18,6 @@ import eventlet import eventlet.event import eventlet.queue import eventlet.timeout -import psutil from neutron.agent.linux import utils from neutron.openstack.common import log as logging @@ -141,12 +140,18 @@ class AsyncProcess(object): # die is to target the child process directly. if self.root_helper: try: - # This assumes that there are not multiple children in any - # level of the process tree under the parent process. - pid = psutil.Process( - self._process.pid).get_children(recursive=True)[-1].pid - except (psutil.NoSuchProcess, IndexError): - pid = None + pid = utils.find_child_pids(pid)[0] + except IndexError: + # Process is already dead + return None + while True: + try: + # We shouldn't have more than one child per process + # so keep getting the children of the first one + pid = utils.find_child_pids(pid)[0] + except IndexError: + # Last process in the tree, return it + break return pid def _kill_process(self, pid): diff --git a/neutron/agent/linux/utils.py b/neutron/agent/linux/utils.py index d6ab603bbe..fdb8d0d4d9 100644 --- a/neutron/agent/linux/utils.py +++ b/neutron/agent/linux/utils.py @@ -108,3 +108,18 @@ def replace_file(file_name, data): tmp_file.close() os.chmod(tmp_file.name, 0o644) os.rename(tmp_file.name, file_name) + + +def find_child_pids(pid): + """Retrieve a list of the pids of child processes of the given pid.""" + + try: + raw_pids = execute(['ps', '--ppid', pid, '-o', 'pid=']) + except RuntimeError as e: + # Exception has already been logged by execute + no_children_found = 'Exit code: 1' in str(e) + if no_children_found: + return [] + # Unexpected errors are the responsibility of the caller + raise + return [x.strip() for x in raw_pids.split('\n') if x.strip()] diff --git a/neutron/tests/unit/agent/linux/test_async_process.py b/neutron/tests/unit/agent/linux/test_async_process.py index 0ea63655a8..d56ad38b9e 100644 --- a/neutron/tests/unit/agent/linux/test_async_process.py +++ b/neutron/tests/unit/agent/linux/test_async_process.py @@ -14,8 +14,6 @@ # License for the specific language governing permissions and limitations # under the License. -import collections - import eventlet.event import eventlet.queue import eventlet.timeout @@ -187,19 +185,20 @@ class TestAsyncProcess(base.BaseTestCase): def _test__get_pid_to_kill(self, expected=_marker, root_helper=None, pids=None): + def _find_child_pids(x): + if not pids: + return [] + pids.pop(0) + return pids + if root_helper: self.proc.root_helper = root_helper - xpid = collections.namedtuple('xpid', ['pid']) - xpids = [xpid(pid) for pid in pids or []] - with mock.patch.object(self.proc, '_process') as mock_process: with mock.patch.object(mock_process, 'pid') as mock_pid: - with mock.patch('psutil.Process') as mock_ps_process: - instance = mock_ps_process.return_value - instance.get_children.return_value = xpids + with mock.patch.object(utils, 'find_child_pids', + side_effect=_find_child_pids): actual = self.proc._get_pid_to_kill() - if expected is _marker: expected = mock_pid self.assertEqual(expected, actual) @@ -208,7 +207,8 @@ class TestAsyncProcess(base.BaseTestCase): self._test__get_pid_to_kill() def test__get_pid_to_kill_returns_child_pid_with_root_helper(self): - self._test__get_pid_to_kill(expected='1', pids=['1'], root_helper='a') + self._test__get_pid_to_kill(expected='2', pids=['1', '2'], + root_helper='a') def test__get_pid_to_kill_returns_last_child_pid_with_root_Helper(self): self._test__get_pid_to_kill(expected='3', pids=['1', '2', '3'], diff --git a/neutron/tests/unit/test_agent_linux_utils.py b/neutron/tests/unit/test_agent_linux_utils.py index cccbf2024f..6b7fbbfd00 100644 --- a/neutron/tests/unit/test_agent_linux_utils.py +++ b/neutron/tests/unit/test_agent_linux_utils.py @@ -17,6 +17,7 @@ import fixtures import mock +import testtools from neutron.agent.linux import utils from neutron.tests import base @@ -106,3 +107,25 @@ class AgentUtilsReplaceFile(base.BaseTestCase): ntf.assert_has_calls(expected) chmod.assert_called_once_with('/baz', 0o644) rename.assert_called_once_with('/baz', '/foo') + + +class TestFindChildPids(base.BaseTestCase): + + def test_returns_empty_list_for_exit_code_1(self): + with mock.patch.object(utils, 'execute', + side_effect=RuntimeError('Exit code: 1')): + self.assertEqual(utils.find_child_pids(-1), []) + + def test_returns_empty_list_for_no_output(self): + with mock.patch.object(utils, 'execute', return_value=''): + self.assertEqual(utils.find_child_pids(-1), []) + + def test_returns_list_of_child_process_ids_for_good_ouput(self): + with mock.patch.object(utils, 'execute', return_value=' 123 \n 185\n'): + self.assertEqual(utils.find_child_pids(-1), ['123', '185']) + + def test_raises_unknown_exception(self): + with testtools.ExpectedException(RuntimeError): + with mock.patch.object(utils, 'execute', + side_effect=RuntimeError()): + utils.find_child_pids(-1) diff --git a/requirements.txt b/requirements.txt index d1c0e6c1cf..74e7f1586b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -16,7 +16,6 @@ jsonrpclib Jinja2 kombu>=2.4.8 netaddr>=0.7.6 -psutil>=0.6.1,<1.0 python-neutronclient>=2.3.0,<3 SQLAlchemy>=0.7.8,<=0.7.99 WebOb>=1.2.3,<1.3