openstack/glance
Code Issues Proposed changes

Fix LP#2054575

Browse Source 
This patch contains a squash of two commits:

COMMIT 1:

Make location URL compatible with cinder backend

While adding location to an image, cinder sends location url as
`cinder://volume_id` for single as well as multistore which is
incompatible with glance multistore and store throws
InvalidLoctation error.

Modifying the location url to be compatible with multistore as
`cinder://store_id/volume_id` to avoid Invalid Location error.

Related-Bug: #2054575
Change-Id: I5f791c58ae857f6c553276dd9808799c3db3aa4f

COMMIT 2:

Fix: optimized upload volume in Cinder store

When Glance is configured to use Cinder store and we upload
volume to Glance in the optimized path, it fails with
InvalidLocation error.
This happens because Cinder is not aware about the store
in which we will create the image and supplies the old
format URL i.e. cinder://<vol-id> whereas Glance expects
new location format i.e. cinder://<store-id>/<vol-id>.

Glance has code to update the format from old location format
to new location format but it isn't triggered in case of
old location APIs.

This patch adds the context to the update store ID request
which calls the Cinder store to provide the updated location,
hence fixing the optimized path for upload volume to image.

Closes-Bug: #2054575
Change-Id: Idd1cb8982b40b85a17821596f76dfa10207f6381

The commits are squashed together to make backport easier.

Change-Id: I9ecdfe08b63c00446dc3e24195e3b8e59b82f55c
(cherry picked from commit 53175262f0f8df303176f4e9cd44e36d442c6cea)
(cherry picked from commit 8540ffc6ee34976285209e9f3c7c728bf649e0ff)
This commit is contained in:
Rajat Dhasmana 2024-07-25 21:16:27 +05:30
parent 3f7f01bb05
commit 6a17e4e2ba
4 changed files with 45 additions and 10 deletions

6
glance/api/v2/images.py

@ -674,7 +674,7 @@ class ImagesController(object):
json_schema_version = change.get('json_schema_version', 10)
if path_root == 'locations':
api_pol.update_locations()
self._do_add_locations(image, path[1], value)
self._do_add_locations(image, path[1], value, req.context)
else:
api_pol.update_property(path_root, value)
if ((hasattr(image, path_root) or
@ -1043,7 +1043,7 @@ class ImagesController(object):
raise webob.exc.HTTPBadRequest(
explanation=encodeutils.exception_to_unicode(ve))
def _do_add_locations(self, image, path_pos, value):
def _do_add_locations(self, image, path_pos, value, context):
if CONF.show_multiple_locations == False:
msg = _("It's not allowed to add locations if locations are "
"invisible.")
@ -1059,7 +1059,7 @@ class ImagesController(object):
updated_location = value
if CONF.enabled_backends:
updated_location = store_utils.get_updated_store_location(
[value])[0]
[value], context=context)[0]
pos = self._get_locations_op_pos(path_pos,
len(image.locations), True)

6
glance/common/store_utils.py

@ -238,8 +238,12 @@ def _update_cinder_location_and_store_id(context, loc):
"due to unknown issues."), uri)
def get_updated_store_location(locations):
def get_updated_store_location(locations, context=None):
for loc in locations:
if loc['url'].startswith("cinder://") and context:
_update_cinder_location_and_store_id(context, loc)
continue
store_id = _get_store_id_from_uri(loc['url'])
if store_id:
loc['metadata']['store'] = store_id

34
glance/tests/unit/common/test_utils.py

@ -102,7 +102,8 @@ class TestCinderStoreUtils(base.MultiStoreClearingUnitTest):
new_callable=mock.PropertyMock)
def _test_update_cinder_store_in_location(self, mock_url_prefix,
mock_associate_store,
is_valid=True):
is_valid=True,
modify_source_url=False):
volume_id = 'db457a25-8f16-4b2c-a644-eae8d17fe224'
store_id = 'fast-cinder'
expected = 'fast-cinder'
@ -117,7 +118,11 @@ class TestCinderStoreUtils(base.MultiStoreClearingUnitTest):
}]
mock_url_prefix.return_value = 'cinder://%s' % store_id
image.locations = locations
store_utils.update_store_in_locations(context, image, image_repo)
if modify_source_url:
updated_location = store_utils.get_updated_store_location(
locations, context=context)
else:
store_utils.update_store_in_locations(context, image, image_repo)
if is_valid:
# This is the case where we found an image that has an
@ -129,10 +134,18 @@ class TestCinderStoreUtils(base.MultiStoreClearingUnitTest):
# format i.e. this is the case when store is valid and location
# url, metadata are updated and image_repo.save is called
expected_url = mock_url_prefix.return_value + '/' + volume_id
self.assertEqual(expected_url, image.locations[0].get('url'))
self.assertEqual(expected, image.locations[0]['metadata'].get(
'store'))
self.assertEqual(1, image_repo.save.call_count)
if modify_source_url:
# This is the case where location url is modified to be
# compatible with multistore as below,
# `cinder://store_id/volume_id` to avoid InvalidLocation error
self.assertEqual(expected_url, updated_location[0].get('url'))
self.assertEqual(expected,
updated_location[0]['metadata'].get('store'))
else:
self.assertEqual(expected_url, image.locations[0].get('url'))
self.assertEqual(expected, image.locations[0]['metadata'].get(
'store'))
self.assertEqual(1, image_repo.save.call_count)
else:
# Here, we've got an image backed by a volume which does
# not have a corresponding store specifying the volume_type.
@ -151,6 +164,15 @@ class TestCinderStoreUtils(base.MultiStoreClearingUnitTest):
def test_update_cinder_store_location_invalid_type(self):
self._test_update_cinder_store_in_location(is_valid=False)
def test_get_updated_cinder_store_location(self):
"""
Test if location url is modified to be compatible
with multistore.
"""
self._test_update_cinder_store_in_location(
modify_source_url=True)
class TestUtils(test_utils.BaseTestCase):
"""Test routines in glance.utils"""

9
releasenotes/notes/make-cinder-url-compatible-with-locations-1f3e938ff7e11c7d.yaml Normal file

@ -0,0 +1,9 @@
---
fixes:
- |
`Bug #2054575 <https://bugs.launchpad.net/glance/+bug/2054575>`_:
Fixed the issue when cinder uploads a volume to glance in the
optimized path and glance rejects the request with invalid location.
Now we convert the old location format sent by cinder into the new
location format supported by multi store, hence allowing volumes to
be uploaded in an optimized way.