From 53c64ff1ac3e92fa1cb8945cfae26b2624f2697d Mon Sep 17 00:00:00 2001 From: Jakub Libosvar Date: Tue, 15 Sep 2015 11:52:03 +0000 Subject: [PATCH] Revert "AsyncProcess: try to kill tender" This change introduced bug 1495937. This reverts commit 470a7d8a106a274e06fb1311c6738f333a98f59c. Change-Id: I84fea4fdac71141da335ccd9e0d4c9d6174dfd86 --- etc/neutron/rootwrap.d/l3.filters | 2 +- .../rootwrap.d/openvswitch-plugin.filters | 2 +- neutron/agent/linux/async_process.py | 34 ++++++++----------- .../agent/linux/test_async_process.py | 2 +- .../unit/agent/linux/test_async_process.py | 28 ++------------- 5 files changed, 19 insertions(+), 49 deletions(-) diff --git a/etc/neutron/rootwrap.d/l3.filters b/etc/neutron/rootwrap.d/l3.filters index c385a1095..0fdf60cd1 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, -15 +kill_ip_monitor: KillFilter, root, ip, -9 # 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 64246588d..c738733bb 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, -15 +kill_ovsdb_client: KillFilter, root, /usr/bin/ovsdb-client, -9 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 4453bbe32..b11c263f2 100644 --- a/neutron/agent/linux/async_process.py +++ b/neutron/agent/linux/async_process.py @@ -161,26 +161,20 @@ class AsyncProcess(object): self._kill_event = None def _kill_process(self, pid): - 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 + 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() 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 bc7f2170f..89c2bec89 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, -15) + self.assertEqual(proc._process.returncode, -9) 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 0bb2fd45b..7d116d14b 100644 --- a/neutron/tests/unit/agent/linux/test_async_process.py +++ b/neutron/tests/unit/agent/linux/test_async_process.py @@ -192,9 +192,8 @@ class TestAsyncProcess(base.BaseTestCase): actual = self.proc._kill_process(pid) self.assertEqual(expected, actual) - mock_execute.assert_has_calls( - [mock.call(['kill', '-15', pid], - run_as_root=self.proc.run_as_root)]) + mock_execute.assert_called_with(['kill', '-9', 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) @@ -205,29 +204,6 @@ 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: -- 2.45.2