diff --git a/cinder/image/image_utils.py b/cinder/image/image_utils.py index 62dddcf9ef3..a1077ea2b1e 100644 --- a/cinder/image/image_utils.py +++ b/cinder/image/image_utils.py @@ -207,7 +207,12 @@ def check_qemu_img_version(minimum_version): def _convert_image(prefix, source, dest, out_format, out_subformat=None, src_format=None, run_as_root=True, cipher_spec=None, passphrase_file=None): - """Convert image to other format.""" + """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. + """ # Check whether O_DIRECT is supported and set '-t none' if it is # This is needed to ensure that all data hit the device before diff --git a/cinder/tests/unit/volume/drivers/test_rbd.py b/cinder/tests/unit/volume/drivers/test_rbd.py index 964ee697ed4..3918b834aa4 100644 --- a/cinder/tests/unit/volume/drivers/test_rbd.py +++ b/cinder/tests/unit/volume/drivers/test_rbd.py @@ -1288,6 +1288,41 @@ 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.drivers.rbd.RBDDriver._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.drivers.rbd.RBDDriver.' + '_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 @@ -1298,6 +1333,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 3a8f5b1e4d4..b9d110ee9de 100644 --- a/cinder/volume/drivers/rbd.py +++ b/cinder/volume/drivers/rbd.py @@ -1533,13 +1533,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