diff --git a/glance/api/v2/images.py b/glance/api/v2/images.py index 95cfe7b9c3..bf74f281b5 100644 --- a/glance/api/v2/images.py +++ b/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) diff --git a/glance/common/store_utils.py b/glance/common/store_utils.py index 552f661f58..e3a94d2784 100644 --- a/glance/common/store_utils.py +++ b/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 diff --git a/glance/tests/unit/common/test_utils.py b/glance/tests/unit/common/test_utils.py index a07b2709d7..8069ef5742 100644 --- a/glance/tests/unit/common/test_utils.py +++ b/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""" diff --git a/releasenotes/notes/make-cinder-url-compatible-with-locations-1f3e938ff7e11c7d.yaml b/releasenotes/notes/make-cinder-url-compatible-with-locations-1f3e938ff7e11c7d.yaml new file mode 100644 index 0000000000..8c9df095cd --- /dev/null +++ b/releasenotes/notes/make-cinder-url-compatible-with-locations-1f3e938ff7e11c7d.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + `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.