From 9b6d38cd02b9a71f87010deabc9bbd386bc4d2f4 Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Tue, 17 Jun 2014 11:46:27 +1200 Subject: [PATCH] Client plugin exception handling methods This change adds common methods which allow resources to handle client exceptions without needing to directly import the exception types. The most common client resource exception handling is: * Detecting if an exception is a 404 * Raising any exception which is not a 404 * Detecting if an exception is a 413 (over limit) * Detecting if an exception was raised by a particular client library Subsequent changes will move to using these methods. Change-Id: Ib2bd55c31e66b562cfa8388beb450be6c06cc4fb --- heat/engine/clients/client_plugin.py | 23 +++ heat/engine/clients/os/ceilometer.py | 9 + heat/engine/clients/os/cinder.py | 9 + heat/engine/clients/os/glance.py | 8 + heat/engine/clients/os/heat_plugin.py | 9 + heat/engine/clients/os/keystone.py | 10 + heat/engine/clients/os/neutron.py | 20 ++ heat/engine/clients/os/nova.py | 12 ++ heat/engine/clients/os/swift.py | 14 ++ heat/engine/clients/os/trove.py | 9 + heat/tests/test_clients.py | 279 ++++++++++++++++++++++++++ 11 files changed, 402 insertions(+) diff --git a/heat/engine/clients/client_plugin.py b/heat/engine/clients/client_plugin.py index 5eff9c4ff6..5548ec2b56 100644 --- a/heat/engine/clients/client_plugin.py +++ b/heat/engine/clients/client_plugin.py @@ -19,6 +19,10 @@ import six @six.add_metaclass(abc.ABCMeta) class ClientPlugin(): + # Module which contains all exceptions classes which the client + # may emit + exceptions_module = None + def __init__(self, context): self.context = context self.clients = context.clients @@ -54,3 +58,22 @@ class ClientPlugin(): except (cfg.NoSuchGroupError, cfg.NoSuchOptError): cfg.CONF.import_opt(option, 'heat.common.config', group='clients') return getattr(cfg.CONF.clients, option) + + def is_client_exception(self, ex): + '''Returns True if the current exception comes from the client.''' + if self.exceptions_module: + return type(ex) in self.exceptions_module.__dict__.values() + return False + + def is_not_found(self, ex): + '''Returns True if the exception is a not-found.''' + return False + + def is_over_limit(self, ex): + '''Returns True if the exception is an over-limit.''' + return False + + def ignore_not_found(self, ex): + '''Raises the exception unless it is a not-found.''' + if not self.is_not_found(ex): + raise ex diff --git a/heat/engine/clients/os/ceilometer.py b/heat/engine/clients/os/ceilometer.py index 159a5a0d33..08dbdff2e0 100644 --- a/heat/engine/clients/os/ceilometer.py +++ b/heat/engine/clients/os/ceilometer.py @@ -12,12 +12,15 @@ # under the License. from ceilometerclient import client as cc +from ceilometerclient import exc from heat.engine.clients import client_plugin class CeilometerClientPlugin(client_plugin.ClientPlugin): + exceptions_module = exc + def _create(self): con = self.context @@ -37,3 +40,9 @@ class CeilometerClientPlugin(client_plugin.ClientPlugin): } return cc.Client('2', endpoint, **args) + + def is_not_found(self, ex): + return isinstance(ex, exc.HTTPNotFound) + + def is_over_limit(self, ex): + return isinstance(ex, exc.HTTPOverLimit) diff --git a/heat/engine/clients/os/cinder.py b/heat/engine/clients/os/cinder.py index f99639b1d6..912730ae12 100644 --- a/heat/engine/clients/os/cinder.py +++ b/heat/engine/clients/os/cinder.py @@ -12,12 +12,15 @@ # under the License. from cinderclient import client as cc +from cinderclient import exceptions from heat.engine.clients import client_plugin class CinderClientPlugin(client_plugin.ClientPlugin): + exceptions_module = exceptions + def _create(self): con = self.context @@ -40,3 +43,9 @@ class CinderClientPlugin(client_plugin.ClientPlugin): client.client.management_url = management_url return client + + def is_not_found(self, ex): + return isinstance(ex, exceptions.NotFound) + + def is_over_limit(self, ex): + return isinstance(ex, exceptions.OverLimit) diff --git a/heat/engine/clients/os/glance.py b/heat/engine/clients/os/glance.py index db99018e92..f51274033f 100644 --- a/heat/engine/clients/os/glance.py +++ b/heat/engine/clients/os/glance.py @@ -26,6 +26,8 @@ LOG = logging.getLogger(__name__) class GlanceClientPlugin(client_plugin.ClientPlugin): + exceptions_module = exc + def _create(self): con = self.context @@ -46,6 +48,12 @@ class GlanceClientPlugin(client_plugin.ClientPlugin): return gc.Client('1', endpoint, **args) + def is_not_found(self, ex): + return isinstance(ex, exc.HTTPNotFound) + + def is_over_limit(self, ex): + return isinstance(ex, exc.HTTPOverLimit) + def get_image_id(self, image_identifier): ''' Return an id for the specified image name or identifier. diff --git a/heat/engine/clients/os/heat_plugin.py b/heat/engine/clients/os/heat_plugin.py index 152b36da94..9ae6d4320c 100644 --- a/heat/engine/clients/os/heat_plugin.py +++ b/heat/engine/clients/os/heat_plugin.py @@ -12,12 +12,15 @@ # under the License. from heatclient import client as hc +from heatclient import exc from heat.engine.clients import client_plugin class HeatClientPlugin(client_plugin.ClientPlugin): + exceptions_module = exc + def _create(self): args = { 'auth_url': self.context.auth_url, @@ -33,6 +36,12 @@ class HeatClientPlugin(client_plugin.ClientPlugin): endpoint = self.get_heat_url() return hc.Client('1', endpoint, **args) + def is_not_found(self, ex): + return isinstance(ex, exc.HTTPNotFound) + + def is_over_limit(self, ex): + return isinstance(ex, exc.HTTPOverLimit) + def get_heat_url(self): heat_url = self._get_client_option('heat', 'url') if heat_url: diff --git a/heat/engine/clients/os/keystone.py b/heat/engine/clients/os/keystone.py index 13f04e2ee4..d9a3a1679a 100644 --- a/heat/engine/clients/os/keystone.py +++ b/heat/engine/clients/os/keystone.py @@ -11,11 +11,21 @@ # License for the specific language governing permissions and limitations # under the License. +from keystoneclient import exceptions + from heat.common import heat_keystoneclient as hkc from heat.engine.clients import client_plugin class KeystoneClientPlugin(client_plugin.ClientPlugin): + exceptions_module = exceptions + def _create(self): return hkc.KeystoneClient(self.context) + + def is_not_found(self, ex): + return isinstance(ex, exceptions.NotFound) + + def is_over_limit(self, ex): + return isinstance(ex, exceptions.RequestEntityTooLarge) diff --git a/heat/engine/clients/os/neutron.py b/heat/engine/clients/os/neutron.py index 3ef22b5962..e5e7dc9792 100644 --- a/heat/engine/clients/os/neutron.py +++ b/heat/engine/clients/os/neutron.py @@ -21,6 +21,8 @@ from heat.engine import constraints class NeutronClientPlugin(client_plugin.ClientPlugin): + exceptions_module = exceptions + def _create(self): con = self.context @@ -39,6 +41,24 @@ class NeutronClientPlugin(client_plugin.ClientPlugin): return nc.Client(**args) + def is_not_found(self, ex): + if isinstance(ex, (exceptions.NotFound, + exceptions.NetworkNotFoundClient, + exceptions.PortNotFoundClient)): + return True + return (isinstance(ex, exceptions.NeutronClientException) and + ex.status_code == 404) + + def is_conflict(self, ex): + if not isinstance(ex, exceptions.NeutronClientException): + return False + return ex.status_code == 409 + + def is_over_limit(self, ex): + if not isinstance(ex, exceptions.NeutronClientException): + return False + return ex.status_code == 413 + class NetworkConstraint(constraints.BaseCustomConstraint): diff --git a/heat/engine/clients/os/nova.py b/heat/engine/clients/os/nova.py index f99fa1bfdc..60dbe11ec7 100644 --- a/heat/engine/clients/os/nova.py +++ b/heat/engine/clients/os/nova.py @@ -12,6 +12,7 @@ # under the License. from novaclient import client as nc +from novaclient import exceptions from novaclient import shell as novashell from heat.engine.clients import client_plugin @@ -19,6 +20,8 @@ from heat.engine.clients import client_plugin class NovaClientPlugin(client_plugin.ClientPlugin): + exceptions_module = exceptions + def _create(self): computeshell = novashell.OpenStackComputeShell() extensions = computeshell._discover_extensions("1.1") @@ -46,3 +49,12 @@ class NovaClientPlugin(client_plugin.ClientPlugin): client.client.management_url = management_url return client + + def is_not_found(self, ex): + return isinstance(ex, exceptions.NotFound) + + def is_over_limit(self, ex): + return isinstance(ex, exceptions.OverLimit) + + def is_bad_request(self, ex): + return isinstance(ex, exceptions.BadRequest) diff --git a/heat/engine/clients/os/swift.py b/heat/engine/clients/os/swift.py index 32d8942574..59abc55836 100644 --- a/heat/engine/clients/os/swift.py +++ b/heat/engine/clients/os/swift.py @@ -12,12 +12,15 @@ # under the License. from swiftclient import client as sc +from swiftclient import exceptions from heat.engine.clients import client_plugin class SwiftClientPlugin(client_plugin.ClientPlugin): + exceptions_module = exceptions + def _create(self): con = self.context @@ -36,3 +39,14 @@ class SwiftClientPlugin(client_plugin.ClientPlugin): 'insecure': self._get_client_option('swift', 'insecure') } return sc.Connection(**args) + + def is_client_exception(self, ex): + return isinstance(ex, exceptions.ClientException) + + def is_not_found(self, ex): + return (isinstance(ex, exceptions.ClientException) and + ex.http_status == 404) + + def is_over_limit(self, ex): + return (isinstance(ex, exceptions.ClientException) and + ex.http_status == 413) diff --git a/heat/engine/clients/os/trove.py b/heat/engine/clients/os/trove.py index 1a5eb9b76b..6c2e7d9e9e 100644 --- a/heat/engine/clients/os/trove.py +++ b/heat/engine/clients/os/trove.py @@ -12,12 +12,15 @@ # under the License. from troveclient import client as tc +from troveclient.client import exceptions from heat.engine.clients import client_plugin class TroveClientPlugin(client_plugin.ClientPlugin): + exceptions_module = exceptions + def _create(self): con = self.context @@ -40,3 +43,9 @@ class TroveClientPlugin(client_plugin.ClientPlugin): client.client.management_url = management_url return client + + def is_not_found(self, ex): + return isinstance(ex, exceptions.NotFound) + + def is_over_limit(self, ex): + return isinstance(ex, exceptions.RequestEntityTooLarge) diff --git a/heat/tests/test_clients.py b/heat/tests/test_clients.py index 50f59583bb..69224c81b7 100644 --- a/heat/tests/test_clients.py +++ b/heat/tests/test_clients.py @@ -11,6 +11,15 @@ # License for the specific language governing permissions and limitations # under the License. +from ceilometerclient import exc as ceil_exc +from cinderclient import exceptions as cinder_exc +from glanceclient import exc as glance_exc +from heatclient import exc as heat_exc +from keystoneclient import exceptions as keystone_exc +from neutronclient.common import exceptions as neutron_exc +from swiftclient import exceptions as swift_exc +from troveclient.client import exceptions as trove_exc + from heatclient import client as heatclient import mock from oslo.config import cfg @@ -19,6 +28,7 @@ from testtools.testcase import skip from heat.engine import clients from heat.engine.clients import client_plugin from heat.tests.common import HeatTestCase +from heat.tests.v1_1 import fakes class ClientsTest(HeatTestCase): @@ -214,3 +224,272 @@ class TestClientPluginsInitialise(HeatTestCase): self.assertEqual(c, plugin.clients) self.assertEqual(con, plugin.context) self.assertIsNone(plugin._client) + + +class TestIsNotFound(HeatTestCase): + + scenarios = [ + ('ceilometer_not_found', dict( + is_not_found=True, + is_over_limit=False, + is_client_exception=True, + plugin='ceilometer', + exception=lambda: ceil_exc.HTTPNotFound(details='gone'), + )), + ('ceilometer_exception', dict( + is_not_found=False, + is_over_limit=False, + is_client_exception=False, + plugin='ceilometer', + exception=lambda: Exception() + )), + ('ceilometer_overlimit', dict( + is_not_found=False, + is_over_limit=True, + is_client_exception=True, + plugin='ceilometer', + exception=lambda: ceil_exc.HTTPOverLimit(details='over'), + )), + ('cinder_not_found', dict( + is_not_found=True, + is_over_limit=False, + is_client_exception=True, + plugin='cinder', + exception=lambda: cinder_exc.NotFound(code=404), + )), + ('cinder_exception', dict( + is_not_found=False, + is_over_limit=False, + is_client_exception=False, + plugin='cinder', + exception=lambda: Exception() + )), + ('cinder_overlimit', dict( + is_not_found=False, + is_over_limit=True, + is_client_exception=True, + plugin='cinder', + exception=lambda: cinder_exc.OverLimit(code=413), + )), + ('glance_not_found', dict( + is_not_found=True, + is_over_limit=False, + is_client_exception=True, + plugin='glance', + exception=lambda: glance_exc.HTTPNotFound(details='gone'), + )), + ('glance_exception', dict( + is_not_found=False, + is_over_limit=False, + is_client_exception=False, + plugin='glance', + exception=lambda: Exception() + )), + ('glance_overlimit', dict( + is_not_found=False, + is_over_limit=True, + is_client_exception=True, + plugin='glance', + exception=lambda: glance_exc.HTTPOverLimit(details='over'), + )), + ('heat_not_found', dict( + is_not_found=True, + is_over_limit=False, + is_client_exception=True, + plugin='heat', + exception=lambda: heat_exc.HTTPNotFound(message='gone'), + )), + ('heat_exception', dict( + is_not_found=False, + is_over_limit=False, + is_client_exception=False, + plugin='heat', + exception=lambda: Exception() + )), + ('heat_overlimit', dict( + is_not_found=False, + is_over_limit=True, + is_client_exception=True, + plugin='heat', + exception=lambda: heat_exc.HTTPOverLimit(message='over'), + )), + ('keystone_not_found', dict( + is_not_found=True, + is_over_limit=False, + is_client_exception=True, + plugin='keystone', + exception=lambda: keystone_exc.NotFound(details='gone'), + )), + ('keystone_exception', dict( + is_not_found=False, + is_over_limit=False, + is_client_exception=False, + plugin='keystone', + exception=lambda: Exception() + )), + ('keystone_overlimit', dict( + is_not_found=False, + is_over_limit=True, + is_client_exception=True, + plugin='keystone', + exception=lambda: keystone_exc.RequestEntityTooLarge( + details='over'), + )), + ('neutron_not_found', dict( + is_not_found=True, + is_over_limit=False, + is_client_exception=True, + plugin='neutron', + exception=lambda: neutron_exc.NotFound, + )), + ('neutron_network_not_found', dict( + is_not_found=True, + is_over_limit=False, + is_client_exception=True, + plugin='neutron', + exception=lambda: neutron_exc.NetworkNotFoundClient(), + )), + ('neutron_port_not_found', dict( + is_not_found=True, + is_over_limit=False, + is_client_exception=True, + plugin='neutron', + exception=lambda: neutron_exc.PortNotFoundClient(), + )), + ('neutron_status_not_found', dict( + is_not_found=True, + is_over_limit=False, + is_client_exception=True, + plugin='neutron', + exception=lambda: neutron_exc.NeutronClientException( + status_code=404), + )), + ('neutron_exception', dict( + is_not_found=False, + is_over_limit=False, + is_client_exception=False, + plugin='neutron', + exception=lambda: Exception() + )), + ('neutron_overlimit', dict( + is_not_found=False, + is_over_limit=True, + is_client_exception=True, + plugin='neutron', + exception=lambda: neutron_exc.NeutronClientException( + status_code=413), + )), + ('nova_not_found', dict( + is_not_found=True, + is_over_limit=False, + is_client_exception=True, + plugin='nova', + exception=lambda: fakes.fake_exception(), + )), + ('nova_exception', dict( + is_not_found=False, + is_over_limit=False, + is_client_exception=False, + plugin='nova', + exception=lambda: Exception() + )), + ('nova_overlimit', dict( + is_not_found=False, + is_over_limit=True, + is_client_exception=True, + plugin='nova', + exception=lambda: fakes.fake_exception(413), + )), + ('swift_not_found', dict( + is_not_found=True, + is_over_limit=False, + is_client_exception=True, + plugin='swift', + exception=lambda: swift_exc.ClientException( + msg='gone', http_status=404), + )), + ('swift_exception', dict( + is_not_found=False, + is_over_limit=False, + is_client_exception=False, + plugin='swift', + exception=lambda: Exception() + )), + ('swift_overlimit', dict( + is_not_found=False, + is_over_limit=True, + is_client_exception=True, + plugin='swift', + exception=lambda: swift_exc.ClientException( + msg='ouch', http_status=413), + )), + ('trove_not_found', dict( + is_not_found=True, + is_over_limit=False, + is_client_exception=True, + plugin='trove', + exception=lambda: trove_exc.NotFound(message='gone'), + )), + ('trove_exception', dict( + is_not_found=False, + is_over_limit=False, + is_client_exception=False, + plugin='trove', + exception=lambda: Exception() + )), + ('trove_overlimit', dict( + is_not_found=False, + is_over_limit=True, + is_client_exception=True, + plugin='trove', + exception=lambda: trove_exc.RequestEntityTooLarge( + message='over'), + )), + ] + + def test_is_not_found(self): + con = mock.Mock() + c = clients.Clients(con) + client_plugin = c.client_plugin(self.plugin) + try: + raise self.exception() + except Exception as e: + if self.is_not_found != client_plugin.is_not_found(e): + raise + + def test_ignore_not_found(self): + con = mock.Mock() + c = clients.Clients(con) + client_plugin = c.client_plugin(self.plugin) + try: + exp = self.exception() + exp_class = exp.__class__ + raise exp + except Exception as e: + if self.is_not_found: + client_plugin.ignore_not_found(e) + else: + self.assertRaises(exp_class, + client_plugin.ignore_not_found, + e) + + def test_is_over_limit(self): + con = mock.Mock() + c = clients.Clients(con) + client_plugin = c.client_plugin(self.plugin) + try: + raise self.exception() + except Exception as e: + if self.is_over_limit != client_plugin.is_over_limit(e): + raise + + def test_is_client_exception(self): + con = mock.Mock() + c = clients.Clients(con) + client_plugin = c.client_plugin(self.plugin) + try: + raise self.exception() + except Exception as e: + ice = self.is_client_exception + if ice != client_plugin.is_client_exception(e): + raise