Merge "Improve lazy loading mechanism for multiple stores"
This commit is contained in:
commit
9c66f46a14
@ -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):
|
||||
|
@ -632,13 +632,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)
|
||||
@ -662,8 +664,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)
|
||||
@ -671,7 +675,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)
|
||||
|
@ -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 = ''
|
||||
|
@ -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):
|
||||
|
@ -5294,7 +5294,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…
x
Reference in New Issue
Block a user