Wait until service thread is done on service stop

In order to be able to perform graceful shutdown of services, we need
to wait for all the current service threads to finish before exiting
from the service process.

Oslo.service provides this facility through it's wait() method, so we
need to call it in Cinder Service stop() method which gets called when
graceful shutdown is requested by sending the process SIGINT or SIGTERM.

Partial-Bug: 1464822
Change-Id: I972b80251cebe353f9d89ff4e05db8d029ef5e73

diff --git a/cinder/service.py b/cinder/service.py
index 13bef8dff206910c31df9db658e10f564229989d..2ad5878ad30590c45a20708b34f4e3e5a1e88af4 100644 (file)
--- a/cinder/service.py
+++ b/cinder/service.py
@@ -263,6 +263,7 @@ class Service(service.Service):
         # errors, go ahead and ignore them.. as we're shutting down anyway
         try:
             self.rpcserver.stop()
+            self.rpcserver.wait()
         except Exception:
             pass
         for x in self.timers:

diff --git a/cinder/tests/unit/test_service.py b/cinder/tests/unit/test_service.py
index 4a4c07ebe5a720f230c0405359d47b8c63bbd849..8e101f0fc0440783bd856b07cbebc3f5f388bfe1 100644 (file)
--- a/cinder/tests/unit/test_service.py
+++ b/cinder/tests/unit/test_service.py
@@ -28,6 +28,7 @@ from cinder import context
 from cinder import db
 from cinder import exception
 from cinder import manager
+from cinder import rpc
 from cinder import service
 from cinder import test
 from cinder import wsgi
@@ -113,25 +114,23 @@ class ServiceTestCase(test.TestCase):
 
     def setUp(self):
         super(ServiceTestCase, self).setUp()
+        self.host = 'foo'
+        self.binary = 'cinder-fake'
+        self.topic = 'fake'
 
     def test_create(self):
-        host = 'foo'
-        binary = 'cinder-fake'
-        topic = 'fake'
-
         # NOTE(vish): Create was moved out of mock replay to make sure that
         #             the looping calls are created in StartService.
-        app = service.Service.create(host=host, binary=binary, topic=topic)
+        app = service.Service.create(host=self.host,
+                                     binary=self.binary,
+                                     topic=self.topic)
 
         self.assertTrue(app)
 
     def test_report_state_newly_disconnected(self):
-        host = 'foo'
-        binary = 'bar'
-        topic = 'test'
-        service_ref = {'host': host,
-                       'binary': binary,
-                       'topic': topic,
+        service_ref = {'host': self.host,
+                       'binary': self.binary,
+                       'topic': self.topic,
                        'report_count': 0,
                        'availability_zone': 'nova',
                        'id': 1}
@@ -141,9 +140,9 @@ class ServiceTestCase(test.TestCase):
             mock_db.service_get.side_effect = db_exc.DBConnectionError()
 
             serv = service.Service(
-                host,
-                binary,
-                topic,
+                self.host,
+                self.binary,
+                self.topic,
                 'cinder.tests.unit.test_service.FakeManager'
             )
             serv.start()
@@ -152,12 +151,9 @@ class ServiceTestCase(test.TestCase):
             self.assertFalse(mock_db.service_update.called)
 
     def test_report_state_newly_connected(self):
-        host = 'foo'
-        binary = 'bar'
-        topic = 'test'
-        service_ref = {'host': host,
-                       'binary': binary,
-                       'topic': topic,
+        service_ref = {'host': self.host,
+                       'binary': self.binary,
+                       'topic': self.topic,
                        'report_count': 0,
                        'availability_zone': 'nova',
                        'id': 1}
@@ -167,9 +163,9 @@ class ServiceTestCase(test.TestCase):
             mock_db.service_get.return_value = service_ref
 
             serv = service.Service(
-                host,
-                binary,
-                topic,
+                self.host,
+                self.binary,
+                self.topic,
                 'cinder.tests.unit.test_service.FakeManager'
             )
             serv.start()
@@ -180,12 +176,9 @@ class ServiceTestCase(test.TestCase):
             self.assertTrue(mock_db.service_update.called)
 
     def test_report_state_manager_not_working(self):
-        host = 'foo'
-        binary = 'bar'
-        topic = 'test'
-        service_ref = {'host': host,
-                       'binary': binary,
-                       'topic': topic,
+        service_ref = {'host': self.host,
+                       'binary': self.binary,
+                       'topic': self.topic,
                        'report_count': 0,
                        'availability_zone': 'nova',
                        'id': 1}
@@ -193,9 +186,9 @@ class ServiceTestCase(test.TestCase):
             mock_db.service_get.return_value = service_ref
 
             serv = service.Service(
-                host,
-                binary,
-                topic,
+                self.host,
+                self.binary,
+                self.topic,
                 'cinder.tests.unit.test_service.FakeManager'
             )
             serv.manager.is_working = mock.Mock(return_value=False)
@@ -213,6 +206,21 @@ class ServiceTestCase(test.TestCase):
             manager="cinder.tests.unit.test_service.FakeManager")
         self.assertEqual(25, CONF.service_down_time)
 
+    @mock.patch.object(rpc, 'get_server')
+    @mock.patch.object(service, 'db')
+    def test_service_stop_waits_for_rpcserver(self, mock_db, mock_rpc):
+        serv = service.Service(
+            self.host,
+            self.binary,
+            self.topic,
+            'cinder.tests.unit.test_service.FakeManager'
+        )
+        serv.start()
+        serv.stop()
+        serv.rpcserver.start.assert_called_once_with()
+        serv.rpcserver.stop.assert_called_once_with()
+        serv.rpcserver.wait.assert_called_once_with()
+
 
 class TestWSGIService(test.TestCase):