RBD: Cleanup temporary file during exception
During copy image to encrypted volume operation, we create a temporary image file to convert source image into an encrypted image and rename it to source image name. If any error occurs during this process, there is no code to cleanup the temporary '.luks' file. This patch adds the following cleanup. Conflicts: cinder/image/image_utils.py cinder/tests/unit/volume/drivers/test_rbd.py Change-Id: I05dfc996a635299990fdafba841ab6c4fd7d32c7 Closes-Bug: 1873738 (cherry picked from commit7c95f3969f
) (cherry picked from commit02de7897d4
) (cherry picked from commit48774c89ab
)
This commit is contained in:
parent
ce4c86a1a3
commit
175eb62813
|
@ -207,7 +207,12 @@ def check_qemu_img_version(minimum_version):
|
||||||
def _convert_image(prefix, source, dest, out_format,
|
def _convert_image(prefix, source, dest, out_format,
|
||||||
out_subformat=None, src_format=None,
|
out_subformat=None, src_format=None,
|
||||||
run_as_root=True, cipher_spec=None, passphrase_file=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
|
# 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
|
# This is needed to ensure that all data hit the device before
|
||||||
|
|
|
@ -1288,6 +1288,41 @@ class RBDTestCase(test.TestCase):
|
||||||
mock_image_service, None]
|
mock_image_service, None]
|
||||||
self.driver.copy_image_to_volume(*args)
|
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
|
@common_mocks
|
||||||
def test_copy_image_no_volume_tmp(self):
|
def test_copy_image_no_volume_tmp(self):
|
||||||
self.cfg.image_conversion_dir = None
|
self.cfg.image_conversion_dir = None
|
||||||
|
@ -1298,6 +1333,11 @@ class RBDTestCase(test.TestCase):
|
||||||
self.cfg.image_conversion_dir = '/var/run/cinder/tmp'
|
self.cfg.image_conversion_dir = '/var/run/cinder/tmp'
|
||||||
self._copy_image()
|
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)
|
@ddt.data(True, False)
|
||||||
@common_mocks
|
@common_mocks
|
||||||
@mock.patch('cinder.volume.drivers.rbd.RBDDriver._get_usage_info')
|
@mock.patch('cinder.volume.drivers.rbd.RBDDriver._get_usage_info')
|
||||||
|
|
|
@ -1533,13 +1533,16 @@ class RBDDriver(driver.CloneableImageVD, driver.MigrateVD,
|
||||||
|
|
||||||
# Convert the raw image to luks
|
# Convert the raw image to luks
|
||||||
dest_image_path = src_image_path + '.luks'
|
dest_image_path = src_image_path + '.luks'
|
||||||
image_utils.convert_image(src_image_path, dest_image_path,
|
try:
|
||||||
'luks', src_format='raw',
|
image_utils.convert_image(src_image_path, dest_image_path,
|
||||||
cipher_spec=cipher_spec,
|
'luks', src_format='raw',
|
||||||
passphrase_file=pass_file.name)
|
cipher_spec=cipher_spec,
|
||||||
|
passphrase_file=pass_file.name)
|
||||||
|
|
||||||
# Replace the original image with the now encrypted image
|
# Replace the original image with the now encrypted image
|
||||||
os.rename(dest_image_path, src_image_path)
|
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,
|
def _copy_image_to_volume(self, context, volume, image_service, image_id,
|
||||||
encrypted=False):
|
encrypted=False):
|
||||||
|
|
|
@ -0,0 +1,6 @@
|
||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
`Bug #1873738 <https://bugs.launchpad.net/cinder/+bug/1873738>`_: RBD Driver:
|
||||||
|
Added cleanup for residue destination file if the copy image to encrypted volume
|
||||||
|
operation fails.
|
Loading…
Reference in New Issue