From cea67763c9f8037f47844e3e057166d6874d801d Mon Sep 17 00:00:00 2001 From: kairat_kushaev Date: Fri, 6 Nov 2015 18:16:30 +0300 Subject: [PATCH] Remove location check from V2 client Glance client has a custom check that generates exception if location has not been returned by image-get request. This check should on server side and it should be managed by policy rules when do location-add action. That also allows to increase possibility of migrating Heat to v2[1]. NOTE: After this patch, we'll raise a HTTPBadRequest from server side instead of HTTPConflict when a user adds a duplicate location. [1]: https://review.openstack.org/#/c/240450/ Co-Authored-By: wangxiyuan Change-Id: I778ad2a97805b4d85eb0430c603c27a0a1c148e0 Closes-bug: #1493026 --- glanceclient/tests/unit/v2/test_images.py | 21 ++++++++------------- glanceclient/v2/images.py | 6 ------ 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/glanceclient/tests/unit/v2/test_images.py b/glanceclient/tests/unit/v2/test_images.py index ca64e553..8ed1bb30 100644 --- a/glanceclient/tests/unit/v2/test_images.py +++ b/glanceclient/tests/unit/v2/test_images.py @@ -1014,17 +1014,13 @@ class TestController(testtools.TestCase): def test_location_ops_when_server_disabled_location_ops(self): # Location operations should not be allowed if server has not - # enabled location related operations + # enabled location related operations. There is no need to check it + # when do location add, because the check would be done in server side. image_id = '3a4560a1-e585-443e-9b39-553b46ec92d1' estr = 'The administrator has disabled API access to image locations' url = 'http://bar.com/' meta = {'bar': 'barmeta'} - e = self.assertRaises(exc.HTTPBadRequest, - self.controller.add_location, - image_id, url, meta) - self.assertIn(estr, str(e)) - e = self.assertRaises(exc.HTTPBadRequest, self.controller.delete_locations, image_id, set([url])) @@ -1052,20 +1048,19 @@ class TestController(testtools.TestCase): add_patch = {'path': '/locations/-', 'value': new_loc, 'op': 'add'} self.controller.add_location(image_id, **new_loc) self.assertEqual(self.api.calls, [ - self._empty_get(image_id), self._patch_req(image_id, [add_patch]), self._empty_get(image_id) ]) - def test_add_duplicate_location(self): + @mock.patch.object(images.Controller, '_send_image_update_request', + side_effect=exc.HTTPBadRequest) + def test_add_duplicate_location(self, mock_request): image_id = 'a2b83adc-888e-11e3-8872-78acc0b951d8' new_loc = {'url': 'http://foo.com/', 'metadata': {'foo': 'newfoo'}} - err_str = 'A location entry at %s already exists' % new_loc['url'] - err = self.assertRaises(exc.HTTPConflict, - self.controller.add_location, - image_id, **new_loc) - self.assertIn(err_str, str(err)) + self.assertRaises(exc.HTTPBadRequest, + self.controller.add_location, + image_id, **new_loc) def test_remove_location(self): image_id = 'a2b83adc-888e-11e3-8872-78acc0b951d8' diff --git a/glanceclient/v2/images.py b/glanceclient/v2/images.py index 053cc641..271dd8a2 100644 --- a/glanceclient/v2/images.py +++ b/glanceclient/v2/images.py @@ -305,12 +305,6 @@ class Controller(object): :param metadata: Metadata associated with the location. :returns: The updated image """ - image = self._get_image_with_locations_or_fail(image_id) - url_list = [l['url'] for l in image.locations] - if url in url_list: - err_str = 'A location entry at %s already exists' % url - raise exc.HTTPConflict(err_str) - add_patch = [{'op': 'add', 'path': '/locations/-', 'value': {'url': url, 'metadata': metadata}}] self._send_image_update_request(image_id, add_patch)