openstack/os-brick
Code Issues Proposed changes

Merge "Fix multipathd dependency in NVMe-oF connections"

Browse Source
This commit is contained in:
Zuul 2024-12-13 15:41:27 +00:00 committed by Gerrit Code Review
commit 1487dc93b6
11 changed files with 154 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
os_brick/initiator/connectors/base.py
View File

@ -112,16 +112,43 @@ class BaseLinuxConnector(initiator_connector.InitiatorConnector):
@staticmethod
def get_connector_properties(root_helper: str, *args, **kwargs) -> dict:
"""The generic connector properties."""
multipath = kwargs['multipath']
enforce_multipath = kwargs['enforce_multipath']
props = {}
# The 'multipath' and 'enforce_multipath' values will be used by
# the caller to verify multipathing in connect_volume.
return {
'multipath': kwargs['multipath'],
'enforce_multipath': kwargs['enforce_multipath'],
}
props['multipath'] = (multipath and
linuxscsi.LinuxSCSI.is_multipath_running(
enforce_multipath, root_helper,
execute=kwargs.get('execute')))
def supports_multipath(self):
"""Generic method to report multipath support.
return props
Each connector, which supports multipath, should override this
method and provide its own implementation of checking the
multipath support. See implementation in iSCSI, FC or NVMe
connectors for reference.
"""
return False
def check_multipath(self, connection_properties):
LOG.debug("Connection properties %s", connection_properties)
multipath = self.use_multipath
# If we are using an old cinder, it will not contain the
# 'enforce_multipath' key and we will default the value to False.
# Unfortunately, there is is no way to know which Cinder
# version we are using when calling get_connector_properties to
# keep backward compatibility.
enforce_multipath = connection_properties.get(
'enforce_multipath', False)
if not self.supports_multipath():
if multipath and enforce_multipath:
raise exception.BrickException(
"Multipathing is enforced but the host doesn't "
"support multipathing.")
if multipath and not enforce_multipath:
LOG.warning(
"Multipathing is requested but the host "
"doesn't support multipathing.")
def check_valid_device(self, path: str, run_as_root: bool = True) -> bool:
return utils.check_valid_device(self, path)

5
os_brick/initiator/connectors/fibre_channel.py
View File

@ -203,6 +203,10 @@ class FibreChannelConnector(base.BaseLinuxConnector):
{'props': connection_properties})
raise exception.VolumePathsNotFound()
def supports_multipath(self):
return self._linuxscsi.is_multipath_running(
root_helper=self._root_helper)
@utils.trace
@utils.connect_volume_prepare_result
@base.synchronized('connect_volume', external=True)
@ -218,6 +222,7 @@ class FibreChannelConnector(base.BaseLinuxConnector):
target_wwn - World Wide Name
target_lun - LUN id of the volume
"""
self.check_multipath(connection_properties)
device_info = {'type': 'block'}
connection_properties = self._add_targets_to_connection_properties(

5
os_brick/initiator/connectors/iscsi.py
View File

@ -510,6 +510,10 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector):
connection_properties)})
raise exception.VolumePathsNotFound()
def supports_multipath(self):
return self._linuxscsi.is_multipath_running(
root_helper=self._root_helper)
@utils.trace
@utils.connect_volume_prepare_result
@base.synchronized('connect_volume', external=True)
@ -529,6 +533,7 @@ class ISCSIConnector(base.BaseLinuxConnector, base_iscsi.BaseISCSIConnector):
target_lun(s) - LUN id of the volume
Note that plural keys may be used when use_multipath=True
"""
self.check_multipath(connection_properties)
try:
if self.use_multipath:
return self._connect_multipath_volume(connection_properties)

7
os_brick/initiator/connectors/nvmeof.py
View File

@ -644,6 +644,7 @@ class NVMeOFConnProps(object):
self.encrypted = conn_props.get('encrypted', False)
self.cacheable = conn_props.get('cacheable', False)
self.discard = conn_props.get('discard', False)
self.enforce_multipath = conn_props.get('enforce_multipath', False)
# old connection properties format
if REPLICAS not in conn_props and NQN not in conn_props:
@ -852,6 +853,9 @@ class NVMeOFConnector(base.BaseLinuxConnector):
# ####### Connect Volume methods ########
def supports_multipath(self):
return self.native_multipath_supported
@utils.trace
@utils.connect_volume_prepare_result
@base.synchronized('connect_volume', external=True)
@ -859,6 +863,9 @@ class NVMeOFConnector(base.BaseLinuxConnector):
def connect_volume(
self, connection_properties: NVMeOFConnProps) -> dict[str, str]:
"""Attach and discover the volume."""
self.check_multipath(
{'enforce_multipath': getattr(
connection_properties, 'enforce_multipath', False)})
try:
if connection_properties.is_replicated is False:
LOG.debug('Starting non replicated connection')

13
os_brick/initiator/linuxscsi.py
View File

@ -277,8 +277,7 @@ class LinuxSCSI(executor.Executor):
return out.strip()
@staticmethod
def is_multipath_running(enforce_multipath,
root_helper,
def is_multipath_running(root_helper,
execute=None) -> bool:
try:
if execute is None:
@ -289,13 +288,9 @@ class LinuxSCSI(executor.Executor):
# There was a bug in multipathd where it didn't return an error
# code and just printed the error message in stdout.
if out and out.startswith('error receiving packet'):
raise putils.ProcessExecutionError('', out, 1, cmd, None)
return False
except putils.ProcessExecutionError as err:
if enforce_multipath:
LOG.error('multipathd is not running: exit code %(err)s',
{'err': err.exit_code})
raise
except putils.ProcessExecutionError:
return False
return True
@ -384,7 +379,7 @@ class LinuxSCSI(executor.Executor):
multipath_running = True
else:
multipath_running = self.is_multipath_running(
enforce_multipath=False, root_helper=self._root_helper)
root_helper=self._root_helper)
for device_name in devices_names:
dev_path = '/dev/' + device_name

10
os_brick/tests/initiator/connectors/test_fibre_channel.py
View File

@ -219,6 +219,8 @@ class FibreChannelConnectorTestCase(test_connector.ConnectorTestCase):
'-fc-0x1234567890123456-lun-1']
self.assertEqual(expected, volume_paths)
@mock.patch.object(
base.BaseLinuxConnector, 'check_multipath', mock.MagicMock())
@mock.patch.object(linuxscsi.LinuxSCSI, 'wait_for_rw')
@mock.patch.object(os.path, 'exists', return_value=True)
@mock.patch.object(os.path, 'realpath', return_value='/dev/sdb')
@ -290,6 +292,8 @@ class FibreChannelConnectorTestCase(test_connector.ConnectorTestCase):
self.connector.connect_volume,
connection_info['data'])
@mock.patch.object(
base.BaseLinuxConnector, 'check_multipath', mock.MagicMock())
@mock.patch.object(linuxscsi.LinuxSCSI, 'find_multipath_device_path')
@mock.patch.object(linuxscsi.LinuxSCSI, 'wait_for_mpath_device')
def _test_connect_volume_multipath(self, get_device_info_mock,
@ -994,3 +998,9 @@ class FibreChannelConnectorTestCase(test_connector.ConnectorTestCase):
mock.sentinel.lun2B),
]
self.assertEqual(expected, res)
@mock.patch.object(linuxscsi.LinuxSCSI, 'is_multipath_running')
def test_supports_multipath(self, mock_mpath_running):
self.connector.supports_multipath()
mock_mpath_running.assert_called_once_with(
root_helper=self.connector._root_helper)

18
os_brick/tests/initiator/connectors/test_iscsi.py
View File

@ -19,6 +19,7 @@ import ddt
from oslo_concurrency import processutils as putils
from os_brick import exception
from os_brick.initiator.connectors import base
from os_brick.initiator.connectors import iscsi
from os_brick.initiator import linuxscsi
from os_brick.initiator import utils
@ -396,6 +397,8 @@ class ISCSIConnectorTestCase(test_connector.ConnectorTestCase):
'multipath_id': FAKE_SCSI_WWN}
self.assertEqual(expected_result, result)
@mock.patch.object(
base.BaseLinuxConnector, 'check_multipath', mock.MagicMock())
@mock.patch.object(iscsi.ISCSIConnector, '_cleanup_connection')
@mock.patch.object(iscsi.ISCSIConnector, '_connect_multipath_volume')
@mock.patch.object(iscsi.ISCSIConnector, '_connect_single_volume')
@ -407,6 +410,8 @@ class ISCSIConnectorTestCase(test_connector.ConnectorTestCase):
con_mp_mock.assert_called_once_with(self.CON_PROPS)
clean_mock.assert_not_called()
@mock.patch.object(
base.BaseLinuxConnector, 'check_multipath', mock.MagicMock())
@mock.patch.object(iscsi.ISCSIConnector, '_cleanup_connection')
@mock.patch.object(iscsi.ISCSIConnector, '_connect_multipath_volume')
@mock.patch.object(iscsi.ISCSIConnector, '_connect_single_volume')
@ -420,6 +425,8 @@ class ISCSIConnectorTestCase(test_connector.ConnectorTestCase):
con_mp_mock.assert_called_once_with(self.CON_PROPS)
clean_mock.assert_called_once_with(self.CON_PROPS, force=True)
@mock.patch.object(
base.BaseLinuxConnector, 'check_multipath', mock.MagicMock())
@mock.patch.object(iscsi.ISCSIConnector, '_cleanup_connection')
@mock.patch.object(iscsi.ISCSIConnector, '_connect_multipath_volume')
@mock.patch.object(iscsi.ISCSIConnector, '_connect_single_volume')
@ -431,6 +438,8 @@ class ISCSIConnectorTestCase(test_connector.ConnectorTestCase):
con_single_mock.assert_called_once_with(self.CON_PROPS)
clean_mock.assert_not_called()
@mock.patch.object(
base.BaseLinuxConnector, 'check_multipath', mock.MagicMock())
@mock.patch.object(iscsi.ISCSIConnector, '_cleanup_connection')
@mock.patch.object(iscsi.ISCSIConnector, '_connect_multipath_volume')
@mock.patch.object(iscsi.ISCSIConnector, '_connect_single_volume')
@ -489,11 +498,12 @@ class ISCSIConnectorTestCase(test_connector.ConnectorTestCase):
# Reset to run with a different transport type
self.cmds = list()
@mock.patch.object(priv_rootwrap, 'execute', return_value=('', ''))
@mock.patch.object(iscsi.ISCSIConnector,
'_run_iscsiadm_update_discoverydb')
@mock.patch.object(os.path, 'exists', return_value=True)
def test_iscsi_portals_with_chap_discovery(
self, exists, update_discoverydb):
self, exists, update_discoverydb, mock_exec):
location = '10.0.2.15:3260'
name = 'volume-00000001'
iqn = 'iqn.2010-10.org.openstack:%s' % name
@ -1777,3 +1787,9 @@ Setting up iSCSI targets: unused
(mock.sentinel.portal2, mock.sentinel.iqn2, mock.sentinel.lun2B)
]
self.assertListEqual(expected, res)
@mock.patch.object(linuxscsi.LinuxSCSI, 'is_multipath_running')
def test_supports_multipath(self, mock_mpath_running):
self.connector.supports_multipath()
mock_mpath_running.assert_called_once_with(
root_helper=self.connector._root_helper)

5
os_brick/tests/initiator/connectors/test_nvmeof.py
View File

@ -2215,3 +2215,8 @@ class NVMeOFConnectorTestCase(test_connector.ConnectorTestCase):
self.connector._try_disconnect(portal)
mock_can_disconnect.assert_called_once_with()
mock_execute.assert_not_called()
@ddt.data(False, True)
def test_supports_multipath(self, ana_support):
self.connector.native_multipath_supported = ana_support
self.assertEqual(ana_support, self.connector.supports_multipath())

83
os_brick/tests/initiator/test_connector.py
View File

@ -16,6 +16,7 @@ import platform
import sys
from unittest import mock
import ddt
from oslo_concurrency import processutils as putils
from oslo_service import loopingcall
@ -82,6 +83,7 @@ class ConnectorUtilsTestCase(test_base.TestCase):
'host': host,
'ip': MY_IP,
'multipath': multipath_result,
'enforce_multipath': enforce_multipath,
'nvme_native_multipath': False,
'os_type': os_type,
'platform': platform,
@ -116,40 +118,25 @@ class ConnectorUtilsTestCase(test_base.TestCase):
@mock.patch.object(priv_rootwrap, 'custom_execute',
side_effect=OSError(2))
@mock.patch.object(priv_rootwrap, 'execute', return_value=('', ''))
def test_brick_get_connector_properties_multipath(self, mock_execute,
def test_brick_get_connector_properties_multipath(self,
mock_custom_execute):
self._test_brick_get_connector_properties(True, True, True)
mock_execute.assert_called_once_with('multipathd', 'show', 'status',
run_as_root=True,
root_helper='sudo')
mock_custom_execute.assert_called_once_with('nvme', 'version')
@mock.patch.object(priv_rootwrap, 'custom_execute',
side_effect=OSError(2))
@mock.patch.object(priv_rootwrap, 'execute',
side_effect=putils.ProcessExecutionError)
def test_brick_get_connector_properties_fallback(self, mock_execute,
def test_brick_get_connector_properties_fallback(self,
mock_custom_execute):
self._test_brick_get_connector_properties(True, False, False)
mock_execute.assert_called_once_with('multipathd', 'show', 'status',
run_as_root=True,
root_helper='sudo')
self._test_brick_get_connector_properties(True, False, True)
mock_custom_execute.assert_called_once_with('nvme', 'version')
@mock.patch.object(priv_rootwrap, 'execute',
side_effect=putils.ProcessExecutionError)
def test_brick_get_connector_properties_raise(self, mock_execute):
self.assertRaises(putils.ProcessExecutionError,
self._test_brick_get_connector_properties,
True, True, None)
def test_brick_connector_properties_override_hostname(self):
override_host = 'myhostname'
self._test_brick_get_connector_properties(False, False, False,
host=override_host)
@ddt.ddt
class ConnectorTestCase(test_base.TestCase):
def setUp(self):
@ -191,7 +178,11 @@ class ConnectorTestCase(test_base.TestCase):
'sudo', multipath=multipath,
enforce_multipath=enforce_multipath)
expected_props = {'multipath': True}
expected_props = {
'multipath': multipath,
'enforce_multipath': enforce_multipath,
}
self.assertEqual(expected_props, props)
multipath = False
@ -200,18 +191,12 @@ class ConnectorTestCase(test_base.TestCase):
'sudo', multipath=multipath,
enforce_multipath=enforce_multipath)
expected_props = {'multipath': False}
self.assertEqual(expected_props, props)
expected_props = {
'multipath': multipath,
'enforce_multipath': enforce_multipath,
}
with mock.patch.object(priv_rootwrap, 'execute',
side_effect=putils.ProcessExecutionError):
multipath = True
enforce_multipath = True
self.assertRaises(
putils.ProcessExecutionError,
base.BaseLinuxConnector.get_connector_properties,
'sudo', multipath=multipath,
enforce_multipath=enforce_multipath)
self.assertEqual(expected_props, props)
@mock.patch('sys.platform', 'win32')
def test_get_connector_mapping_win32(self):
@ -304,3 +289,39 @@ class ConnectorTestCase(test_base.TestCase):
with mock.patch.object(self.connector, '_execute',
side_effect=putils.ProcessExecutionError):
self.assertFalse(self.connector.check_valid_device('/dev'))
@ddt.data(
(False, False, False),
(False, False, True),
(False, True, False),
(False, True, True),
(True, False, False),
(True, False, True),
(True, True, False),
(True, True, True),
)
@ddt.unpack
@mock.patch.object(base, 'LOG')
@mock.patch.object(base.BaseLinuxConnector, 'supports_multipath')
def test_check_multipath(
self, mpath, enforce_mpath, supports_mpath, mock_supports_mpath,
mock_log):
self.connector = fake.FakeConnector(None)
fake_conn_props = self.fake_connection()
self.connector.use_multipath = mpath
mock_supports_mpath.return_value = supports_mpath
fake_conn_props['data']['enforce_multipath'] = enforce_mpath
if mpath and enforce_mpath and not supports_mpath:
self.assertRaises(
exception.BrickException,
self.connector.check_multipath,
fake_conn_props['data']
)
else:
self.connector.check_multipath(fake_conn_props['data'])
if mpath and not enforce_mpath and not supports_mpath:
mock_log.warning.assert_called_once_with(
"Multipathing is requested but the host "
"doesn't support multipathing."
)
mock_supports_mpath.assert_called_once_with()

23
os_brick/tests/initiator/test_linuxscsi.py
View File

@ -1020,34 +1020,21 @@ loop0 0"""
@mock.patch('os_brick.privileged.rootwrap.execute', return_value=('', ''))
def test_is_multipath_running(self, mock_exec):
res = linuxscsi.LinuxSCSI.is_multipath_running(False, None, mock_exec)
res = linuxscsi.LinuxSCSI.is_multipath_running(None, mock_exec)
self.assertTrue(res)
mock_exec.assert_called_once_with(
'multipathd', 'show', 'status', run_as_root=True, root_helper=None)
@mock.patch.object(linuxscsi, 'LOG')
@mock.patch('os_brick.privileged.rootwrap.execute')
def test_is_multipath_running_failure(
self, mock_exec, mock_log
):
mock_exec.side_effect = putils.ProcessExecutionError()
self.assertRaises(putils.ProcessExecutionError,
linuxscsi.LinuxSCSI.is_multipath_running,
True, None, mock_exec)
mock_log.error.assert_called_once()
@mock.patch.object(linuxscsi, 'LOG')
@mock.patch('os_brick.privileged.rootwrap.execute')
def test_is_multipath_running_failure_exit_code_0(
self, mock_exec, mock_log
):
mock_exec.return_value = ('error receiving packet', '')
self.assertRaises(putils.ProcessExecutionError,
linuxscsi.LinuxSCSI.is_multipath_running,
True, None, mock_exec)
res = linuxscsi.LinuxSCSI.is_multipath_running(None, mock_exec)
mock_exec.assert_called_once_with(
'multipathd', 'show', 'status', run_as_root=True, root_helper=None)
mock_log.error.assert_called_once()
self.assertFalse(res)
@mock.patch.object(linuxscsi, 'LOG')
@mock.patch('os_brick.privileged.rootwrap.execute')
@ -1055,7 +1042,7 @@ loop0 0"""
self, mock_exec, mock_log
):
mock_exec.side_effect = putils.ProcessExecutionError()
res = linuxscsi.LinuxSCSI.is_multipath_running(False, None, mock_exec)
res = linuxscsi.LinuxSCSI.is_multipath_running(None, mock_exec)
mock_exec.assert_called_once_with(
'multipathd', 'show', 'status', run_as_root=True, root_helper=None)
self.assertFalse(res)
@ -1067,7 +1054,7 @@ loop0 0"""
self, mock_exec, mock_log
):
mock_exec.return_value = ('error receiving packet', '')
res = linuxscsi.LinuxSCSI.is_multipath_running(False, None, mock_exec)
res = linuxscsi.LinuxSCSI.is_multipath_running(None, mock_exec)
mock_exec.assert_called_once_with(
'multipathd', 'show', 'status', run_as_root=True, root_helper=None)
self.assertFalse(res)

9
releasenotes/notes/fix-multipathd-dependency-nvmeof-d61187f1ab3808c2.yaml Normal file
View File

@ -0,0 +1,9 @@
---
fixes:
- |
NVMe-oF connector `bug #2085013
<https://bugs.launchpad.net/os-brick/+bug/2085013>`_ : Removed
``multipathd`` dependency for NVMe-oF connections.
Previously, multipathd had to be installed to leverage native
NVMe multipathing (ANA) but now that dependency is removed.