Merge "Fix LP#2054575" into stable/2024.1

Author: Zuul

glance/api/v2/images.py

@@ -673,7 +673,7 @@ def _do_add(self, req, image, api_pol, change):
         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
@@ -1042,7 +1042,7 @@ def _do_replace_locations(self, image, value):
             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.")
@@ -1058,7 +1058,7 @@ def _do_add_locations(self, image, path_pos, value):
         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)

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

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 @@ def _test_update_cinder_store_in_location(self, mock_url_prefix,
         }]
         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 @@ def _test_update_cinder_store_in_location(self, mock_url_prefix,
             # 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 @@ def test_update_cinder_store_location_valid_type(self):
     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"""

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

@@ -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.