diff --git a/cinder/image/image_utils.py b/cinder/image/image_utils.py index 11b8160f97b..237c4dcbbb1 100644 --- a/cinder/image/image_utils.py +++ b/cinder/image/image_utils.py @@ -294,6 +294,10 @@ def _convert_image(prefix, source, dest, out_format, src_passphrase_file=None): """Convert image to other format. + NOTE: If the qemu-img convert command fails and this function raises an + exception, a non-empty dest file may be left in the filesystem. + It is the responsibility of the caller to decide what to do with this file. + :param prefix: command prefix, i.e. cgexec for throttling :param source: source filename :param dest: destination filename diff --git a/cinder/tests/unit/volume/drivers/test_rbd.py b/cinder/tests/unit/volume/drivers/test_rbd.py index 15b36c4ed9b..49109beda4e 100644 --- a/cinder/tests/unit/volume/drivers/test_rbd.py +++ b/cinder/tests/unit/volume/drivers/test_rbd.py @@ -1309,6 +1309,39 @@ class RBDTestCase(test.TestCase): mock_image_service, None] self.driver.copy_image_to_volume(*args) + @mock.patch('cinder.volume.drivers.rbd.fileutils.delete_if_exists') + @mock.patch('cinder.volume.volume_utils.check_encryption_provider', + return_value={'encryption_key_id': fake.ENCRYPTION_KEY_ID}) + @mock.patch('cinder.image.image_utils.convert_image') + def _copy_image_encrypted(self, mock_convert, mock_encrypt_key, + mock_temp_delete): + key_mgr = fake_keymgr.fake_api() + self.mock_object(castellan.key_manager, 'API', return_value=key_mgr) + key_id = key_mgr.store(self.context, KeyObject()) + self.volume_a.encryption_key_id = key_id + + enc_info = {'encryption_key_id': key_id, + 'cipher': 'aes-xts-essiv', + 'key_size': 256} + with mock.patch('cinder.volume.volume_utils.check_encryption_provider', + return_value=enc_info), \ + mock.patch('cinder.volume.drivers.rbd.open'), \ + mock.patch('os.rename'): + with mock.patch.object(tempfile, 'NamedTemporaryFile'): + with mock.patch.object(os.path, 'exists') as mock_exists: + mock_exists.return_value = True + with mock.patch.object(image_utils, 'fetch_to_raw'): + with mock.patch.object(self.driver, 'delete_volume'): + with mock.patch.object(self.driver, '_resize'): + mock_image_service = mock.MagicMock() + args = [self.context, self.volume_a, + mock_image_service, None] + self.driver.copy_image_to_encrypted_volume( + *args) + mock_temp_delete.assert_called() + self.assertEqual(1, + mock_temp_delete.call_count) + @common_mocks def test_copy_image_no_volume_tmp(self): self.cfg.image_conversion_dir = None @@ -1319,6 +1352,11 @@ class RBDTestCase(test.TestCase): self.cfg.image_conversion_dir = '/var/run/cinder/tmp' self._copy_image() + @common_mocks + def test_copy_image_volume_tmp_encrypted(self): + self.cfg.image_conversion_dir = '/var/run/cinder/tmp' + self._copy_image_encrypted() + @ddt.data(True, False) @common_mocks @mock.patch('cinder.volume.drivers.rbd.RBDDriver._get_usage_info') diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py index bb8015a60e0..a5c901a2851 100644 --- a/cinder/volume/drivers/rbd.py +++ b/cinder/volume/drivers/rbd.py @@ -1571,13 +1571,16 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD, # Convert the raw image to luks dest_image_path = src_image_path + '.luks' - image_utils.convert_image(src_image_path, dest_image_path, - 'luks', src_format='raw', - cipher_spec=cipher_spec, - passphrase_file=pass_file.name) + try: + image_utils.convert_image(src_image_path, dest_image_path, + 'luks', src_format='raw', + cipher_spec=cipher_spec, + passphrase_file=pass_file.name) - # Replace the original image with the now encrypted image - os.rename(dest_image_path, src_image_path) + # Replace the original image with the now encrypted image + os.rename(dest_image_path, src_image_path) + finally: + fileutils.delete_if_exists(dest_image_path) def _copy_image_to_volume(self, context, volume, image_service, image_id, encrypted=False): diff --git a/releasenotes/notes/cleanup-rbd-temp-file-during-convert-fail-3848e9dbe7e15fc6.yaml b/releasenotes/notes/cleanup-rbd-temp-file-during-convert-fail-3848e9dbe7e15fc6.yaml new file mode 100644 index 00000000000..de9a4868ca1 --- /dev/null +++ b/releasenotes/notes/cleanup-rbd-temp-file-during-convert-fail-3848e9dbe7e15fc6.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + `Bug #1873738 `_: RBD Driver: + Added cleanup for residue destination file if the copy image to encrypted volume + operation fails. \ No newline at end of file