From 3a34bae0fe8824fc597e81f1f43e32c54f2cf4ca Mon Sep 17 00:00:00 2001 From: Sebastian Wagner Date: Tue, 24 Apr 2018 16:52:35 +0200 Subject: [PATCH] mgr/dashboard: Applied Exception handling to RBDs * DashboardTestCase: Removed waiter-thread Signed-off-by: Sebastian Wagner --- qa/tasks/mgr/dashboard/helper.py | 62 +++++++++------------ qa/tasks/mgr/dashboard/test_rbd.py | 20 ++++--- src/pybind/mgr/dashboard/controllers/rbd.py | 29 +++++----- 3 files changed, 53 insertions(+), 58 deletions(-) diff --git a/qa/tasks/mgr/dashboard/helper.py b/qa/tasks/mgr/dashboard/helper.py index 4640fc9b98139..f6f524b51f68c 100644 --- a/qa/tasks/mgr/dashboard/helper.py +++ b/qa/tasks/mgr/dashboard/helper.py @@ -166,53 +166,41 @@ class DashboardTestCase(MgrTestCase): task_name = res['name'] task_metadata = res['metadata'] - class Waiter(threading.Thread): - def __init__(self, task_name, task_metadata): - super(Waiter, self).__init__() - self.task_name = task_name - self.task_metadata = task_metadata - self.ev = threading.Event() - self.abort = False - self.res_task = None - - def run(self): - running = True - while running and not self.abort: - log.info("task (%s, %s) is still executing", self.task_name, - self.task_metadata) - time.sleep(1) - res = cls._get('/api/task?name={}'.format(self.task_name)) - for task in res['finished_tasks']: - if task['metadata'] == self.task_metadata: - # task finished - running = False - self.res_task = task - self.ev.set() - - thread = Waiter(task_name, task_metadata) - thread.start() - status = thread.ev.wait(timeout) - if not status: - # timeout expired - thread.abort = True - thread.join() - raise Exception("Waiting for task ({}, {}) to finish timed out" - .format(task_name, task_metadata)) + retries = int(timeout) + res_task = None + while retries > 0 and not res_task: + retries -= 1 + log.info("task (%s, %s) is still executing", task_name, + task_metadata) + time.sleep(1) + _res = cls._get('/api/task?name={}'.format(task_name)) + cls._assertEq(cls._resp.status_code, 200) + executing_tasks = [task for task in _res['executing_tasks'] if + task['metadata'] == task_metadata] + finished_tasks = [task for task in _res['finished_tasks'] if + task['metadata'] == task_metadata] + if not executing_tasks and finished_tasks: + res_task = finished_tasks[0] + + if retries <= 0: + raise Exception("Waiting for task ({}, {}) to finish timed out. {}" + .format(task_name, task_metadata, _res)) + log.info("task (%s, %s) finished", task_name, task_metadata) - if thread.res_task['success']: + if res_task['success']: if method == 'POST': cls._resp.status_code = 201 elif method == 'PUT': cls._resp.status_code = 200 elif method == 'DELETE': cls._resp.status_code = 204 - return thread.res_task['ret_value'] + return res_task['ret_value'] else: - if 'status' in thread.res_task['exception']: - cls._resp.status_code = thread.res_task['exception']['status'] + if 'status' in res_task['exception']: + cls._resp.status_code = res_task['exception']['status'] else: cls._resp.status_code = 500 - return thread.res_task['exception'] + return res_task['exception'] @classmethod def _task_post(cls, url, data=None, timeout=60): diff --git a/qa/tasks/mgr/dashboard/test_rbd.py b/qa/tasks/mgr/dashboard/test_rbd.py index d8977bb264dc3..80705aaac9113 100644 --- a/qa/tasks/mgr/dashboard/test_rbd.py +++ b/qa/tasks/mgr/dashboard/test_rbd.py @@ -261,8 +261,11 @@ class RbdTest(DashboardTestCase): res = self.create_image('rbd', 'test_rbd_twice', 10240) self.assertStatus(400) - self.assertEqual(res, {"errno": 17, "status": 400, "component": "rbd", - "detail": "[errno 17] error creating image"}) + self.assertEqual(res, {"code": '17', 'status': 400, "component": "rbd", + "detail": "[errno 17] error creating image", + 'task': {'name': 'rbd/create', + 'metadata': {'pool_name': 'rbd', + 'image_name': 'test_rbd_twice'}}}) self.remove_image('rbd', 'test_rbd_twice') self.assertStatus(204) @@ -317,8 +320,11 @@ class RbdTest(DashboardTestCase): def test_delete_non_existent_image(self): res = self.remove_image('rbd', 'i_dont_exist') self.assertStatus(400) - self.assertEqual(res, {"errno": 2, "status": 400, "component": "rbd", - "detail": "[errno 2] error removing image"}) + self.assertEqual(res, {u'code': u'2', "status": 400, "component": "rbd", + "detail": "[errno 2] error removing image", + 'task': {'name': 'rbd/delete', + 'metadata': {'pool_name': 'rbd', + 'image_name': 'i_dont_exist'}}}) def test_image_delete(self): self.create_image('rbd', 'delete_me', 2**30) @@ -473,8 +479,8 @@ class RbdTest(DashboardTestCase): res = self.remove_image('rbd', 'cimg') self.assertStatus(400) - self.assertIn('errno', res) - self.assertEqual(res['errno'], 39) + self.assertIn('code', res) + self.assertEqual(res['code'], '39') self.remove_image('rbd', 'cimg-clone') self.assertStatus(204) @@ -525,7 +531,7 @@ class RbdTest(DashboardTestCase): self.assertIsNotNone(img['parent']) self.flatten_image('rbd_iscsi', 'img1_snapf_clone') - self.assertStatus(200) + self.assertStatus([200, 201]) img = self._get('/api/block/image/rbd_iscsi/img1_snapf_clone') self.assertStatus(200) diff --git a/src/pybind/mgr/dashboard/controllers/rbd.py b/src/pybind/mgr/dashboard/controllers/rbd.py index e0b8cf536e943..557e3c6af379d 100644 --- a/src/pybind/mgr/dashboard/controllers/rbd.py +++ b/src/pybind/mgr/dashboard/controllers/rbd.py @@ -4,30 +4,27 @@ from __future__ import absolute_import import math +from functools import partial + import cherrypy -import rados import rbd from . import ApiController, AuthRequired, RESTController, Task from .. import mgr from ..services.ceph_service import CephService from ..tools import ViewCache +from ..services.exception import handle_rados_error, handle_rbd_error, \ + serialize_dashboard_exception -# pylint: disable=inconsistent-return-statements -def _rbd_exception_handler(ex): - if isinstance(ex, rbd.OSError): - return {'status': 409, 'detail': str(ex), 'errno': ex.errno, - 'component': 'rbd'} - elif isinstance(ex, rados.OSError): - return {'status': 409, 'detail': str(ex), 'errno': ex.errno, - 'component': 'rados'} - raise ex - - +# pylint: disable=not-callable def RbdTask(name, metadata, wait_for): - return Task("rbd/{}".format(name), metadata, wait_for, - _rbd_exception_handler) + def composed_decorator(func): + func = handle_rados_error('pool')(func) + func = handle_rbd_error()(func) + return Task("rbd/{}".format(name), metadata, wait_for, + partial(serialize_dashboard_exception, include_http_status=True))(func) + return composed_decorator def _rbd_call(pool_name, func, *args, **kwargs): @@ -254,9 +251,13 @@ class Rbd(RESTController): result.append({'status': status, 'value': value, 'pool_name': pool}) return result + @handle_rbd_error() + @handle_rados_error('pool') def list(self, pool_name=None): return self._rbd_list(pool_name) + @handle_rbd_error() + @handle_rados_error('pool') def get(self, pool_name, image_name): ioctx = mgr.rados.open_ioctx(pool_name) try: -- 2.39.5