diff --git a/glance/db/__init__.py b/glance/db/__init__.py index 50e3e2ff3f..8b8665b4a1 100644 --- a/glance/db/__init__.py +++ b/glance/db/__init__.py @@ -353,8 +353,22 @@ class ImageMemberRepo(object): raise exception.Duplicate(msg) image_member_values = self._format_image_member_to_db(image_member) - new_values = self.db_api.image_member_create(self.context, - image_member_values) + # Note(shalq): find the image member including the member marked with + # deleted. We will use only one record to represent membership between + # the same image and member. The record of the deleted image member + # will be reused, if it exists, update its properties instead of + # creating a new one. + members = self.db_api.image_member_find(self.context, + image_id=self.image.image_id, + member=image_member.member_id, + include_deleted=True) + if members: + new_values = self.db_api.image_member_update(self.context, + members[0]['id'], + image_member_values) + else: + new_values = self.db_api.image_member_create(self.context, + image_member_values) image_member.created_at = new_values['created_at'] image_member.updated_at = new_values['updated_at'] image_member.id = new_values['id'] diff --git a/glance/db/registry/api.py b/glance/db/registry/api.py index ff1820d835..c236492870 100644 --- a/glance/db/registry/api.py +++ b/glance/db/registry/api.py @@ -188,15 +188,24 @@ def image_member_delete(client, memb_id, session=None): @_get_client -def image_member_find(client, image_id=None, member=None, status=None): - """Find all members that meet the given criteria +def image_member_find(client, image_id=None, member=None, status=None, + include_deleted=False): + """Find all members that meet the given criteria. + + Note, currently include_deleted should be true only when create a new + image membership, as there may be a deleted image membership between + the same image and tenant, the membership will be reused in this case. + It should be false in other cases. :param image_id: identifier of image entity :param member: tenant to which membership has been granted + :include_deleted: A boolean indicating whether the result should include + the deleted record of image member """ return client.image_member_find(image_id=image_id, member=member, - status=status) + status=status, + include_deleted=include_deleted) @_get_client diff --git a/glance/db/simple/api.py b/glance/db/simple/api.py index 9f5f62afa0..527b251373 100644 --- a/glance/db/simple/api.py +++ b/glance/db/simple/api.py @@ -454,7 +454,8 @@ def image_property_delete(context, prop_ref, image_ref): @log_call -def image_member_find(context, image_id=None, member=None, status=None): +def image_member_find(context, image_id=None, member=None, + status=None, include_deleted=False): filters = [] images = DATA['images'] members = DATA['members'] diff --git a/glance/db/sqlalchemy/api.py b/glance/db/sqlalchemy/api.py index 52a9b9882f..e3d3bd4855 100644 --- a/glance/db/sqlalchemy/api.py +++ b/glance/db/sqlalchemy/api.py @@ -1075,21 +1075,31 @@ def _image_member_get(context, memb_id, session): return query.one() -def image_member_find(context, image_id=None, member=None, status=None): - """Find all members that meet the given criteria +def image_member_find(context, image_id=None, member=None, + status=None, include_deleted=False): + """Find all members that meet the given criteria. + + Note, currently include_deleted should be true only when create a new + image membership, as there may be a deleted image membership between + the same image and tenant, the membership will be reused in this case. + It should be false in other cases. :param image_id: identifier of image entity :param member: tenant to which membership has been granted + :include_deleted: A boolean indicating whether the result should include + the deleted record of image member """ session = get_session() - members = _image_member_find(context, session, image_id, member, status) + members = _image_member_find(context, session, image_id, + member, status, include_deleted) return [_image_member_format(m) for m in members] def _image_member_find(context, session, image_id=None, - member=None, status=None): + member=None, status=None, include_deleted=False): query = session.query(models.ImageMember) - query = query.filter_by(deleted=False) + if not include_deleted: + query = query.filter_by(deleted=False) if not context.is_admin: query = query.join(models.Image) diff --git a/glance/registry/api/v1/members.py b/glance/registry/api/v1/members.py index d006c777d3..252bc0486e 100644 --- a/glance/registry/api/v1/members.py +++ b/glance/registry/api/v1/members.py @@ -158,7 +158,8 @@ class Controller(object): # Try to find the corresponding membership members = self.db_api.image_member_find(req.context, image_id=datum['image_id'], - member=datum['member']) + member=datum['member'], + include_deleted=True) try: member = members[0] except IndexError: @@ -257,7 +258,8 @@ class Controller(object): # Look up an existing membership... members = self.db_api.image_member_find(req.context, image_id=image_id, - member=id) + member=id, + include_deleted=True) if members: if can_share is not None: values = dict(can_share=can_share) diff --git a/glance/tests/unit/v1/test_registry_api.py b/glance/tests/unit/v1/test_registry_api.py index f0e615d8a8..afaa27143e 100644 --- a/glance/tests/unit/v1/test_registry_api.py +++ b/glance/tests/unit/v1/test_registry_api.py @@ -1766,6 +1766,49 @@ class TestRegistryAPI(base.IsolatedUnitTest, test_utils.RegistryAPIMixIn): res = req.get_response(api) self.assertEqual(res.status_int, 403) + def test_add_member_delete_create(self): + """ + Test check that the same member can be successfully added after delete + it, and the same record will be reused for the same membership. + """ + # add a member + UUID8 = _gen_uuid() + extra_fixture = self.get_fixture(id=UUID8, size=19, protected=False, + owner='test user') + + db_api.image_create(self.context, extra_fixture) + fixture = dict(can_share=True) + test_uri = '/images/%s/members/test_add_member_delete_create' + body = jsonutils.dumps(dict(member=fixture)) + self.get_api_response_ext(204, url=test_uri % UUID8, + method='PUT', body=body, + content_type='json') + memb_list = db_api.image_member_find(self.context, image_id=UUID8) + self.assertEqual(1, len(memb_list)) + memb_list2 = db_api.image_member_find(self.context, + image_id=UUID8, + include_deleted=True) + self.assertEqual(1, len(memb_list2)) + # delete the member + self.get_api_response_ext(204, method='DELETE', + url=test_uri % UUID8) + memb_list = db_api.image_member_find(self.context, image_id=UUID8) + self.assertEqual(0, len(memb_list)) + memb_list2 = db_api.image_member_find(self.context, + image_id=UUID8, + include_deleted=True) + self.assertEqual(1, len(memb_list2)) + # create it again + self.get_api_response_ext(204, url=test_uri % UUID8, + method='PUT', body=body, + content_type='json') + memb_list = db_api.image_member_find(self.context, image_id=UUID8) + self.assertEqual(1, len(memb_list)) + memb_list2 = db_api.image_member_find(self.context, + image_id=UUID8, + include_deleted=True) + self.assertEqual(1, len(memb_list2)) + def test_get_on_image_member(self): """ Test GET on image members raises 405 and produces correct Allow headers diff --git a/glance/tests/unit/v2/test_registry_client.py b/glance/tests/unit/v2/test_registry_client.py index e958c7740f..fc174acafe 100644 --- a/glance/tests/unit/v2/test_registry_client.py +++ b/glance/tests/unit/v2/test_registry_client.py @@ -677,6 +677,31 @@ class TestRegistryV2Client(base.IsolatedUnitTest, num_members = len(memb_list) self.assertEqual(0, num_members) + def test_image_member_find_include_deleted(self): + """Tests getting member images include the delted member""" + values = dict(image_id=UUID2, member='pattieblack') + # create a member + member = self.client.image_member_create(values=values) + memb_list = self.client.image_member_find(member='pattieblack') + memb_list2 = self.client.image_member_find(member='pattieblack', + include_deleted=True) + self.assertEqual(1, len(memb_list)) + self.assertEqual(1, len(memb_list2)) + # delete the member + self.client.image_member_delete(memb_id=member['id']) + memb_list = self.client.image_member_find(member='pattieblack') + memb_list2 = self.client.image_member_find(member='pattieblack', + include_deleted=True) + self.assertEqual(0, len(memb_list)) + self.assertEqual(1, len(memb_list2)) + # create it again + member = self.client.image_member_create(values=values) + memb_list = self.client.image_member_find(member='pattieblack') + memb_list2 = self.client.image_member_find(member='pattieblack', + include_deleted=True) + self.assertEqual(1, len(memb_list)) + self.assertEqual(2, len(memb_list2)) + def test_add_update_members(self): """Tests updating image members""" values = dict(image_id=UUID2, member='pattieblack')