libvirt: Skip encryption metadata lookups if secret already exists on host

When connecting an encrypted volume to a host the _attach_encryptor
method will be called in order to either call a legacy os-brick
encryptor *or* configure a libvirt secret used by libvirt and QEMU to
natively decrypt LUKSv1 encrypted volumes. To create this libvirt secret
the configured key manager will be queried to provide and then decode
the associated secret before this is stashed within libvirt.

This change simply skips the above when an existing libvirt secret
associated with the target volume is found on the host already.

While this obviously optimises basic instance lifecycle flows such as a
simple power off and on it additionally resolves a more convoluted use
case when the ``[DEFAULT]/resume_guests_state_on_host_boot``
configurable is enabled. In this case the compute service has no request
context with which to query the key manager when attempting to restart
instances with encrypted volumes attached. As a result any attempt by
the compute service to restart an instance with an attached encrypted
volume would previously fail.

Closes-Bug: #1905701
Change-Id: Ia2007bc63ef09931ea0197cef29d6a5614ed821a
(cherry picked from commit a107a50)

Author: lyarwood

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

@@ -9136,6 +9136,9 @@ def test_connect_volume_luks(self, mock_is_volume_luks, mock_host,
                       'encryption_key_id': uuids.encryption_key_id}
         instance = mock.sentinel.instance
 
+        # Mock out find_secret so we don't skip ahead
+        drvr._host.find_secret.return_value = None
+
         # Mock out the encryptors
         mock_encryptor = mock.Mock()
         mock_get_volume_encryptor.return_value = mock_encryptor
@@ -10196,6 +10199,21 @@ def test_attach_encryptor_encrypted_native_luks_serial(self,
         crt_scrt.assert_called_once_with(
             'volume', uuids.serial, password=key)
 
+    @mock.patch.object(key_manager, 'API')
+    def test_attach_encryptor_secret_exists(self, mock_key_manager_api):
+        connection_info = {'data': {'volume_id': uuids.volume_id}}
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        with test.nested(
+            mock.patch.object(drvr, '_get_volume_encryption'),
+            mock.patch.object(drvr._host, 'find_secret')
+        ) as (mock_get_volume_encryption, mock_find_secret):
+            drvr._attach_encryptor(self.context, connection_info, None)
+
+        # Assert we called find_secret and nothing else
+        mock_find_secret.assert_called_once_with('volume', uuids.volume_id)
+        mock_get_volume_encryption.assert_not_called()
+        mock_key_manager_api.assert_not_called()
+
     @mock.patch('os_brick.encryptors.get_encryption_metadata')
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor')
     def test_detach_encryptor_connection_info_incomplete(self,

nova/virt/libvirt/driver.py

@@ -1791,6 +1791,17 @@ def _attach_encryptor(self, context, connection_info, encryption):
         to determine if an attempt to attach the encryptor should be made.
 
         """
+        # NOTE(lyarwood): Skip any attempt to fetch encryption metadata or the
+        # actual passphrase from the key manager if a libvirt secert already
+        # exists locally for the volume. This suggests that the instance was
+        # only powered off or the underlying host rebooted.
+        volume_id = driver_block_device.get_volume_id(connection_info)
+        if self._host.find_secret('volume', volume_id):
+            LOG.debug("A libvirt secret for volume %s has been found on the "
+                      "host, skipping any attempt to create another or attach "
+                      "an os-brick encryptor.", volume_id)
+            return
+
         if encryption is None:
             encryption = self._get_volume_encryption(context, connection_info)
 
@@ -1822,7 +1833,6 @@ def _attach_encryptor(self, context, connection_info, encryption):
             # NOTE(lyarwood): Store the passphrase as a libvirt secret locally
             # on the compute node. This secret is used later when generating
             # the volume config.
-            volume_id = driver_block_device.get_volume_id(connection_info)
             self._host.create_secret('volume', volume_id, password=passphrase)
         elif encryption:
             encryptor = self._get_volume_encryptor(connection_info,

releasenotes/notes/bug_1905701-fdc7402ffe70d104.yaml

@@ -0,0 +1,13 @@
+---
+fixes:
+  - |
+    The libvirt virt driver will no longer attempt to fetch volume
+    encryption metadata or the associated secret key when attaching ``LUKSv1``
+    encrypted volumes if a libvirt secret already exists on the host.
+
+    This resolves `bug 1905701`_ where instances with ``LUKSv1`` encrypted
+    volumes could not be restarted automatically by the ``nova-compute``
+    service after a host reboot when the
+    ``[DEFAULT]/resume_guests_state_on_host_boot`` configurable was enabled.
+
+    .. _bug 1905701: https://launchpad.net/bugs/1905701