Merge "RBD: Cleanup temporary file during exception"

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

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')

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):

releasenotes/notes/cleanup-rbd-temp-file-during-convert-fail-3848e9dbe7e15fc6.yaml

@@ -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.