Don't mask exceptions in is_service_available

We seem to be masking client exceptions when checking
resource availability, it would be good to provide the
proper error message to the user.

Change-Id: I9b82b3d5d8382f2c253febf84b8d68ab7f128c5d
Closes-Bug: #1542361

heat/common/exception.py

@@ -33,8 +33,8 @@ LOG = logging.getLogger(__name__)
 
 # TODO(kanagaraj-manickam): Expose this to user via REST API
 ERROR_CODE_MAP = {
-    '99001': _("Service %(service_name)s does not have required endpoint in "
-               "service catalog for the resource type %(resource_type)s")
+    '99001': _("Service %(service_name)s is not available for resource "
+               "type %(resource_type)s, reason: %(reason)s")
 }
 
 

heat/engine/resource.py

@@ -150,14 +150,23 @@ class Resource(object):
 
     @classmethod
     def _validate_service_availability(cls, context, resource_type):
-        if not cls.is_service_available(context):
+        try:
+            svc_available = cls.is_service_available(context)
+        except Exception as exc:
             ex = exception.ResourceTypeUnavailable(
+                resource_type=resource_type,
                 service_name=cls.default_client_name,
-                resource_type=resource_type
-                )
-            LOG.info(six.text_type(ex))
-
+                reason=six.text_type(exc))
+            LOG.exception(exc)
             raise ex
+        else:
+            if not svc_available:
+                ex = exception.ResourceTypeUnavailable(
+                    resource_type=resource_type,
+                    service_name=cls.default_client_name,
+                    reason='Service endpoint not in service catalog.')
+                LOG.info(six.text_type(ex))
+                raise ex
 
     def _init_attributes(self):
         """The method that defines attribute initialization for a resource.
@@ -558,31 +567,26 @@ class Resource(object):
         # resources as they are implemented within the engine.
         if cls.default_client_name is None:
             return True
+        client_plugin = clients.Clients(context).client_plugin(
+            cls.default_client_name)
 
-        try:
-            client_plugin = clients.Clients(context).client_plugin(
-                cls.default_client_name)
-
-            service_types = client_plugin.service_types
-            if not service_types:
-                return True
-
-            # NOTE(kanagaraj-manickam): if one of the service_type does
-            # exist in the keystone, then considered it as available.
-            for service_type in service_types:
-                endpoint_exists = client_plugin.does_endpoint_exist(
-                    service_type=service_type,
-                    service_name=cls.default_client_name)
-                if endpoint_exists:
-                    req_extension = cls.required_service_extension
-                    is_ext_available = (
-                        not req_extension or client_plugin.has_extension(
-                            req_extension))
-                    if is_ext_available:
-                        return True
-        except Exception as ex:
-            LOG.exception(ex)
+        service_types = client_plugin.service_types
+        if not service_types:
+            return True
 
+        # NOTE(kanagaraj-manickam): if one of the service_type does
+        # exist in the keystone, then considered it as available.
+        for service_type in service_types:
+            endpoint_exists = client_plugin.does_endpoint_exist(
+                service_type=service_type,
+                service_name=cls.default_client_name)
+            if endpoint_exists:
+                req_extension = cls.required_service_extension
+                is_ext_available = (
+                    not req_extension or client_plugin.has_extension(
+                        req_extension))
+                if is_ext_available:
+                    return True
         return False
 
     def keystone(self):

heat/engine/service.py

@@ -1322,11 +1322,19 @@ class EngineService(service.Service):
         if resource_class.support_status.status == support.HIDDEN:
             raise exception.NotSupported(type_name)
 
-        if not resource_class.is_service_available(cnxt):
+        try:
+            svc_available = resource_class.is_service_available(cnxt)
+        except Exception as exc:
             raise exception.ResourceTypeUnavailable(
                 service_name=resource_class.default_client_name,
-                resource_type=type_name
-            )
+                resource_type=type_name,
+                reason=six.text_type(exc))
+        else:
+            if not svc_available:
+                raise exception.ResourceTypeUnavailable(
+                    service_name=resource_class.default_client_name,
+                    resource_type=type_name,
+                    reason='Service endpoint not in service catalog.')
 
         def properties_schema():
             for name, schema_dict in resource_class.properties_schema.items():

heat/tests/engine/test_resource_type.py

@@ -166,10 +166,9 @@ class ResourceTypeTest(common.HeatTestCase):
                                    self.eng.resource_schema,
                                    self.ctx,
                                    type_name)
-
-            msg = ('HEAT-E99001 Service sample does not have required endpoint'
-                   ' in service catalog for the resource type'
-                   ' ResourceWithDefaultClientName')
+            msg = ('HEAT-E99001 Service sample is not available for resource '
+                   'type ResourceWithDefaultClientName, reason: '
+                   'Service endpoint not in service catalog.')
             self.assertEqual(msg,
                              six.text_type(ex),
                              'invalid exception message')

heat/tests/test_resource.py

@@ -3070,12 +3070,13 @@ class ResourceAvailabilityTest(common.HeatTestCase):
             ['test_type']
         )
         mock_client_plugin.has_extension = mock.Mock(
-            side_effect=Exception("error"))
+            side_effect=exception.AuthorizationFailure())
         mock_client_plugin_method.return_value = mock_client_plugin
 
-        self.assertFalse(
-            generic_rsrc.ResourceWithDefaultClientNameExt.is_service_available(
-                context=mock.Mock()))
+        self.assertRaises(
+            exception.AuthorizationFailure,
+            generic_rsrc.ResourceWithDefaultClientNameExt.is_service_available,
+            context=mock.Mock())
         mock_client_plugin_method.assert_called_once_with(
             generic_rsrc.ResourceWithDefaultClientName.default_client_name)
         mock_service_types.assert_called_once_with()
@@ -3112,8 +3113,8 @@ class ResourceAvailabilityTest(common.HeatTestCase):
             service_name=(generic_rsrc.ResourceWithDefaultClientName
                           .default_client_name))
 
-    def test_service_not_deployed_throws_exception(self):
-        """Test raising exception when the service is not deployed.
+    def test_service_not_available_returns_false(self):
+        """Test when the service is not in service catalog.
 
         When the service is not deployed, make sure resource is throwing
         ResourceTypeUnavailable exception.
@@ -3138,9 +3139,45 @@ class ResourceAvailabilityTest(common.HeatTestCase):
                 definition=definition,
                 stack=mock_stack)
 
-            msg = ('HEAT-E99001 Service sample does not have required endpoint'
-                   ' in service catalog for the resource type'
-                   ' UnavailableResourceType')
+            msg = ('HEAT-E99001 Service sample is not available for resource '
+                   'type UnavailableResourceType, reason: '
+                   'Service endpoint not in service catalog.')
+            self.assertEqual(msg,
+                             six.text_type(ex),
+                             'invalid exception message')
+
+            # Make sure is_service_available is called on the right class
+            mock_method.assert_called_once_with(mock_stack.context)
+
+    def test_service_not_available_throws_exception(self):
+        """Test for other exceptions when checking for service availability
+
+        Ex. when client throws an error, make sure resource is throwing
+        ResourceTypeUnavailable that contains the orginal exception message.
+        """
+        with mock.patch.object(
+                generic_rsrc.ResourceWithDefaultClientName,
+                'is_service_available') as mock_method:
+            mock_method.side_effect = exception.AuthorizationFailure()
+
+            definition = rsrc_defn.ResourceDefinition(
+                name='Test Resource',
+                resource_type='UnavailableResourceType')
+
+            mock_stack = mock.MagicMock()
+            mock_stack.service_check_defer = False
+
+            ex = self.assertRaises(
+                exception.ResourceTypeUnavailable,
+                generic_rsrc.ResourceWithDefaultClientName.__new__,
+                cls=generic_rsrc.ResourceWithDefaultClientName,
+                name='test_stack',
+                definition=definition,
+                stack=mock_stack)
+
+            msg = ('HEAT-E99001 Service sample is not available for resource '
+                   'type UnavailableResourceType, reason: '
+                   'Authorization failed.')
             self.assertEqual(msg,
                              six.text_type(ex),
                              'invalid exception message')