From 4adf35778b3aae3db99a90a1bbb94e668ec7963d Mon Sep 17 00:00:00 2001 From: John Griffith Date: Wed, 23 Oct 2013 20:08:30 +0000 Subject: [PATCH] Handle NotFound exceptions in API There were a number of calls in the API that weren't catching NotFound exceptions. The result is unhandled exception traces and errors in the logs on good runs in tempest. Basicly any negative test that requests a non-existent element would result in an unhandled exception. This patch adds try/except around the volume_api.getxxx calls in cinder/api/... methods to clean this up. Closes-bug: #1243485 Change-Id: I902acc7f4fdbc20fdb1a68697679417694c5533e --- cinder/api/contrib/volume_actions.py | 60 +++++++++++++++++++++++----- cinder/api/v1/volumes.py | 18 +++++++-- cinder/api/v2/volumes.py | 17 ++++++-- 3 files changed, 77 insertions(+), 18 deletions(-) diff --git a/cinder/api/contrib/volume_actions.py b/cinder/api/contrib/volume_actions.py index 34e53e319..5b482f453 100644 --- a/cinder/api/contrib/volume_actions.py +++ b/cinder/api/contrib/volume_actions.py @@ -75,7 +75,11 @@ class VolumeActionsController(wsgi.Controller): def _attach(self, req, id, body): """Add attachment metadata.""" context = req.environ['cinder.context'] - volume = self.volume_api.get(context, id) + try: + volume = self.volume_api.get(context, id) + except exception.VolumeNotFound as error: + raise webob.exc.HTTPNotFound(explanation=error.msg) + # instance uuid is an option now instance_uuid = None if 'instance_uuid' in body['os-attach']: @@ -115,7 +119,11 @@ class VolumeActionsController(wsgi.Controller): def _detach(self, req, id, body): """Clear attachment metadata.""" context = req.environ['cinder.context'] - volume = self.volume_api.get(context, id) + try: + volume = self.volume_api.get(context, id) + except exception.VolumeNotFound as error: + raise webob.exc.HTTPNotFound(explanation=error.msg) + self.volume_api.detach(context, volume) return webob.Response(status_int=202) @@ -123,7 +131,11 @@ class VolumeActionsController(wsgi.Controller): def _reserve(self, req, id, body): """Mark volume as reserved.""" context = req.environ['cinder.context'] - volume = self.volume_api.get(context, id) + try: + volume = self.volume_api.get(context, id) + except exception.VolumeNotFound as error: + raise webob.exc.HTTPNotFound(explanation=error.msg) + self.volume_api.reserve_volume(context, volume) return webob.Response(status_int=202) @@ -131,7 +143,11 @@ class VolumeActionsController(wsgi.Controller): def _unreserve(self, req, id, body): """Unmark volume as reserved.""" context = req.environ['cinder.context'] - volume = self.volume_api.get(context, id) + try: + volume = self.volume_api.get(context, id) + except exception.VolumeNotFound as error: + raise webob.exc.HTTPNotFound(explanation=error.msg) + self.volume_api.unreserve_volume(context, volume) return webob.Response(status_int=202) @@ -139,7 +155,11 @@ class VolumeActionsController(wsgi.Controller): def _begin_detaching(self, req, id, body): """Update volume status to 'detaching'.""" context = req.environ['cinder.context'] - volume = self.volume_api.get(context, id) + try: + volume = self.volume_api.get(context, id) + except exception.VolumeNotFound as error: + raise webob.exc.HTTPNotFound(explanation=error.msg) + self.volume_api.begin_detaching(context, volume) return webob.Response(status_int=202) @@ -147,7 +167,11 @@ class VolumeActionsController(wsgi.Controller): def _roll_detaching(self, req, id, body): """Roll back volume status to 'in-use'.""" context = req.environ['cinder.context'] - volume = self.volume_api.get(context, id) + try: + volume = self.volume_api.get(context, id) + except exception.VolumeNotFound as error: + raise webob.exc.HTTPNotFound(explanation=error.msg) + self.volume_api.roll_detaching(context, volume) return webob.Response(status_int=202) @@ -155,7 +179,11 @@ class VolumeActionsController(wsgi.Controller): def _initialize_connection(self, req, id, body): """Initialize volume attachment.""" context = req.environ['cinder.context'] - volume = self.volume_api.get(context, id) + try: + volume = self.volume_api.get(context, id) + except exception.VolumeNotFound as error: + raise webob.exc.HTTPNotFound(explanation=error.msg) + connector = body['os-initialize_connection']['connector'] info = self.volume_api.initialize_connection(context, volume, @@ -166,7 +194,11 @@ class VolumeActionsController(wsgi.Controller): def _terminate_connection(self, req, id, body): """Terminate volume attachment.""" context = req.environ['cinder.context'] - volume = self.volume_api.get(context, id) + try: + volume = self.volume_api.get(context, id) + except exception.VolumeNotFound as error: + raise webob.exc.HTTPNotFound(explanation=error.msg) + connector = body['os-terminate_connection']['connector'] self.volume_api.terminate_connection(context, volume, connector) return webob.Response(status_int=202) @@ -193,6 +225,7 @@ class VolumeActionsController(wsgi.Controller): volume = self.volume_api.get(context, id) except exception.VolumeNotFound as error: raise webob.exc.HTTPNotFound(explanation=error.msg) + authorize(context, "upload_image") image_metadata = {"container_format": params.get("container_format", "bare"), @@ -217,7 +250,11 @@ class VolumeActionsController(wsgi.Controller): def _extend(self, req, id, body): """Extend size of volume.""" context = req.environ['cinder.context'] - volume = self.volume_api.get(context, id) + try: + volume = self.volume_api.get(context, id) + except exception.VolumeNotFound as error: + raise webob.exc.HTTPNotFound(explanation=error.msg) + try: _val = int(body['os-extend']['new_size']) except (KeyError, ValueError): @@ -232,7 +269,10 @@ class VolumeActionsController(wsgi.Controller): def _volume_readonly_update(self, req, id, body): """Update volume readonly flag.""" context = req.environ['cinder.context'] - volume = self.volume_api.get(context, id) + try: + volume = self.volume_api.get(context, id) + except exception.VolumeNotFound as error: + raise webob.exc.HTTPNotFound(explanation=error.msg) if not self.is_valid_body(body, 'os-update_readonly_flag'): msg = _("No 'os-update_readonly_flag' was specified " diff --git a/cinder/api/v1/volumes.py b/cinder/api/v1/volumes.py index 13a4363f8..9fe18f8f3 100644 --- a/cinder/api/v1/volumes.py +++ b/cinder/api/v1/volumes.py @@ -379,15 +379,25 @@ class VolumeController(wsgi.Controller): snapshot_id = volume.get('snapshot_id') if snapshot_id is not None: - kwargs['snapshot'] = self.volume_api.get_snapshot(context, - snapshot_id) + try: + kwargs['snapshot'] = self.volume_api.get_snapshot(context, + snapshot_id) + except exception.NotFound: + explanation = _('snapshot id:%s not found') % snapshot_id + raise exc.HTTPNotFound(explanation=explanation) + else: kwargs['snapshot'] = None source_volid = volume.get('source_volid') if source_volid is not None: - kwargs['source_volume'] = self.volume_api.get_volume(context, - source_volid) + try: + kwargs['source_volume'] = \ + self.volume_api.get_volume(context, + source_volid) + except exception.NotFound: + explanation = _('source vol id:%s not found') % source_volid + raise exc.HTTPNotFound(explanation=explanation) else: kwargs['source_volume'] = None diff --git a/cinder/api/v2/volumes.py b/cinder/api/v2/volumes.py index 71cd4a272..5d9dd70f2 100644 --- a/cinder/api/v2/volumes.py +++ b/cinder/api/v2/volumes.py @@ -345,15 +345,24 @@ class VolumeController(wsgi.Controller): snapshot_id = volume.get('snapshot_id') if snapshot_id is not None: - kwargs['snapshot'] = self.volume_api.get_snapshot(context, - snapshot_id) + try: + kwargs['snapshot'] = self.volume_api.get_snapshot(context, + snapshot_id) + except exception.NotFound: + explanation = _('snapshot id:%s not found') % snapshot_id + raise exc.HTTPNotFound(explanation=explanation) else: kwargs['snapshot'] = None source_volid = volume.get('source_volid') if source_volid is not None: - kwargs['source_volume'] = self.volume_api.get_volume(context, - source_volid) + try: + kwargs['source_volume'] = \ + self.volume_api.get_volume(context, + source_volid) + except exception.NotFound: + explanation = _('source volume id:%s not found') % source_volid + raise exc.HTTPNotFound(explanation=explanation) else: kwargs['source_volume'] = None -- 2.45.2