summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorZuul <zuul@review.opendev.org>2022-05-30 06:02:46 +0000
committerGerrit Code Review <review@openstack.org>2022-05-30 06:02:46 +0000
commitbe8c195488e710af815d1a44400c307227f88a04 (patch)
treeac0fa298d5d4bda3a68a7d7ef99b8d2e275698b7
parent65899728b127d411a02747af6005f4c973cff24e (diff)
parent668cd217967c6d114cec2241e97d3cfaee1a7a74 (diff)
downloadironic-python-agent-be8c195488e710af815d1a44400c307227f88a04.tar.gz
Merge "Multipath Hardware path handling" into stable/wallaby
-rw-r--r--ironic_python_agent/hardware.py152
-rw-r--r--ironic_python_agent/tests/unit/extensions/test_image.py2
-rw-r--r--ironic_python_agent/tests/unit/samples/hardware_samples.py59
-rw-r--r--ironic_python_agent/tests/unit/test_agent.py21
-rw-r--r--ironic_python_agent/tests/unit/test_hardware.py537
-rw-r--r--ironic_python_agent/tests/unit/test_utils.py6
-rw-r--r--ironic_python_agent/utils.py1
-rw-r--r--releasenotes/notes/multipath-handling-00a5b412d2cf2e4e.yaml18
8 files changed, 740 insertions, 56 deletions
diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py
index c0a8e85a..8341394f 100644
--- a/ironic_python_agent/hardware.py
+++ b/ironic_python_agent/hardware.py
@@ -80,6 +80,8 @@ RAID_APPLY_CONFIGURATION_ARGSINFO = {
}
}
+MULTIPATH_ENABLED = None
+
def _get_device_info(dev, devclass, field):
"""Get the device info according to device class and field."""
@@ -126,6 +128,35 @@ def _udev_settle():
return
+def _load_multipath_modules():
+ """Load multipath modules
+
+ This is required to be able to collect multipath information.
+ Two separate paths exist, one with a helper utility for Centos/RHEL
+ and another which is just load the modules, and trust multipathd
+ will do the needful.
+ """
+ if (os.path.isfile('/usr/sbin/mpathconf')
+ and not os.path.isfile('/etc/multipath.conf')):
+ # For Centos/Rhel/Etc which uses mpathconf, this does
+ # a couple different things, including configuration generation...
+ # which is not *really* required.. at least *shouldn't* be.
+ # WARNING(TheJulia): This command explicitly replaces local
+ # configuration.
+ il_utils.try_execute('/usr/sbin/mpathconf', '--enable',
+ '--find_multipaths', 'yes',
+ '--with_module', 'y',
+ '--with_multipathd', 'y')
+ else:
+ # Ensure modules are loaded. Configuration is not required
+ # and implied based upon compiled in defaults.
+ # NOTE(TheJulia): Debian/Ubuntu specifically just document
+ # using `multipath -t` output to start a new configuration
+ # file, if needed.
+ il_utils.try_execute('modprobe', 'dm_multipath')
+ il_utils.try_execute('modprobe', 'multipath')
+
+
def _check_for_iscsi():
"""Connect iSCSI shared connected via iBFT or OF.
@@ -169,6 +200,84 @@ def _get_md_uuid(raid_device):
return match.group(1)
+def _enable_multipath():
+ """Initialize multipath IO if possible.
+
+ :returns: True if the multipathd daemon and multipath command to enumerate
+ devices was scucessfully able to be called.
+ """
+ try:
+ _load_multipath_modules()
+ # This might not work, ideally it *should* already be running...
+ # NOTE(TheJulia): Testing locally, a prior running multipathd, the
+ # explicit multipathd start just appears to silently exit with a
+ # result code of 0.
+ utils.execute('multipathd')
+ # This is mainly to get the system to actually do the needful and
+ # identify/enumerate paths by combining what it can detect and what
+ # it already knows. This may be useful, and in theory this should be
+ # logged in the IPA log should it be needed.
+ utils.execute('multipath', '-ll')
+ return True
+ except FileNotFoundError as e:
+ LOG.warning('Attempted to determine if multipath tools were present. '
+ 'Not detected. Error recorded: %s', e)
+ return False
+ except processutils.ProcessExecutionError as e:
+ LOG.warning('Attempted to invoke multipath utilities, but we '
+ 'encountered an error: %s', e)
+ return False
+
+
+def _get_multipath_parent_device(device):
+ """Check and return a multipath device."""
+ if not device:
+ # if lsblk provides invalid output, this can be None.
+ return
+ check_device = os.path.join('/dev', str(device))
+ try:
+ # Explicitly run the check as regardless of if the device is mpath or
+ # not, multipath tools when using list always exits with a return
+ # code of 0.
+ utils.execute('multipath', '-c', check_device)
+ # path check with return an exit code of 1 if you send it a multipath
+ # device mapper device, like dm-0.
+ # NOTE(TheJulia): -ll is supposed to load from all available
+ # information, but may not force a rescan. It may be -f if we need
+ # that. That being said, it has been about a decade since I was
+ # running multipath tools on SAN connected gear, so my memory is
+ # definitely fuzzy.
+ out, _ = utils.execute('multipath', '-ll', check_device)
+ except processutils.ProcessExecutionError as e:
+ # FileNotFoundError if the utility does not exist.
+ # -1 return code if the device is not valid.
+ LOG.debug('Checked device %(dev)s and determined it was '
+ 'not a multipath device. %(error)s',
+ {'dev': check_device,
+ 'error': e})
+ return
+ except FileNotFoundError:
+ # This should never happen, as MULTIPATH_ENABLED would be False
+ # before this occurs.
+ LOG.warning('Attempted to check multipathing status, however '
+ 'the \'multipath\' binary is missing or not in the '
+ 'execution PATH.')
+ return
+ # Data format:
+ # MPATHDEVICENAME dm-0 TYPE,HUMANNAME
+ # size=56G features='1 retain_attached_hw_handler' hwhandler='0' wp=rw
+ # `-+- policy='service-time 0' prio=1 status=active
+ # `- 0:0:0:0 sda 8:0 active ready running
+ try:
+ lines = out.splitlines()
+ mpath_device = lines[0].split(' ')[1]
+ # give back something like dm-0 so we can log it.
+ return mpath_device
+ except IndexError:
+ # We didn't get any command output, so Nope.
+ pass
+
+
def _get_component_devices(raid_device):
"""Get the component devices of a Software RAID device.
@@ -359,7 +468,8 @@ def _md_scan_and_assemble():
def list_all_block_devices(block_type='disk',
ignore_raid=False,
ignore_floppy=True,
- ignore_empty=True):
+ ignore_empty=True,
+ ignore_multipath=False):
"""List all physical block devices
The switches we use for lsblk: P for KEY="value" output, b for size output
@@ -376,6 +486,9 @@ def list_all_block_devices(block_type='disk',
:param ignore_floppy: Ignore floppy disk devices in the block device
list. By default, these devices are filtered out.
:param ignore_empty: Whether to ignore disks with size equal 0.
+ :param ignore_multipath: Whether to ignore devices backing multipath
+ devices. Default is to consider multipath
+ devices, if possible.
:return: A list of BlockDevices
"""
@@ -386,6 +499,8 @@ def list_all_block_devices(block_type='disk',
return True
return False
+ check_multipath = not ignore_multipath and get_multipath_status()
+
_udev_settle()
# map device names to /dev/disk/by-path symbolic links that points to it
@@ -416,7 +531,6 @@ def list_all_block_devices(block_type='disk',
'-o{}'.format(','.join(columns)))[0]
lines = report.splitlines()
context = pyudev.Context()
-
devices = []
for line in lines:
device = {}
@@ -438,16 +552,31 @@ def list_all_block_devices(block_type='disk',
LOG.debug('Ignoring floppy disk device %s', device)
continue
+ dev_kname = device.get('KNAME')
+ if check_multipath:
+ # Net effect is we ignore base devices, and their base devices
+ # to what would be the mapped device name which would not pass the
+ # validation, but would otherwise be match-able.
+ mpath_parent_dev = _get_multipath_parent_device(dev_kname)
+ if mpath_parent_dev:
+ LOG.warning(
+ "We have identified a multipath device %(device)s, this "
+ "is being ignored in favor of %(mpath_device)s and its "
+ "related child devices.",
+ {'device': dev_kname,
+ 'mpath_device': mpath_parent_dev})
+ continue
# Search for raid in the reply type, as RAID is a
# disk device, and we should honor it if is present.
# Other possible type values, which we skip recording:
# lvm, part, rom, loop
+
if devtype != block_type:
if devtype is None or ignore_raid:
LOG.debug("Skipping: {!r}".format(line))
continue
elif ('raid' in devtype
- and block_type in ['raid', 'disk']):
+ and block_type in ['raid', 'disk', 'mpath']):
LOG.debug(
"TYPE detected to contain 'raid', signifying a "
"RAID volume. Found: {!r}".format(line))
@@ -461,6 +590,11 @@ def list_all_block_devices(block_type='disk',
LOG.debug(
"TYPE detected to contain 'md', signifying a "
"RAID partition. Found: {!r}".format(line))
+ elif devtype == 'mpath' and block_type == 'disk':
+ LOG.debug(
+ "TYPE detected to contain 'mpath', "
+ "signifing a device mapper multipath device. "
+ "Found: %s", line)
else:
LOG.debug(
"TYPE did not match. Wanted: {!r} but found: {!r}".format(
@@ -974,6 +1108,10 @@ class GenericHardwareManager(HardwareManager):
# Do some initialization before we declare ourself ready
_check_for_iscsi()
_md_scan_and_assemble()
+ global MULTIPATH_ENABLED
+ if MULTIPATH_ENABLED is None:
+ MULTIPATH_ENABLED = _enable_multipath()
+
self.wait_for_disks()
return HardwareSupport.GENERIC
@@ -2609,3 +2747,11 @@ def deduplicate_steps(candidate_steps):
deduped_steps[manager].append(winning_step)
return deduped_steps
+
+
+def get_multipath_status():
+ """Return the status of multipath initialization."""
+ # NOTE(TheJulia): Provides a nice place to mock out and simplify testing
+ # as if we directly try and work with the global var, we will be racing
+ # tests endlessly.
+ return MULTIPATH_ENABLED
diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py
index 6778527c..2cce89b3 100644
--- a/ironic_python_agent/tests/unit/extensions/test_image.py
+++ b/ironic_python_agent/tests/unit/extensions/test_image.py
@@ -787,6 +787,7 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640
mock_append_to_fstab.assert_called_with(self.fake_dir,
self.fake_efi_system_part_uuid)
+ @mock.patch.object(hardware, 'get_multipath_status', lambda *_: False)
@mock.patch.object(os.path, 'ismount', lambda *_: False)
@mock.patch.object(image, '_is_bootloader_loaded', lambda *_: True)
@mock.patch.object(os.path, 'exists', autospec=True)
@@ -898,6 +899,7 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640
uuid=self.fake_efi_system_part_uuid)
self.assertFalse(mock_dispatch.called)
+ @mock.patch.object(hardware, 'get_multipath_status', lambda *_: False)
@mock.patch.object(image, '_efi_boot_setup', lambda *_: False)
@mock.patch.object(os.path, 'ismount', lambda *_: False)
@mock.patch.object(image, '_is_bootloader_loaded', lambda *_: True)
diff --git a/ironic_python_agent/tests/unit/samples/hardware_samples.py b/ironic_python_agent/tests/unit/samples/hardware_samples.py
index 660c61b5..e402af45 100644
--- a/ironic_python_agent/tests/unit/samples/hardware_samples.py
+++ b/ironic_python_agent/tests/unit/samples/hardware_samples.py
@@ -121,7 +121,10 @@ BLK_DEVICE_TEMPLATE = (
'KNAME="fd1" MODEL="magic" SIZE="4096" ROTA="1" TYPE="disk" UUID="" '
'PARTUUID=""\n'
'KNAME="sdf" MODEL="virtual floppy" SIZE="0" ROTA="1" TYPE="disk" UUID="" '
- 'PARTUUID=""'
+ 'PARTUUID=""\n'
+ 'KNAME="dm-0" MODEL="NWD-BLP4-1600 " SIZE="1765517033472" '
+ ' ROTA="0" TYPE="mpath" UUID="" PARTUUID=""\n'
+
)
# NOTE(pas-ha) largest device is 1 byte smaller than 4GiB
@@ -160,6 +163,49 @@ RAID_BLK_DEVICE_TEMPLATE = (
'PARTUUID=""'
)
+MULTIPATH_BLK_DEVICE_TEMPLATE = (
+ 'KNAME="sda" MODEL="INTEL_SSDSC2CT060A3" SIZE="60022480896" ROTA="0" '
+ 'TYPE="disk" UUID="" PARTUUID=""\n'
+ 'KNAME="sda2" MODEL="" SIZE="59162722304" ROTA="0" TYPE="part" '
+ 'UUID="f8b55d59-96c3-3982-b129-1b6b2ee8da86" '
+ 'PARTUUID="c97c8aac-7796-4433-b1fc-9b5fac43edf3"\n'
+ 'KNAME="sda3" MODEL="" SIZE="650002432" ROTA="0" TYPE="part" '
+ 'UUID="b3b03565-5f13-3c93-b2a6-6d90e25be926" '
+ 'PARTUUID="6c85beff-b2bd-4a1c-91b7-8abb5256459d"\n'
+ 'KNAME="sda1" MODEL="" SIZE="209715200" ROTA="0" TYPE="part" '
+ 'UUID="0a83355d-7500-3f5f-9abd-66f6fd03714c" '
+ 'PARTUUID="eba28b26-b76a-402c-94dd-0b66a523a485"\n'
+ 'KNAME="dm-0" MODEL="" SIZE="60022480896" ROTA="0" TYPE="mpath" '
+ 'UUID="" PARTUUID=""\n'
+ 'KNAME="dm-4" MODEL="" SIZE="650002432" ROTA="0" TYPE="part" '
+ 'UUID="b3b03565-5f13-3c93-b2a6-6d90e25be926" '
+ 'PARTUUID="6c85beff-b2bd-4a1c-91b7-8abb5256459d"\n'
+ 'KNAME="dm-2" MODEL="" SIZE="209715200" ROTA="0" TYPE="part" '
+ 'UUID="0a83355d-7500-3f5f-9abd-66f6fd03714c" '
+ 'PARTUUID="eba28b26-b76a-402c-94dd-0b66a523a485"\n'
+ 'KNAME="dm-3" MODEL="" SIZE="59162722304" ROTA="0" TYPE="part" '
+ 'UUID="f8b55d59-96c3-3982-b129-1b6b2ee8da86" '
+ 'PARTUUID="c97c8aac-7796-4433-b1fc-9b5fac43edf3"\n'
+ 'KNAME="sdb" MODEL="INTEL_SSDSC2CT060A3" SIZE="60022480896" '
+ 'ROTA="0" TYPE="disk" UUID="" PARTUUID=""\n'
+ 'KNAME="sdb2" MODEL="" SIZE="59162722304" ROTA="0" TYPE="part" '
+ 'UUID="f8b55d59-96c3-3982-b129-1b6b2ee8da86" '
+ 'PARTUUID="c97c8aac-7796-4433-b1fc-9b5fac43edf3"\n'
+ 'KNAME="sdb3" MODEL="" SIZE="650002432" ROTA="0" TYPE="part" '
+ 'UUID="b3b03565-5f13-3c93-b2a6-6d90e25be926" '
+ 'PARTUUID="6c85beff-b2bd-4a1c-91b7-8abb5256459d"\n'
+ 'KNAME="sdb1" MODEL="" SIZE="209715200" ROTA="0" TYPE="part" '
+ 'UUID="0a83355d-7500-3f5f-9abd-66f6fd03714c" '
+ 'PARTUUID="eba28b26-b76a-402c-94dd-0b66a523a485"\n'
+ 'KNAME="sdc" MODEL="ST1000DM003-1CH162" SIZE="1000204886016" '
+ 'ROTA="1" TYPE="disk" UUID="" PARTUUID=""\n'
+ 'KNAME="sdc1" MODEL="" SIZE="899999072256" ROTA="1" TYPE="part" '
+ 'UUID="457f7d3c-9376-4997-89bd-d1a7c8b04060" '
+ 'PARTUUID="c9433d2e-3bbc-47b4-92bf-43c1d80f06e0"\n'
+ 'KNAME="dm-1" MODEL="" SIZE="1000204886016" ROTA="0" TYPE="mpath" '
+ 'UUID="" PARTUUID=""\n'
+)
+
PARTUUID_DEVICE_TEMPLATE = (
'KNAME="sda" MODEL="DRIVE 0" SIZE="1765517033472" '
'ROTA="1" TYPE="disk" UUID="" PARTUUID=""\n'
@@ -1501,7 +1547,6 @@ NVME_CLI_INFO_TEMPLATE_FORMAT_UNSUPPORTED = ("""
}
""")
-
SGDISK_INFO_TEMPLATE = ("""
Partition GUID code: C12A7328-F81F-11D2-BA4B-00A0C93EC93B (EFI system partition)
Partition unique GUID: FAED7408-6D92-4FC6-883B-9069E2274ECA
@@ -1511,3 +1556,13 @@ Partition size: 1048576 sectors (512.0 MiB)
Attribute flags: 0000000000000000
Partition name: 'EFI System Partition'
""") # noqa
+
+MULTIPATH_VALID_PATH = '%s is a valid multipath device path'
+MULTIPATH_INVALID_PATH = '%s is not a valid multipath device path'
+
+MULTIPATH_LINKS_DM = (
+ 'SUPER_FRIENDLY_NAME %s ATA,INTEL SSDSC2CT06\n'
+ 'size=56G features=\'1 retain_attached_hw_handler\' hwhandler=\'0\' wp=rw\n' # noqa
+ ' `-+- policy=\'service-time 0\' prio=1 status=active\n'
+ ' `- 0:0:0:0 device s 8:0 active ready running\n'
+)
diff --git a/ironic_python_agent/tests/unit/test_agent.py b/ironic_python_agent/tests/unit/test_agent.py
index 897d023c..b0a6915c 100644
--- a/ironic_python_agent/tests/unit/test_agent.py
+++ b/ironic_python_agent/tests/unit/test_agent.py
@@ -538,6 +538,7 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest):
wsgi_server.start.assert_called_once_with()
self.agent.heartbeater.start.assert_called_once_with()
+ @mock.patch.object(hardware, '_enable_multipath', autospec=True)
@mock.patch('ironic_python_agent.hardware_managers.cna._detect_cna_card',
mock.Mock())
@mock.patch.object(agent.IronicPythonAgent,
@@ -548,7 +549,8 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest):
@mock.patch.object(hardware.HardwareManager, 'list_hardware_info',
autospec=True)
def test_run_with_inspection(self, mock_list_hardware, mock_wsgi,
- mock_dispatch, mock_inspector, mock_wait):
+ mock_dispatch, mock_inspector, mock_wait,
+ mock_mpath):
CONF.set_override('inspection_callback_url', 'http://foo/bar')
def set_serve_api():
@@ -589,6 +591,7 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest):
mock_dispatch.call_args_list)
self.agent.heartbeater.start.assert_called_once_with()
+ @mock.patch.object(hardware, '_enable_multipath', autospec=True)
@mock.patch('ironic_lib.mdns.get_endpoint', autospec=True)
@mock.patch(
'ironic_python_agent.hardware_managers.cna._detect_cna_card',
@@ -606,7 +609,8 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest):
mock_dispatch,
mock_inspector,
mock_wait,
- mock_mdns):
+ mock_mdns,
+ mock_mpath):
mock_mdns.side_effect = lib_exc.ServiceLookupFailure()
# If inspection_callback_url is configured and api_url is not when the
# agent starts, ensure that the inspection will be called and wsgi
@@ -646,6 +650,7 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest):
self.assertTrue(mock_wait.called)
self.assertFalse(mock_dispatch.called)
+ @mock.patch.object(hardware, '_enable_multipath', autospec=True)
@mock.patch('ironic_lib.mdns.get_endpoint', autospec=True)
@mock.patch(
'ironic_python_agent.hardware_managers.cna._detect_cna_card',
@@ -663,7 +668,8 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest):
mock_dispatch,
mock_inspector,
mock_wait,
- mock_mdns):
+ mock_mdns,
+ mock_mpath):
mock_mdns.side_effect = lib_exc.ServiceLookupFailure()
# If both api_url and inspection_callback_url are not configured when
# the agent starts, ensure that the inspection will be skipped and wsgi
@@ -994,12 +1000,13 @@ class TestAdvertiseAddress(ironic_agent_base.IronicAgentTest):
self.assertFalse(mock_exec.called)
self.assertFalse(mock_gethostbyname.called)
+ @mock.patch.object(hardware, '_enable_multipath', autospec=True)
@mock.patch.object(netutils, 'get_ipv4_addr',
autospec=True)
@mock.patch('ironic_python_agent.hardware_managers.cna._detect_cna_card',
autospec=True)
- def test_with_network_interface(self, mock_cna, mock_get_ipv4, mock_exec,
- mock_gethostbyname):
+ def test_with_network_interface(self, mock_cna, mock_get_ipv4, mock_mpath,
+ mock_exec, mock_gethostbyname):
self.agent.network_interface = 'em1'
mock_get_ipv4.return_value = '1.2.3.4'
mock_cna.return_value = False
@@ -1012,12 +1019,14 @@ class TestAdvertiseAddress(ironic_agent_base.IronicAgentTest):
self.assertFalse(mock_exec.called)
self.assertFalse(mock_gethostbyname.called)
+ @mock.patch.object(hardware, '_enable_multipath', autospec=True)
@mock.patch.object(netutils, 'get_ipv4_addr',
autospec=True)
@mock.patch('ironic_python_agent.hardware_managers.cna._detect_cna_card',
autospec=True)
def test_with_network_interface_failed(self, mock_cna, mock_get_ipv4,
- mock_exec, mock_gethostbyname):
+ mock_mpath, mock_exec,
+ mock_gethostbyname):
self.agent.network_interface = 'em1'
mock_get_ipv4.return_value = None
mock_cna.return_value = False
diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py
index 01825d05..b5b10876 100644
--- a/ironic_python_agent/tests/unit/test_hardware.py
+++ b/ironic_python_agent/tests/unit/test_hardware.py
@@ -723,60 +723,234 @@ class TestGenericHardwareManager(base.IronicAgentTest):
self.assertEqual('eth1.102', interfaces[4].name)
self.assertEqual('eth1.103', interfaces[5].name)
+ @mock.patch.object(hardware, 'get_multipath_status', autospec=True)
@mock.patch.object(os, 'readlink', autospec=True)
@mock.patch.object(os, 'listdir', autospec=True)
@mock.patch.object(hardware, 'get_cached_node', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True)
def test_get_os_install_device(self, mocked_execute, mock_cached_node,
- mocked_listdir, mocked_readlink):
+ mocked_listdir, mocked_readlink,
+ mocked_mpath):
mocked_readlink.return_value = '../../sda'
mocked_listdir.return_value = ['1:0:0:0']
mock_cached_node.return_value = None
- mocked_execute.return_value = (hws.BLK_DEVICE_TEMPLATE, '')
+ mocked_mpath.return_value = False
+ mocked_execute.side_effect = [
+ (hws.BLK_DEVICE_TEMPLATE, ''),
+ ]
self.assertEqual('/dev/sdb', self.hardware.get_os_install_device())
- mocked_execute.assert_called_once_with(
- 'lsblk', '-Pbia', '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID')
+ mock_cached_node.assert_called_once_with()
+ self.assertEqual(1, mocked_mpath.call_count)
+
+ @mock.patch.object(hardware, 'get_multipath_status', autospec=True)
+ @mock.patch.object(os, 'readlink', autospec=True)
+ @mock.patch.object(os, 'listdir', autospec=True)
+ @mock.patch.object(hardware, 'get_cached_node', autospec=True)
+ @mock.patch.object(utils, 'execute', autospec=True)
+ def test_get_os_install_device_multipath(
+ self, mocked_execute, mock_cached_node,
+ mocked_listdir, mocked_readlink,
+ mocked_mpath):
+ mocked_mpath.return_value = True
+ mocked_readlink.return_value = '../../sda'
+ mocked_listdir.return_value = ['1:0:0:0']
+ mock_cached_node.return_value = None
+ mocked_execute.side_effect = [
+ (hws.MULTIPATH_BLK_DEVICE_TEMPLATE, ''),
+ (hws.MULTIPATH_VALID_PATH % '/dev/sda', ''),
+ (hws.MULTIPATH_LINKS_DM % 'dm-0', ''),
+ (hws.MULTIPATH_VALID_PATH % '/dev/sda2', ''),
+ (hws.MULTIPATH_LINKS_DM % 'dm-0', ''),
+ (hws.MULTIPATH_VALID_PATH % '/dev/sda3', ''),
+ (hws.MULTIPATH_LINKS_DM % 'dm-0', ''),
+ (hws.MULTIPATH_VALID_PATH % '/dev/sda1', ''),
+ (hws.MULTIPATH_LINKS_DM % 'dm-0', ''),
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # dm-0
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # dm-4
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # dm-2
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # dm-3
+ (hws.MULTIPATH_VALID_PATH % '/dev/sdb', ''),
+ (hws.MULTIPATH_LINKS_DM % 'dm-0', ''),
+ (hws.MULTIPATH_VALID_PATH % '/dev/sdb2', ''),
+ (hws.MULTIPATH_LINKS_DM % 'dm-0', ''),
+ (hws.MULTIPATH_VALID_PATH % '/dev/sdb3', ''),
+ (hws.MULTIPATH_LINKS_DM % 'dm-0', ''),
+ (hws.MULTIPATH_VALID_PATH % '/dev/sdb1', ''),
+ (hws.MULTIPATH_LINKS_DM % 'dm-0', ''),
+ processutils.ProcessExecutionError(
+ stderr=hws.MULTIPATH_INVALID_PATH % '/dev/sdc'), # sdc
+ processutils.ProcessExecutionError(
+ stderr=hws.MULTIPATH_INVALID_PATH % '/dev/sdc1'), # sdc1
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # dm-1
+ ]
+ expected = [
+ mock.call('lsblk', '-Pbia',
+ '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'),
+ mock.call('multipath', '-c', '/dev/sda'),
+ mock.call('multipath', '-ll', '/dev/sda'),
+ mock.call('multipath', '-c', '/dev/sda2'),
+ mock.call('multipath', '-ll', '/dev/sda2'),
+ mock.call('multipath', '-c', '/dev/sda3'),
+ mock.call('multipath', '-ll', '/dev/sda3'),
+ mock.call('multipath', '-c', '/dev/sda1'),
+ mock.call('multipath', '-ll', '/dev/sda1'),
+ mock.call('multipath', '-c', '/dev/dm-0'),
+ mock.call('multipath', '-c', '/dev/dm-4'),
+ mock.call('multipath', '-c', '/dev/dm-2'),
+ mock.call('multipath', '-c', '/dev/dm-3'),
+ mock.call('multipath', '-c', '/dev/sdb'),
+ mock.call('multipath', '-ll', '/dev/sdb'),
+ mock.call('multipath', '-c', '/dev/sdb2'),
+ mock.call('multipath', '-ll', '/dev/sdb2'),
+ mock.call('multipath', '-c', '/dev/sdb3'),
+ mock.call('multipath', '-ll', '/dev/sdb3'),
+ mock.call('multipath', '-c', '/dev/sdb1'),
+ mock.call('multipath', '-ll', '/dev/sdb1'),
+ mock.call('multipath', '-c', '/dev/sdc'),
+ mock.call('multipath', '-c', '/dev/sdc1'),
+ mock.call('multipath', '-c', '/dev/dm-1'),
+ ]
+ self.assertEqual('/dev/dm-0', self.hardware.get_os_install_device())
+ mocked_execute.assert_has_calls(expected)
mock_cached_node.assert_called_once_with()
+ @mock.patch.object(hardware, 'get_multipath_status', autospec=True)
+ @mock.patch.object(os, 'readlink', autospec=True)
+ @mock.patch.object(os, 'listdir', autospec=True)
+ @mock.patch.object(hardware, 'get_cached_node', autospec=True)
+ @mock.patch.object(utils, 'execute', autospec=True)
+ def test_get_os_install_device_not_multipath(
+ self, mocked_execute, mock_cached_node,
+ mocked_listdir, mocked_readlink, mocked_mpath):
+ mocked_readlink.return_value = '../../sda'
+ mocked_listdir.return_value = ['1:0:0:0']
+ mocked_mpath.return_value = True
+ hint = {'size': '>900'}
+ mock_cached_node.return_value = {'properties': {'root_device': hint},
+ 'uuid': 'node1',
+ 'instance_info': {}}
+ mocked_execute.side_effect = [
+ (hws.MULTIPATH_BLK_DEVICE_TEMPLATE, ''),
+ (hws.MULTIPATH_VALID_PATH % '/dev/sda', ''),
+ (hws.MULTIPATH_LINKS_DM % 'dm-0', ''),
+ (hws.MULTIPATH_VALID_PATH % '/dev/sda2', ''),
+ (hws.MULTIPATH_LINKS_DM % 'dm-0', ''),
+ (hws.MULTIPATH_VALID_PATH % '/dev/sda3', ''),
+ (hws.MULTIPATH_LINKS_DM % 'dm-0', ''),
+ (hws.MULTIPATH_VALID_PATH % '/dev/sda1', ''),
+ (hws.MULTIPATH_LINKS_DM % 'dm-0', ''),
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # dm-0
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # dm-4
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # dm-2
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # dm-3
+ (hws.MULTIPATH_VALID_PATH % '/dev/sdb', ''),
+ (hws.MULTIPATH_LINKS_DM % 'dm-0', ''),
+ (hws.MULTIPATH_VALID_PATH % '/dev/sdb2', ''),
+ (hws.MULTIPATH_LINKS_DM % 'dm-0', ''),
+ (hws.MULTIPATH_VALID_PATH % '/dev/sdb3', ''),
+ (hws.MULTIPATH_LINKS_DM % 'dm-0', ''),
+ (hws.MULTIPATH_VALID_PATH % '/dev/sdb1', ''),
+ (hws.MULTIPATH_LINKS_DM % 'dm-0', ''),
+ processutils.ProcessExecutionError(
+ stderr=hws.MULTIPATH_INVALID_PATH % '/dev/sdc'), # sdc
+ processutils.ProcessExecutionError(
+ stderr=hws.MULTIPATH_INVALID_PATH % '/dev/sdc1'), # sdc1
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # dm-1
+ ]
+ expected = [
+ mock.call('lsblk', '-Pbia',
+ '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'),
+ mock.call('multipath', '-c', '/dev/sda'),
+ mock.call('multipath', '-ll', '/dev/sda'),
+ mock.call('multipath', '-c', '/dev/sda2'),
+ mock.call('multipath', '-ll', '/dev/sda2'),
+ mock.call('multipath', '-c', '/dev/sda3'),
+ mock.call('multipath', '-ll', '/dev/sda3'),
+ mock.call('multipath', '-c', '/dev/sda1'),
+ mock.call('multipath', '-ll', '/dev/sda1'),
+ mock.call('multipath', '-c', '/dev/dm-0'),
+ mock.call('multipath', '-c', '/dev/dm-4'),
+ mock.call('multipath', '-c', '/dev/dm-2'),
+ mock.call('multipath', '-c', '/dev/dm-3'),
+ mock.call('multipath', '-c', '/dev/sdb'),
+ mock.call('multipath', '-ll', '/dev/sdb'),
+ mock.call('multipath', '-c', '/dev/sdb2'),
+ mock.call('multipath', '-ll', '/dev/sdb2'),
+ mock.call('multipath', '-c', '/dev/sdb3'),
+ mock.call('multipath', '-ll', '/dev/sdb3'),
+ mock.call('multipath', '-c', '/dev/sdb1'),
+ mock.call('multipath', '-ll', '/dev/sdb1'),
+ mock.call('multipath', '-c', '/dev/sdc'),
+ mock.call('multipath', '-c', '/dev/sdc1'),
+ mock.call('multipath', '-c', '/dev/dm-1'),
+ ]
+ self.assertEqual('/dev/sdc', self.hardware.get_os_install_device())
+ mocked_execute.assert_has_calls(expected)
+ mock_cached_node.assert_called_once_with()
+ mocked_mpath.assert_called_once_with()
+
+ @mock.patch.object(hardware, 'get_multipath_status', autospec=True)
@mock.patch.object(os, 'readlink', autospec=True)
@mock.patch.object(os, 'listdir', autospec=True)
@mock.patch.object(hardware, 'get_cached_node', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True)
def test_get_os_install_device_raid(self, mocked_execute,
mock_cached_node, mocked_listdir,
- mocked_readlink):
+ mocked_readlink, mocked_mpath):
# NOTE(TheJulia): The readlink and listdir mocks are just to satisfy
# what is functionally an available path check and that information
# is stored in the returned result for use by root device hints.
- mocked_readlink.side_effect = '../../sda'
+ mocked_readlink.return_value = '../../sda'
mocked_listdir.return_value = ['1:0:0:0']
+ mocked_mpath.return_value = False
mock_cached_node.return_value = None
- mocked_execute.return_value = (hws.RAID_BLK_DEVICE_TEMPLATE, '')
- # This should ideally select the smallest device and in theory raid
- # should always be smaller
- self.assertEqual('/dev/md0', self.hardware.get_os_install_device())
- mocked_execute.assert_called_once_with(
- 'lsblk', '-Pbia', '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID')
+ mocked_execute.return_value = (hws.BLK_DEVICE_TEMPLATE_SMALL, '')
+ ex = self.assertRaises(errors.DeviceNotFound,
+ self.hardware.get_os_install_device)
+ expected = [
+ mock.call('lsblk', '-Pbia',
+ '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'),
+ ]
+ mocked_execute.assert_has_calls(expected)
+ self.assertIn(str(4 * units.Gi), ex.details)
mock_cached_node.assert_called_once_with()
+ self.assertEqual(1, mocked_mpath.call_count)
+ @mock.patch.object(hardware, 'get_multipath_status', autospec=True)
@mock.patch.object(os, 'readlink', autospec=True)
@mock.patch.object(os, 'listdir', autospec=True)
@mock.patch.object(hardware, 'get_cached_node', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True)
def test_get_os_install_device_fails(self, mocked_execute,
mock_cached_node,
- mocked_listdir, mocked_readlink):
+ mocked_listdir, mocked_readlink,
+ mocked_mpath):
"""Fail to find device >=4GB w/o root device hints"""
mocked_readlink.return_value = '../../sda'
mocked_listdir.return_value = ['1:0:0:0']
+ mocked_mpath.return_value = False
mock_cached_node.return_value = None
mocked_execute.return_value = (hws.BLK_DEVICE_TEMPLATE_SMALL, '')
ex = self.assertRaises(errors.DeviceNotFound,
self.hardware.get_os_install_device)
- mocked_execute.assert_called_once_with(
- 'lsblk', '-Pbia', '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID')
+ expected = [
+ mock.call('lsblk', '-Pbia',
+ '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'),
+ ]
+ mocked_execute.assert_has_calls(expected)
self.assertIn(str(4 * units.Gi), ex.details)
mock_cached_node.assert_called_once_with()
+ self.assertEqual(1, mocked_mpath.call_count)
@mock.patch.object(hardware, 'list_all_block_devices', autospec=True)
@mock.patch.object(hardware, 'get_cached_node', autospec=True)
@@ -1183,6 +1357,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
ignore_raid=True)],
list_mock.call_args_list)
+ @mock.patch.object(hardware, 'get_multipath_status', lambda *_: True)
@mock.patch.object(os, 'readlink', autospec=True)
@mock.patch.object(os, 'listdir', autospec=True)
@mock.patch.object(hardware, '_get_device_info', autospec=True)
@@ -1200,7 +1375,34 @@ class TestGenericHardwareManager(base.IronicAgentTest):
mock_readlink.side_effect = lambda x, m=by_path_map: m[x]
mock_listdir.return_value = [os.path.basename(x)
for x in sorted(by_path_map)]
- mocked_execute.return_value = (hws.BLK_DEVICE_TEMPLATE, '')
+ mocked_execute.side_effect = [
+ (hws.BLK_DEVICE_TEMPLATE, ''),
+ processutils.ProcessExecutionError(
+ stderr=hws.MULTIPATH_INVALID_PATH % '/dev/sda'),
+ processutils.ProcessExecutionError(
+ stderr=hws.MULTIPATH_INVALID_PATH % '/dev/sdb'),
+ processutils.ProcessExecutionError(
+ stderr=hws.MULTIPATH_INVALID_PATH % '/dev/sdc'),
+ # Pretend sdd is a multipath device... because why not.
+ (hws.MULTIPATH_VALID_PATH % '/dev/sdd', ''),
+ (hws.MULTIPATH_LINKS_DM % 'dm-0', ''),
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # loop0
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # zram0
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # ram0
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # ram1
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # ram2
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # ram3
+ processutils.ProcessExecutionError(
+ stderr=hws.MULTIPATH_INVALID_PATH % '/dev/sdf'),
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # dm-0
+ ]
mocked_udev.side_effect = [pyudev.DeviceNotFoundByFileError(),
pyudev.DeviceNotFoundByNumberError('block',
1234),
@@ -1230,7 +1432,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
vendor='Super Vendor',
hctl='1:0:0:0',
by_path='/dev/disk/by-path/1:0:0:2'),
- hardware.BlockDevice(name='/dev/sdd',
+ hardware.BlockDevice(name='/dev/dm-0',
model='NWD-BLP4-1600',
size=1765517033472,
rotational=False,
@@ -1246,21 +1448,41 @@ class TestGenericHardwareManager(base.IronicAgentTest):
self.assertEqual(getattr(expected, attr),
getattr(device, attr))
expected_calls = [mock.call('/sys/block/%s/device/scsi_device' % dev)
- for dev in ('sda', 'sdb', 'sdc', 'sdd')]
+ for dev in ('sda', 'sdb', 'sdc', 'dm-0')]
mock_listdir.assert_has_calls(expected_calls)
-
expected_calls = [mock.call('/dev/disk/by-path/1:0:0:%d' % dev)
for dev in range(3)]
mock_readlink.assert_has_calls(expected_calls)
+ expected_calls = [
+ mock.call('lsblk', '-Pbia',
+ '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'),
+ mock.call('multipath', '-c', '/dev/sda'),
+ mock.call('multipath', '-c', '/dev/sdb'),
+ mock.call('multipath', '-c', '/dev/sdc'),
+ mock.call('multipath', '-c', '/dev/sdd'),
+ mock.call('multipath', '-ll', '/dev/sdd'),
+ mock.call('multipath', '-c', '/dev/loop0'),
+ mock.call('multipath', '-c', '/dev/zram0'),
+ mock.call('multipath', '-c', '/dev/ram0'),
+ mock.call('multipath', '-c', '/dev/ram1'),
+ mock.call('multipath', '-c', '/dev/ram2'),
+ mock.call('multipath', '-c', '/dev/ram3'),
+ mock.call('multipath', '-c', '/dev/sdf'),
+ mock.call('multipath', '-c', '/dev/dm-0')
+ ]
+ mocked_execute.assert_has_calls(expected_calls)
+ @mock.patch.object(hardware, 'get_multipath_status', autospec=True)
@mock.patch.object(os, 'listdir', autospec=True)
@mock.patch.object(hardware, '_get_device_info', autospec=True)
@mock.patch.object(pyudev.Devices, 'from_device_file', autospec=False)
@mock.patch.object(utils, 'execute', autospec=True)
def test_list_all_block_device_hctl_fail(self, mocked_execute, mocked_udev,
mocked_dev_vendor,
- mocked_listdir):
+ mocked_listdir,
+ mocked_mpath):
mocked_listdir.side_effect = (OSError, OSError, IndexError)
+ mocked_mpath.return_value = False
mocked_execute.return_value = (hws.BLK_DEVICE_TEMPLATE_SMALL, '')
mocked_dev_vendor.return_value = 'Super Vendor'
devices = hardware.list_all_block_devices()
@@ -1272,6 +1494,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
]
self.assertEqual(expected_calls, mocked_listdir.call_args_list)
+ @mock.patch.object(hardware, 'get_multipath_status', autospec=True)
@mock.patch.object(os, 'readlink', autospec=True)
@mock.patch.object(os, 'listdir', autospec=True)
@mock.patch.object(hardware, '_get_device_info', autospec=True)
@@ -1279,15 +1502,62 @@ class TestGenericHardwareManager(base.IronicAgentTest):
@mock.patch.object(utils, 'execute', autospec=True)
def test_list_all_block_device_with_udev(self, mocked_execute, mocked_udev,
mocked_dev_vendor, mocked_listdir,
- mocked_readlink):
+ mocked_readlink, mocked_mpath):
mocked_readlink.return_value = '../../sda'
mocked_listdir.return_value = ['1:0:0:0']
- mocked_execute.return_value = (hws.BLK_DEVICE_TEMPLATE, '')
+ mocked_execute.side_effect = [
+ (hws.BLK_DEVICE_TEMPLATE, ''),
+ processutils.ProcessExecutionError(
+ stderr=hws.MULTIPATH_INVALID_PATH % '/dev/sda'),
+ processutils.ProcessExecutionError(
+ stderr=hws.MULTIPATH_INVALID_PATH % '/dev/sdb'),
+ processutils.ProcessExecutionError(
+ stderr=hws.MULTIPATH_INVALID_PATH % '/dev/sdc'),
+ # Pretend sdd is a multipath device... because why not.
+ (hws.MULTIPATH_VALID_PATH % '/dev/sdd', ''),
+ (hws.MULTIPATH_LINKS_DM % 'dm-0', ''),
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # loop0
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # zram0
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # ram0
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # ram1
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # ram2
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # ram3
+ processutils.ProcessExecutionError(
+ stderr=hws.MULTIPATH_INVALID_PATH % '/dev/sdf'),
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # dm-0
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ]
+
+ mocked_mpath.return_value = False
mocked_udev.side_effect = iter([
{'ID_WWN': 'wwn%d' % i, 'ID_SERIAL_SHORT': 'serial%d' % i,
'ID_WWN_WITH_EXTENSION': 'wwn-ext%d' % i,
'ID_WWN_VENDOR_EXTENSION': 'wwn-vendor-ext%d' % i}
- for i in range(4)
+ for i in range(5)
])
mocked_dev_vendor.return_value = 'Super Vendor'
devices = hardware.list_all_block_devices()
@@ -1345,6 +1615,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
expected_calls = [mock.call('/sys/block/%s/device/scsi_device' % dev)
for dev in ('sda', 'sdb', 'sdc', 'sdd')]
mocked_listdir.assert_has_calls(expected_calls)
+ mocked_mpath.assert_called_once_with()
@mock.patch.object(hardware, 'ThreadPool', autospec=True)
@mock.patch.object(hardware, 'dispatch_to_managers', autospec=True)
@@ -3988,6 +4259,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
self.hardware._nvme_erase, block_device)
+@mock.patch.object(hardware, '_enable_multipath', autospec=True)
@mock.patch.object(hardware.GenericHardwareManager,
'get_os_install_device', autospec=True)
@mock.patch.object(hardware, '_md_scan_and_assemble', autospec=True)
@@ -4000,7 +4272,7 @@ class TestEvaluateHardwareSupport(base.IronicAgentTest):
def test_evaluate_hw_waits_for_disks(
self, mocked_sleep, mocked_check_for_iscsi,
- mocked_md_assemble, mocked_get_inst_dev):
+ mocked_md_assemble, mocked_get_inst_dev, mocked_enable_mpath):
mocked_get_inst_dev.side_effect = [
errors.DeviceNotFound('boom'),
None
@@ -4018,7 +4290,7 @@ class TestEvaluateHardwareSupport(base.IronicAgentTest):
@mock.patch.object(hardware, 'LOG', autospec=True)
def test_evaluate_hw_no_wait_for_disks(
self, mocked_log, mocked_sleep, mocked_check_for_iscsi,
- mocked_md_assemble, mocked_get_inst_dev):
+ mocked_md_assemble, mocked_get_inst_dev, mocked_enable_mpath):
CONF.set_override('disk_wait_attempts', '0')
result = self.hardware.evaluate_hardware_support()
@@ -4032,7 +4304,7 @@ class TestEvaluateHardwareSupport(base.IronicAgentTest):
@mock.patch.object(hardware, 'LOG', autospec=True)
def test_evaluate_hw_waits_for_disks_nonconfigured(
self, mocked_log, mocked_sleep, mocked_check_for_iscsi,
- mocked_md_assemble, mocked_get_inst_dev):
+ mocked_md_assemble, mocked_get_inst_dev, mocked_enable_mpath):
mocked_get_inst_dev.side_effect = [
errors.DeviceNotFound('boom'),
errors.DeviceNotFound('boom'),
@@ -4063,7 +4335,8 @@ class TestEvaluateHardwareSupport(base.IronicAgentTest):
mocked_sleep,
mocked_check_for_iscsi,
mocked_md_assemble,
- mocked_get_inst_dev):
+ mocked_get_inst_dev,
+ mocked_enable_mpath):
CONF.set_override('disk_wait_attempts', '1')
mocked_get_inst_dev.side_effect = [
@@ -4083,7 +4356,8 @@ class TestEvaluateHardwareSupport(base.IronicAgentTest):
def test_evaluate_hw_disks_timeout_unconfigured(self, mocked_sleep,
mocked_check_for_iscsi,
mocked_md_assemble,
- mocked_get_inst_dev):
+ mocked_get_inst_dev,
+ mocked_enable_mpath):
mocked_get_inst_dev.side_effect = errors.DeviceNotFound('boom')
self.hardware.evaluate_hardware_support()
mocked_sleep.assert_called_with(3)
@@ -4091,7 +4365,8 @@ class TestEvaluateHardwareSupport(base.IronicAgentTest):
def test_evaluate_hw_disks_timeout_configured(self, mocked_sleep,
mocked_check_for_iscsi,
mocked_md_assemble,
- mocked_root_dev):
+ mocked_root_dev,
+ mocked_enable_mpath):
CONF.set_override('disk_wait_delay', '5')
mocked_root_dev.side_effect = errors.DeviceNotFound('boom')
@@ -4100,7 +4375,7 @@ class TestEvaluateHardwareSupport(base.IronicAgentTest):
def test_evaluate_hw_disks_timeout(
self, mocked_sleep, mocked_check_for_iscsi,
- mocked_md_assemble, mocked_get_inst_dev):
+ mocked_md_assemble, mocked_get_inst_dev, mocked_enable_mpath):
mocked_get_inst_dev.side_effect = errors.DeviceNotFound('boom')
result = self.hardware.evaluate_hardware_support()
self.assertEqual(hardware.HardwareSupport.GENERIC, result)
@@ -4114,6 +4389,7 @@ class TestEvaluateHardwareSupport(base.IronicAgentTest):
@mock.patch.object(utils, 'execute', autospec=True)
class TestModuleFunctions(base.IronicAgentTest):
+ @mock.patch.object(hardware, 'get_multipath_status', autospec=True)
@mock.patch.object(os, 'readlink', autospec=True)
@mock.patch.object(hardware, '_get_device_info',
lambda x, y, z: 'FooTastic')
@@ -4122,16 +4398,30 @@ class TestModuleFunctions(base.IronicAgentTest):
autospec=False)
def test_list_all_block_devices_success(self, mocked_fromdevfile,
mocked_udev, mocked_readlink,
- mocked_execute):
+ mocked_mpath, mocked_execute):
+ mocked_mpath.return_value = True
mocked_readlink.return_value = '../../sda'
mocked_fromdevfile.return_value = {}
- mocked_execute.return_value = (hws.BLK_DEVICE_TEMPLATE_SMALL, '')
+ mocked_execute.side_effect = [
+ (hws.BLK_DEVICE_TEMPLATE_SMALL, ''),
+ processutils.ProcessExecutionError(
+ stderr=hws.MULTIPATH_INVALID_PATH % '/dev/sda'),
+ processutils.ProcessExecutionError(
+ stderr=hws.MULTIPATH_INVALID_PATH % '/dev/sdb'),
+ ]
result = hardware.list_all_block_devices()
- mocked_execute.assert_called_once_with(
- 'lsblk', '-Pbia', '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID')
+ expected_calls = [
+ mock.call('lsblk', '-Pbia',
+ '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'),
+ mock.call('multipath', '-c', '/dev/sda'),
+ mock.call('multipath', '-c', '/dev/sdb')
+ ]
+ mocked_execute.assert_has_calls(expected_calls)
self.assertEqual(BLK_DEVICE_TEMPLATE_SMALL_DEVICES, result)
mocked_udev.assert_called_once_with()
+ mocked_mpath.assert_called_once_with()
+ @mock.patch.object(hardware, 'get_multipath_status', autospec=True)
@mock.patch.object(os, 'readlink', autospec=True)
@mock.patch.object(hardware, '_get_device_info',
lambda x, y, z: 'FooTastic')
@@ -4140,16 +4430,49 @@ class TestModuleFunctions(base.IronicAgentTest):
autospec=False)
def test_list_all_block_devices_success_raid(self, mocked_fromdevfile,
mocked_udev, mocked_readlink,
- mocked_execute):
+ mocked_mpath, mocked_execute):
mocked_readlink.return_value = '../../sda'
mocked_fromdevfile.return_value = {}
- mocked_execute.return_value = (hws.RAID_BLK_DEVICE_TEMPLATE, '')
+ mocked_mpath.return_value = True
+ mocked_execute.side_effect = [
+ (hws.RAID_BLK_DEVICE_TEMPLATE, ''),
+ processutils.ProcessExecutionError(
+ stderr=hws.MULTIPATH_INVALID_PATH % '/dev/sda'),
+ processutils.ProcessExecutionError(
+ stderr=hws.MULTIPATH_INVALID_PATH % '/dev/sda1'),
+ processutils.ProcessExecutionError(
+ stderr=hws.MULTIPATH_INVALID_PATH % '/dev/sdb'),
+ processutils.ProcessExecutionError(
+ stderr=hws.MULTIPATH_INVALID_PATH % '/dev/sdb1'),
+ processutils.ProcessExecutionError(
+ stderr=hws.MULTIPATH_INVALID_PATH % '/dev/sda'),
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # md0p1
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # md0
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # md0
+ processutils.ProcessExecutionError(
+ stderr='the -c option requires a path to check'), # md1
+ ]
+ expected_calls = [
+ mock.call('lsblk', '-Pbia',
+ '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'),
+ mock.call('multipath', '-c', '/dev/sda'),
+ mock.call('multipath', '-c', '/dev/sda1'),
+ mock.call('multipath', '-c', '/dev/sdb'),
+ mock.call('multipath', '-c', '/dev/sdb1'),
+ mock.call('multipath', '-c', '/dev/md0p1'),
+ mock.call('multipath', '-c', '/dev/md0'),
+ mock.call('multipath', '-c', '/dev/md1'),
+ ]
+
result = hardware.list_all_block_devices(ignore_empty=False)
- mocked_execute.assert_called_once_with(
- 'lsblk', '-Pbia', '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID')
+ mocked_execute.assert_has_calls(expected_calls)
self.assertEqual(RAID_BLK_DEVICE_TEMPLATE_DEVICES, result)
mocked_udev.assert_called_once_with()
+ @mock.patch.object(hardware, 'get_multipath_status', autospec=True)
@mock.patch.object(os, 'readlink', autospec=True)
@mock.patch.object(hardware, '_get_device_info',
lambda x, y, z: 'FooTastic')
@@ -4159,21 +4482,37 @@ class TestModuleFunctions(base.IronicAgentTest):
def test_list_all_block_devices_partuuid_success(
self, mocked_fromdevfile,
mocked_udev, mocked_readlink,
- mocked_execute):
+ mocked_mpath, mocked_execute):
mocked_readlink.return_value = '../../sda'
mocked_fromdevfile.return_value = {}
- mocked_execute.return_value = (hws.PARTUUID_DEVICE_TEMPLATE, '')
+ mocked_mpath.return_value = True
+ mocked_execute.side_effect = [
+ (hws.PARTUUID_DEVICE_TEMPLATE, ''),
+ processutils.ProcessExecutionError(
+ stderr=hws.MULTIPATH_INVALID_PATH % '/dev/sda'),
+ processutils.ProcessExecutionError(
+ stderr=hws.MULTIPATH_INVALID_PATH % '/dev/sdb'),
+ ]
result = hardware.list_all_block_devices(block_type='part')
- mocked_execute.assert_called_once_with(
- 'lsblk', '-Pbia', '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID')
+ expected_calls = [
+ mock.call('lsblk', '-Pbia',
+ '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'),
+ mock.call('multipath', '-c', '/dev/sda'),
+ mock.call('multipath', '-c', '/dev/sda1'),
+ ]
+ mocked_execute.assert_has_calls(expected_calls)
self.assertEqual(BLK_DEVICE_TEMPLATE_PARTUUID_DEVICE, result)
mocked_udev.assert_called_once_with()
+ mocked_mpath.assert_called_once_with()
+ @mock.patch.object(hardware, 'get_multipath_status', autospec=True)
@mock.patch.object(hardware, '_get_device_info',
lambda x, y: "FooTastic")
@mock.patch.object(hardware, '_udev_settle', autospec=True)
def test_list_all_block_devices_wrong_block_type(self, mocked_udev,
+ mock_mpath_enabled,
mocked_execute):
+ mock_mpath_enabled.return_value = False
mocked_execute.return_value = ('TYPE="foo" MODEL="model"', '')
result = hardware.list_all_block_devices()
mocked_execute.assert_called_once_with(
@@ -4181,17 +4520,32 @@ class TestModuleFunctions(base.IronicAgentTest):
self.assertEqual([], result)
mocked_udev.assert_called_once_with()
+ @mock.patch.object(hardware, 'get_multipath_status', autospec=True)
@mock.patch.object(hardware, '_udev_settle', autospec=True)
def test_list_all_block_devices_missing(self, mocked_udev,
+ mocked_mpath,
mocked_execute):
"""Test for missing values returned from lsblk"""
- mocked_execute.return_value = ('TYPE="disk" MODEL="model"', '')
+ mocked_mpath.return_value = False
+ mocked_execute.side_effect = [
+ ('TYPE="disk" MODEL="model"', ''),
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ]
+ expected_calls = [
+ mock.call('lsblk', '-Pbia',
+ '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'),
+ ]
+
self.assertRaisesRegex(
errors.BlockDeviceError,
r'^Block device caused unknown error: KNAME, PARTUUID, ROTA, '
r'SIZE, UUID must be returned by lsblk.$',
hardware.list_all_block_devices)
mocked_udev.assert_called_once_with()
+ mocked_execute.assert_has_calls(expected_calls)
def test__udev_settle(self, mocked_execute):
hardware._udev_settle()
@@ -4210,6 +4564,103 @@ class TestModuleFunctions(base.IronicAgentTest):
mock.call('iscsistart', '-f')])
+@mock.patch.object(utils, 'execute', autospec=True)
+@mock.patch.object(il_utils, 'try_execute', autospec=True)
+class TestMultipathEnabled(base.IronicAgentTest):
+
+ @mock.patch.object(os.path, 'isfile', autospec=True)
+ def test_enable_multipath_with_config(self, mock_isfile, mock_try_exec,
+ mocked_execute):
+ mock_isfile.side_effect = [True, True]
+ mock_try_exec.side_effect = [
+ ('', ''),
+ ('', ''),
+ ]
+ mocked_execute.side_effect = [
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ]
+ self.assertTrue(hardware._enable_multipath())
+ mock_try_exec.assert_has_calls([
+ mock.call('modprobe', 'dm_multipath'),
+ mock.call('modprobe', 'multipath'),
+ ])
+ mocked_execute.assert_has_calls([
+ mock.call('multipathd'),
+ mock.call('multipath', '-ll'),
+ ])
+
+ @mock.patch.object(os.path, 'isfile', autospec=True)
+ def test_enable_multipath_mpathconf(self, mock_isfile, mock_try_exec,
+ mocked_execute):
+ mock_isfile.side_effect = [True, False]
+ mock_try_exec.side_effect = [
+ ('', ''),
+ ('', ''),
+ ]
+ mocked_execute.side_effect = [
+ ('', ''),
+ ('', ''),
+ ]
+ self.assertTrue(hardware._enable_multipath())
+ mock_try_exec.assert_has_calls([
+ mock.call('/usr/sbin/mpathconf', '--enable',
+ '--find_multipaths', 'yes',
+ '--with_module', 'y',
+ '--with_multipathd', 'y'),
+ ])
+ mocked_execute.assert_has_calls([
+ mock.call('multipathd'),
+ mock.call('multipath', '-ll'),
+ ])
+
+ @mock.patch.object(os.path, 'isfile', autospec=True)
+ def test_enable_multipath_no_multipath(self, mock_isfile, mock_try_exec,
+ mocked_execute):
+ mock_isfile.return_value = False
+ mock_try_exec.side_effect = [
+ ('', ''),
+ ('', ''),
+ ]
+ mocked_execute.side_effect = [
+ ('', ''),
+ ('', ''),
+ ]
+ self.assertTrue(hardware._enable_multipath())
+ mock_try_exec.assert_has_calls([
+ mock.call('modprobe', 'dm_multipath'),
+ mock.call('modprobe', 'multipath'),
+ ])
+ mocked_execute.assert_has_calls([
+ mock.call('multipathd'),
+ mock.call('multipath', '-ll'),
+ ])
+
+ @mock.patch.object(hardware, '_load_multipath_modules', autospec=True)
+ def test_enable_multipath_not_found_mpath_config(self,
+ mock_modules,
+ mock_try_exec,
+ mocked_execute):
+ mocked_execute.side_effect = FileNotFoundError()
+ self.assertFalse(hardware._enable_multipath())
+ self.assertEqual(1, mocked_execute.call_count)
+ self.assertEqual(1, mock_modules.call_count)
+
+ @mock.patch.object(hardware, '_load_multipath_modules', autospec=True)
+ def test_enable_multipath_lacking_support(self,
+ mock_modules,
+ mock_try_exec,
+ mocked_execute):
+ mocked_execute.side_effect = [
+ ('', ''), # Help will of course work.
+ processutils.ProcessExecutionError('lacking kernel support')
+ ]
+ self.assertFalse(hardware._enable_multipath())
+ self.assertEqual(2, mocked_execute.call_count)
+ self.assertEqual(1, mock_modules.call_count)
+
+
def create_hdparm_info(supported=False, enabled=False, locked=False,
frozen=False, enhanced_erase=False):
diff --git a/ironic_python_agent/tests/unit/test_utils.py b/ironic_python_agent/tests/unit/test_utils.py
index bfad67f0..a8d62118 100644
--- a/ironic_python_agent/tests/unit/test_utils.py
+++ b/ironic_python_agent/tests/unit/test_utils.py
@@ -444,7 +444,8 @@ class TestUtils(testtools.TestCase):
file_list=[],
io_dict={'journal': mock.ANY, 'ip_addr': mock.ANY, 'ps': mock.ANY,
'df': mock.ANY, 'iptables': mock.ANY, 'lshw': mock.ANY,
- 'lsblk': mock.ANY, 'mdstat': mock.ANY})
+ 'lsblk': mock.ANY, 'mdstat': mock.ANY,
+ 'multipath': mock.ANY})
@mock.patch.object(utils, 'gzip_and_b64encode', autospec=True)
@mock.patch.object(utils, 'is_journalctl_present', autospec=True)
@@ -466,7 +467,8 @@ class TestUtils(testtools.TestCase):
file_list=['/var/log'],
io_dict={'iptables': mock.ANY, 'ip_addr': mock.ANY, 'ps': mock.ANY,
'dmesg': mock.ANY, 'df': mock.ANY, 'lshw': mock.ANY,
- 'lsblk': mock.ANY, 'mdstat': mock.ANY})
+ 'lsblk': mock.ANY, 'mdstat': mock.ANY,
+ 'multipath': mock.ANY})
def test_get_ssl_client_options(self):
# defaults
diff --git a/ironic_python_agent/utils.py b/ironic_python_agent/utils.py
index 2fc60ff1..77de3cbe 100644
--- a/ironic_python_agent/utils.py
+++ b/ironic_python_agent/utils.py
@@ -70,6 +70,7 @@ COLLECT_LOGS_COMMANDS = {
'lshw': ['lshw', '-quiet', '-json'],
'lsblk': ['lsblk', '--all', '-o%s' % ','.join(LSBLK_COLUMNS)],
'mdstat': ['cat', '/proc/mdstat'],
+ 'multipath': ['multipath', '-ll'],
}
diff --git a/releasenotes/notes/multipath-handling-00a5b412d2cf2e4e.yaml b/releasenotes/notes/multipath-handling-00a5b412d2cf2e4e.yaml
new file mode 100644
index 00000000..19f299ca
--- /dev/null
+++ b/releasenotes/notes/multipath-handling-00a5b412d2cf2e4e.yaml
@@ -0,0 +1,18 @@
+---
+fixes:
+ - |
+ Fixes failures with handling of Multipath IO devices where Active/Passive
+ storage arrays are in use. Previously, "standby" paths could result in
+ IO errors causing cleaning to terminate. The agent now explicitly attempts
+ to handle and account for multipaths based upon the MPIO data available.
+ This requires the ``multipath`` and ``multipathd`` utility to be present
+ in the ramdisk. These are supplied by the ``device-mapper-multipath`` or
+ ``multipath-tools`` packages, and are not requried for the agent's use.
+ - |
+ Fixes non-ideal behavior when performing cleaning where Active/Active
+ MPIO devices would ultimately be cleaned once per IO path, instead of
+ once per backend device.
+other:
+ - |
+ The agent will now attempt to collect any multipath path information
+ and upload it to the agent ramdisk, if the tooling is present.