diff --git a/cinder/tests/unit/volume/test_image.py b/cinder/tests/unit/volume/test_image.py index ebc7904c40a..2b1b9c3923f 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 341e7326fea..9a7a50ff61f 100644 --- a/cinder/tests/unit/volume/test_volume_manager.py +++ b/cinder/tests/unit/volume/test_volume_manager.py @@ -342,3 +342,163 @@ class VolumeManagerTestCase(base.BaseVolumeTestCase): res = manager._driver_shares_targets() self.assertFalse(res) + + @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:// 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:/// 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 23817b9a6f0..98c4ddc4700 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -1723,7 +1723,6 @@ class VolumeManager(manager.CleanableManager, image_volume_meta, False) - uri = 'cinder://%s' % image_volume.id image_registered = None # retrieve store information from extra-specs @@ -1732,6 +1731,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 00000000000..04a909d3d3d --- /dev/null +++ b/releasenotes/notes/bug-1978020-glance-upload-uri-8fbc70c442ac620c.yaml @@ -0,0 +1,11 @@ +--- +fixes: + - | + `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.