Merge "Change get_root_helper_child_pid to stop when it finds cmd"
This commit is contained in:
Jenkins
Gerrit Code Review
commit
3791469ceb
@ -155,6 +155,7 @@ class AsyncProcess(object):
|
||||
if self._process:
|
||||
return utils.get_root_helper_child_pid(
|
||||
self._process.pid,
|
||||
self.cmd_without_namespace,
|
||||
run_as_root=self.run_as_root)
|
||||
|
||||
def _kill(self, kill_signal):
|
||||
|
||||
@ -217,15 +217,16 @@ def remove_conf_files(cfg_root, uuid):
|
||||
os.unlink(file_path)
|
||||
|
||||
|
||||
def get_root_helper_child_pid(pid, run_as_root=False):
|
||||
def get_root_helper_child_pid(pid, expected_cmd, run_as_root=False):
|
||||
"""
|
||||
Get the lowest child pid in the process hierarchy
|
||||
Get the first non root_helper child pid in the process hierarchy.
|
||||
|
||||
If root helper was used, two or more processes would be created:
|
||||
|
||||
- a root helper process (e.g. sudo myscript)
|
||||
- possibly a rootwrap script (e.g. neutron-rootwrap)
|
||||
- a child process (e.g. myscript)
|
||||
- possibly its child processes
|
||||
|
||||
Killing the root helper process will leave the child process
|
||||
running, re-parented to init, so the only way to ensure that both
|
||||
@ -233,18 +234,17 @@ def get_root_helper_child_pid(pid, run_as_root=False):
|
||||
"""
|
||||
pid = str(pid)
|
||||
if run_as_root:
|
||||
try:
|
||||
pid = 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 = find_child_pids(pid)[0]
|
||||
except IndexError:
|
||||
# Last process in the tree, return it
|
||||
return # We never found the child pid with expected_cmd
|
||||
|
||||
# If we've found a pid with no root helper, return it.
|
||||
# If we continue, we can find transient children.
|
||||
if pid_invoked_with_cmdline(pid, expected_cmd):
|
||||
break
|
||||
return pid
|
||||
|
||||
|
||||
@ -250,7 +250,7 @@ class RootHelperProcess(subprocess.Popen):
|
||||
sleep=CHILD_PROCESS_SLEEP):
|
||||
def child_is_running():
|
||||
child_pid = utils.get_root_helper_child_pid(
|
||||
self.pid, run_as_root=True)
|
||||
self.pid, self.cmd, run_as_root=True)
|
||||
if utils.pid_invoked_with_cmdline(child_pid, self.cmd):
|
||||
return True
|
||||
|
||||
@ -260,7 +260,7 @@ class RootHelperProcess(subprocess.Popen):
|
||||
exception=RuntimeError("Process %s hasn't been spawned "
|
||||
"in %d seconds" % (self.cmd, timeout)))
|
||||
self.child_pid = utils.get_root_helper_child_pid(
|
||||
self.pid, run_as_root=True)
|
||||
self.pid, self.cmd, run_as_root=True)
|
||||
|
||||
@property
|
||||
def is_running(self):
|
||||
|
||||
@ -37,3 +37,7 @@ touch_filter2: RegExpFilter, /usr/bin/touch, root, touch, /etc/netns/qdhcp-[0-9a
|
||||
|
||||
# needed by test_ovs_flows which runs ovs-appctl ofproto/trace
|
||||
ovstrace_filter: RegExpFilter, ovs-appctl, root, ovs-appctl, ofproto/trace, .*, .*
|
||||
|
||||
# needed for TestGetRootHelperChildPid
|
||||
bash_filter: RegExpFilter, /bin/bash, root, bash, -c, \(sleep 100\)
|
||||
sleep_kill: KillFilter, root, sleep, -9
|
||||
|
||||
@ -12,12 +12,15 @@
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import functools
|
||||
|
||||
import eventlet
|
||||
import testtools
|
||||
|
||||
from neutron.agent.linux import async_process
|
||||
from neutron.agent.linux import utils
|
||||
from neutron.tests.functional.agent.linux import test_async_process
|
||||
from neutron.tests.functional import base as functional_base
|
||||
|
||||
|
||||
class TestPIDHelpers(test_async_process.AsyncProcessTestFramework):
|
||||
@ -38,3 +41,61 @@ class TestPIDHelpers(test_async_process.AsyncProcessTestFramework):
|
||||
def test_wait_until_true_predicate_fails(self):
|
||||
with testtools.ExpectedException(eventlet.timeout.Timeout):
|
||||
utils.wait_until_true(lambda: False, 2)
|
||||
|
||||
|
||||
class TestGetRootHelperChildPid(functional_base.BaseSudoTestCase):
|
||||
def _addcleanup_sleep_process(self, parent_pid):
|
||||
sleep_pid = utils.execute(
|
||||
['ps', '--ppid', parent_pid, '-o', 'pid=']).strip()
|
||||
self.addCleanup(
|
||||
utils.execute,
|
||||
['kill', '-9', sleep_pid],
|
||||
check_exit_code=False,
|
||||
run_as_root=True)
|
||||
|
||||
def test_get_root_helper_child_pid_returns_first_child(self):
|
||||
"""Test that the first child, not lowest child pid is returned.
|
||||
|
||||
Test creates following proccess tree:
|
||||
sudo +
|
||||
|
|
||||
+--rootwrap +
|
||||
|
|
||||
+--bash+
|
||||
|
|
||||
+--sleep 100
|
||||
|
||||
and tests that pid of `bash' command is returned.
|
||||
"""
|
||||
|
||||
def wait_for_sleep_is_spawned(parent_pid):
|
||||
proc_tree = utils.execute(
|
||||
['pstree', parent_pid], check_exit_code=False)
|
||||
processes = [command.strip() for command in proc_tree.split('---')
|
||||
if command]
|
||||
return 'sleep' == processes[-1]
|
||||
|
||||
cmd = ['bash', '-c', '(sleep 100)']
|
||||
proc = async_process.AsyncProcess(cmd, run_as_root=True)
|
||||
proc.start()
|
||||
|
||||
# root helpers spawn their child processes asynchronously, and we
|
||||
# don't want to use proc.start(block=True) as that uses
|
||||
# get_root_helper_child_pid (The method under test) internally.
|
||||
sudo_pid = proc._process.pid
|
||||
utils.wait_until_true(
|
||||
functools.partial(
|
||||
wait_for_sleep_is_spawned,
|
||||
sudo_pid),
|
||||
sleep=0.1)
|
||||
|
||||
child_pid = utils.get_root_helper_child_pid(
|
||||
sudo_pid, cmd, run_as_root=True)
|
||||
self.assertIsNotNone(
|
||||
child_pid,
|
||||
"get_root_helper_child_pid is expected to return the pid of the "
|
||||
"bash process")
|
||||
self._addcleanup_sleep_process(child_pid)
|
||||
with open('/proc/%s/cmdline' % child_pid, 'r') as f_proc_cmdline:
|
||||
cmdline = f_proc_cmdline.readline().split('\0')[0]
|
||||
self.assertIn('bash', cmdline)
|
||||
|
||||
@ -206,7 +206,8 @@ class TestFindChildPids(base.BaseTestCase):
|
||||
|
||||
class TestGetRoothelperChildPid(base.BaseTestCase):
|
||||
def _test_get_root_helper_child_pid(self, expected=_marker,
|
||||
run_as_root=False, pids=None):
|
||||
run_as_root=False, pids=None,
|
||||
cmds=None):
|
||||
def _find_child_pids(x):
|
||||
if not pids:
|
||||
return []
|
||||
@ -214,9 +215,17 @@ class TestGetRoothelperChildPid(base.BaseTestCase):
|
||||
return pids
|
||||
|
||||
mock_pid = object()
|
||||
pid_invoked_with_cmdline = {}
|
||||
if cmds:
|
||||
pid_invoked_with_cmdline['side_effect'] = cmds
|
||||
else:
|
||||
pid_invoked_with_cmdline['return_value'] = False
|
||||
with mock.patch.object(utils, 'find_child_pids',
|
||||
side_effect=_find_child_pids):
|
||||
actual = utils.get_root_helper_child_pid(mock_pid, run_as_root)
|
||||
side_effect=_find_child_pids), \
|
||||
mock.patch.object(utils, 'pid_invoked_with_cmdline',
|
||||
**pid_invoked_with_cmdline):
|
||||
actual = utils.get_root_helper_child_pid(
|
||||
mock_pid, mock.ANY, run_as_root)
|
||||
if expected is _marker:
|
||||
expected = str(mock_pid)
|
||||
self.assertEqual(expected, actual)
|
||||
@ -226,12 +235,21 @@ class TestGetRoothelperChildPid(base.BaseTestCase):
|
||||
|
||||
def test_returns_child_pid_as_root(self):
|
||||
self._test_get_root_helper_child_pid(expected='2', pids=['1', '2'],
|
||||
run_as_root=True)
|
||||
run_as_root=True,
|
||||
cmds=[True])
|
||||
|
||||
def test_returns_last_child_pid_as_root(self):
|
||||
self._test_get_root_helper_child_pid(expected='3',
|
||||
pids=['1', '2', '3'],
|
||||
run_as_root=True)
|
||||
run_as_root=True,
|
||||
cmds=[False, True])
|
||||
|
||||
def test_returns_first_non_root_helper_child(self):
|
||||
self._test_get_root_helper_child_pid(
|
||||
expected='2',
|
||||
pids=['1', '2', '3'],
|
||||
run_as_root=True,
|
||||
cmds=[True, False])
|
||||
|
||||
def test_returns_none_as_root(self):
|
||||
self._test_get_root_helper_child_pid(expected=None, run_as_root=True)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user