From 281fadc15c5e257e72514957701a5571a83d8c38 Mon Sep 17 00:00:00 2001 From: Abhishek Kekane Date: Mon, 15 Feb 2021 09:04:17 +0000 Subject: [PATCH] New API /v2/images/{id}/tasks Added new API /v2/images/{id}/tasks to show tasks associated with image. This API will return list of tasks associated for valid image else returns 404 not found if image is not present. This API also initiates task scrubbing before returning tasks to user. Implements: blueprint messages-api Change-Id: Ib3cacb4dd4d75de32e539f8a3b48bdaa762e6d8e --- glance/api/v2/images.py | 13 +++ glance/api/v2/router.py | 5 + glance/db/simple/api.py | 20 +++- glance/db/sqlalchemy/api.py | 37 ++++++++ glance/tests/functional/db/base.py | 71 ++++++++++++++ glance/tests/functional/v2/test_images.py | 29 ++++++ glance/tests/unit/test_db.py | 83 ++++++++++++++-- glance/tests/unit/v2/test_images_resource.py | 99 ++++++++++++++++++++ 8 files changed, 347 insertions(+), 10 deletions(-) diff --git a/glance/api/v2/images.py b/glance/api/v2/images.py index a5b383c12e..be52973f1f 100644 --- a/glance/api/v2/images.py +++ b/glance/api/v2/images.py @@ -441,6 +441,19 @@ class ImagesController(object): except exception.NotAuthenticated as e: raise webob.exc.HTTPUnauthorized(explanation=e.msg) + def get_task_info(self, req, image_id): + image_repo = self.gateway.get_repo(req.context) + try: + # NOTE (abhishekk): Just to check image is valid + image = image_repo.get(image_id) + except (exception.NotFound, exception.Forbidden): + raise webob.exc.HTTPNotFound() + + tasks = self.db_api.tasks_get_by_image(req.context, + image.image_id) + + return {"tasks": tasks} + @utils.mutating def update(self, req, image_id, changes): image_repo = self.gateway.get_repo(req.context) diff --git a/glance/api/v2/router.py b/glance/api/v2/router.py index 22e8713187..2055d5cbdc 100644 --- a/glance/api/v2/router.py +++ b/glance/api/v2/router.py @@ -416,6 +416,11 @@ class API(wsgi.Router): action='show', conditions={'method': ['GET']}, body_reject=True) + mapper.connect('/images/{image_id}/tasks', + controller=images_resource, + action='get_task_info', + conditions={'method': ['GET']}, + body_reject=True) mapper.connect('/images/{image_id}', controller=images_resource, action='delete', diff --git a/glance/db/simple/api.py b/glance/db/simple/api.py index 2bdc633a6b..b5d416b229 100644 --- a/glance/db/simple/api.py +++ b/glance/db/simple/api.py @@ -170,16 +170,16 @@ def _task_format(task_id, **values): task = { 'id': task_id, 'type': 'import', - 'status': 'pending', + 'status': values.get('status', 'pending'), 'owner': None, 'expires_at': None, 'created_at': dt, 'updated_at': dt, 'deleted_at': None, 'deleted': False, - 'image_id': None, - 'request_id': None, - 'user_id': None, + 'image_id': values.get('image_id', None), + 'request_id': values.get('request_id', None), + 'user_id': values.get('user_id', None), } task.update(values) return task @@ -487,6 +487,18 @@ def image_get(context, image_id, session=None, force_show_deleted=False, return image +@log_call +def tasks_get_by_image(context, image_id): + db_tasks = DATA['tasks'] + tasks = [] + for task in db_tasks: + if db_tasks[task]['image_id'] == image_id: + if _is_task_visible(context, db_tasks[task]): + tasks.append(db_tasks[task]) + + return tasks + + @log_call def image_get_all(context, filters=None, marker=None, limit=None, sort_key=None, sort_dir=None, diff --git a/glance/db/sqlalchemy/api.py b/glance/db/sqlalchemy/api.py index 33f747e33e..15698e3357 100644 --- a/glance/db/sqlalchemy/api.py +++ b/glance/db/sqlalchemy/api.py @@ -1661,6 +1661,43 @@ def task_get(context, task_id, session=None, force_show_deleted=False): return _task_format(task_ref, task_ref.info) +def tasks_get_by_image(context, image_id, session=None): + """Fetch all tasks associated with image_id""" + tasks = [] + session = session or get_session() + _task_soft_delete(context, session=session) + + query = session.query(models.Task).options( + sa_orm.joinedload(models.Task.info) + ).filter_by(image_id=image_id) + + expires_at = models.Task.expires_at + query = query.filter(expires_at >= timeutils.utcnow()) + updated_at = models.Task.updated_at + query.filter( + updated_at <= (timeutils.utcnow() + + datetime.timedelta(hours=CONF.task.task_time_to_live))) + + if not context.can_see_deleted: + query = query.filter_by(deleted=False) + + try: + task_refs = query.all() + except sa_orm.exc.NoResultFound: + LOG.debug("No task found for image with ID %s", image_id) + return tasks + + for task_ref in task_refs: + # Make sure the task is visible + if not _is_task_visible(context, task_ref): + msg = "Task %s is not visible, excluding" % task_ref.id + LOG.debug(msg) + continue + tasks.append(_task_format(task_ref, task_ref.info)) + + return tasks + + def task_delete(context, task_id, session=None): """Delete a task""" session = session or get_session() diff --git a/glance/tests/functional/db/base.py b/glance/tests/functional/db/base.py index 66731a511f..1897edc146 100644 --- a/glance/tests/functional/db/base.py +++ b/glance/tests/functional/db/base.py @@ -1726,6 +1726,77 @@ class TaskTests(test_utils.BaseTestCase): self.assertEqual(fixture['message'], task['message']) self.assertEqual(expires_at, task['expires_at']) + def _test_task_get_by_image(self, expired=False, deleted=False, + other_owner=False): + expires_at = timeutils.utcnow() + if expired is False: + expires_at += datetime.timedelta(hours=1) + elif expired is None: + # This is the case where we haven't even processed the task + # to give it an expiry time. + expires_at = None + image_id = str(uuid.uuid4()) + fixture = { + 'owner': other_owner and 'notme!' or self.context.owner, + 'type': 'import', + 'status': 'pending', + 'input': '{"loc": "fake"}', + 'result': "{'image_id': %s}" % image_id, + 'message': 'blah', + 'expires_at': expires_at, + 'image_id': image_id, + 'user_id': 'me', + 'request_id': 'reqid', + } + + new_task = self.db_api.task_create(self.adm_context, fixture) + if deleted: + self.db_api.task_delete(self.context, new_task['id']) + return (new_task['id'], + self.db_api.tasks_get_by_image(self.context, image_id)) + + def test_task_get_by_image_not_expired(self): + # Make sure we get back the task + task_id, tasks = self._test_task_get_by_image(expired=False) + self.assertEqual(1, len(tasks)) + self.assertEqual(task_id, tasks[0]['id']) + + def test_task_get_by_image_expired(self): + # Make sure we do not retrieve the expired task + task_id, tasks = self._test_task_get_by_image(expired=True) + self.assertEqual(0, len(tasks)) + + # We should have deleted the task while querying for it, so make + # sure that our task is now marked as deleted. + tasks = self.db_api.task_get_all(self.adm_context) + self.assertEqual(1, len(tasks)) + self.assertEqual(task_id, tasks[0]['id']) + self.assertTrue(tasks[0]['deleted']) + + def test_task_get_by_image_no_expiry(self): + # Make sure we do not retrieve the expired task + task_id, tasks = self._test_task_get_by_image(expired=None) + self.assertEqual(0, len(tasks)) + + # The task should not have been retrieved at all above, + # but it's also not deleted because it doesn't have an expiry, + # so it should still be in the DB. + tasks = self.db_api.task_get_all(self.adm_context) + self.assertEqual(1, len(tasks)) + self.assertEqual(task_id, tasks[0]['id']) + self.assertFalse(tasks[0]['deleted']) + self.assertIsNone(tasks[0]['expires_at']) + + def test_task_get_by_image_deleted(self): + task_id, tasks = self._test_task_get_by_image(deleted=True) + # We cannot see the deleted tasks + self.assertEqual(0, len(tasks)) + + def test_task_get_by_image_not_mine(self): + task_id, tasks = self._test_task_get_by_image(other_owner=True) + # We cannot see tasks we do not own + self.assertEqual(0, len(tasks)) + def test_task_get_all(self): now = timeutils.utcnow() then = now + datetime.timedelta(days=365) diff --git a/glance/tests/functional/v2/test_images.py b/glance/tests/functional/v2/test_images.py index f32a549681..baa3084511 100644 --- a/glance/tests/functional/v2/test_images.py +++ b/glance/tests/functional/v2/test_images.py @@ -5434,6 +5434,7 @@ class TestImagesMultipleBackend(functional.MultipleBackendFunctionalTest): 'stores': ['file1']}) response = requests.post(path, headers=headers, data=data) self.assertEqual(http.ACCEPTED, response.status_code) + import_reqid = response.headers['X-Openstack-Request-Id'] # Verify image is in active state and checksum is set # NOTE(abhishekk): As import is a async call we need to provide @@ -5465,6 +5466,19 @@ class TestImagesMultipleBackend(functional.MultipleBackendFunctionalTest): self.assertEqual(http.OK, response.status_code) self.assertIn('file1', jsonutils.loads(response.text)['stores']) + # Ensure image has one task associated with it + path = self._url('/v2/images/%s/tasks' % image_id) + response = requests.get(path, headers=self._headers()) + self.assertEqual(http.OK, response.status_code) + tasks = jsonutils.loads(response.text)['tasks'] + self.assertEqual(1, len(tasks)) + for task in tasks: + self.assertEqual(image_id, task['image_id']) + user_id = response.request.headers.get( + 'X-User-Id') + self.assertEqual(user_id, task['user_id']) + self.assertEqual(import_reqid, task['request_id']) + # Copy newly created image to file2 and file3 stores path = self._url('/v2/images/%s/import' % image_id) headers = self._headers({ @@ -5477,6 +5491,7 @@ class TestImagesMultipleBackend(functional.MultipleBackendFunctionalTest): 'stores': ['file2', 'file3']}) response = requests.post(path, headers=headers, data=data) self.assertEqual(http.ACCEPTED, response.status_code) + copy_reqid = response.headers['X-Openstack-Request-Id'] # Verify image is copied # NOTE(abhishekk): As import is a async call we need to provide @@ -5496,6 +5511,20 @@ class TestImagesMultipleBackend(functional.MultipleBackendFunctionalTest): self.assertIn('file2', jsonutils.loads(response.text)['stores']) self.assertIn('file3', jsonutils.loads(response.text)['stores']) + # Ensure image has two tasks associated with it + path = self._url('/v2/images/%s/tasks' % image_id) + response = requests.get(path, headers=self._headers()) + self.assertEqual(http.OK, response.status_code) + tasks = jsonutils.loads(response.text)['tasks'] + self.assertEqual(2, len(tasks)) + expected_reqids = [copy_reqid, import_reqid] + for task in tasks: + self.assertEqual(image_id, task['image_id']) + user_id = response.request.headers.get( + 'X-User-Id') + self.assertEqual(user_id, task['user_id']) + self.assertEqual(expected_reqids.pop(), task['request_id']) + # Deleting image should work path = self._url('/v2/images/%s' % image_id) response = requests.delete(path, headers=self._headers()) diff --git a/glance/tests/unit/test_db.py b/glance/tests/unit/test_db.py index 6c6ba0fa16..7742bc7cd2 100644 --- a/glance/tests/unit/test_db.py +++ b/glance/tests/unit/test_db.py @@ -55,6 +55,11 @@ CHECKSUM = '93264c3edf5972c9f1cb309543d38a5c' CHCKSUM1 = '43264c3edf4972c9f1cb309543d38a55' +TASK_ID_1 = 'b3006bd0-461e-4228-88ea-431c14e918b4' +TASK_ID_2 = '07b6b562-6770-4c8b-a649-37a515144ce9' +TASK_ID_3 = '72d16bb6-4d70-48a5-83fe-14bb842dc737' + + def _db_fixture(id, **kwargs): obj = { 'id': id, @@ -89,17 +94,24 @@ def _db_image_member_fixture(image_id, member_id, **kwargs): return obj -def _db_task_fixture(task_id, type, status, **kwargs): +def _db_task_fixture(task_id, **kwargs): + default_datetime = timeutils.utcnow() obj = { 'id': task_id, - 'type': type, - 'status': status, - 'input': None, + 'status': kwargs.get('status', 'pending'), + 'type': 'import', + 'input': kwargs.get('input', {}), 'result': None, 'owner': None, + 'image_id': kwargs.get('image_id'), + 'user_id': kwargs.get('user_id'), + 'request_id': kwargs.get('request_id'), 'message': None, - 'deleted': False, - 'expires_at': timeutils.utcnow() + datetime.timedelta(days=365) + 'expires_at': default_datetime + datetime.timedelta(days=365), + 'created_at': default_datetime, + 'updated_at': default_datetime, + 'deleted_at': None, + 'deleted': False } obj.update(kwargs) return obj @@ -136,6 +148,53 @@ class TestImageRepo(test_utils.BaseTestCase): ] [self.db.image_create(None, image) for image in self.images] + # Create tasks associated with image + self.tasks = [ + _db_task_fixture( + TASK_ID_1, image_id=UUID1, status='completed', + input={ + "image_id": UUID1, + "import_req": { + "method": { + "name": "glance-direct" + }, + "backend": ["fake-store"] + }, + }, + user_id=USER1, + request_id='fake-request-id', + ), + _db_task_fixture( + TASK_ID_2, image_id=UUID1, status='completed', + input={ + "image_id": UUID1, + "import_req": { + "method": { + "name": "copy-image" + }, + "all_stores": True, + "all_stores_must_succeed": False, + "backend": ["fake-store", "fake_store_1"] + }, + }, + user_id=USER1, + request_id='fake-request-id', + ), + _db_task_fixture( + TASK_ID_3, status='completed', + input={ + "image_id": UUID2, + "import_req": { + "method": { + "name": "glance-direct" + }, + "backend": ["fake-store"] + }, + }, + ), + ] + [self.db.task_create(None, task) for task in self.tasks] + self.db.image_tag_set_all(None, UUID1, ['ping', 'pong']) def _create_image_members(self): @@ -156,6 +215,18 @@ class TestImageRepo(test_utils.BaseTestCase): self.assertEqual(256, image.size) self.assertEqual(TENANT1, image.owner) + def test_tasks_get_by_image(self): + tasks = self.db.tasks_get_by_image(self.context, UUID1) + self.assertEqual(2, len(tasks)) + for task in tasks: + self.assertEqual(USER1, task['user_id']) + self.assertEqual('fake-request-id', task['request_id']) + self.assertEqual(UUID1, task['image_id']) + + def test_tasks_get_by_image_not_exists(self): + tasks = self.db.tasks_get_by_image(self.context, UUID3) + self.assertEqual(0, len(tasks)) + def test_location_value(self): image = self.image_repo.get(UUID3) self.assertEqual(UUID3_LOCATION, image.locations[0]['url']) diff --git a/glance/tests/unit/v2/test_images_resource.py b/glance/tests/unit/v2/test_images_resource.py index 2ba7656531..ab79df51cb 100644 --- a/glance/tests/unit/v2/test_images_resource.py +++ b/glance/tests/unit/v2/test_images_resource.py @@ -35,6 +35,7 @@ import glance.api.v2.image_actions import glance.api.v2.images from glance.common import exception from glance.common import store_utils +from glance.common import timeutils from glance import domain import glance.schema from glance.tests.unit import base @@ -73,6 +74,11 @@ MULTIHASH1 = hashlib.sha512(b'glance').hexdigest() MULTIHASH2 = hashlib.sha512(b'image_service').hexdigest() +TASK_ID_1 = 'b3006bd0-461e-4228-88ea-431c14e918b4' +TASK_ID_2 = '07b6b562-6770-4c8b-a649-37a515144ce9' +TASK_ID_3 = '72d16bb6-4d70-48a5-83fe-14bb842dc737' + + def _db_fixture(id, **kwargs): obj = { 'id': id, @@ -99,6 +105,29 @@ def _db_fixture(id, **kwargs): return obj +def _db_task_fixtures(task_id, **kwargs): + default_datetime = timeutils.utcnow() + obj = { + 'id': task_id, + 'status': kwargs.get('status', 'pending'), + 'type': 'import', + 'input': kwargs.get('input', {}), + 'result': None, + 'owner': None, + 'image_id': kwargs.get('image_id'), + 'user_id': kwargs.get('user_id'), + 'request_id': kwargs.get('request_id'), + 'message': None, + 'expires_at': default_datetime + datetime.timedelta(days=365), + 'created_at': default_datetime, + 'updated_at': default_datetime, + 'deleted_at': None, + 'deleted': False + } + obj.update(kwargs) + return obj + + def _domain_fixture(id, **kwargs): properties = { 'image_id': id, @@ -227,6 +256,53 @@ class TestImagesController(base.IsolatedUnitTest): ] [self.db.image_create(None, image) for image in self.images] + # Create tasks associated with image + self.tasks = [ + _db_task_fixtures( + TASK_ID_1, image_id=UUID1, status='completed', + input={ + "image_id": UUID1, + "import_req": { + "method": { + "name": "glance-direct" + }, + "backend": ["fake-store"] + }, + }, + user_id='fake-user-id', + request_id='fake-request-id', + ), + _db_task_fixtures( + TASK_ID_2, image_id=UUID1, status='completed', + input={ + "image_id": UUID1, + "import_req": { + "method": { + "name": "copy-image" + }, + "all_stores": True, + "all_stores_must_succeed": False, + "backend": ["fake-store", "fake_store_1"] + }, + }, + user_id='fake-user-id', + request_id='fake-request-id', + ), + _db_task_fixtures( + TASK_ID_3, status='completed', + input={ + "image_id": UUID2, + "import_req": { + "method": { + "name": "glance-direct" + }, + "backend": ["fake-store"] + }, + }, + ), + ] + [self.db.task_create(None, task) for task in self.tasks] + self.db.image_tag_set_all(None, UUID1, ['ping', 'pong']) def _create_image_members(self): @@ -697,6 +773,29 @@ class TestImagesController(base.IsolatedUnitTest): self.assertRaises(webob.exc.HTTPNotFound, self.controller.show, request, UUID4) + def test_get_task_info(self): + request = unit_test_utils.get_fake_request() + output = self.controller.get_task_info(request, image_id=UUID1) + # NOTE Here we have only tasks associated with the image and not + # other task which has not stored image_id, user_id and + # request_id in tasks database table. + self.assertEqual(2, len(output['tasks'])) + for task in output['tasks']: + self.assertEqual(UUID1, task['image_id']) + self.assertEqual('fake-user-id', task['user_id']) + self.assertEqual('fake-request-id', task['request_id']) + + def test_get_task_info_no_tasks(self): + request = unit_test_utils.get_fake_request() + output = self.controller.get_task_info(request, image_id=UUID2) + self.assertEqual([], output['tasks']) + + def test_get_task_info_raises_not_found(self): + request = unit_test_utils.get_fake_request() + self.assertRaises(webob.exc.HTTPNotFound, + self.controller.get_task_info, request, + 'fake-image-id') + def test_image_import_raises_conflict_if_container_format_is_none(self): request = unit_test_utils.get_fake_request()