From: IWAMOTO Toshihiro Date: Fri, 11 Sep 2015 10:01:20 +0000 (+0900) Subject: AsyncProcess: try to kill tender X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=470a7d8a106a274e06fb1311c6738f333a98f59c;p=openstack-build%2Fneutron-build.git AsyncProcess: try to kill tender _kill_process kills processes with SIGKILL, which prevents the processes' cleanup from running. Issue SIGTERM first and wait a bit. Change-Id: Ie7b94011bbd11b1d672c95e3be19bb3c84ef77ec Closes-bug: 1494363 --- diff --git a/etc/neutron/rootwrap.d/l3.filters b/etc/neutron/rootwrap.d/l3.filters index 0fdf60cd1..c385a1095 100644 --- a/etc/neutron/rootwrap.d/l3.filters +++ b/etc/neutron/rootwrap.d/l3.filters @@ -30,7 +30,7 @@ find: RegExpFilter, find, root, find, /sys/class/net, -maxdepth, 1, -type, l, -p ip_exec: IpNetnsExecFilter, ip, root # For ip monitor -kill_ip_monitor: KillFilter, root, ip, -9 +kill_ip_monitor: KillFilter, root, ip, -9, -15 # ovs_lib (if OVSInterfaceDriver is used) ovs-vsctl: CommandFilter, ovs-vsctl, root diff --git a/etc/neutron/rootwrap.d/openvswitch-plugin.filters b/etc/neutron/rootwrap.d/openvswitch-plugin.filters index c738733bb..64246588d 100644 --- a/etc/neutron/rootwrap.d/openvswitch-plugin.filters +++ b/etc/neutron/rootwrap.d/openvswitch-plugin.filters @@ -14,7 +14,7 @@ ovs-vsctl: CommandFilter, ovs-vsctl, root # NOTE(yamamoto): of_interface=native doesn't use ovs-ofctl ovs-ofctl: CommandFilter, ovs-ofctl, root -kill_ovsdb_client: KillFilter, root, /usr/bin/ovsdb-client, -9 +kill_ovsdb_client: KillFilter, root, /usr/bin/ovsdb-client, -9, -15 ovsdb-client: CommandFilter, ovsdb-client, root xe: CommandFilter, xe, root diff --git a/neutron/agent/linux/async_process.py b/neutron/agent/linux/async_process.py index b11c263f2..4453bbe32 100644 --- a/neutron/agent/linux/async_process.py +++ b/neutron/agent/linux/async_process.py @@ -161,20 +161,26 @@ class AsyncProcess(object): self._kill_event = None def _kill_process(self, pid): - try: - # A process started by a root helper will be running as - # root and need to be killed via the same helper. - utils.execute(['kill', '-9', pid], run_as_root=self.run_as_root) - except Exception as ex: - stale_pid = (isinstance(ex, RuntimeError) and - 'No such process' in str(ex)) - if not stale_pid: - LOG.exception(_LE('An error occurred while killing [%s].'), - self.cmd) - return False - - if self._process: - self._process.wait() + for sig, timeout in [('-15', 5), ('-9', -1)]: + try: + # A process started by a root helper will be running as + # root and need to be killed via the same helper. + utils.execute(['kill', sig, pid], run_as_root=self.run_as_root) + except Exception as ex: + stale_pid = (isinstance(ex, RuntimeError) and + 'No such process' in str(ex)) + if not stale_pid: + LOG.exception(_LE('An error occurred while killing [%s].'), + self.cmd) + return False + return True + + if self._process: + while timeout != 0: + if self._process.poll() is not None: + return True + eventlet.sleep(1) + timeout -= 1 return True def _handle_process_error(self): diff --git a/neutron/tests/functional/agent/linux/test_async_process.py b/neutron/tests/functional/agent/linux/test_async_process.py index 89c2bec89..bc7f2170f 100644 --- a/neutron/tests/functional/agent/linux/test_async_process.py +++ b/neutron/tests/functional/agent/linux/test_async_process.py @@ -54,7 +54,7 @@ class TestAsyncProcess(AsyncProcessTestFramework): # Ensure that the process and greenthreads have stopped proc._process.wait() - self.assertEqual(proc._process.returncode, -9) + self.assertEqual(proc._process.returncode, -15) for watcher in proc._watchers: watcher.wait() diff --git a/neutron/tests/unit/agent/linux/test_async_process.py b/neutron/tests/unit/agent/linux/test_async_process.py index 7d116d14b..0bb2fd45b 100644 --- a/neutron/tests/unit/agent/linux/test_async_process.py +++ b/neutron/tests/unit/agent/linux/test_async_process.py @@ -192,8 +192,9 @@ class TestAsyncProcess(base.BaseTestCase): actual = self.proc._kill_process(pid) self.assertEqual(expected, actual) - mock_execute.assert_called_with(['kill', '-9', pid], - run_as_root=self.proc.run_as_root) + mock_execute.assert_has_calls( + [mock.call(['kill', '-15', pid], + run_as_root=self.proc.run_as_root)]) def test__kill_process_returns_true_for_valid_pid(self): self._test__kill_process('1', True) @@ -204,6 +205,29 @@ class TestAsyncProcess(base.BaseTestCase): def test__kill_process_returns_false_for_execute_exception(self): self._test__kill_process('1', False, 'Invalid') + def test__kill_process_with_9(self): + class Mockpoll(object): + # This must be larger than timeout (5) + i = 6 + + def __call__(self): + self.i -= 1 + return None if self.i >= 0 else 0 + + self.proc.run_as_root = True + pid = '1' + with mock.patch.object(utils, 'execute') as mock_execute,\ + mock.patch.object(self.proc, '_process') as mock_process,\ + mock.patch.object(mock_process, 'poll', new=Mockpoll()): + actual = self.proc._kill_process(pid) + + self.assertTrue(actual) + self.assertEqual([mock.call(['kill', '-15', pid], + run_as_root=self.proc.run_as_root), + mock.call(['kill', '-9', pid], + run_as_root=self.proc.run_as_root)], + mock_execute.mock_calls) + def test_stop_calls_kill(self): self.proc._kill_event = True with mock.patch.object(self.proc, '_kill') as mock_kill: