Wait for periodic tasks to stop on exit

When a Service in Cinder exits it stops all operations and waits for
them to complete, and it also stops all periodica tasks created with
FixedIntervalLoop, but it doesn't wait until these are completed.

This patch will wait for all periodic tasks that received the stop
request without raising an exception before exiting.

Change-Id: I984b8aebd40f9482f0cf8098146d1ccd635dc1d6
Closes-Bug: #1546234

diff --git a/cinder/service.py b/cinder/service.py
index 08ca100d570b70b629fdae73de18f54d5cb9279d..8ab77544cfcc6e540415a7ced6717e015931ded6 100644 (file)
--- a/cinder/service.py
+++ b/cinder/service.py
@@ -280,20 +280,23 @@ class Service(service.Service):
             self.rpcserver.stop()
         except Exception:
             pass
+
+        self.timers_skip = []
         for x in self.timers:
             try:
                 x.stop()
             except Exception:
-                pass
-        self.timers = []
+                self.timers_skip.append(x)
         super(Service, self).stop()
 
     def wait(self):
+        skip = getattr(self, 'timers_skip', [])
         for x in self.timers:
-            try:
-                x.wait()
-            except Exception:
-                pass
+            if x not in skip:
+                try:
+                    x.wait()
+                except Exception:
+                    pass
         if self.rpcserver:
             self.rpcserver.wait()
 

diff --git a/cinder/tests/unit/test_service.py b/cinder/tests/unit/test_service.py
index 585287f0b08b3cf37b3b60295cf0bde43d338782..d144e3010865653fbbcb030a429488cb70039730 100644 (file)
--- a/cinder/tests/unit/test_service.py
+++ b/cinder/tests/unit/test_service.py
@@ -245,6 +245,47 @@ class ServiceTestCase(test.TestCase):
         serv.rpcserver.stop.assert_called_once_with()
         serv.rpcserver.wait.assert_called_once_with()
 
+    @mock.patch('cinder.service.Service.report_state')
+    @mock.patch('cinder.service.Service.periodic_tasks')
+    @mock.patch.object(service.loopingcall, 'FixedIntervalLoopingCall')
+    @mock.patch.object(rpc, 'get_server')
+    @mock.patch('cinder.db')
+    def test_service_stop_waits_for_timers(self, mock_db, mock_rpc,
+                                           mock_loopcall, mock_periodic,
+                                           mock_report):
+        """Test that we wait for loopcalls only if stop succeeds."""
+        serv = service.Service(
+            self.host,
+            self.binary,
+            self.topic,
+            'cinder.tests.unit.test_service.FakeManager',
+            report_interval=5,
+            periodic_interval=10,
+        )
+
+        # One of the loopcalls will raise an exception on stop
+        mock_loopcall.side_effect = (
+            mock.Mock(**{'stop.side_effect': Exception}),
+            mock.Mock())
+
+        serv.start()
+        serv.stop()
+        serv.wait()
+        serv.rpcserver.start.assert_called_once_with()
+        serv.rpcserver.stop.assert_called_once_with()
+        serv.rpcserver.wait.assert_called_once_with()
+
+        # The first loopcall will have failed on the stop call, so we will not
+        # have waited for it to stop
+        self.assertEqual(1, serv.timers[0].start.call_count)
+        self.assertEqual(1, serv.timers[0].stop.call_count)
+        self.assertFalse(serv.timers[0].wait.called)
+
+        # We will wait for the second loopcall
+        self.assertEqual(1, serv.timers[1].start.call_count)
+        self.assertEqual(1, serv.timers[1].stop.call_count)
+        self.assertEqual(1, serv.timers[1].wait.call_count)
+
 
 class TestWSGIService(test.TestCase):