From 4aab14a09fa072d2b3a5adfbeb752956a93c5118 Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Tue, 14 Jun 2022 10:22:45 +0900 Subject: [PATCH] libvirt: Add new option to enforce multipath volume connections Currently, even when [libvirt] volume_use_multipath is set to True, volume attachment silently falls back to single path if multipathd is not running in the host. This sometimes prevents operators from noticing the misconfiguration, until they face any issue caused by missing redundancy. Introduce the new [libvirt] volume_enforce_multipath option, which makes the attachment process fail if multipathd is not running. A similar parameter (enforce_multipath_for_image_xfer) was already implemented in cinder and this change follows how the parameter is implemented there. Also add the check in init phase to detect lack of mulitipath daemon during initializing driver. Min version of os-brick has to be bumped due to the interface change made by 8d919696a9f1b1361f00aac7032647b5e1656082 . Implements: blueprint enforce-multipath Change-Id: I828de70ca7b343a4562ace4049d2b3857dbf900a --- nova/conf/libvirt.py | 16 ++++++ nova/tests/unit/virt/libvirt/test_driver.py | 53 +++++++++++++++++++ .../unit/virt/libvirt/volume/test_nvme.py | 18 ++++++- nova/virt/libvirt/driver.py | 19 +++++++ nova/virt/libvirt/volume/fibrechannel.py | 3 +- nova/virt/libvirt/volume/iscsi.py | 3 +- nova/virt/libvirt/volume/iser.py | 3 +- nova/virt/libvirt/volume/nvme.py | 3 +- ...me_enforce_multipath-c790e98b9b05848e.yaml | 9 ++++ requirements.txt | 2 +- 10 files changed, 122 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/volume_enforce_multipath-c790e98b9b05848e.yaml diff --git a/nova/conf/libvirt.py b/nova/conf/libvirt.py index 4ac488969f..b77986b915 100644 --- a/nova/conf/libvirt.py +++ b/nova/conf/libvirt.py @@ -1112,6 +1112,22 @@ Use multipath connection of the iSCSI or FC volume Volumes can be connected in the LibVirt as multipath devices. This will provide high availability and fault tolerance. +"""), + cfg.BoolOpt('volume_enforce_multipath', + default=False, + help=""" +Require multipathd when attaching a volume to an instance. + +When enabled, attachment of volumes will be aborted when multipathd is not +running. Otherwise, it will fallback to single path without error. + +When enabled, the libvirt driver checks availability of mulitpathd when it is +initialized, and the compute service fails to start if multipathd is not +running. + +Related options: + +* volume_use_multipath must be True when this is True """), cfg.IntOpt('num_volume_scan_tries', deprecated_name='num_iscsi_scan_tries', diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 9f75a8cadc..436c3a04c1 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -1661,6 +1661,59 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock_which.assert_not_called() + def test__check_multipath_misconfiguration(self): + self.flags(volume_use_multipath=False, volume_enforce_multipath=True, + group='libvirt') + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + exc = self.assertRaises( + exception.InvalidConfiguration, + drvr._check_multipath) + + self.assertIn( + "The 'volume_use_multipath' option should be 'True' when " + "the 'volume_enforce_multipath' option is 'True'.", + str(exc), + ) + + @mock.patch('os_brick.initiator.linuxscsi.LinuxSCSI.is_multipath_running') + def test__check_multipath_disabled(self, is_multipath_running): + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + drvr._check_multipath() + is_multipath_running.assert_not_called() + + @mock.patch('os_brick.initiator.linuxscsi.LinuxSCSI.is_multipath_running') + def test__check_multipath_optional(self, is_multipath_running): + self.flags(volume_use_multipath=True, group='libvirt') + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + drvr._check_multipath() + is_multipath_running.assert_not_called() + + @mock.patch('os_brick.initiator.linuxscsi.LinuxSCSI.is_multipath_running') + def test__check_multipath_enforced(self, is_multipath_running): + is_multipath_running.return_value = True + self.flags(volume_use_multipath=True, volume_enforce_multipath=True, + group='libvirt') + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + drvr._check_multipath() + is_multipath_running.assert_called_once_with(root_helper=mock.ANY) + + @mock.patch('os_brick.initiator.linuxscsi.LinuxSCSI.is_multipath_running') + def test__check_multipath_enforced_missing(self, is_multipath_running): + is_multipath_running.return_value = False + self.flags(volume_use_multipath=True, volume_enforce_multipath=True, + group='libvirt') + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + exc = self.assertRaises( + exception.InvalidConfiguration, + drvr._check_multipath) + + self.assertIn( + "The 'volume_enforce_multipath' option is 'True' but " + "multipathd is not running.", + str(exc), + ) + is_multipath_running.assert_called_once_with(root_helper=mock.ANY) + @mock.patch.object(libvirt_driver.LOG, 'warning') def test_check_cpu_set_configuration__no_configuration(self, mock_log): """Test that configuring no CPU option results no errors or logs. diff --git a/nova/tests/unit/virt/libvirt/volume/test_nvme.py b/nova/tests/unit/virt/libvirt/volume/test_nvme.py index 42ef0adc8d..5413c3ba16 100644 --- a/nova/tests/unit/virt/libvirt/volume/test_nvme.py +++ b/nova/tests/unit/virt/libvirt/volume/test_nvme.py @@ -30,7 +30,7 @@ class LibvirtNVMEVolumeDriverTestCase(test_volume.LibvirtVolumeBaseTestCase): nvme.LibvirtNVMEVolumeDriver(self.fake_host) mock_factory.assert_called_once_with( initiator.NVME, 'sudo', use_multipath=False, - device_scan_attempts=3) + device_scan_attempts=3, enforce_multipath=False) @mock.patch('os.path.exists', return_value=True) @mock.patch('nova.utils.get_root_helper') @@ -44,7 +44,21 @@ class LibvirtNVMEVolumeDriverTestCase(test_volume.LibvirtVolumeBaseTestCase): nvme.LibvirtNVMEVolumeDriver(self.fake_host) mock_factory.assert_called_once_with( initiator.NVME, 'sudo', use_multipath=True, - device_scan_attempts=3) + device_scan_attempts=3, enforce_multipath=False) + + @mock.patch('os.path.exists', return_value=True) + @mock.patch('nova.utils.get_root_helper') + @mock.patch('os_brick.initiator.connector.InitiatorConnector.factory') + def test_libvirt_nvme_driver_multipath_enforced(self, mock_factory, + mock_helper, exists): + self.flags(num_nvme_discover_tries=3, volume_use_multipath=True, + volume_enforce_multipath=True, group='libvirt') + mock_helper.return_value = 'sudo' + + nvme.LibvirtNVMEVolumeDriver(self.fake_host) + mock_factory.assert_called_once_with( + initiator.NVME, 'sudo', use_multipath=True, + device_scan_attempts=3, enforce_multipath=True) @mock.patch('os_brick.initiator.connector.InitiatorConnector.factory', new=mock.Mock(return_value=mock.Mock())) diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index c4b06b47be..0f22b9c856 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -56,6 +56,7 @@ from os_brick import encryptors from os_brick.encryptors import luks as luks_encryptor from os_brick import exception as brick_exception from os_brick.initiator import connector +from os_brick.initiator import linuxscsi import os_resource_classes as orc import os_traits as ot from oslo_concurrency import processutils @@ -900,6 +901,8 @@ class LibvirtDriver(driver.ComputeDriver): self._check_vtpm_support() + self._check_multipath() + # Set REGISTER_IMAGE_PROPERTY_DEFAULTS in the instance system_metadata # to default values for properties that have not already been set. self._register_all_undefined_instance_details() @@ -1164,6 +1167,22 @@ class LibvirtDriver(driver.ComputeDriver): LOG.debug('Enabling emulated TPM support') + def _check_multipath(self) -> None: + if not CONF.libvirt.volume_enforce_multipath: + return + + if not CONF.libvirt.volume_use_multipath: + msg = _("The 'volume_use_multipath' option should be 'True' when " + "the 'volume_enforce_multipath' option is 'True'.") + raise exception.InvalidConfiguration(msg) + + multipath_running = linuxscsi.LinuxSCSI.is_multipath_running( + root_helper=utils.get_root_helper()) + if not multipath_running: + msg = _("The 'volume_enforce_multipath' option is 'True' but " + "multipathd is not running.") + raise exception.InvalidConfiguration(msg) + def _start_inactive_mediated_devices(self): # Get a list of inactive mdevs so we can start them and make them # active. We need to start inactive mdevs even if they are not diff --git a/nova/virt/libvirt/volume/fibrechannel.py b/nova/virt/libvirt/volume/fibrechannel.py index 1752f6d0cc..36e7d8904a 100644 --- a/nova/virt/libvirt/volume/fibrechannel.py +++ b/nova/virt/libvirt/volume/fibrechannel.py @@ -35,7 +35,8 @@ class LibvirtFibreChannelVolumeDriver(libvirt_volume.LibvirtBaseVolumeDriver): self.connector = connector.InitiatorConnector.factory( initiator.FIBRE_CHANNEL, utils.get_root_helper(), use_multipath=CONF.libvirt.volume_use_multipath, - device_scan_attempts=CONF.libvirt.num_volume_scan_tries) + device_scan_attempts=CONF.libvirt.num_volume_scan_tries, + enforce_multipath=CONF.libvirt.volume_enforce_multipath) def get_config(self, connection_info, disk_info): """Returns xml for libvirt.""" diff --git a/nova/virt/libvirt/volume/iscsi.py b/nova/virt/libvirt/volume/iscsi.py index 2b25972a49..7c99615c38 100644 --- a/nova/virt/libvirt/volume/iscsi.py +++ b/nova/virt/libvirt/volume/iscsi.py @@ -38,7 +38,8 @@ class LibvirtISCSIVolumeDriver(libvirt_volume.LibvirtBaseVolumeDriver): initiator.ISCSI, utils.get_root_helper(), use_multipath=CONF.libvirt.volume_use_multipath, device_scan_attempts=CONF.libvirt.num_volume_scan_tries, - transport=self._get_transport()) + transport=self._get_transport(), + enforce_multipath=CONF.libvirt.volume_enforce_multipath) def _get_transport(self): if CONF.libvirt.iscsi_iface: diff --git a/nova/virt/libvirt/volume/iser.py b/nova/virt/libvirt/volume/iser.py index b3cbddb3d4..e71548a7d2 100644 --- a/nova/virt/libvirt/volume/iser.py +++ b/nova/virt/libvirt/volume/iser.py @@ -33,7 +33,8 @@ class LibvirtISERVolumeDriver(iscsi.LibvirtISCSIVolumeDriver): initiator.ISER, utils.get_root_helper(), use_multipath=CONF.libvirt.iser_use_multipath, device_scan_attempts=CONF.libvirt.num_iser_scan_tries, - transport=self._get_transport()) + transport=self._get_transport(), + enforce_multipath=CONF.libvirt.volume_enforce_multipath) def _get_transport(self): return 'iser' diff --git a/nova/virt/libvirt/volume/nvme.py b/nova/virt/libvirt/volume/nvme.py index e2977c3572..b1829bcbef 100644 --- a/nova/virt/libvirt/volume/nvme.py +++ b/nova/virt/libvirt/volume/nvme.py @@ -34,7 +34,8 @@ class LibvirtNVMEVolumeDriver(libvirt_volume.LibvirtVolumeDriver): self.connector = connector.InitiatorConnector.factory( initiator.NVME, utils.get_root_helper(), use_multipath=CONF.libvirt.volume_use_multipath, - device_scan_attempts=CONF.libvirt.num_nvme_discover_tries) + device_scan_attempts=CONF.libvirt.num_nvme_discover_tries, + enforce_multipath=CONF.libvirt.volume_enforce_multipath) def connect_volume(self, connection_info, instance): diff --git a/releasenotes/notes/volume_enforce_multipath-c790e98b9b05848e.yaml b/releasenotes/notes/volume_enforce_multipath-c790e98b9b05848e.yaml new file mode 100644 index 0000000000..e8aa9aa5d1 --- /dev/null +++ b/releasenotes/notes/volume_enforce_multipath-c790e98b9b05848e.yaml @@ -0,0 +1,9 @@ +--- +features: + - | + The new ``[libvirt] volume_enforce_multipath`` option has been added. When + this option is set to ``True``, attachment of volumes is aborted when + multipathd is not running in the host. Otherwise it falls back to single + path. This option also makes the libvirt driver to check multipathd during + initialization, and the compute service fails to start if mulitipathd is + not running. diff --git a/requirements.txt b/requirements.txt index 65ea56c459..18eaf62f34 100644 --- a/requirements.txt +++ b/requirements.txt @@ -49,7 +49,7 @@ rfc3986>=1.2.0 # Apache-2.0 oslo.middleware>=3.31.0 # Apache-2.0 psutil>=3.2.2 # BSD oslo.versionedobjects>=1.35.0 # Apache-2.0 -os-brick>=6.0 # Apache-2.0 +os-brick>=6.10.0 # Apache-2.0 os-resource-classes>=1.1.0 # Apache-2.0 os-traits>=3.3.0 # Apache-2.0 os-vif>=3.1.0 # Apache-2.0