LUKS: Support extending host attached volumes

Patch fixing bug #1861071 resolved the issue of extending LUKS v1
volumes when nova connects them via libvirt instead of through os-brick,
but nova side still fails to extend LUKSv2 in-use volumes when they
don't go through libvirt.

The logs will show a very similar error, but the user won't know that
this has happened and Cinder will show the new size:

 libvirt.libvirtError: internal error: unable to execute QEMU command
 'block_resize': Cannot grow device files

There are 2 parts to this problem:

- The device mapper device is not automatically extended.
- Nova tries to use the encrypted block device size as the size of the
  decrypted device.

This patch adds new functionality to the encryptors so that they can
extend decrypted volumes to match the size of the encrypted device.

New method added to encryptors is called "extend_volume", and should be
called after the homonymous method in the connector has been called, and
the value returned by the encryptor's extend_volume method is the real
size of the decrypted volume (encrypted volume - headers).

The patch only adds functionality for LUKS and LUKSv2 volumes, not to
cryptsetup volumes.

Related-Bug: #1967157
Change-Id: I351f1a7769c9f915e4cd280f05a8b8b87f40df84

os_brick/encryptors/base.py

@@ -60,3 +60,8 @@ class VolumeEncryptor(executor.Executor, metaclass=abc.ABCMeta):
 
         """
         pass
+
+    @abc.abstractmethod
+    def extend_volume(self, context, **kwargs):
+        """Extend an encrypted volume and return the decrypted volume size."""
+        pass

os_brick/encryptors/cryptsetup.py

@@ -169,3 +169,6 @@ class CryptsetupEncryptor(base.VolumeEncryptor):
     def detach_volume(self, **kwargs):
         """Removes the dm-crypt mapping for the device."""
         self._close_volume(**kwargs)
+
+    def extend_volume(self, context, **kwargs):
+        raise NotImplementedError()

os_brick/encryptors/luks.py

@@ -18,6 +18,7 @@ from oslo_log import log as logging
 
 from os_brick.encryptors import cryptsetup
 from os_brick.privileged import rootwrap as priv_rootwrap
+from os_brick import utils
 
 LOG = logging.getLogger(__name__)
 
@@ -159,6 +160,20 @@ class LuksEncryptor(cryptsetup.CryptsetupEncryptor):
                       root_helper=self._root_helper,
                       attempts=3)
 
+    def extend_volume(self, context, **kwargs):
+        """Extend an encrypted volume and return the decrypted volume size."""
+        symlink = self.symlink_path
+        LOG.debug('Resizing mapping %s to match underlying device', symlink)
+        key = self._get_key(context).get_encoded()
+        passphrase = self._get_passphrase(key)
+        self._execute('cryptsetup', 'resize', symlink,
+                      process_input=passphrase,
+                      run_as_root=True, check_exit_code=True,
+                      root_helper=self._root_helper)
+        res = utils.get_device_size(self, symlink)
+        LOG.debug('New size of mapping is %s', res)
+        return res
+
 
 class Luks2Encryptor(LuksEncryptor):
     """A VolumeEncryptor based on LUKS v2.

os_brick/encryptors/nop.py

@@ -41,3 +41,6 @@ class NoOpEncryptor(base.VolumeEncryptor):
 
     def detach_volume(self, **kwargs):
         pass
+
+    def extend_volume(self, context, **kwargs):
+        pass

os_brick/initiator/connectors/nvmeof.py

@@ -437,7 +437,7 @@ class NVMeOFConnector(base.BaseLinuxConnector):
         if volume_paths:
             if connection_properties.get('volume_nguid'):
                 for path in volume_paths:
-                    return self._linuxscsi.get_device_size(path)
+                    return utils.get_device_size(self, path)
             return self._linuxscsi.extend_volume(
                 volume_paths, use_multipath=self.use_multipath)
         else:
@@ -531,7 +531,7 @@ class NVMeOFConnector(base.BaseLinuxConnector):
             device_path = NVMeOFConnector.get_nvme_device_path(
                 self, target_nqn, vol_uuid)
 
-        return self._linuxscsi.get_device_size(device_path)
+        return utils.get_device_size(self, device_path)
 
     def _connect_target_volume(self, target_nqn, vol_uuid, portals):
         nvme_ctrls = NVMeOFConnector.rescan(self, target_nqn)

os_brick/initiator/connectors/scaleio.py

@@ -537,22 +537,9 @@ class ScaleIOConnector(base.BaseLinuxConnector):
         self._rescan_vols()
         volume_paths = self.get_volume_paths(connection_properties)
         if volume_paths:
-            return self.get_device_size(volume_paths[0])
+            return utils.get_device_size(self, volume_paths[0])
 
         # if we got here, the volume is not mapped
         msg = (_("Error extending ScaleIO volume"))
         LOG.error(msg)
         raise exception.BrickException(message=msg)
-
-    def get_device_size(self, device):
-        """Get the size in bytes of a volume."""
-        (out, _err) = self._execute('blockdev', '--getsize64',
-                                    device, run_as_root=True,
-                                    root_helper=self._root_helper)
-        var = str(out.strip())
-        LOG.debug("Device %(dev)s size: %(var)s",
-                  {'dev': device, 'var': var})
-        if var.isnumeric():
-            return int(var)
-        else:
-            return None

os_brick/initiator/connectors/storpool.py

@@ -229,12 +229,12 @@ class StorPoolConnector(base.BaseLinuxConnector):
         # Wait for the StorPool client to update the size of the local device
         path = '/dev/storpool/' + volume
         for _ in range(10):
-            size = self._get_device_size(path)
+            size = utils.get_device_size(self, path)
             LOG.debug('Got local size %(size)d', {'size': size})
             if size == vdata.size:
                 return size
             time.sleep(0.1)
         else:
-            size = self._get_device_size(path)
+            size = utils.get_device_size(self, path)
             LOG.debug('Last attempt: local size %(size)d', {'size': size})
             return size

os_brick/initiator/linuxscsi.py

@@ -549,17 +549,6 @@ class LinuxSCSI(executor.Executor):
             return info
         return None
 
-    def get_device_size(self, device):
-        """Get the size in bytes of a volume."""
-        (out, _err) = self._execute('blockdev', '--getsize64',
-                                    device, run_as_root=True,
-                                    root_helper=self._root_helper)
-        var = str(out.strip())
-        if var.isnumeric():
-            return int(var)
-        else:
-            return None
-
     def multipath_reconfigure(self):
         """Issue a multipathd reconfigure.
 
@@ -632,13 +621,13 @@ class LinuxSCSI(executor.Executor):
             scsi_path = ("/sys/bus/scsi/drivers/sd/%(device_id)s" %
                          {'device_id': device_id})
 
-            size = self.get_device_size(volume_path)
+            size = utils.get_device_size(self, volume_path)
             LOG.debug("Starting size: %s", size)
 
             # now issue the device rescan
             rescan_path = "%(scsi_path)s/rescan" % {'scsi_path': scsi_path}
             self.echo_scsi_command(rescan_path, "1")
-            new_size = self.get_device_size(volume_path)
+            new_size = utils.get_device_size(self, volume_path)
             LOG.debug("volume size after scsi device rescan %s", new_size)
 
         scsi_wwn = self.get_scsi_wwn(volume_paths[0])
@@ -648,13 +637,13 @@ class LinuxSCSI(executor.Executor):
                 # Force a reconfigure so that resize works
                 self.multipath_reconfigure()
 
-                size = self.get_device_size(mpath_device)
+                size = utils.get_device_size(self, mpath_device)
                 LOG.info("mpath(%(device)s) current size %(size)s",
                          {'device': mpath_device, 'size': size})
 
                 self.multipath_resize_map(scsi_wwn)
 
-                new_size = self.get_device_size(mpath_device)
+                new_size = utils.get_device_size(self, mpath_device)
                 LOG.info("mpath(%(device)s) new size %(size)s",
                          {'device': mpath_device, 'size': new_size})
 

os_brick/tests/encryptors/test_cryptsetup.py

@@ -153,3 +153,7 @@ class CryptsetupEncryptorTestCase(test_base.VolumeEncryptorTestCase):
             mock.call('/dev/mapper/%s' % wwn)])
         mock_execute.assert_called_once_with(
             'cryptsetup', 'status', wwn, run_as_root=True)
+
+    def test_extend_volume(self):
+        self.assertRaises(NotImplementedError,
+                          self.encryptor.extend_volume, mock.sentinel.context)

os_brick/tests/encryptors/test_luks.py

@@ -17,6 +17,7 @@ from unittest import mock
 
 from oslo_concurrency import processutils as putils
 
+from os_brick.encryptors import cryptsetup
 from os_brick.encryptors import luks
 from os_brick.tests.encryptors import test_cryptsetup
 
@@ -182,6 +183,25 @@ class LuksEncryptorTestCase(test_cryptsetup.CryptsetupEncryptorTestCase):
                       attempts=3, run_as_root=True, check_exit_code=[0, 4]),
         ])
 
+    @mock.patch('os_brick.utils.get_device_size')
+    @mock.patch.object(cryptsetup.CryptsetupEncryptor, '_execute')
+    @mock.patch.object(cryptsetup.CryptsetupEncryptor, '_get_passphrase')
+    @mock.patch.object(cryptsetup.CryptsetupEncryptor, '_get_key')
+    def test_extend_volume(self, mock_key, mock_pass, mock_exec, mock_size):
+        encryptor = self.encryptor
+        res = encryptor.extend_volume(mock.sentinel.context)
+        self.assertEqual(mock_size.return_value, res)
+
+        mock_key.assert_called_once_with(mock.sentinel.context)
+        mock_key.return_value.get_encoded.assert_called_once_with()
+        key = mock_key.return_value.get_encoded.return_value
+        mock_pass.assert_called_once_with(key)
+        mock_exec.assert_called_once_with(
+            'cryptsetup', 'resize', encryptor.dev_path,
+            process_input=mock_pass.return_value, run_as_root=True,
+            check_exit_code=True, root_helper=encryptor._root_helper)
+        mock_size.assert_called_once_with(encryptor, encryptor.dev_path)
+
 
 class Luks2EncryptorTestCase(LuksEncryptorTestCase):
     def _create(self):

os_brick/tests/encryptors/test_nop.py

@@ -36,3 +36,7 @@ class NoOpEncryptorTestCase(test_base.VolumeEncryptorTestCase):
             'provider': 'NoOpEncryptor',
         }
         self.encryptor.detach_volume(**test_args)
+
+    def test_extend_volume(self):
+        # Test that it exists and doesn't break on call
+        self.encryptor.extend_volume('context', anything=1, goes='asdf')

os_brick/tests/initiator/connectors/test_nvmeof.py

@@ -439,7 +439,7 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase):
         mock_end_raid.assert_called_with(self.connector, '/dev/md/md1')
 
     @mock.patch.object(nvmeof.NVMeOFConnector, 'get_nvme_device_path')
-    @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_size')
+    @mock.patch('os_brick.utils.get_device_size')
     def test_extend_volume_unreplicated(
             self, mock_device_size, mock_device_path):
         connection_properties = {
@@ -456,10 +456,10 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase):
         mock_device_path.assert_called_with(
             self.connector, volume_replicas[0]['target_nqn'],
             volume_replicas[0]['vol_uuid'])
-        mock_device_size.assert_called_with('/dev/nvme0n1')
+        mock_device_size.assert_called_with(self.connector, '/dev/nvme0n1')
 
     @mock.patch.object(nvmeof.NVMeOFConnector, 'get_nvme_device_path')
-    @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_size')
+    @mock.patch('os_brick.utils.get_device_size')
     def test_extend_volume_unreplicated_no_replica(
             self, mock_device_size, mock_device_path):
         connection_properties = {
@@ -473,10 +473,10 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase):
                 connection_properties), 100)
         mock_device_path.assert_called_with(
             self.connector, 'fakenqn', 'fakeuuid')
-        mock_device_size.assert_called_with('/dev/nvme0n1')
+        mock_device_size.assert_called_with(self.connector, '/dev/nvme0n1')
 
     @mock.patch.object(nvmeof.NVMeOFConnector, 'run_mdadm')
-    @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_size')
+    @mock.patch('os_brick.utils.get_device_size')
     def test_extend_volume_replicated(
             self, mock_device_size, mock_mdadm):
         mock_device_size.return_value = 100
@@ -486,9 +486,9 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase):
         device_path = '/dev/md/' + connection_properties['alias']
         mock_mdadm.assert_called_with(
             self.connector, ['mdadm', '--grow', '--size', 'max', device_path])
-        mock_device_size.assert_called_with(device_path)
+        mock_device_size.assert_called_with(self.connector, device_path)
 
-    @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_size')
+    @mock.patch('os_brick.utils.get_device_size')
     def test_extend_volume_with_nguid(self, mock_device_size):
         device_path = '/dev/nvme0n1'
         connection_properties = {
@@ -500,7 +500,7 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase):
             self.connector.extend_volume(connection_properties),
             100
         )
-        mock_device_size.assert_called_with(device_path)
+        mock_device_size.assert_called_with(self.connector, device_path)
 
     @mock.patch.object(nvmeof.NVMeOFConnector, 'rescan')
     @mock.patch.object(nvmeof.NVMeOFConnector, 'get_nvme_device_path')

os_brick/tests/initiator/connectors/test_scaleio.py

@@ -306,7 +306,7 @@ class ScaleIOConnectorTestCase(test_connector.ConnectorTestCase):
 
     @mock.patch.object(os.path, 'exists', return_value=True)
     @mock.patch.object(scaleio.ScaleIOConnector, '_find_volume_path')
-    @mock.patch.object(scaleio.ScaleIOConnector, 'get_device_size')
+    @mock.patch('os_brick.utils.get_device_size')
     def test_extend_volume(self,
                            mock_device_size,
                            mock_find_volume_path,

os_brick/tests/initiator/test_linuxscsi.py

@@ -759,26 +759,6 @@ loop0                                     0"""
         self.assertEqual("0", info['devices'][1]['id'])
         self.assertEqual("3", info['devices'][1]['lun'])
 
-    def test_get_device_size(self):
-        mock_execute = mock.Mock()
-        self.linuxscsi._execute = mock_execute
-        size = '1024'
-        mock_execute.return_value = (size, None)
-
-        ret_size = self.linuxscsi.get_device_size('/dev/fake')
-        self.assertEqual(int(size), ret_size)
-
-        size = 'junk'
-        mock_execute.return_value = (size, None)
-        ret_size = self.linuxscsi.get_device_size('/dev/fake')
-        self.assertIsNone(ret_size)
-
-        size_bad = '1024\n'
-        size_good = 1024
-        mock_execute.return_value = (size_bad, None)
-        ret_size = self.linuxscsi.get_device_size('/dev/fake')
-        self.assertEqual(size_good, ret_size)
-
     def test_multipath_reconfigure(self):
         self.linuxscsi.multipath_reconfigure()
         expected_commands = ['multipathd reconfigure']
@@ -792,7 +772,7 @@ loop0                                     0"""
 
     @mock.patch.object(linuxscsi.LinuxSCSI, 'find_multipath_device_path')
     @mock.patch.object(linuxscsi.LinuxSCSI, 'get_scsi_wwn')
-    @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_size')
+    @mock.patch('os_brick.utils.get_device_size')
     @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_info')
     def test_extend_volume_no_mpath(self, mock_device_info,
                                     mock_device_size,
@@ -822,7 +802,7 @@ loop0                                     0"""
 
     @mock.patch.object(linuxscsi.LinuxSCSI, 'find_multipath_device_path')
     @mock.patch.object(linuxscsi.LinuxSCSI, 'get_scsi_wwn')
-    @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_size')
+    @mock.patch('os_brick.utils.get_device_size')
     @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_info')
     def test_extend_volume_with_mpath(self, mock_device_info,
                                       mock_device_size,
@@ -854,7 +834,7 @@ loop0                                     0"""
     @mock.patch.object(linuxscsi.LinuxSCSI, '_multipath_resize_map')
     @mock.patch.object(linuxscsi.LinuxSCSI, 'find_multipath_device_path')
     @mock.patch.object(linuxscsi.LinuxSCSI, 'get_scsi_wwn')
-    @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_size')
+    @mock.patch('os_brick.utils.get_device_size')
     @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_info')
     def test_extend_volume_with_mpath_fail(self, mock_device_info,
                                            mock_device_size,
@@ -892,7 +872,7 @@ loop0                                     0"""
     @mock.patch.object(linuxscsi.LinuxSCSI, '_multipath_resize_map')
     @mock.patch.object(linuxscsi.LinuxSCSI, 'find_multipath_device_path')
     @mock.patch.object(linuxscsi.LinuxSCSI, 'get_scsi_wwn')
-    @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_size')
+    @mock.patch('os_brick.utils.get_device_size')
     @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_info')
     def test_extend_volume_with_mpath_pending(self, mock_device_info,
                                               mock_device_size,
@@ -931,7 +911,7 @@ loop0                                     0"""
     @mock.patch.object(linuxscsi.LinuxSCSI, '_multipath_resize_map')
     @mock.patch.object(linuxscsi.LinuxSCSI, 'find_multipath_device_path')
     @mock.patch.object(linuxscsi.LinuxSCSI, 'get_scsi_wwn')
-    @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_size')
+    @mock.patch('os_brick.utils.get_device_size')
     @mock.patch.object(linuxscsi.LinuxSCSI, 'get_device_info')
     def test_extend_volume_with_mpath_timeout(self, mock_device_info,
                                               mock_device_size,

os_brick/tests/test_utils.py

@@ -28,6 +28,22 @@ class WrongException(exception.BrickException):
     pass
 
 
+@ddt.ddt
+class TestUtils(base.TestCase):
+    @ddt.data(('1024', 1024), ('junk', None), ('2048\n', 2048))
+    @ddt.unpack
+    def test_get_device_size(self, cmd_out, expected):
+        mock_execute = mock.Mock()
+        mock_execute._execute.return_value = (cmd_out, None)
+
+        device = '/dev/fake'
+        ret_size = utils.get_device_size(mock_execute, device)
+        self.assertEqual(expected, ret_size)
+        mock_execute._execute.assert_called_once_with(
+            'blockdev', '--getsize64', device,
+            run_as_root=True, root_helper=mock_execute._root_helper)
+
+
 class TestRetryDecorator(base.TestCase):
 
     def test_no_retry_required(self):

os_brick/utils.py

@@ -394,3 +394,15 @@ def connect_volume_undo_prepare_result(f=None, unlink_after=False):
     if f:
         return decorator(f)
     return decorator
+
+
+def get_device_size(executor, device):
+    """Get the size in bytes of a volume."""
+    (out, _err) = executor._execute('blockdev', '--getsize64',
+                                    device, run_as_root=True,
+                                    root_helper=executor._root_helper)
+    var = str(out.strip())
+    if var.isnumeric():
+        return int(var)
+    else:
+        return None

releasenotes/notes/extend-encrypted-in-use-ac3f7a1994ec3a38.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    `Bug #1967157 <https://bugs.launchpad.net/nova/+bug/1967157>`_: Fixed
+    extending LUKS and LUKSv2 host attached encrypted volumes. Only LUKS v1
+    volumes decrypted via libvirt were working, but now all LUKS based in-use
+    encrypted volumes can be extended.