Improve lazy loading mechanism for multiple stores
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
)
This commit is contained in:
parent
2df2792fee
commit
9a104acbae
|
@ -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):
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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 = ''
|
||||
|
|
|
@ -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):
|
||||
|
|
|
@ -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):
|
||||
|
|
Loading…
Reference in New Issue