Introduce kill_signal parameter to AsynProcess.stop()

All stop() calls of instances of AsyncProcess class were sending
hardcoded SIGKILL signal to its process. This patch leaves the default
behavior to SIGKILL but offers any number to be sent to kill command.

Note: Internal private methods also got a new parameter which is not
      appended. Given that those methods are private and thus not used
      outside of the class, we can afford it.

Change-Id: Ib7b0273c134d59c6a50173d4c2eb35761fcd3d62
Related-Bug: #1487548

diff --git a/neutron/agent/linux/async_process.py b/neutron/agent/linux/async_process.py
index b11c263f27ad23a21ab09ce312b022cfea22fc2e..c31210bd45ac88dde01514d730663e56059382f4 100644 (file)
--- a/neutron/agent/linux/async_process.py
+++ b/neutron/agent/linux/async_process.py
@@ -12,6 +12,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import signal
+
 import eventlet
 import eventlet.event
 import eventlet.queue
@@ -103,16 +105,18 @@ class AsyncProcess(object):
         if block:
             utils.wait_until_true(self.is_active)
 
-    def stop(self, block=False):
+    def stop(self, block=False, kill_signal=signal.SIGKILL):
         """Halt the process and watcher threads.
 
         :param block: Block until the process has stopped.
+        :param kill_signal: Number of signal that will be sent to the process
+                            when terminating the process
         :raises eventlet.timeout.Timeout if blocking is True and the process
                 did not stop in time.
         """
         if self._kill_event:
             LOG.debug('Halting async process [%s].', self.cmd)
-            self._kill()
+            self._kill(kill_signal)
         else:
             raise AsyncProcessException(_('Process is not running.'))
 
@@ -142,7 +146,7 @@ class AsyncProcess(object):
                 self._process.pid,
                 run_as_root=self.run_as_root)
 
-    def _kill(self, respawning=False):
+    def _kill(self, kill_signal, respawning=False):
         """Kill the process and the associated watcher greenthreads.
 
         :param respawning: Optional, whether respawn will be subsequently
@@ -153,18 +157,19 @@ class AsyncProcess(object):
 
         pid = self.pid
         if pid:
-            self._kill_process(pid)
+            self._kill_process(pid, kill_signal)
 
         if not respawning:
             # Clear the kill event to ensure the process can be
             # explicitly started again.
             self._kill_event = None
 
-    def _kill_process(self, pid):
+    def _kill_process(self, pid, kill_signal):
         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)
+            utils.execute(['kill', '-%d' % kill_signal, pid],
+                          run_as_root=self.run_as_root)
         except Exception as ex:
             stale_pid = (isinstance(ex, RuntimeError) and
                          'No such process' in str(ex))
@@ -185,7 +190,7 @@ class AsyncProcess(object):
             respawning = True
         else:
             respawning = False
-        self._kill(respawning=respawning)
+        self._kill(signal.SIGKILL, respawning=respawning)
         if respawning:
             eventlet.sleep(self.respawn_interval)
             LOG.debug('Respawning async process [%s].', self.cmd)

diff --git a/neutron/tests/functional/agent/linux/test_ovsdb_monitor.py b/neutron/tests/functional/agent/linux/test_ovsdb_monitor.py
index e88329df43c66ae7ee8d6e4b78071d2e57181a23..53a074ccd5012d96f6fbd1505dae3a92a70e98d7 100644 (file)
--- a/neutron/tests/functional/agent/linux/test_ovsdb_monitor.py
+++ b/neutron/tests/functional/agent/linux/test_ovsdb_monitor.py
@@ -22,6 +22,8 @@ Tests in this module will be skipped unless:
  - sudo testing is enabled (see neutron.tests.functional.base for details)
 """
 
+import signal
+
 import eventlet
 from oslo_config import cfg
 
@@ -82,7 +84,7 @@ class TestOvsdbMonitor(BaseMonitorTest):
         old_pid = self.monitor._process.pid
         output1 = self.collect_initial_output()
         pid = utils.get_root_helper_child_pid(old_pid, run_as_root=True)
-        self.monitor._kill_process(pid)
+        self.monitor._kill_process(pid, signal.SIGKILL)
         self.monitor._reset_queues()
         while (self.monitor._process.pid == old_pid):
             eventlet.sleep(0.01)

diff --git a/neutron/tests/unit/agent/linux/test_async_process.py b/neutron/tests/unit/agent/linux/test_async_process.py
index 7d116d14ba3226626687e1315b86b722ac495cdd..5bf9097592a1b1afd7f5f5c53455b21d7c59d201 100644 (file)
--- a/neutron/tests/unit/agent/linux/test_async_process.py
+++ b/neutron/tests/unit/agent/linux/test_async_process.py
@@ -12,6 +12,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import signal
+
 import eventlet.event
 import eventlet.queue
 import eventlet.timeout
@@ -57,7 +59,7 @@ class TestAsyncProcess(base.BaseTestCase):
         with mock.patch.object(self.proc, '_kill') as kill:
             self.proc._handle_process_error()
 
-        kill.assert_has_calls([mock.call(respawning=False)])
+        kill.assert_has_calls([mock.call(signal.SIGKILL, respawning=False)])
 
     def test__handle_process_error_kills_without_respawn(self):
         self.proc.respawn_interval = 1
@@ -66,7 +68,7 @@ class TestAsyncProcess(base.BaseTestCase):
                 with mock.patch('eventlet.sleep') as sleep:
                     self.proc._handle_process_error()
 
-        kill.assert_has_calls([mock.call(respawning=True)])
+        kill.assert_has_calls([mock.call(signal.SIGKILL, respawning=True)])
         sleep.assert_has_calls([mock.call(self.proc.respawn_interval)])
         spawn.assert_called_once_with()
 
@@ -161,7 +163,7 @@ class TestAsyncProcess(base.BaseTestCase):
                 mock.patch.object(self.proc, '_kill_process'
                                   ) as mock_kill_process,\
                 mock.patch.object(self.proc, '_process'):
-            self.proc._kill(respawning)
+            self.proc._kill(signal.SIGKILL, respawning)
 
             if respawning:
                 self.assertIsNotNone(self.proc._kill_event)
@@ -170,7 +172,7 @@ class TestAsyncProcess(base.BaseTestCase):
 
         mock_kill_event.send.assert_called_once_with()
         if pid:
-            mock_kill_process.assert_called_once_with(pid)
+            mock_kill_process.assert_called_once_with(pid, signal.SIGKILL)
 
     def test__kill_when_respawning_does_not_clear_kill_event(self):
         self._test__kill(True)
@@ -181,7 +183,8 @@ class TestAsyncProcess(base.BaseTestCase):
     def test__kill_targets_process_for_pid(self):
         self._test__kill(False, pid='1')
 
-    def _test__kill_process(self, pid, expected, exception_message=None):
+    def _test__kill_process(self, pid, expected, exception_message=None,
+                            kill_signal=signal.SIGKILL):
         self.proc.run_as_root = True
         if exception_message:
             exc = RuntimeError(exception_message)
@@ -189,10 +192,10 @@ class TestAsyncProcess(base.BaseTestCase):
             exc = None
         with mock.patch.object(utils, 'execute',
                                side_effect=exc) as mock_execute:
-            actual = self.proc._kill_process(pid)
+            actual = self.proc._kill_process(pid, kill_signal)
 
         self.assertEqual(expected, actual)
-        mock_execute.assert_called_with(['kill', '-9', pid],
+        mock_execute.assert_called_with(['kill', '-%d' % kill_signal, pid],
                                         run_as_root=self.proc.run_as_root)
 
     def test__kill_process_returns_true_for_valid_pid(self):
@@ -204,11 +207,14 @@ class TestAsyncProcess(base.BaseTestCase):
     def test__kill_process_returns_false_for_execute_exception(self):
         self._test__kill_process('1', False, 'Invalid')
 
-    def test_stop_calls_kill(self):
+    def test_kill_process_with_different_signal(self):
+        self._test__kill_process('1', True, kill_signal=signal.SIGTERM)
+
+    def test_stop_calls_kill_with_provided_signal_number(self):
         self.proc._kill_event = True
         with mock.patch.object(self.proc, '_kill') as mock_kill:
-            self.proc.stop()
-        mock_kill.assert_called_once_with()
+            self.proc.stop(kill_signal=signal.SIGTERM)
+        mock_kill.assert_called_once_with(signal.SIGTERM)
 
     def test_stop_raises_exception_if_already_started(self):
         with testtools.ExpectedException(async_process.AsyncProcessException):