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 0fdf60cd1ecd39f718e2ec8a9dd89552d7c6bb29..c385a10953f907c58c52ec6d4f2229cbcba8a9fd 100644 (file)
--- 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 c738733bb42f75a4b7951c2d426de461e491abc5..64246588dac6fc581e1059e929839af906154e7b 100644 (file)
--- 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 b11c263f27ad23a21ab09ce312b022cfea22fc2e..4453bbe3222ca2e76d3cbe63e4e6e3def0388860 100644 (file)
--- 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 89c2bec890f254c428a6830faa47b623d5af914c..bc7f2170fc38ae612b5298a4ac7f334a5cd79c7c 100644 (file)
--- 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 7d116d14ba3226626687e1315b86b722ac495cdd..0bb2fd45b1ea3435604beaa39c503432c70c2bb1 100644 (file)
--- 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: