diff options
author | Jenkins <jenkins@review.openstack.org> | 2016-08-06 00:35:53 +0000 |
---|---|---|
committer | Gerrit Code Review <review@openstack.org> | 2016-08-06 00:35:53 +0000 |
commit | abd7f362133c7bc3c15df2fb3947a3b0f51211c8 (patch) | |
tree | 4272c9803f12ca5af37b20b6a115b6853d25512c | |
parent | 395f34d47ddbe7776deecf4b98a6270a963849de (diff) | |
parent | d7c8e53d9008ae9a9338caea84634e1ad57bed0c (diff) | |
download | neutron-abd7f362133c7bc3c15df2fb3947a3b0f51211c8.tar.gz |
Merge "Change get_root_helper_child_pid to stop when it finds cmd" into stable/liberty
-rw-r--r-- | neutron/agent/linux/async_process.py | 1 | ||||
-rw-r--r-- | neutron/agent/linux/utils.py | 16 | ||||
-rw-r--r-- | neutron/tests/common/net_helpers.py | 4 | ||||
-rw-r--r-- | neutron/tests/contrib/functional-testing.filters | 4 | ||||
-rw-r--r-- | neutron/tests/functional/agent/linux/test_utils.py | 61 | ||||
-rw-r--r-- | neutron/tests/unit/agent/linux/test_utils.py | 28 |
6 files changed, 99 insertions, 15 deletions
diff --git a/neutron/agent/linux/async_process.py b/neutron/agent/linux/async_process.py index 9a114b7c98..91cb4a96aa 100644 --- a/neutron/agent/linux/async_process.py +++ b/neutron/agent/linux/async_process.py @@ -154,6 +154,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): diff --git a/neutron/agent/linux/utils.py b/neutron/agent/linux/utils.py index 3b614958f4..93524b92a8 100644 --- a/neutron/agent/linux/utils.py +++ b/neutron/agent/linux/utils.py @@ -257,15 +257,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 @@ -273,18 +274,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 diff --git a/neutron/tests/common/net_helpers.py b/neutron/tests/common/net_helpers.py index ebf89e3fa8..0d3b48ec14 100644 --- a/neutron/tests/common/net_helpers.py +++ b/neutron/tests/common/net_helpers.py @@ -259,7 +259,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 @@ -269,7 +269,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) class NetcatTester(object): diff --git a/neutron/tests/contrib/functional-testing.filters b/neutron/tests/contrib/functional-testing.filters index fdf3061718..7b0f988c92 100644 --- a/neutron/tests/contrib/functional-testing.filters +++ b/neutron/tests/contrib/functional-testing.filters @@ -30,3 +30,7 @@ mkdir_filter: RegExpFilter, /bin/mkdir, root, mkdir, -p, /etc/netns/qdhcp-[0-9a- rm_filter: RegExpFilter, /bin/rm, root, rm, -r, /etc/netns/qdhcp-[0-9a-z./-]+ touch_filter: RegExpFilter, /bin/touch, root, touch, /etc/netns/qdhcp-[0-9a-z./-]+/resolv.conf touch_filter: RegExpFilter, /usr/bin/touch, root, touch, /etc/netns/qdhcp-[0-9a-z./-]+/resolv.conf + +# needed for TestGetRootHelperChildPid +bash_filter: RegExpFilter, /bin/bash, root, bash, -c, \(sleep 100\) +sleep_kill: KillFilter, root, sleep, -9 diff --git a/neutron/tests/functional/agent/linux/test_utils.py b/neutron/tests/functional/agent/linux/test_utils.py index 5508457218..c5c07e72c0 100644 --- a/neutron/tests/functional/agent/linux/test_utils.py +++ b/neutron/tests/functional/agent/linux/test_utils.py @@ -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) diff --git a/neutron/tests/unit/agent/linux/test_utils.py b/neutron/tests/unit/agent/linux/test_utils.py index 71a7b8959a..15c4af9f67 100644 --- a/neutron/tests/unit/agent/linux/test_utils.py +++ b/neutron/tests/unit/agent/linux/test_utils.py @@ -239,7 +239,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 [] @@ -247,9 +248,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) @@ -259,12 +268,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) |