openstack
/
neutron
Code Issues Proposed changes

Ensure get_pid_to_kill works with rootwrap script 

To ensure that correct process is killed when using a rootwrap
script, we must recursively list the children of our top-level
process and kill the last one. This patch uses the psutil python
module which is already used in the heat-cfntools project.

Change-Id: I702bb9dd794c08fcaab637284ee303de1778cbb9
This commit is contained in:
Terry Wilson 2013-10-22 13:22:18 -05:00
parent 43b22ddb7d
commit 61e9268bd4
6 changed files with 28 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
neutron/agent/linux/async_process.py
View File

@ -18,6 +18,7 @@ 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
@ -129,21 +130,22 @@ class AsyncProcess(object):
def _get_pid_to_kill(self):
pid = self._process.pid
# If root helper was used, two processes will be created:
# If root helper was used, two or more processes will be created:
#
# - a root helper process (e.g. sudo myscript)
# - possibly a rootwrap script (e.g. neutron-rootwrap)
# - a child process (e.g. myscript)
#
# Killing the root helper process will leave the child process
# as a zombie, so the only way to ensure that both die is to
# target the child process directly.
# running, re-parented to init, so the only way to ensure that both
# die is to target the child process directly.
if self.root_helper:
pids = utils.find_child_pids(pid)
if pids:
# The root helper will only ever launch a single child.
pid = pids[0]
else:
# Process is already dead.
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
return pid

14
neutron/agent/linux/utils.py
View File

@ -108,17 +108,3 @@ 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()]

3
neutron/tests/functional/agent/linux/test_ovsdb_monitor.py
View File

@ -69,7 +69,8 @@ class BaseMonitorTest(base.BaseTestCase):
self._check_test_requirements()
self.root_helper = 'sudo'
# Emulate using a rootwrap script with sudo
self.root_helper = 'sudo sudo'
self.ovs = ovs_lib.BaseOVS(self.root_helper)
self.bridge = create_ovs_resource('test-br-', self.ovs.add_bridge)

16
neutron/tests/unit/agent/linux/test_async_process.py
View File

@ -14,6 +14,8 @@
# License for the specific language governing permissions and limitations
# under the License.
import collections
import eventlet.event
import eventlet.queue
import eventlet.timeout
@ -187,11 +189,17 @@ class TestAsyncProcess(base.BaseTestCase):
root_helper=None, pids=None):
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.object(utils, 'find_child_pids',
return_value=pids):
with mock.patch('psutil.Process') as mock_ps_process:
instance = mock_ps_process.return_value
instance.get_children.return_value = xpids
actual = self.proc._get_pid_to_kill()
if expected is _marker:
expected = mock_pid
self.assertEqual(expected, actual)
@ -202,6 +210,10 @@ class TestAsyncProcess(base.BaseTestCase):
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')
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'],
root_helper='a')
def test__get_pid_to_kill_returns_none_with_root_helper(self):
self._test__get_pid_to_kill(expected=None, root_helper='a')

23
neutron/tests/unit/test_agent_linux_utils.py
View File

@ -17,7 +17,6 @@
import fixtures
import mock
import testtools
from neutron.agent.linux import utils
from neutron.tests import base
@ -107,25 +106,3 @@ 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)

1
requirements.txt
View File

@ -16,6 +16,7 @@ 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