diff --git a/tricircle/cinder_apigw/controllers/volume.py b/tricircle/cinder_apigw/controllers/volume.py index d8d1591..979fc97 100644 --- a/tricircle/cinder_apigw/controllers/volume.py +++ b/tricircle/cinder_apigw/controllers/volume.py @@ -15,11 +15,9 @@ import urlparse -import pecan from pecan import expose from pecan import request from pecan import response -from pecan import Response from pecan import rest from oslo_log import log as logging @@ -31,6 +29,7 @@ import tricircle.common.context as t_context from tricircle.common import httpclient as hclient from tricircle.common.i18n import _ from tricircle.common.i18n import _LE +from tricircle.common import utils import tricircle.db.api as db_api from tricircle.db import core @@ -49,8 +48,8 @@ class VolumeController(rest.RestController): context = t_context.extract_context_from_environ() if 'volume' not in kw: - pecan.abort(400, _('Volume not found in request body')) - return + return utils.format_cinder_error( + 400, _("Missing required element 'volume' in request body.")) az = kw['volume'].get('availability_zone', '') pod, pod_az = az_ag.get_pod_by_az_tenant( @@ -58,15 +57,14 @@ class VolumeController(rest.RestController): az_name=az, tenant_id=self.tenant_id) if not pod: - pecan.abort(500, _('Pod not configured or scheduling failure')) LOG.error(_LE("Pod not configured or scheduling failure")) - return + return utils.format_cinder_error( + 500, _('Pod not configured or scheduling failure')) t_pod = db_api.get_top_pod(context) if not t_pod: - pecan.abort(500, _('Top Pod not configured')) LOG.error(_LE("Top Pod not configured")) - return + return utils.format_cinder_error(500, _('Top Pod not configured')) # TODO(joehuang): get release from pod configuration, # to convert the content @@ -82,10 +80,10 @@ class VolumeController(rest.RestController): s_type=cons.ST_CINDER) if s_ctx['b_url'] == '': - pecan.abort(500, _('bottom pod endpoint incorrect')) - LOG.error(_LE("bottom pod endpoint incorrect %s") % + LOG.error(_LE("Bottom Pod endpoint incorrect %s") % pod['pod_name']) - return + return utils.format_cinder_error( + 500, _('Bottom Pod endpoint incorrect')) b_headers = self._convert_header(t_release, b_release, @@ -141,8 +139,8 @@ class VolumeController(rest.RestController): 'bottom_id': b_vol_ret['id'], 'pod_id': pod['pod_id'], 'exception': e}) - return Response(_('Failed to create volume ' - 'resource routing'), 500) + return utils.format_cinder_error( + 500, _('Failed to create volume resource routing')) ret_vol = self._convert_object(b_release, t_release, b_vol_ret, @@ -152,7 +150,7 @@ class VolumeController(rest.RestController): return {'volume': ret_vol} - return {'error': b_ret_body} + return b_ret_body @expose(generic=True, template='json') def get_one(self, _id): @@ -171,10 +169,12 @@ class VolumeController(rest.RestController): s_ctx = self._get_res_routing_ref(context, _id, request.url) if not s_ctx: - return Response(_('Failed to find resource'), 404) + return utils.format_cinder_error( + 404, _('Volume %s could not be found.') % _id) if s_ctx['b_url'] == '': - return Response(_('bottom pod endpoint incorrect'), 404) + return utils.format_cinder_error( + 404, _('Bottom Pod endpoint incorrect')) resp = hclient.forward_req(context, 'GET', b_headers, @@ -290,10 +290,12 @@ class VolumeController(rest.RestController): s_ctx = self._get_res_routing_ref(context, _id, request.url) if not s_ctx: - return Response(_('Resource not found'), 404) + return utils.format_cinder_error( + 404, _('Volume %s could not be found.') % _id) if s_ctx['b_url'] == '': - return Response(_('Bottom pod endpoint incorrect'), 404) + return utils.format_cinder_error( + 404, _('Bottom Pod endpoint incorrect')) b_headers = self._convert_header(t_release, b_release, @@ -351,10 +353,12 @@ class VolumeController(rest.RestController): s_ctx = self._get_res_routing_ref(context, _id, request.url) if not s_ctx: - return Response(_('Failed to find resource'), 404) + return utils.format_cinder_error( + 404, _('Volume %s could not be found.') % _id) if s_ctx['b_url'] == '': - return Response(_('bottom pod endpoint incorrect'), 404) + return utils.format_cinder_error( + 404, _('Bottom Pod endpoint incorrect')) b_headers = self._convert_header(t_release, b_release, diff --git a/tricircle/nova_apigw/controllers/action.py b/tricircle/nova_apigw/controllers/action.py index 6e1f1d3..998244b 100644 --- a/tricircle/nova_apigw/controllers/action.py +++ b/tricircle/nova_apigw/controllers/action.py @@ -23,6 +23,7 @@ import tricircle.common.client as t_client from tricircle.common import constants import tricircle.common.context as t_context from tricircle.common.i18n import _ +from tricircle.common import utils import tricircle.db.api as db_api LOG = logging.getLogger(__name__) @@ -52,13 +53,6 @@ class ActionController(rest.RestController): client = self._get_client(pod_name) return client.action_servers(context, 'stop', self.server_id) - @staticmethod - def _format_error(code, message): - pecan.response.status = code - # format error message in this form so nova client can - # correctly parse it - return {'Error': {'message': message, 'code': code}} - @expose(generic=True, template='json') def post(self, **kw): context = t_context.extract_context_from_environ() @@ -70,12 +64,14 @@ class ActionController(rest.RestController): action_handle = self.handle_map[_type] action_type = _type if not action_handle: - return self._format_error(400, _('Server action not supported')) + return utils.format_nova_error( + 400, _('Server action not supported')) server_mappings = db_api.get_bottom_mappings_by_top_id( context, self.server_id, constants.RT_SERVER) if not server_mappings: - return self._format_error(404, _('Server not found')) + return utils.format_nova_error( + 404, _('Server %s could not be found') % self.server_id) pod_name = server_mappings[0][0]['pod_name'] try: @@ -96,4 +92,4 @@ class ActionController(rest.RestController): if ex_message: message = ex_message LOG.error(message) - return self._format_error(code, message) + return utils.format_nova_error(code, message) diff --git a/tricircle/nova_apigw/controllers/aggregate.py b/tricircle/nova_apigw/controllers/aggregate.py index 55ab4c4..6842d3f 100644 --- a/tricircle/nova_apigw/controllers/aggregate.py +++ b/tricircle/nova_apigw/controllers/aggregate.py @@ -22,6 +22,8 @@ import oslo_db.exception as db_exc from tricircle.common import az_ag import tricircle.common.context as t_context import tricircle.common.exceptions as t_exc +from tricircle.common.i18n import _ +from tricircle.common import utils from tricircle.db import core from tricircle.db import models @@ -36,20 +38,25 @@ class AggregateActionController(rest.RestController): def post(self, **kw): context = t_context.extract_context_from_environ() if not context.is_admin: - pecan.abort(400, 'Admin role required to operate aggregates') - return + return utils.format_nova_error( + 403, _("Policy doesn't allow os_compute_api:os-aggregates:" + "index to be performed.")) try: with context.session.begin(): core.get_resource(context, models.Aggregate, self.aggregate_id) except t_exc.ResourceNotFound: - pecan.abort(400, 'Aggregate not found') - return + return utils.format_nova_error( + 404, _('Aggregate %s could not be found.') % self.aggregate_id) if 'add_host' in kw or 'remove_host' in kw: - pecan.abort(400, 'Add and remove host action not supported') - return + return utils.format_nova_error( + 400, _('Add and remove host action not supported')) # TODO(zhiyuan) handle aggregate metadata updating - aggregate = az_ag.get_one_ag(context, self.aggregate_id) - return {'aggregate': aggregate} + try: + aggregate = az_ag.get_one_ag(context, self.aggregate_id) + return {'aggregate': aggregate} + except Exception: + return utils.format_nova_error( + 500, _('Aggregate operation on %s failed') % self.aggregate_id) class AggregateController(rest.RestController): @@ -67,11 +74,12 @@ class AggregateController(rest.RestController): def post(self, **kw): context = t_context.extract_context_from_environ() if not context.is_admin: - pecan.abort(400, 'Admin role required to create aggregates') - return + return utils.format_nova_error( + 403, _("Policy doesn't allow os_compute_api:os-aggregates:" + "index to be performed.")) if 'aggregate' not in kw: - pecan.abort(400, 'Request body not found') - return + return utils.format_nova_error( + 400, _('aggregate is not set')) host_aggregate = kw['aggregate'] name = host_aggregate['name'].strip() @@ -85,11 +93,11 @@ class AggregateController(rest.RestController): ag_name=name, az_name=avail_zone) except db_exc.DBDuplicateEntry: - pecan.abort(409, 'Aggregate already exists') - return + return utils.format_nova_error( + 409, _('Aggregate %s already exists.') % name) except Exception: - pecan.abort(500, 'Fail to create host aggregate') - return + return utils.format_nova_error( + 500, _('Fail to create aggregate')) return {'aggregate': aggregate} @@ -101,8 +109,11 @@ class AggregateController(rest.RestController): aggregate = az_ag.get_one_ag(context, _id) return {'aggregate': aggregate} except t_exc.ResourceNotFound: - pecan.abort(404, 'Aggregate not found') - return + return utils.format_nova_error( + 404, _('Aggregate %s could not be found.') % _id) + except Exception: + return utils.format_nova_error( + 500, _('Fail to get aggregate %s') % _id) @expose(generic=True, template='json') def get_all(self): @@ -112,8 +123,7 @@ class AggregateController(rest.RestController): with context.session.begin(): aggregates = az_ag.get_all_ag(context) except Exception: - pecan.abort(500, 'Fail to get all host aggregates') - return + return utils.format_nova_error(500, _('Fail to list aggregates')) return {'aggregates': aggregates} @expose(generic=True, template='json') @@ -124,5 +134,8 @@ class AggregateController(rest.RestController): az_ag.delete_ag(context, _id) pecan.response.status = 200 except t_exc.ResourceNotFound: - pecan.abort(404, 'Aggregate not found') - return + return utils.format_nova_error( + 404, _('Aggregate %s could not be found.') % _id) + except Exception: + return utils.format_nova_error( + 500, _('Fail to delete aggregate %s') % _id) diff --git a/tricircle/nova_apigw/controllers/flavor.py b/tricircle/nova_apigw/controllers/flavor.py index 79746b9..fcd179c 100644 --- a/tricircle/nova_apigw/controllers/flavor.py +++ b/tricircle/nova_apigw/controllers/flavor.py @@ -20,6 +20,7 @@ from pecan import rest import oslo_db.exception as db_exc import tricircle.common.context as t_context +from tricircle.common.i18n import _ from tricircle.common import utils from tricircle.db import core from tricircle.db import models @@ -37,15 +38,17 @@ class FlavorManageController(rest.RestController): def post(self, **kw): context = t_context.extract_context_from_environ() if not context.is_admin: - pecan.abort(400, 'Admin role required to create flavors') - return + return utils.format_nova_error( + 403, _("Policy doesn't allow os_compute_api:os-flavor-manage " + "to be performed.")) required_fields = ['name', 'ram', 'vcpus', 'disk'] if 'flavor' not in kw: - pass + utils.format_nova_error(400, _('flavor is not set')) if not utils.validate_required_fields_set(kw['flavor'], required_fields): - pass + utils.format_nova_error( + 400, _('Invalid input for field/attribute flavor.')) flavor_dict = { 'name': kw['flavor']['name'], @@ -63,28 +66,36 @@ class FlavorManageController(rest.RestController): with context.session.begin(): flavor = core.create_resource( context, models.InstanceTypes, flavor_dict) - except db_exc.DBDuplicateEntry: - pecan.abort(409, 'Flavor already exists') - return + except db_exc.DBDuplicateEntry as e: + if 'flavorid' in e.columns: + return utils.format_nova_error( + 409, _('Flavor with ID %s already ' + 'exists.') % flavor_dict['flavorid']) + else: + return utils.format_nova_error( + 409, _('Flavor with name %s already ' + 'exists.') % flavor_dict['name']) except Exception: - pecan.abort(500, 'Fail to create flavor') - return + return utils.format_nova_error(500, _('Failed to create flavor')) return {'flavor': flavor} @expose(generic=True, template='json') def delete(self, _id): context = t_context.extract_context_from_environ() - with context.session.begin(): - flavors = core.query_resource(context, models.InstanceTypes, - [{'key': 'flavorid', - 'comparator': 'eq', - 'value': _id}], []) - if not flavors: - pecan.abort(404, 'Flavor not found') - return - core.delete_resource(context, - models.InstanceTypes, flavors[0]['id']) + try: + with context.session.begin(): + flavors = core.query_resource(context, models.InstanceTypes, + [{'key': 'flavorid', + 'comparator': 'eq', + 'value': _id}], []) + if not flavors: + return utils.format_nova_error( + 404, _('Flavor %s could not be found') % _id) + core.delete_resource(context, models.InstanceTypes, + flavors[0]['id']) + except Exception: + return utils.format_nova_error(500, _('Failed to delete flavor')) pecan.response.status = 202 return @@ -103,17 +114,17 @@ class FlavorController(rest.RestController): def post(self, **kw): context = t_context.extract_context_from_environ() if not context.is_admin: - pecan.abort(400, 'Admin role required to create flavors') - return + return utils.format_nova_error( + 403, _("Policy doesn't allow os_compute_api:os-flavor-manage " + "to be performed.")) required_fields = ['name', 'ram', 'vcpus', 'disk'] if 'flavor' not in kw: - pecan.abort(400, 'Request body not found') - return + utils.format_nova_error(400, _('flavor is not set')) if not utils.validate_required_fields_set(kw['flavor'], required_fields): - pecan.abort(400, 'Required field not set') - return + utils.format_nova_error( + 400, _('Invalid input for field/attribute flavor.')) flavor_dict = { 'name': kw['flavor']['name'], @@ -131,12 +142,17 @@ class FlavorController(rest.RestController): with context.session.begin(): flavor = core.create_resource( context, models.InstanceTypes, flavor_dict) - except db_exc.DBDuplicateEntry: - pecan.abort(409, 'Flavor already exists') - return + except db_exc.DBDuplicateEntry as e: + if 'flavorid' in e.columns: + return utils.format_nova_error( + 409, _('Flavor with ID %s already ' + 'exists.') % flavor_dict['flavorid']) + else: + return utils.format_nova_error( + 409, _('Flavor with name %s already ' + 'exists.') % flavor_dict['name']) except Exception: - pecan.abort(500, 'Fail to create flavor') - return + utils.format_nova_error(500, _('Failed to create flavor')) flavor['id'] = flavor['flavorid'] del flavor['flavorid'] @@ -163,8 +179,8 @@ class FlavorController(rest.RestController): 'comparator': 'eq', 'value': _id}], []) if not flavors: - pecan.abort(404, 'Flavor not found') - return + return utils.format_nova_error( + 404, _('Flavor %s could not be found') % _id) flavor = flavors[0] flavor['id'] = flavor['flavorid'] del flavor['flavorid'] @@ -184,15 +200,18 @@ class FlavorController(rest.RestController): def delete(self, _id): # TODO(zhiyuan) handle foreign key constraint context = t_context.extract_context_from_environ() - with context.session.begin(): - flavors = core.query_resource(context, models.InstanceTypes, - [{'key': 'flavorid', - 'comparator': 'eq', - 'value': _id}], []) - if not flavors: - pecan.abort(404, 'Flavor not found') - return - core.delete_resource(context, - models.InstanceTypes, flavors[0]['id']) + try: + with context.session.begin(): + flavors = core.query_resource(context, models.InstanceTypes, + [{'key': 'flavorid', + 'comparator': 'eq', + 'value': _id}], []) + if not flavors: + return utils.format_nova_error( + 404, _('Flavor %s could not be found') % _id) + core.delete_resource(context, + models.InstanceTypes, flavors[0]['id']) + except Exception: + return utils.format_nova_error(500, _('Failed to delete flavor')) pecan.response.status = 202 return diff --git a/tricircle/nova_apigw/controllers/image.py b/tricircle/nova_apigw/controllers/image.py index 9896c3f..9537486 100644 --- a/tricircle/nova_apigw/controllers/image.py +++ b/tricircle/nova_apigw/controllers/image.py @@ -13,7 +13,6 @@ # License for the specific language governing permissions and limitations # under the License. -import pecan from pecan import expose from pecan import rest import re @@ -22,6 +21,8 @@ import urlparse import tricircle.common.client as t_client from tricircle.common import constants import tricircle.common.context as t_context +from tricircle.common.i18n import _ +from tricircle.common import utils import tricircle.db.api as db_api @@ -141,8 +142,7 @@ class ImageController(rest.RestController): context = t_context.extract_context_from_environ() image = self.client.get_images(context, _id) if not image: - pecan.abort(404, 'Image not found') - return + return utils.format_nova_error(404, _('Image not found')) return {'image': self._construct_show_image_entry(context, image)} @expose(generic=True, template='json') diff --git a/tricircle/nova_apigw/controllers/network.py b/tricircle/nova_apigw/controllers/network.py index abbaa61..78b6a0d 100644 --- a/tricircle/nova_apigw/controllers/network.py +++ b/tricircle/nova_apigw/controllers/network.py @@ -13,12 +13,13 @@ # License for the specific language governing permissions and limitations # under the License. -import pecan from pecan import expose from pecan import rest import tricircle.common.client as t_client import tricircle.common.context as t_context +from tricircle.common.i18n import _ +from tricircle.common import utils class NetworkController(rest.RestController): @@ -38,8 +39,7 @@ class NetworkController(rest.RestController): context = t_context.extract_context_from_environ() network = self.client.get_networks(context, _id) if not network: - pecan.abort(404, 'Network not found') - return + return utils.format_nova_error(404, _('Network not found')) return {'network': self._construct_network_entry(network)} @expose(generic=True, template='json') diff --git a/tricircle/nova_apigw/controllers/server.py b/tricircle/nova_apigw/controllers/server.py index 9807bc5..30a3177 100644 --- a/tricircle/nova_apigw/controllers/server.py +++ b/tricircle/nova_apigw/controllers/server.py @@ -99,15 +99,14 @@ class ServerController(rest.RestController): mappings = db_api.get_bottom_mappings_by_top_id( context, _id, constants.RT_SERVER) if not mappings: - pecan.abort(404, 'Server not found') - return + return utils.format_nova_error( + 404, _('Instance %s could not be found.') % _id) pod, bottom_id = mappings[0] client = self._get_client(pod['pod_name']) server = client.get_servers(context, bottom_id) if not server: - self._remove_stale_mapping(context, _id) - pecan.abort(404, 'Server not found') - return + return utils.format_nova_error( + 404, _('Instance %s could not be found.') % _id) else: self._transform_network_name(server) return {'server': server} @@ -123,18 +122,18 @@ class ServerController(rest.RestController): context = t_context.extract_context_from_environ() if 'server' not in kw: - pecan.abort(400, 'Request body not found') - return + return utils.format_nova_error( + 400, _('server is not set')) if 'availability_zone' not in kw['server']: - pecan.abort(400, 'Availability zone not set') - return + return utils.format_nova_error( + 400, _('availability zone is not set')) pod, b_az = az_ag.get_pod_by_az_tenant( context, kw['server']['availability_zone'], self.project_id) if not pod: - pecan.abort(400, 'No pod bound to availability zone') - return + return utils.format_nova_error( + 500, _('Pod not configured or scheduling failure')) t_server_dict = kw['server'] self._process_metadata_quota(context, t_server_dict) @@ -155,12 +154,12 @@ class ServerController(rest.RestController): security_groups = [] for sg in kw['server']['security_groups']: if 'name' not in sg: - pecan.abort(404, 'Security group name not specified') - return + return utils.format_nova_error( + 400, _('Invalid input for field/attribute')) if sg['name'] not in top_sg_map: - pecan.abort(404, - 'Security group %s not found' % sg['name']) - return + return utils.format_nova_error( + 400, _('Unable to find security_group with name or id ' + '%s') % sg['name']) security_groups.append(sg['name']) t_sg_ids, b_sg_ids, is_news = self._handle_security_group( context, pod, top_sg_map, security_groups) @@ -172,30 +171,32 @@ class ServerController(rest.RestController): network = top_client.get_networks(context, net_info['uuid']) if not network: - pecan.abort(400, 'Network not found') - return + return utils.format_nova_error( + 400, _('Network %s could not be ' + 'found') % net_info['uuid']) if not self._check_network_server_the_same_az( network, kw['server']['availability_zone']): - pecan.abort(400, 'Network and server not in the same ' - 'availability zone') - return + return utils.format_nova_error( + 400, _('Network and server not in the same ' + 'availability zone')) subnets = top_client.list_subnets( context, [{'key': 'network_id', 'comparator': 'eq', 'value': network['id']}]) if not subnets: - pecan.abort(400, 'Network not contain subnets') - return + return utils.format_nova_error( + 400, _('Network does not contain any subnets')) t_port_id, b_port_id = self._handle_network( context, pod, network, subnets, top_sg_ids=t_sg_ids, bottom_sg_ids=b_sg_ids) elif 'port' in net_info: port = top_client.get_ports(context, net_info['port']) if not port: - pecan.abort(400, 'Port not found') - return + return utils.format_nova_error( + 400, _('Port %s could not be ' + 'found') % net_info['port']) t_port_id, b_port_id = self._handle_port( context, pod, port) server_body['networks'].append({'port': b_port_id}) @@ -749,7 +750,7 @@ class ServerController(rest.RestController): msg = str(e) LOG.exception(_LE('Quota exceeded %(msg)s'), {'msg': msg}) - pecan.abort(400, _('Quota exceeded %s') % msg) + return utils.format_nova_error(400, _('Quota exceeded %s') % msg) def _check_injected_file_quota(self, context, injected_files): """Enforce quota limits on injected files. @@ -796,15 +797,16 @@ class ServerController(rest.RestController): except t_exceptions.InvalidMetadata as e1: LOG.exception(_LE('Invalid metadata %(exception)s'), {'exception': str(e1)}) - pecan.abort(400, _('Invalid metadata')) + return utils.format_nova_error(400, _('Invalid metadata')) except t_exceptions.InvalidMetadataSize as e2: LOG.exception(_LE('Invalid metadata size %(exception)s'), {'exception': str(e2)}) - pecan.abort(400, _('Invalid metadata size')) + return utils.format_nova_error(400, _('Invalid metadata size')) except t_exceptions.MetadataLimitExceeded as e3: LOG.exception(_LE('Quota exceeded %(exception)s'), {'exception': str(e3)}) - pecan.abort(400, _('Quota exceeded in metadata')) + return utils.format_nova_error(400, + _('Quota exceeded in metadata')) def _check_metadata_properties_quota(self, context, metadata=None): """Enforce quota limits on metadata properties.""" diff --git a/tricircle/nova_apigw/controllers/volume.py b/tricircle/nova_apigw/controllers/volume.py index 1e68aec..5a8bd9b 100644 --- a/tricircle/nova_apigw/controllers/volume.py +++ b/tricircle/nova_apigw/controllers/volume.py @@ -13,7 +13,6 @@ # License for the specific language governing permissions and limitations # under the License. -import pecan from pecan import expose from pecan import rest import re @@ -23,7 +22,9 @@ from oslo_log import log as logging import tricircle.common.client as t_client from tricircle.common import constants import tricircle.common.context as t_context +from tricircle.common.i18n import _ from tricircle.common.i18n import _LE +from tricircle.common import utils import tricircle.db.api as db_api LOG = logging.getLogger(__name__) @@ -46,23 +47,23 @@ class VolumeController(rest.RestController): context = t_context.extract_context_from_environ() if 'volumeAttachment' not in kw: - pecan.abort(400, 'Request body not found') - return + return utils.format_nova_error( + 400, _('volumeAttachment is not set')) body = kw['volumeAttachment'] if 'volumeId' not in body: - pecan.abort(400, 'Volume not set') - return + return utils.format_nova_error( + 400, _('Invalid input for field/attribute volumeAttachment')) server_mappings = db_api.get_bottom_mappings_by_top_id( context, self.server_id, constants.RT_SERVER) if not server_mappings: - pecan.abort(404, 'Server not found') - return + return utils.format_nova_error(404, _('Instance %s could not be ' + 'found.') % self.server_id) volume_mappings = db_api.get_bottom_mappings_by_top_id( context, body['volumeId'], constants.RT_VOLUME) if not volume_mappings: - pecan.abort(404, 'Volume not found') - return + return utils.format_nova_error( + 404, _('Volume %s could not be found') % body['volumeId']) server_pod_name = server_mappings[0][0]['pod_name'] volume_pod_name = volume_mappings[0][0]['pod_name'] @@ -74,8 +75,8 @@ class VolumeController(rest.RestController): 'server_pod': server_pod_name, 'volume': body['volumeId'], 'volume_pod': volume_pod_name}) - pecan.abort(400, 'Server and volume not in the same pod') - return + return utils.format_nova_error( + 400, _('Server and volume not in the same pod')) device = None if 'device' in body: @@ -84,8 +85,9 @@ class VolumeController(rest.RestController): match = re.match('(^/dev/x{0,1}[a-z]{0,1}d{0,1})([a-z]+)[0-9]*$', device) if not match: - pecan.abort(400, 'Invalid device path') - return + return utils.format_nova_error( + 400, _('The supplied device path (%s) is ' + 'invalid.') % device) client = self._get_client(server_pod_name) volume = client.action_server_volumes( diff --git a/tricircle/tests/unit/nova_apigw/controllers/test_action.py b/tricircle/tests/unit/nova_apigw/controllers/test_action.py index 79f5d0d..1c1666b 100644 --- a/tricircle/tests/unit/nova_apigw/controllers/test_action.py +++ b/tricircle/tests/unit/nova_apigw/controllers/test_action.py @@ -74,6 +74,9 @@ class ActionTest(unittest.TestCase): 'resource_type': constants.RT_SERVER}) return t_server_id + def _validate_error_code(self, res, code): + self.assertEqual(code, res[res.keys()[0]]['code']) + @patch.object(pecan, 'response', new=FakeResponse) @patch.object(context, 'extract_context_from_environ') def test_action_not_supported(self, mock_context): @@ -81,9 +84,7 @@ class ActionTest(unittest.TestCase): body = {'unsupported_action': ''} res = self.controller.post(**body) - self.assertEqual('Server action not supported', - res['Error']['message']) - self.assertEqual(400, res['Error']['code']) + self._validate_error_code(res, 400) @patch.object(pecan, 'response', new=FakeResponse) @patch.object(context, 'extract_context_from_environ') @@ -92,8 +93,7 @@ class ActionTest(unittest.TestCase): body = {'os-start': ''} res = self.controller.post(**body) - self.assertEqual('Server not found', res['Error']['message']) - self.assertEqual(404, res['Error']['code']) + self._validate_error_code(res, 404) @patch.object(pecan, 'response', new=FakeResponse) @patch.object(client.Client, 'action_resources') @@ -109,27 +109,17 @@ class ActionTest(unittest.TestCase): msg='Server operation forbidden') body = {'os-start': ''} res = self.controller.post(**body) - # this is the message of HTTPForbiddenError exception - self.assertEqual('Server operation forbidden', res['Error']['message']) - # this is the code of HTTPForbiddenError exception - self.assertEqual(403, res['Error']['code']) + self._validate_error_code(res, 403) mock_action.side_effect = exceptions.ServiceUnavailable body = {'os-start': ''} res = self.controller.post(**body) - # this is the message of ServiceUnavailable exception - self.assertEqual('The service is unavailable', res['Error']['message']) - # code is 500 by default - self.assertEqual(500, res['Error']['code']) + self._validate_error_code(res, 500) mock_action.side_effect = Exception body = {'os-start': ''} res = self.controller.post(**body) - # use default message if exception's message is empty - self.assertEqual('Action os-start on server %s fails' % t_server_id, - res['Error']['message']) - # code is 500 by default - self.assertEqual(500, res['Error']['code']) + self._validate_error_code(res, 500) @patch.object(pecan, 'response', new=FakeResponse) @patch.object(client.Client, 'action_resources') diff --git a/tricircle/tests/unit/nova_apigw/controllers/test_server.py b/tricircle/tests/unit/nova_apigw/controllers/test_server.py index 3f2d7e5..0b98715 100644 --- a/tricircle/tests/unit/nova_apigw/controllers/test_server.py +++ b/tricircle/tests/unit/nova_apigw/controllers/test_server.py @@ -26,7 +26,6 @@ from oslo_utils import uuidutils from tricircle.common import constants from tricircle.common import context import tricircle.common.exceptions as t_exceptions -from tricircle.common.i18n import _ from tricircle.common import lock_handle from tricircle.common import xrpcapi from tricircle.db import api @@ -319,6 +318,9 @@ class ServerTest(unittest.TestCase): b_pods.append(b_pod) return t_pod, b_pods + def _validate_error_code(self, res, code): + self.assertEqual(code, res[res.keys()[0]]['code']) + def test_get_or_create_route(self): t_pod, b_pod = self._prepare_pod() route, is_own = self.controller._get_or_create_route( @@ -378,7 +380,7 @@ class ServerTest(unittest.TestCase): t_pod, b_pod = self._prepare_pod() port = {'id': 'top_port_id'} body = {'port': {'name': 'top_port_id'}} - _, bottom_port_id = self.controller._prepare_neutron_element( + is_new, bottom_port_id = self.controller._prepare_neutron_element( self.context, b_pod, port, 'port', body) mappings = api.get_bottom_mappings_by_top_id(self.context, 'top_port_id', 'port') @@ -526,10 +528,9 @@ class ServerTest(unittest.TestCase): self._test_handle_network_dhcp_port('10.0.0.4') @patch.object(pecan, 'response', new=FakeResponse) - @patch.object(pecan, 'abort') @patch.object(FakeClient, 'create_servers') @patch.object(context, 'extract_context_from_environ') - def test_post_with_network_az(self, mock_ctx, mock_create, mock_abort): + def test_post_with_network_az(self, mock_ctx, mock_create): t_pod, b_pod = self._prepare_pod() top_net_id = 'top_net_id' top_subnet_id = 'top_subnet_id' @@ -586,16 +587,13 @@ class ServerTest(unittest.TestCase): # update top net for test purpose, wrong az TOP_NETS[0]['availability_zone_hints'] = ['fake_az'] - self.controller.post(**body) + res = self.controller.post(**body) + self._validate_error_code(res, 400) # update top net for test purpose, correct az and wrong az TOP_NETS[0]['availability_zone_hints'] = ['b_az', 'fake_az'] - self.controller.post(**body) - - msg = 'Network and server not in the same availability zone' - # abort two times - calls = [mock.call(400, msg), mock.call(400, msg)] - mock_abort.assert_has_calls(calls) + res = self.controller.post(**body) + self._validate_error_code(res, 400) @patch.object(pecan, 'response', new=FakeResponse) @patch.object(FakeClient, 'create_servers') @@ -979,8 +977,8 @@ class ServerTest(unittest.TestCase): res['Error']['message']) self.assertEqual(404, res['Error']['code']) - @patch.object(pecan, 'abort') - def test_process_injected_file_quota(self, mock_abort): + @patch.object(pecan, 'response', new=FakeResponse) + def test_process_injected_file_quota(self): ctx = self.context.elevated() def _update_default_quota(num1, len1, len2): @@ -1017,11 +1015,8 @@ class ServerTest(unittest.TestCase): self.controller._check_injected_file_quota, ctx, injected_files) - self.controller._process_injected_file_quota(ctx, t_server_dict) - msg = _('Quota exceeded %s') % \ - t_exceptions.OnsetFileLimitExceeded.message - calls = [mock.call(400, msg)] - mock_abort.assert_has_calls(calls) + res = self.controller._process_injected_file_quota(ctx, t_server_dict) + self._validate_error_code(res, 400) _update_default_quota(len(injected_files), max_path + 1, @@ -1035,11 +1030,8 @@ class ServerTest(unittest.TestCase): self.controller._check_injected_file_quota, ctx, injected_files) - self.controller._process_injected_file_quota(ctx, t_server_dict) - msg = _('Quota exceeded %s') % \ - t_exceptions.OnsetFilePathLimitExceeded.message - calls = [mock.call(400, msg)] - mock_abort.assert_has_calls(calls) + res = self.controller._process_injected_file_quota(ctx, t_server_dict) + self._validate_error_code(res, 400) _update_default_quota(len(injected_files) + 1, max_path, @@ -1053,19 +1045,16 @@ class ServerTest(unittest.TestCase): self.controller._check_injected_file_quota, ctx, injected_files) - self.controller._process_injected_file_quota(ctx, t_server_dict) - msg = _('Quota exceeded %s') % \ - t_exceptions.OnsetFileContentLimitExceeded.message - calls = [mock.call(400, msg)] - mock_abort.assert_has_calls(calls) + res = self.controller._process_injected_file_quota(ctx, t_server_dict) + self._validate_error_code(res, 400) _update_default_quota(len(injected_files) + 1, max_path + 1, max_content) self.controller._check_injected_file_quota(ctx, injected_files) - @patch.object(pecan, 'abort') - def test_process_metadata_quota(self, mock_abort): + @patch.object(pecan, 'response', new=FakeResponse) + def test_process_metadata_quota(self): ctx = self.context.elevated() def _update_default_quota(num): @@ -1092,10 +1081,8 @@ class ServerTest(unittest.TestCase): self.assertRaises(t_exceptions.InvalidMetadata, self.controller._check_metadata_properties_quota, ctx, meta_data_items) - self.controller._process_metadata_quota(ctx, t_server_dict) - msg = _('Invalid metadata') - calls = [mock.call(400, msg)] - mock_abort.assert_has_calls(calls) + res = self.controller._process_metadata_quota(ctx, t_server_dict) + self._validate_error_code(res, 400) meta_data_items['A'] = '1' _update_default_quota(len(meta_data_items)) @@ -1110,10 +1097,8 @@ class ServerTest(unittest.TestCase): self.controller._check_metadata_properties_quota, ctx, meta_data_items) - self.controller._process_metadata_quota(ctx, t_server_dict) - msg = _('Quota exceeded in metadata') - calls = [mock.call(400, msg)] - mock_abort.assert_has_calls(calls) + res = self.controller._process_metadata_quota(ctx, t_server_dict) + self._validate_error_code(res, 400) _update_default_quota(len(meta_data_items) + 1) @@ -1125,10 +1110,8 @@ class ServerTest(unittest.TestCase): self.controller._check_metadata_properties_quota, ctx, meta_data_items) - self.controller._process_metadata_quota(ctx, t_server_dict) - msg = _('Invalid metadata size') - calls = [mock.call(400, msg)] - mock_abort.assert_has_calls(calls) + res = self.controller._process_metadata_quota(ctx, t_server_dict) + self._validate_error_code(res, 400) meta_data_items['C'] = '3' meta_data_items[string_exceed_MAX_METADATA_LEGNGTH] = '4' @@ -1136,10 +1119,8 @@ class ServerTest(unittest.TestCase): self.controller._check_metadata_properties_quota, ctx, meta_data_items) - self.controller._process_metadata_quota(ctx, t_server_dict) - msg = _('Invalid metadata size') - calls = [mock.call(400, msg)] - mock_abort.assert_has_calls(calls) + res = self.controller._process_metadata_quota(ctx, t_server_dict) + self._validate_error_code(res, 400) @patch.object(pecan, 'response', new=FakeResponse) @patch.object(context, 'extract_context_from_environ') diff --git a/tricircle/tests/unit/nova_apigw/controllers/test_volume.py b/tricircle/tests/unit/nova_apigw/controllers/test_volume.py index efac02a..f4f8c22 100644 --- a/tricircle/tests/unit/nova_apigw/controllers/test_volume.py +++ b/tricircle/tests/unit/nova_apigw/controllers/test_volume.py @@ -29,6 +29,13 @@ from tricircle.db import models from tricircle.nova_apigw.controllers import volume +class FakeResponse(object): + def __new__(cls, code=500): + cls.status = code + cls.status_code = code + return super(FakeResponse, cls).__new__(cls) + + class FakeVolume(object): def to_dict(self): pass @@ -60,10 +67,13 @@ class VolumeTest(unittest.TestCase): b_pods.append(b_pod) return t_pod, b_pods - @patch.object(pecan, 'abort') + def _validate_error_code(self, res, code): + self.assertEqual(code, res[res.keys()[0]]['code']) + + @patch.object(pecan, 'response', new=FakeResponse) @patch.object(client.Client, 'action_resources') @patch.object(context, 'extract_context_from_environ') - def test_attach_volume(self, mock_context, mock_action, mock_abort): + def test_attach_volume(self, mock_context, mock_action): mock_context.return_value = self.context mock_action.return_value = FakeVolume() @@ -112,36 +122,40 @@ class VolumeTest(unittest.TestCase): # failure case, bad request body = {'volumeAttachment': {'volumeId': t_volume2_id}} - self.controller.post(**body) + res = self.controller.post(**body) + self._validate_error_code(res, 400) + body = {'fakePara': ''} - self.controller.post(**body) + res = self.controller.post(**body) + self._validate_error_code(res, 400) + body = {'volumeAttachment': {}} - self.controller.post(**body) + res = self.controller.post(**body) + self._validate_error_code(res, 400) + # each part of path should not start with digit body = {'volumeAttachment': {'volumeId': t_volume1_id, 'device': '/dev/001disk'}} - self.controller.post(**body) + res = self.controller.post(**body) + self._validate_error_code(res, 400) + # the first part should be "dev", and only two parts are allowed body = {'volumeAttachment': {'volumeId': t_volume1_id, 'device': '/dev/vdb/disk'}} - self.controller.post(**body) + res = self.controller.post(**body) + self._validate_error_code(res, 400) + body = {'volumeAttachment': {'volumeId': t_volume1_id, 'device': '/disk/vdb'}} - self.controller.post(**body) - calls = [mock.call(400, 'Server and volume not in the same pod'), - mock.call(400, 'Request body not found'), - mock.call(400, 'Volume not set'), - mock.call(400, 'Invalid device path'), - mock.call(400, 'Invalid device path'), - mock.call(400, 'Invalid device path')] - mock_abort.assert_has_calls(calls) + res = self.controller.post(**body) + self._validate_error_code(res, 400) # failure case, resource not found body = {'volumeAttachment': {'volumeId': 'fake_volume_id'}} - self.controller.post(**body) + res = self.controller.post(**body) + self._validate_error_code(res, 404) + self.controller.server_id = 'fake_server_id' body = {'volumeAttachment': {'volumeId': t_volume1_id}} - self.controller.post(**body) - calls = [mock.call(404, 'Volume not found'), - mock.call(404, 'Server not found')] - mock_abort.assert_has_calls(calls) + res = self.controller.post(**body) + self._validate_error_code(res, 404)