diff --git a/tricircle/api/controllers/pod.py b/tricircle/api/controllers/pod.py index 5454997..a99b61e 100644 --- a/tricircle/api/controllers/pod.py +++ b/tricircle/api/controllers/pod.py @@ -105,13 +105,23 @@ class PodsController(rest.RestController): 'dc_name': dc_name, 'az_name': az_name}) except db_exc.DBDuplicateEntry as e1: - LOG.error(_LE('Record already exists: %(exception)s'), - {'exception': e1}) + LOG.exception(_LE('Record already exists on %(pod_name)s: ' + '%(exception)s'), + {'pod_name': pod_name, + 'exception': e1}) return Response(_('Record already exists'), 409) except Exception as e2: - LOG.error(_LE('Fail to create pod: %(exception)s'), - {'exception': e2}) - return Response(_('Fail to create pod'), 500) + LOG.exception(_LE('Failed to create pod: %(pod_name)s,' + 'pod_az_name: %(pod_az_name)s,' + 'dc_name: %(dc_name)s,' + 'az_name: %(az_name)s' + '%(exception)s '), + {'pod_name': pod_name, + 'pod_az_name': pod_az_name, + 'dc_name': dc_name, + 'az_name': az_name, + 'exception': e2}) + return Response(_('Failed to create pod'), 500) return {'pod': new_pod} @@ -140,9 +150,10 @@ class PodsController(rest.RestController): try: return {'pods': db_api.list_pods(context)} except Exception as e: - LOG.error(_LE('Fail to list pod: %(exception)s'), - {'exception': e}) - pecan.abort(500, _('Fail to list pod')) + LOG.exception(_LE('Failed to list all pods: %(exception)s '), + {'exception': e}) + + pecan.abort(500, _('Failed to list pods')) return @expose(generic=True, template='json') @@ -166,9 +177,12 @@ class PodsController(rest.RestController): except t_exc.ResourceNotFound: return Response(_('Pod not found'), 404) except Exception as e: - LOG.error(_LE('Fail to delete pod: %(exception)s'), - {'exception': e}) - return Response(_('Fail to delete pod'), 500) + LOG.exception(_LE('Failed to delete pod: %(pod_id)s,' + '%(exception)s'), + {'pod_id': _id, + 'exception': e}) + + return Response(_('Failed to delete pod'), 500) def _get_top_region(self, ctx): top_region_name = '' @@ -179,7 +193,10 @@ class PodsController(rest.RestController): for pod in pods: if pod['az_name'] == '' and pod['pod_name'] != '': return pod['pod_name'] - except Exception: + except Exception as e: + LOG.exception(_LE('Failed to get top region: %(exception)s '), + {'exception': e}) + return top_region_name return top_region_name @@ -222,9 +239,12 @@ class BindingsController(rest.RestController): except t_exc.ResourceNotFound: return Response(_('pod_id not found in pod'), 422) except Exception as e: - LOG.error(_LE('Fail to create pod binding: %(exception)s'), - {'exception': e}) - pecan.abort(500, _('Fail to create pod binding')) + LOG.exception(_LE('Failed to get_resource for pod_id: ' + '%(pod_id)s ,' + '%(exception)s '), + {'pod_id': pod_id, + 'exception': e}) + pecan.abort(500, _('Failed to create pod binding')) return try: @@ -240,9 +260,9 @@ class BindingsController(rest.RestController): except db_exc.DBReferenceError: return Response(_('DB reference not exists in pod'), 422) except Exception as e: - LOG.error(_LE('Fail to create pod binding: %(exception)s'), - {'exception': e}) - pecan.abort(500, _('Fail to create pod binding')) + LOG.exception(_LE('Failed to create pod binding: %(exception)s '), + {'exception': e}) + pecan.abort(500, _('Failed to create pod binding')) return return {'pod_binding': pod_binding} diff --git a/tricircle/cinder_apigw/controllers/volume.py b/tricircle/cinder_apigw/controllers/volume.py index 51cb959..7a1854b 100644 --- a/tricircle/cinder_apigw/controllers/volume.py +++ b/tricircle/cinder_apigw/controllers/volume.py @@ -132,9 +132,18 @@ class VolumeController(rest.RestController): 'project_id': self.tenant_id, 'resource_type': cons.RT_VOLUME}) except Exception as e: - LOG.error(_LE('Fail to create volume: %(exception)s'), - {'exception': e}) - return Response(_('Failed to create volume'), 500) + LOG.exception(_LE('Failed to create volume ' + 'resource routing' + 'top_id: %(top_id)s ,' + 'bottom_id: %(bottom_id)s ,' + 'pod_id: %(pod_id)s ,' + '%(exception)s '), + {'top_id': b_vol_ret['id'], + 'bottom_id': b_vol_ret['id'], + 'pod_id': pod['pod_id'], + 'exception': e}) + return Response(_('Failed to create volume ' + 'resource routing'), 500) ret_vol = self._convert_object(b_release, t_release, b_vol_ret, diff --git a/tricircle/tests/functional/api/controllers/test_pod.py b/tricircle/tests/functional/api/controllers/test_pod.py index 231d5ee..a581f9e 100644 --- a/tricircle/tests/functional/api/controllers/test_pod.py +++ b/tricircle/tests/functional/api/controllers/test_pod.py @@ -12,15 +12,14 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. - from mock import patch - import pecan from pecan.configuration import set_config from pecan.testing import load_test_app from oslo_config import cfg from oslo_config import fixture as fixture_config +import oslo_db.exception as db_exc from tricircle.api import app from tricircle.common import az_ag @@ -107,6 +106,52 @@ class TestPodController(API_FunctionalTest): self.assertEqual(response.status_int, test_pod['expected_error']) + def fake_create_ag_az(context, ag_name, az_name): + raise db_exc.DBDuplicateEntry + + @patch.object(context, 'is_admin_context', + new=fake_is_admin) + @patch.object(az_ag, 'create_ag_az', + new=fake_create_ag_az) + def test_post_dup_db_exception(self): + pods = [ + { + "pod": + { + "pod_name": "Pod1", + "pod_az_name": "az1", + "dc_name": "dc1", + "az_name": "AZ1" + }, + "expected_error": 409 + }, + ] + + self._test_and_check(pods) + + def fake_create_ag_az_exp(context, ag_name, az_name): + raise Exception + + @patch.object(context, 'is_admin_context', + new=fake_is_admin) + @patch.object(core, 'create_resource', + new=fake_create_ag_az_exp) + def test_post_exception(self): + pods = [ + { + "pod": + { + "pod_name": "Pod1", + "pod_az_name": "az1", + "dc_name": "dc1", + "az_name": "AZ1" + }, + "expected_error": 500 + }, + ] + + self._test_and_check(pods) + @patch.object(context, 'is_admin_context', new=fake_is_admin) def test_post_invalid_input(self): diff --git a/tricircle/tests/functional/cinder_apigw/controllers/test_volume.py b/tricircle/tests/functional/cinder_apigw/controllers/test_volume.py index b669168..edfe1d6 100644 --- a/tricircle/tests/functional/cinder_apigw/controllers/test_volume.py +++ b/tricircle/tests/functional/cinder_apigw/controllers/test_volume.py @@ -219,7 +219,6 @@ class TestVolumeController(CinderVolumeFunctionalTest): def test_post_error_case(self): volumes = [ - # no 'volume' parameter { "volume_xxx": { @@ -260,6 +259,32 @@ class TestVolumeController(CinderVolumeFunctionalTest): self._test_and_check(volumes, 'my_tenant_id') + def fake_create_resource(context, ag_name, az_name): + raise Exception + + @patch.object(hclient, 'forward_req', + new=fake_volumes_forward_req) + @patch.object(core, 'create_resource', + new=fake_create_resource) + def test_post_exception(self): + volumes = [ + # no 'volume' parameter + { + "volume": + { + "name": 'vol_1', + "availability_zone": FAKE_AZ, + "attach_status": "detached", + "volume_type": '', + "project_id": 'my_tenant_id', + "metadata": {} + }, + "expected_error": 500 + } + ] + + self._test_and_check(volumes, 'my_tenant_id') + @patch.object(hclient, 'forward_req', new=fake_volumes_forward_req) def test_post_one_and_get_one(self): diff --git a/tricircle/tests/unit/network/test_plugin.py b/tricircle/tests/unit/network/test_plugin.py index 2ed8a0e..52968f3 100644 --- a/tricircle/tests/unit/network/test_plugin.py +++ b/tricircle/tests/unit/network/test_plugin.py @@ -381,7 +381,12 @@ class FakeQuery(object): keys = [] values = [] for e in criteria: - if not isinstance(e.right, elements.Null): + if not hasattr(e, 'right') and isinstance(e, elements.False_): + # filter is a single False value, set key to a 'INVALID_FIELD' + # then no records will be returned + keys.append('INVALID_FIELD') + values.append(False) + elif not isinstance(e.right, elements.Null): _filter.append(e) else: if e.left.name == 'network_id' and (