From 598e35c337c6acf0f7558a874e85c727fc0fbde2 Mon Sep 17 00:00:00 2001
From: Gorka Eguileor <geguileo@redhat.com>
Date: Tue, 16 Feb 2016 19:03:49 +0100
Subject: [PATCH] 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
---
 cinder/service.py                 | 15 ++++++-----
 cinder/tests/unit/test_service.py | 41 +++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/cinder/service.py b/cinder/service.py
index 08ca100d5..8ab77544c 100644
--- 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 585287f0b..d144e3010 100644
--- 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):
 
-- 
2.45.2