| From 8f0964a893b98faa6f8fc751a67601c6b58366ed Mon Sep 17 00:00:00 2001 |
| From: Peter Penchev <openstack-dev@storpool.com> |
| Date: Thu, 9 Jun 2022 01:43:28 +0300 |
| Subject: [PATCH 1/8] Send the correct location URI to the Glance v2 API |
| |
| When uploading a volume to an image, send the new-style |
| cinder://<store-id>/<volume-id> URL to the Glance API if |
| image_service:store_id is present in the volume type extra specs. |
| |
| Closes-Bug: #1978020 |
| Co-Authored-By: Rajat Dhasmana <rajatdhasmana@gmail.com> |
| Change-Id: I815706f691a7d1e5a0c54eb15222417008ef1f34 |
| --- |
| cinder/tests/unit/volume/test_image.py | 2 +- |
| .../tests/unit/volume/test_volume_manager.py | 160 ++++++++++++++++++ |
| cinder/volume/manager.py | 4 +- |
| ...20-glance-upload-uri-8fbc70c442ac620c.yaml | 11 ++ |
| 4 files changed, 175 insertions(+), 2 deletions(-) |
| create mode 100644 releasenotes/notes/bug-1978020-glance-upload-uri-8fbc70c442ac620c.yaml |
| |
| diff --git a/cinder/tests/unit/volume/test_image.py b/cinder/tests/unit/volume/test_image.py |
| index ebc7904c4..2b1b9c392 100644 |
| --- a/cinder/tests/unit/volume/test_image.py |
| +++ b/cinder/tests/unit/volume/test_image.py |
| @@ -366,7 +366,7 @@ class CopyVolumeToImageTestCase(base.BaseVolumeTestCase): |
| def test_copy_volume_to_image_with_image_volume(self): |
| image = self._test_copy_volume_to_image_with_image_volume() |
| self.assertTrue(image['locations'][0]['url'].startswith('cinder://')) |
| - image_volume_id = image['locations'][0]['url'][9:] |
| + image_volume_id = image['locations'][0]['url'].split('/')[-1] |
| # The image volume does NOT include the snapshot_id, and include the |
| # source_volid which is the uploaded-volume id. |
| vol_ref = db.volume_get(self.context, image_volume_id) |
| diff --git a/cinder/tests/unit/volume/test_volume_manager.py b/cinder/tests/unit/volume/test_volume_manager.py |
| index 3b751d2cd..547a48fdd 100644 |
| --- a/cinder/tests/unit/volume/test_volume_manager.py |
| +++ b/cinder/tests/unit/volume/test_volume_manager.py |
| @@ -287,3 +287,163 @@ class VolumeManagerTestCase(base.BaseVolumeTestCase): |
| manager._parse_connection_options(ctxt, vol, conn_info) |
| self.assertIn('cacheable', conn_info['data']) |
| self.assertIs(conn_info['data']['cacheable'], False) |
| + |
| + @mock.patch('cinder.message.api.API.create') |
| + @mock.patch('cinder.volume.volume_utils.require_driver_initialized') |
| + @mock.patch('cinder.volume.manager.VolumeManager._clone_image_volume') |
| + @mock.patch('cinder.db.volume_metadata_update') |
| + def test_clone_image_no_volume(self, |
| + fake_update, |
| + fake_clone, |
| + fake_msg_create, |
| + fake_init): |
| + """Make sure nothing happens if no volume was created.""" |
| + manager = vol_manager.VolumeManager() |
| + |
| + ctx = mock.sentinel.context |
| + volume = fake_volume.fake_volume_obj(ctx) |
| + image_service = mock.MagicMock(spec=[]) |
| + |
| + fake_clone.return_value = None |
| + |
| + image_meta = {'disk_format': 'raw', 'container_format': 'ova'} |
| + manager._clone_image_volume_and_add_location(ctx, volume, |
| + image_service, image_meta) |
| + fake_clone.assert_not_called() |
| + fake_update.assert_not_called() |
| + |
| + image_meta = {'disk_format': 'qcow2', 'container_format': 'bare'} |
| + manager._clone_image_volume_and_add_location(ctx, volume, |
| + image_service, image_meta) |
| + fake_clone.assert_not_called() |
| + fake_update.assert_not_called() |
| + |
| + image_meta = {'disk_format': 'raw', 'container_format': 'bare'} |
| + manager._clone_image_volume_and_add_location(ctx, volume, |
| + image_service, image_meta) |
| + fake_clone.assert_called_once_with(ctx, volume, image_meta) |
| + fake_update.assert_not_called() |
| + |
| + @mock.patch('cinder.message.api.API.create') |
| + @mock.patch('cinder.objects.VolumeType.get_by_id') |
| + @mock.patch('cinder.volume.volume_utils.require_driver_initialized') |
| + @mock.patch('cinder.volume.manager.VolumeManager._clone_image_volume') |
| + @mock.patch('cinder.db.volume_metadata_update') |
| + def test_clone_image_no_store_id(self, |
| + fake_update, |
| + fake_clone, |
| + fake_msg_create, |
| + fake_volume_type_get, |
| + fake_init): |
| + """Send a cinder://<volume-id> URL if no store ID in extra specs.""" |
| + manager = vol_manager.VolumeManager() |
| + |
| + project_id = fake.PROJECT_ID |
| + |
| + ctx = mock.MagicMock() |
| + ctx.elevated.return_value = ctx |
| + ctx.project_id = project_id |
| + |
| + vol_type = fake_volume.fake_volume_type_obj( |
| + ctx, |
| + id=fake.VOLUME_TYPE_ID, |
| + name=fake.VOLUME_TYPE_NAME, |
| + extra_specs={'volume_type_backend': 'unknown'}) |
| + fake_volume_type_get.return_value = vol_type |
| + |
| + volume = fake_volume.fake_volume_obj(ctx, |
| + id=fake.VOLUME_ID, |
| + volume_type_id=vol_type.id) |
| + |
| + image_volume_id = fake.VOLUME2_ID |
| + image_volume = fake_volume.fake_volume_obj(ctx, id=image_volume_id) |
| + url = 'cinder://%(vol)s' % {'vol': image_volume_id} |
| + |
| + image_service = mock.MagicMock(spec=['add_location']) |
| + image_meta_id = fake.IMAGE_ID |
| + image_meta = { |
| + 'id': image_meta_id, |
| + 'disk_format': 'raw', |
| + 'container_format': 'bare', |
| + } |
| + image_volume_meta = { |
| + 'image_owner': project_id, |
| + 'glance_image_id': image_meta_id, |
| + } |
| + |
| + fake_clone.return_value = image_volume |
| + |
| + manager._clone_image_volume_and_add_location(ctx, volume, |
| + image_service, image_meta) |
| + fake_clone.assert_called_once_with(ctx, volume, image_meta) |
| + fake_update.assert_called_with(ctx, image_volume_id, |
| + image_volume_meta, False) |
| + image_service.add_location.assert_called_once_with(ctx, image_meta_id, |
| + url, {}) |
| + |
| + @mock.patch('cinder.message.api.API.create') |
| + @mock.patch('cinder.objects.VolumeType.get_by_id') |
| + @mock.patch('cinder.volume.volume_utils.require_driver_initialized') |
| + @mock.patch('cinder.volume.manager.VolumeManager._clone_image_volume') |
| + @mock.patch('cinder.db.volume_metadata_update') |
| + def test_clone_image_with_store_id(self, |
| + fake_update, |
| + fake_clone, |
| + fake_msg_create, |
| + fake_volume_type_get, |
| + fake_init): |
| + """Send a cinder://<store-id>/<volume-id> URL.""" |
| + manager = vol_manager.VolumeManager() |
| + |
| + project_id = fake.PROJECT_ID |
| + |
| + ctx = mock.MagicMock() |
| + ctx.elevated.return_value = ctx |
| + ctx.project_id = project_id |
| + |
| + store_id = 'muninn' |
| + vol_type = fake_volume.fake_volume_type_obj( |
| + ctx, |
| + id=fake.VOLUME_TYPE_ID, |
| + name=fake.VOLUME_TYPE_NAME, |
| + extra_specs={ |
| + 'volume_type_backend': 'unknown', |
| + 'image_service:store_id': store_id, |
| + }) |
| + fake_volume_type_get.return_value = vol_type |
| + |
| + volume = fake_volume.fake_volume_obj(ctx, |
| + id=fake.VOLUME_ID, |
| + volume_type_id=vol_type.id) |
| + |
| + image_volume_id = '42' |
| + image_volume = mock.MagicMock(spec=['id']) |
| + image_volume.id = image_volume_id |
| + url = 'cinder://%(store)s/%(vol)s' % { |
| + 'store': store_id, |
| + 'vol': image_volume_id, |
| + } |
| + |
| + image_service = mock.MagicMock(spec=['add_location']) |
| + image_meta_id = fake.IMAGE_ID |
| + image_meta = { |
| + 'id': image_meta_id, |
| + 'disk_format': 'raw', |
| + 'container_format': 'bare', |
| + } |
| + image_volume_meta = { |
| + 'image_owner': project_id, |
| + 'glance_image_id': image_meta_id, |
| + } |
| + |
| + fake_clone.return_value = image_volume |
| + |
| + manager._clone_image_volume_and_add_location(ctx, volume, |
| + image_service, image_meta) |
| + fake_clone.assert_called_once_with(ctx, volume, image_meta) |
| + fake_update.assert_called_with(ctx, image_volume_id, |
| + image_volume_meta, False) |
| + image_service.add_location.assert_called_once_with(ctx, |
| + image_meta_id, |
| + url, |
| + {'store': store_id}) |
| diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py |
| index dbb73d894..804a1c6cb 100644 |
| --- a/cinder/volume/manager.py |
| +++ b/cinder/volume/manager.py |
| @@ -1683,7 +1683,6 @@ class VolumeManager(manager.CleanableManager, |
| image_volume_meta, |
| False) |
| |
| - uri = 'cinder://%s' % image_volume.id |
| image_registered = None |
| |
| # retrieve store information from extra-specs |
| @@ -1692,6 +1691,9 @@ class VolumeManager(manager.CleanableManager, |
| |
| if store_id: |
| location_metadata['store'] = store_id |
| + uri = 'cinder://%s/%s' % (store_id, image_volume.id) |
| + else: |
| + uri = 'cinder://%s' % image_volume.id |
| |
| try: |
| image_registered = image_service.add_location( |
| diff --git a/releasenotes/notes/bug-1978020-glance-upload-uri-8fbc70c442ac620c.yaml b/releasenotes/notes/bug-1978020-glance-upload-uri-8fbc70c442ac620c.yaml |
| new file mode 100644 |
| index 000000000..04a909d3d |
| --- /dev/null |
| +++ b/releasenotes/notes/bug-1978020-glance-upload-uri-8fbc70c442ac620c.yaml |
| @@ -0,0 +1,11 @@ |
| +--- |
| +fixes: |
| + - | |
| + `Bug #1978020 <https://bugs.launchpad.net/cinder/+bug/1978020>`_: Fixed |
| + uploading a volume to a Cinder-backed Glance image; if a store name is set |
| + in the volume type's extra specs, it must also be sent to Glance as part of |
| + the new image location URI. |
| + Please note that while the `image_service:store_id` extra spec is |
| + validated when it is set for the volume type, it is not validated later; |
| + it is the operator's responsibility to make sure that the Glance store is |
| + not renamed or removed or that the volume types are updated accordingly. |
| -- |
| 2.35.1 |
| |