Change get_root_helper_child_pid to stop when it finds cmd
get_root_helper_child_pid recursively finds the child of pid, until it can no longer find a child. However, the intention is not to find the deepest child, but to strip away root helpers. For example 'sudo neutron-rootwrap x' is supposed to find the pid of x. However, in cases 'x' spawned quick lived children of its own (For example: ip / brctl / ovs invocations), get_root_helper_child_pid returned those pids if called in the wrong time. Change-Id: I582aa5c931c8bfe57f49df6899445698270bb33e Closes-Bug: #1558819
This commit is contained in:
parent
98af306043
commit
fd93e19f2a
|
@ -155,6 +155,7 @@ class AsyncProcess(object):
|
||||||
if self._process:
|
if self._process:
|
||||||
return utils.get_root_helper_child_pid(
|
return utils.get_root_helper_child_pid(
|
||||||
self._process.pid,
|
self._process.pid,
|
||||||
|
self.cmd_without_namespace,
|
||||||
run_as_root=self.run_as_root)
|
run_as_root=self.run_as_root)
|
||||||
|
|
||||||
def _kill(self, kill_signal):
|
def _kill(self, kill_signal):
|
||||||
|
|
|
@ -217,15 +217,16 @@ def remove_conf_files(cfg_root, uuid):
|
||||||
os.unlink(file_path)
|
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:
|
If root helper was used, two or more processes would be created:
|
||||||
|
|
||||||
- a root helper process (e.g. sudo myscript)
|
- a root helper process (e.g. sudo myscript)
|
||||||
- possibly a rootwrap script (e.g. neutron-rootwrap)
|
- possibly a rootwrap script (e.g. neutron-rootwrap)
|
||||||
- a child process (e.g. myscript)
|
- a child process (e.g. myscript)
|
||||||
|
- possibly its child processes
|
||||||
|
|
||||||
Killing the root helper process will leave the child process
|
Killing the root helper process will leave the child process
|
||||||
running, re-parented to init, so the only way to ensure that both
|
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)
|
pid = str(pid)
|
||||||
if run_as_root:
|
if run_as_root:
|
||||||
try:
|
|
||||||
pid = find_child_pids(pid)[0]
|
|
||||||
except IndexError:
|
|
||||||
# Process is already dead
|
|
||||||
return None
|
|
||||||
while True:
|
while True:
|
||||||
try:
|
try:
|
||||||
# We shouldn't have more than one child per process
|
# We shouldn't have more than one child per process
|
||||||
# so keep getting the children of the first one
|
# so keep getting the children of the first one
|
||||||
pid = find_child_pids(pid)[0]
|
pid = find_child_pids(pid)[0]
|
||||||
except IndexError:
|
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
|
break
|
||||||
return pid
|
return pid
|
||||||
|
|
||||||
|
|
|
@ -250,7 +250,7 @@ class RootHelperProcess(subprocess.Popen):
|
||||||
sleep=CHILD_PROCESS_SLEEP):
|
sleep=CHILD_PROCESS_SLEEP):
|
||||||
def child_is_running():
|
def child_is_running():
|
||||||
child_pid = utils.get_root_helper_child_pid(
|
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):
|
if utils.pid_invoked_with_cmdline(child_pid, self.cmd):
|
||||||
return True
|
return True
|
||||||
|
|
||||||
|
@ -260,7 +260,7 @@ class RootHelperProcess(subprocess.Popen):
|
||||||
exception=RuntimeError("Process %s hasn't been spawned "
|
exception=RuntimeError("Process %s hasn't been spawned "
|
||||||
"in %d seconds" % (self.cmd, timeout)))
|
"in %d seconds" % (self.cmd, timeout)))
|
||||||
self.child_pid = utils.get_root_helper_child_pid(
|
self.child_pid = utils.get_root_helper_child_pid(
|
||||||
self.pid, run_as_root=True)
|
self.pid, self.cmd, run_as_root=True)
|
||||||
|
|
||||||
@property
|
@property
|
||||||
def is_running(self):
|
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
|
# needed by test_ovs_flows which runs ovs-appctl ofproto/trace
|
||||||
ovstrace_filter: RegExpFilter, ovs-appctl, root, 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
|
# License for the specific language governing permissions and limitations
|
||||||
# under the License.
|
# under the License.
|
||||||
|
|
||||||
|
import functools
|
||||||
|
|
||||||
import eventlet
|
import eventlet
|
||||||
import testtools
|
import testtools
|
||||||
|
|
||||||
from neutron.agent.linux import async_process
|
from neutron.agent.linux import async_process
|
||||||
from neutron.agent.linux import utils
|
from neutron.agent.linux import utils
|
||||||
from neutron.tests.functional.agent.linux import test_async_process
|
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):
|
class TestPIDHelpers(test_async_process.AsyncProcessTestFramework):
|
||||||
|
@ -38,3 +41,61 @@ class TestPIDHelpers(test_async_process.AsyncProcessTestFramework):
|
||||||
def test_wait_until_true_predicate_fails(self):
|
def test_wait_until_true_predicate_fails(self):
|
||||||
with testtools.ExpectedException(eventlet.timeout.Timeout):
|
with testtools.ExpectedException(eventlet.timeout.Timeout):
|
||||||
utils.wait_until_true(lambda: False, 2)
|
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):
|
class TestGetRoothelperChildPid(base.BaseTestCase):
|
||||||
def _test_get_root_helper_child_pid(self, expected=_marker,
|
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):
|
def _find_child_pids(x):
|
||||||
if not pids:
|
if not pids:
|
||||||
return []
|
return []
|
||||||
|
@ -214,9 +215,17 @@ class TestGetRoothelperChildPid(base.BaseTestCase):
|
||||||
return pids
|
return pids
|
||||||
|
|
||||||
mock_pid = object()
|
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',
|
with mock.patch.object(utils, 'find_child_pids',
|
||||||
side_effect=_find_child_pids):
|
side_effect=_find_child_pids), \
|
||||||
actual = utils.get_root_helper_child_pid(mock_pid, run_as_root)
|
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:
|
if expected is _marker:
|
||||||
expected = str(mock_pid)
|
expected = str(mock_pid)
|
||||||
self.assertEqual(expected, actual)
|
self.assertEqual(expected, actual)
|
||||||
|
@ -226,12 +235,21 @@ class TestGetRoothelperChildPid(base.BaseTestCase):
|
||||||
|
|
||||||
def test_returns_child_pid_as_root(self):
|
def test_returns_child_pid_as_root(self):
|
||||||
self._test_get_root_helper_child_pid(expected='2', pids=['1', '2'],
|
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):
|
def test_returns_last_child_pid_as_root(self):
|
||||||
self._test_get_root_helper_child_pid(expected='3',
|
self._test_get_root_helper_child_pid(expected='3',
|
||||||
pids=['1', '2', '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):
|
def test_returns_none_as_root(self):
|
||||||
self._test_get_root_helper_child_pid(expected=None, run_as_root=True)
|
self._test_get_root_helper_child_pid(expected=None, run_as_root=True)
|
||||||
|
|
Loading…
Reference in New Issue