From c7809384532d98496fc3fa844d425193c87cfea5 Mon Sep 17 00:00:00 2001
From: LingxianKong <anlin.kong@gmail.com>
Date: Fri, 14 Aug 2015 15:17:18 +0800
Subject: [PATCH] Raise user-friendly exception in case of connection failed

In the scenario of network interruption or connection manually shutdown,
the service api can't be used any more, we should catch the exception.

Change-Id: I276cdb70e35b14504c7139604066098703d9edec
Closes-Bug: #1484804
---
 mistral/api/controllers/v2/service.py      | 20 ++++++++++++++-----
 mistral/coordination.py                    |  7 +++++++
 mistral/tests/unit/api/v2/test_services.py | 23 +++++++++++++++++++++-
 3 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/mistral/api/controllers/v2/service.py b/mistral/api/controllers/v2/service.py
index 4413875b..4c5bbc80 100644
--- a/mistral/api/controllers/v2/service.py
+++ b/mistral/api/controllers/v2/service.py
@@ -15,6 +15,8 @@
 from oslo_config import cfg
 from oslo_log import log as logging
 from pecan import rest
+import six
+import tooz.coordination
 from wsme import types as wtypes
 import wsmeext.pecan as wsme_pecan
 
@@ -72,11 +74,19 @@ class ServicesController(rest.RestController):
         services_list = []
         service_group = ['%s_group' % i for i in launch.LAUNCH_OPTIONS]
 
-        for group in service_group:
-            members = service_coordinator.get_members(group)
-            services_list.extend(
-                [Service.from_dict({'type': group, 'name': member})
-                 for member in members]
+        try:
+            for group in service_group:
+                members = service_coordinator.get_members(group)
+                services_list.extend(
+                    [Service.from_dict({'type': group, 'name': member})
+                     for member in members]
+                )
+        except tooz.coordination.ToozError as e:
+            # In the scenario of network interruption or manually shutdown
+            # connection shutdown, ToozError will be raised.
+            raise exc.CoordinationException(
+                "Failed to get service members from coordination backend. %s"
+                % six.text_type(e)
             )
 
         return Services(services=services_list)
diff --git a/mistral/coordination.py b/mistral/coordination.py
index c2076eed..d8697068 100644
--- a/mistral/coordination.py
+++ b/mistral/coordination.py
@@ -93,6 +93,8 @@ class ServiceCoordinator(object):
             LOG.exception('Error sending a heartbeat to coordination '
                           'backend. %s', six.text_type(e))
 
+            self._started = False
+
     @retry(stop_max_attempt_number=5)
     def join_group(self, group_id):
         if not self.is_active() or not group_id:
@@ -133,6 +135,11 @@ class ServiceCoordinator(object):
             )
 
     def get_members(self, group_id):
+        """Gets members of coordination group.
+
+        ToozError exception must be handled when this function is invoded, we
+        leave it to the invoker for the handling decision.
+        """
         if not self.is_active():
             return []
 
diff --git a/mistral/tests/unit/api/v2/test_services.py b/mistral/tests/unit/api/v2/test_services.py
index 3665122b..1d138502 100644
--- a/mistral/tests/unit/api/v2/test_services.py
+++ b/mistral/tests/unit/api/v2/test_services.py
@@ -12,7 +12,9 @@
 #    See the License for the specific language governing permissions and
 #    limitations under the License.
 
+import mock
 from oslo_config import cfg
+import tooz.coordination
 from webtest import app as webtest_app
 
 from mistral import coordination
@@ -38,7 +40,7 @@ class TestServicesController(base.FunctionalTest):
         srv_ret = [{"name": "service1", "type": "api_group"}]
         self.assertItemsEqual(srv_ret, resp.json['services'])
 
-    def test_get_all_raise(self):
+    def test_get_all_without_backend(self):
         cfg.CONF.set_default('backend_url', None, 'coordination')
 
         coordination.cleanup_service_coordinator()
@@ -51,3 +53,22 @@ class TestServicesController(base.FunctionalTest):
         )
 
         self.assertIn('Service API is not supported', context.message)
+
+    @mock.patch('mistral.coordination.ServiceCoordinator.get_members',
+                side_effect=tooz.coordination.ToozError('error message'))
+    def test_get_all_with_get_members_error(self, mock_get_members):
+        cfg.CONF.set_default('backend_url', 'zake://', 'coordination')
+
+        coordination.cleanup_service_coordinator()
+        coordination.get_service_coordinator()
+
+        context = self.assertRaises(
+            webtest_app.AppError,
+            self.app.get,
+            '/v2/services',
+        )
+
+        self.assertIn(
+            'Failed to get service members from coordination backend',
+            context.message
+        )