diff --git a/glance/api/authorization.py b/glance/api/authorization.py index b498c8bb77..791c537187 100644 --- a/glance/api/authorization.py +++ b/glance/api/authorization.py @@ -32,12 +32,12 @@ LOG = logging.getLogger(__name__) def lazy_update_store_info(func): """Update store information in location metadata""" @functools.wraps(func) - def wrapped(context, image, **kwargs): + def wrapped(context, image, image_repo, **kwargs): if CONF.enabled_backends: store_utils.update_store_in_locations( - image.locations, image.image_id) + image, image_repo) - return func(context, image, **kwargs) + return func(context, image, image_repo, **kwargs) return wrapped @@ -54,7 +54,7 @@ def is_image_mutable(context, image): @lazy_update_store_info -def proxy_image(context, image): +def proxy_image(context, image, image_repo): if is_image_mutable(context, image): return ImageProxy(image, context) else: @@ -127,11 +127,11 @@ class ImageRepoProxy(glance.domain.proxy.Repo): def get(self, image_id): image = self.image_repo.get(image_id) - return proxy_image(self.context, image) + return proxy_image(self.context, image, self.image_repo) def list(self, *args, **kwargs): images = self.image_repo.list(*args, **kwargs) - return [proxy_image(self.context, i) for i in images] + return [proxy_image(self.context, i, self.image_repo) for i in images] def _validate_image_accepts_members(visibility): diff --git a/glance/api/v2/images.py b/glance/api/v2/images.py index 8acb8e81f0..a3ae18f51b 100644 --- a/glance/api/v2/images.py +++ b/glance/api/v2/images.py @@ -622,13 +622,15 @@ class ImagesController(object): val_data = self._validate_validation_data(image, value) # NOTE(abhishekk): get glance store based on location uri + updated_location = value if CONF.enabled_backends: - store_utils.update_store_in_locations(value, image.image_id) + updated_location = store_utils.get_updated_store_location( + value) try: # NOTE(flwang): _locations_proxy's setattr method will check if # the update is acceptable. - image.locations = value + image.locations = updated_location if image.status == 'queued': for k, v in val_data.items(): setattr(image, k, v) @@ -652,8 +654,10 @@ class ImagesController(object): val_data = self._validate_validation_data(image, [value]) # NOTE(abhishekk): get glance store based on location uri + updated_location = value if CONF.enabled_backends: - store_utils.update_store_in_locations([value], image.image_id) + updated_location = store_utils.get_updated_store_location( + [value])[0] pos = self._get_locations_op_pos(path_pos, len(image.locations), True) @@ -661,7 +665,7 @@ class ImagesController(object): msg = _("Invalid position for adding a location.") raise webob.exc.HTTPBadRequest(explanation=msg) try: - image.locations.insert(pos, value) + image.locations.insert(pos, updated_location) if image.status == 'queued': for k, v in val_data.items(): setattr(image, k, v) diff --git a/glance/common/store_utils.py b/glance/common/store_utils.py index 8e47371525..ade5898b17 100644 --- a/glance/common/store_utils.py +++ b/glance/common/store_utils.py @@ -180,22 +180,35 @@ def _get_store_id_from_uri(uri): return -def update_store_in_locations(locations, image_id): +def update_store_in_locations(image, image_repo): + for loc in image.locations: + if (not loc['metadata'].get( + 'store') or loc['metadata'].get( + 'store') not in CONF.enabled_backends): + store_id = _get_store_id_from_uri(loc['url']) + if store_id: + if 'store' in loc['metadata']: + old_store = loc['metadata']['store'] + if old_store != store_id: + LOG.debug("Store '%(old)s' has changed to " + "'%(new)s' by operator, updating " + "the same in the location of image " + "'%(id)s'", {'old': old_store, + 'new': store_id, + 'id': image.image_id}) + + loc['metadata']['store'] = store_id + image_repo.save(image) + + +def get_updated_store_location(locations): for loc in locations: store_id = _get_store_id_from_uri(loc['url']) if store_id: - if 'store' in loc['metadata']: - old_store = loc['metadata']['store'] - if old_store != store_id: - LOG.debug("Store '%(old)s' has changed to " - "'%(new)s' by operator, updating " - "the same in the location of image " - "'%(id)s'", {'old': old_store, - 'new': store_id, - 'id': image_id}) - loc['metadata']['store'] = store_id + return locations + def get_dir_separator(): separator = '' diff --git a/glance/tests/unit/common/test_utils.py b/glance/tests/unit/common/test_utils.py index ebbba9e3bd..9e2cae56f3 100644 --- a/glance/tests/unit/common/test_utils.py +++ b/glance/tests/unit/common/test_utils.py @@ -35,28 +35,61 @@ CONF = cfg.CONF class TestStoreUtils(test_utils.BaseTestCase): """Test glance.common.store_utils module""" - def _test_update_store_in_location(self, metadata, store_id, expected): + def _test_update_store_in_location(self, metadata, store_id, expected, + store_id_call_count=1, + save_call_count=1): + image = mock.Mock() + image_repo = mock.Mock() + image_repo.save = mock.Mock() locations = [{ 'url': 'rbd://aaaaaaaa/images/id', 'metadata': metadata }] + image.locations = locations with mock.patch.object( store_utils, '_get_store_id_from_uri') as mock_get_store_id: mock_get_store_id.return_value = store_id - store_utils.update_store_in_locations(locations, mock.Mock()) - self.assertEqual(locations[0]['metadata'].get('store'), expected) + store_utils.update_store_in_locations(image, image_repo) + self.assertEqual(image.locations[0]['metadata'].get( + 'store'), expected) + self.assertEqual(store_id_call_count, mock_get_store_id.call_count) + self.assertEqual(save_call_count, image_repo.save.call_count) def test_update_store_location_with_no_store(self): + enabled_backends = { + "rbd1": "rbd", + "rbd2": "rbd" + } + self.config(enabled_backends=enabled_backends) self._test_update_store_in_location({}, 'rbd1', 'rbd1') def test_update_store_location_with_different_store(self): - self._test_update_store_in_location({'store': 'rbd2'}, 'rbd1', 'rbd1') + enabled_backends = { + "ceph1": "rbd", + "ceph2": "rbd" + } + self.config(enabled_backends=enabled_backends) + self._test_update_store_in_location( + {'store': 'rbd2'}, 'ceph1', 'ceph1') def test_update_store_location_with_same_store(self): - self._test_update_store_in_location({'store': 'rbd1'}, 'rbd1', 'rbd1') + enabled_backends = { + "rbd1": "rbd", + "rbd2": "rbd" + } + self.config(enabled_backends=enabled_backends) + self._test_update_store_in_location({'store': 'rbd1'}, 'rbd1', 'rbd1', + store_id_call_count=0, + save_call_count=0) def test_update_store_location_with_store_none(self): - self._test_update_store_in_location({}, None, None) + enabled_backends = { + "rbd1": "rbd", + "rbd2": "rbd" + } + self.config(enabled_backends=enabled_backends) + self._test_update_store_in_location({}, None, None, + save_call_count=0) class TestUtils(test_utils.BaseTestCase): diff --git a/glance/tests/unit/v2/test_images_resource.py b/glance/tests/unit/v2/test_images_resource.py index 75cf61a694..a6c8d7b074 100644 --- a/glance/tests/unit/v2/test_images_resource.py +++ b/glance/tests/unit/v2/test_images_resource.py @@ -5248,7 +5248,7 @@ class TestMultiImagesController(base.MultiIsolatedUnitTest): request = unit_test_utils.get_fake_request() self.assertRaises(webob.exc.HTTPConflict, self.controller.import_image, - request, UUID1, + request, UUID2, {'method': {'name': 'glance-direct'}}) def test_delete_from_store_as_non_owner(self):