openstack/glance
Code Issues Proposed changes

Improve lazy loading mechanism for multiple stores

Browse Source 
Glance has a facility lazy loading for legacy images which will be called
on get/list api calls to add store information in image's location metadata
based on location URL of image. Even if admin decides to change the store
names in glance-api.conf same will also be updated in location metadata
for all images related to that particular store. Current implementation of
legacy image performs this operation on each get/list call as location metadata
is not getting updated in database or it doesn't handle to perform store name
check in glance-api.conf.

Improvements done:
1. Save updated location metadata information in database permenantly
2. Add logic to perform lazy loading only if store information is not present
in location metadata or store present in location metadata is not defined in
glance's enbaled_backends configuration option.

Change-Id: I789fa7adfb459e7861c90a51f418a635c0c22244
Closes-Bug: #1886374
(cherry picked from commit ab0e5268a9)
(cherry picked from commit 9a104acbae)
This commit is contained in:
Abhishek Kekane 2020-07-06 07:49:31 +00:00
parent bdba1feacb
commit baf6b5ebf0
5 changed files with 78 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
glance/api/authorization.py
View File

@ -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):

12
glance/api/v2/images.py
View File

@ -523,13 +523,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)
@ -553,8 +555,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)
@ -562,7 +566,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)

35
glance/common/store_utils.py
View File

@ -178,22 +178,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 = ''

45
glance/tests/unit/common/test_utils.py
View File

@ -31,28 +31,61 @@ from glance.tests import utils as test_utils
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):

2
glance/tests/unit/v2/test_images_resource.py
View File

@ -5172,7 +5172,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_image_lazy_loading_store(self):