libvirt: disconnect volume when encryption fails

The compute node has left the attachment information after the
encryption fails. This patch fixes this by ensuring the volume is
disconnected when an exception occurs.

Change-Id: I9d652f182d83a2557ff6ed0b21c69a735c3f9840
Closes-Bug: #1812945

Author: yenai

nova/tests/unit/virt/libvirt/test_driver.py

@@ -7507,6 +7507,66 @@ def test_get_volume_config(self, _set_cache_mode, get_config):
         _set_cache_mode.assert_called_once_with(config)
         self.assertEqual(config_guest_disk.to_xml(), config.to_xml())
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_driver')
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_attach_encryptor')
+    def test_connect_volume_encryption_success(
+            self, mock_attach_encryptor, mock_get_volume_driver):
+
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        mock_volume_driver = mock.MagicMock(
+            spec=volume_drivers.LibvirtBaseVolumeDriver)
+        mock_get_volume_driver.return_value = mock_volume_driver
+
+        connection_info = {'driver_volume_type': 'fake',
+                           'data': {'device_path': '/fake',
+                                    'access_mode': 'rw',
+                                    'volume_id': uuids.volume_id}}
+        encryption = {'provider': encryptors.LUKS,
+                      'encryption_key_id': uuids.encryption_key_id}
+        instance = mock.sentinel.instance
+
+        drvr._connect_volume(self.context, connection_info, instance,
+                             encryption=encryption)
+
+        mock_get_volume_driver.assert_called_once_with(connection_info)
+        mock_volume_driver.connect_volume.assert_called_once_with(
+            connection_info, instance)
+        mock_attach_encryptor.assert_called_once_with(
+            self.context, connection_info, encryption, True)
+        mock_volume_driver.disconnect_volume.assert_not_called()
+
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_driver')
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_attach_encryptor')
+    def test_connect_volume_encryption_fail(
+            self, mock_attach_encryptor, mock_get_volume_driver):
+
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        mock_volume_driver = mock.MagicMock(
+            spec=volume_drivers.LibvirtBaseVolumeDriver)
+        mock_get_volume_driver.return_value = mock_volume_driver
+
+        connection_info = {'driver_volume_type': 'fake',
+                           'data': {'device_path': '/fake',
+                                    'access_mode': 'rw',
+                                    'volume_id': uuids.volume_id}}
+        encryption = {'provider': encryptors.LUKS,
+                      'encryption_key_id': uuids.encryption_key_id}
+        instance = mock.sentinel.instance
+        mock_attach_encryptor.side_effect = processutils.ProcessExecutionError
+
+        self.assertRaises(processutils.ProcessExecutionError,
+                          drvr._connect_volume,
+                          self.context, connection_info, instance,
+                          encryption=encryption)
+
+        mock_get_volume_driver.assert_called_once_with(connection_info)
+        mock_volume_driver.connect_volume.assert_called_once_with(
+            connection_info, instance)
+        mock_attach_encryptor.assert_called_once_with(
+            self.context, connection_info, encryption, True)
+        mock_volume_driver.disconnect_volume.assert_called_once_with(
+            connection_info, instance)
+
     @mock.patch.object(key_manager, 'API')
     @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryption')
     @mock.patch.object(libvirt_driver.LibvirtDriver, '_use_native_luks')

nova/virt/libvirt/driver.py

@@ -1256,8 +1256,15 @@ def _connect_volume(self, context, connection_info, instance,
                         encryption=None, allow_native_luks=True):
         vol_driver = self._get_volume_driver(connection_info)
         vol_driver.connect_volume(connection_info, instance)
-        self._attach_encryptor(context, connection_info, encryption,
-                               allow_native_luks)
+        try:
+            self._attach_encryptor(
+                context, connection_info, encryption, allow_native_luks)
+        except Exception:
+            # Encryption failed so rollback the volume connection.
+            with excutils.save_and_reraise_exception(logger=LOG):
+                LOG.exception("Failure attaching encryptor; rolling back "
+                              "volume connection", instance=instance)
+                vol_driver.disconnect_volume(connection_info, instance)
 
     def _should_disconnect_target(self, context, connection_info, instance):
         connection_count = 0