From: Mitsuhiro Tanino <mitsuhiro.tanino@hds.com>
Date: Sat, 13 Jun 2015 04:23:40 +0000 (-0400)
Subject: Wait until service thread is done on service stop
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=fcb5068e79490351359afdec9f05f42ebb022edf;p=openstack-build%2Fcinder-build.git

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 13bef8dff..2ad5878ad 100644
--- 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 4a4c07ebe..8e101f0fc 100644
--- 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):