From 5bffcf05edca3c2a3b5e1c1b6a536e5491b4bcac Mon Sep 17 00:00:00 2001 From: Tomoki Sekiyama Date: Thu, 25 Jun 2015 17:41:47 -0400 Subject: [PATCH] Fix multipath iSCSI encrypted volume attach failure Currently iSCSI volume attachment fails if iscsi_use_multipath is set to True. This is because the encryptor requests cryptsetup to create the symlink to the LUKS device with the same name of the device-mapper multipath device. To avoid the name collision, this patch adds the 'crypt-' prefix to the symlink. Change-Id: I01a3104025a9c479e40933ca24024355df9dfb8b Closes-Bug: #1439869 --- .../unit/volume/encryptors/test_cryptsetup.py | 40 ++++++++++++++++++- nova/volume/encryptors/cryptsetup.py | 39 +++++++++++++++++- 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/nova/tests/unit/volume/encryptors/test_cryptsetup.py b/nova/tests/unit/volume/encryptors/test_cryptsetup.py index 0d8957d93..e524f40d8 100644 --- a/nova/tests/unit/volume/encryptors/test_cryptsetup.py +++ b/nova/tests/unit/volume/encryptors/test_cryptsetup.py @@ -15,6 +15,7 @@ import binascii +import copy from castellan.common.objects import symmetric_key as key import mock @@ -33,14 +34,15 @@ def fake__get_key(context): class CryptsetupEncryptorTestCase(test_base.VolumeEncryptorTestCase): - def _create(self, connection_info): + @mock.patch('os.path.exists', return_value=False) + def _create(self, connection_info, mock_exists): return cryptsetup.CryptsetupEncryptor(connection_info) def setUp(self): super(CryptsetupEncryptorTestCase, self).setUp() self.dev_path = self.connection_info['data']['device_path'] - self.dev_name = self.dev_path.split('/')[-1] + self.dev_name = 'crypt-%s' % self.dev_path.split('/')[-1] self.symlink_path = self.dev_path @@ -102,3 +104,37 @@ class CryptsetupEncryptorTestCase(test_base.VolumeEncryptorTestCase): cryptsetup.CryptsetupEncryptor, connection_info) self.assertIn(type, six.text_type(exc)) + + @mock.patch('os.path.exists', return_value=True) + @mock.patch('nova.utils.execute') + def test_init_volume_encryption_with_old_name(self, mock_execute, + mock_exists): + # If an old name crypt device exists, dev_path should be the old name. + old_dev_name = self.dev_path.split('/')[-1] + encryptor = cryptsetup.CryptsetupEncryptor(self.connection_info) + self.assertFalse(encryptor.dev_name.startswith('crypt-')) + self.assertEqual(old_dev_name, encryptor.dev_name) + self.assertEqual(self.dev_path, encryptor.dev_path) + self.assertEqual(self.symlink_path, encryptor.symlink_path) + mock_exists.assert_called_once_with('/dev/mapper/%s' % old_dev_name) + mock_execute.assert_called_once_with( + 'cryptsetup', 'status', old_dev_name, run_as_root=True) + + @mock.patch('os.path.exists', side_effect=[False, True]) + @mock.patch('nova.utils.execute') + def test_init_volume_encryption_with_wwn(self, mock_execute, mock_exists): + # If an wwn name crypt device exists, dev_path should be based on wwn. + old_dev_name = self.dev_path.split('/')[-1] + wwn = 'fake_wwn' + connection_info = copy.deepcopy(self.connection_info) + connection_info['data']['multipath_id'] = wwn + encryptor = cryptsetup.CryptsetupEncryptor(connection_info) + self.assertFalse(encryptor.dev_name.startswith('crypt-')) + self.assertEqual(wwn, encryptor.dev_name) + self.assertEqual(self.dev_path, encryptor.dev_path) + self.assertEqual(self.symlink_path, encryptor.symlink_path) + mock_exists.assert_has_calls([ + mock.call('/dev/mapper/%s' % old_dev_name), + mock.call('/dev/mapper/%s' % wwn)]) + mock_execute.assert_called_once_with( + 'cryptsetup', 'status', wwn, run_as_root=True) diff --git a/nova/volume/encryptors/cryptsetup.py b/nova/volume/encryptors/cryptsetup.py index 72cdd183e..32bea39a2 100644 --- a/nova/volume/encryptors/cryptsetup.py +++ b/nova/volume/encryptors/cryptsetup.py @@ -17,9 +17,11 @@ import binascii import os +from oslo_concurrency import processutils from oslo_log import log as logging from nova import exception +from nova.i18n import _LW from nova import utils from nova.volume.encryptors import base @@ -49,10 +51,45 @@ class CryptsetupEncryptor(base.VolumeEncryptor): self.symlink_path = connection_info['data']['device_path'] # a unique name for the volume -- e.g., the iSCSI participant name - self.dev_name = self.symlink_path.split('/')[-1] + self.dev_name = 'crypt-%s' % self.symlink_path.split('/')[-1] + + # NOTE(tsekiyama): In older version of nova, dev_name was the same + # as the symlink name. Now it has 'crypt-' prefix to avoid conflict + # with multipath device symlink. To enable rolling update, we use the + # old name when the encrypted volume already exists. + old_dev_name = self.symlink_path.split('/')[-1] + wwn = data.get('multipath_id') + if self._is_crypt_device_available(old_dev_name): + self.dev_name = old_dev_name + LOG.debug("Using old encrypted volume name: %s", self.dev_name) + elif wwn and wwn != old_dev_name: + # FibreChannel device could be named '/dev/mapper/'. + if self._is_crypt_device_available(wwn): + self.dev_name = wwn + LOG.debug("Using encrypted volume name from wwn: %s", + self.dev_name) + # the device's actual path on the compute host -- e.g., /dev/sd_ self.dev_path = os.path.realpath(self.symlink_path) + def _is_crypt_device_available(self, dev_name): + if not os.path.exists('/dev/mapper/%s' % dev_name): + return False + + try: + utils.execute('cryptsetup', 'status', dev_name, run_as_root=True) + except processutils.ProcessExecutionError as e: + # If /dev/mapper/ is a non-crypt block device (such as a + # normal disk or multipath device), exit_code will be 1. In the + # case, we will omit the warning message. + if e.exit_code != 1: + LOG.warning(_LW('cryptsetup status %(dev_name) exited ' + 'abnormally (status %(exit_code)s): %(err)s'), + {"dev_name": dev_name, "exit_code": e.exit_code, + "err": e.stderr}) + return False + return True + def _get_passphrase(self, key): """Convert raw key to string.""" return binascii.hexlify(key).decode('utf-8')