summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJulia Kreger <juliaashleykreger@gmail.com>2022-04-07 15:15:17 -0700
committerIury Gregory Melo Ferreira <imelofer@redhat.com>2022-05-18 20:26:39 -0300
commit014d37743a3b5694e0e2a3cabfafe885417172d5 (patch)
tree110f09fd260655586fba4e307c9923a0a8160dc4
parent2c22e60b45509d949db1ca03092c0b4575e00727 (diff)
downloadironic-python-agent-014d37743a3b5694e0e2a3cabfafe885417172d5.tar.gz
Multipath Hardware path handling
Removes multipath base devices from consideration by default, and instead allows the device-mapper device managed by multipath to be picked up and utilized instead. In effect, allowing us to ignore standby paths *and* leverage multiple concurrent IO paths if so offered via ALUA. In reality, anyone who has previously built IPA with multipath tooling might not have encountered issues previously because they used Active/Active SAN storage environments. They would have worked because the IO lock would have been exchanged between controllers and paths. However, Active/Passive environments will block passive paths from access, ultimately preventing new locks from being established without proper negotiation. Ultimately requiring multipathing *and* the agent to be smart enough to know to disqualify underlying paths to backend storage volumes. An additional benefit of this is active/active MPIO devices will, as long as ``multipath`` is present inside the ramdisk, no longer possibly result in duplicate IO wipes occuring accross numerous devices. Story: #2010003 Task: #45108 Resolves: rhbz#2076622 Resolves: rhbz#2070519 Change-Id: I0fd6356f036d5ff17510fb838eaf418164cdfc92
-rw-r--r--ironic_python_agent/hardware.py153
-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.py515
-rw-r--r--ironic_python_agent/tests/unit/test_utils.py36
-rw-r--r--ironic_python_agent/utils.py1
-rw-r--r--releasenotes/notes/multipath-handling-00a5b412d2cf2e4e.yaml18
8 files changed, 748 insertions, 57 deletions
diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py
index 54de8d72..a4cc60fd 100644
--- a/ironic_python_agent/hardware.py
+++ b/ironic_python_agent/hardware.py
@@ -81,6 +81,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."""
@@ -138,6 +140,36 @@ def _load_ipmi_modules():
il_utils.try_execute('modprobe', 'ipmi_si')
+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.
@@ -181,6 +213,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.
+ il_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.
+ il_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.
+ il_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, _ = il_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.
@@ -371,7 +481,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
@@ -388,6 +499,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
"""
@@ -398,6 +512,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
@@ -428,7 +544,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 = {}
@@ -450,10 +565,25 @@ def list_all_block_devices(block_type='disk',
LOG.debug('Ignoring floppy disk device: %s', line)
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(
@@ -462,7 +592,7 @@ def list_all_block_devices(block_type='disk',
{'block_type': block_type, 'line': 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: %s", line)
@@ -476,6 +606,11 @@ def list_all_block_devices(block_type='disk',
LOG.debug(
"TYPE detected to contain 'md', signifying a "
"RAID partition. Found: %s", 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: %(block_type)s but found: "
@@ -1001,6 +1136,10 @@ class GenericHardwareManager(HardwareManager):
_check_for_iscsi()
_md_scan_and_assemble()
_load_ipmi_modules()
+ global MULTIPATH_ENABLED
+ if MULTIPATH_ENABLED is None:
+ MULTIPATH_ENABLED = _enable_multipath()
+
self.wait_for_disks()
return HardwareSupport.GENERIC
@@ -2787,3 +2926,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 e04fb3bc..2444e30d 100644
--- a/ironic_python_agent/tests/unit/extensions/test_image.py
+++ b/ironic_python_agent/tests/unit/extensions/test_image.py
@@ -737,6 +737,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)
@@ -848,6 +849,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 0c568028..d90b0414 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
@@ -988,12 +994,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
@@ -1006,12 +1013,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 0db92354..1eb0f80a 100644
--- a/ironic_python_agent/tests/unit/test_hardware.py
+++ b/ironic_python_agent/tests/unit/test_hardware.py
@@ -758,60 +758,238 @@ 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(il_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(il_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(il_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(il_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_listdir.return_value = ['1:0:0:0']
mock_cached_node.return_value = None
- mocked_execute.return_value = (hws.RAID_BLK_DEVICE_TEMPLATE, '')
+ mocked_mpath.return_value = False
+ mocked_execute.side_effect = [
+ (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')
+ expected = [
+ mock.call('lsblk', '-Pbia',
+ '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'),
+ ]
+
+ 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(il_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)
@@ -1215,6 +1393,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)
@@ -1232,7 +1411,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),
@@ -1262,7 +1468,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,
@@ -1278,21 +1484,42 @@ 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(il_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()
@@ -1304,6 +1531,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)
@@ -1311,15 +1539,62 @@ class TestGenericHardwareManager(base.IronicAgentTest):
@mock.patch.object(il_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()
@@ -1377,6 +1652,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)
@@ -4160,6 +4436,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
self.hardware._nvme_erase, block_device)
+@mock.patch.object(hardware, '_enable_multipath', autospec=True)
@mock.patch.object(hardware, '_load_ipmi_modules', autospec=True)
@mock.patch.object(hardware.GenericHardwareManager,
'get_os_install_device', autospec=True)
@@ -4174,7 +4451,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_load_ipmi_modules):
+ mocked_load_ipmi_modules, mocked_enable_mpath):
mocked_get_inst_dev.side_effect = [
errors.DeviceNotFound('boom'),
None
@@ -4194,7 +4471,7 @@ class TestEvaluateHardwareSupport(base.IronicAgentTest):
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_load_ipmi_modules):
+ mocked_load_ipmi_modules, mocked_enable_mpath):
CONF.set_override('disk_wait_attempts', '0')
result = self.hardware.evaluate_hardware_support()
@@ -4209,7 +4486,7 @@ class TestEvaluateHardwareSupport(base.IronicAgentTest):
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_load_ipmi_modules):
+ mocked_load_ipmi_modules, mocked_enable_mpath):
mocked_get_inst_dev.side_effect = [
errors.DeviceNotFound('boom'),
errors.DeviceNotFound('boom'),
@@ -4241,7 +4518,8 @@ class TestEvaluateHardwareSupport(base.IronicAgentTest):
mocked_check_for_iscsi,
mocked_md_assemble,
mocked_get_inst_dev,
- mocked_load_ipmi_modules):
+ mocked_load_ipmi_modules,
+ mocked_enable_mpath):
CONF.set_override('disk_wait_attempts', '1')
mocked_get_inst_dev.side_effect = [
@@ -4262,7 +4540,8 @@ class TestEvaluateHardwareSupport(base.IronicAgentTest):
mocked_check_for_iscsi,
mocked_md_assemble,
mocked_get_inst_dev,
- mocked_load_ipmi_modules):
+ mocked_load_ipmi_modules,
+ mocked_enable_mpath):
mocked_get_inst_dev.side_effect = errors.DeviceNotFound('boom')
self.hardware.evaluate_hardware_support()
mocked_sleep.assert_called_with(3)
@@ -4271,7 +4550,8 @@ class TestEvaluateHardwareSupport(base.IronicAgentTest):
mocked_check_for_iscsi,
mocked_md_assemble,
mocked_root_dev,
- mocked_load_ipmi_modules):
+ mocked_load_ipmi_modules,
+ mocked_enable_mpath):
CONF.set_override('disk_wait_delay', '5')
mocked_root_dev.side_effect = errors.DeviceNotFound('boom')
@@ -4281,7 +4561,8 @@ 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_load_ipmi_modules):
+ mocked_load_ipmi_modules,
+ mocked_enable_mpath):
mocked_get_inst_dev.side_effect = errors.DeviceNotFound('boom')
result = self.hardware.evaluate_hardware_support()
self.assertEqual(hardware.HardwareSupport.GENERIC, result)
@@ -4295,6 +4576,7 @@ class TestEvaluateHardwareSupport(base.IronicAgentTest):
@mock.patch.object(il_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')
@@ -4303,16 +4585,31 @@ 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')
@@ -4321,16 +4618,48 @@ 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')
@@ -4340,21 +4669,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(
@@ -4362,17 +4707,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()
@@ -4399,6 +4759,85 @@ class TestModuleFunctions(base.IronicAgentTest):
mock.call('modprobe', 'ipmi_si')])
+@mock.patch.object(il_utils, 'execute', autospec=True)
+class TestMultipathEnabled(base.IronicAgentTest):
+
+ @mock.patch.object(os.path, 'isfile', autospec=True)
+ def test_enable_multipath_with_config(self, mock_isfile, mocked_execute):
+ mock_isfile.side_effect = [True, True]
+ mocked_execute.side_effect = [
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ]
+ self.assertTrue(hardware._enable_multipath())
+ mocked_execute.assert_has_calls([
+ mock.call('modprobe', 'dm_multipath'),
+ mock.call('modprobe', 'multipath'),
+ mock.call('multipathd'),
+ mock.call('multipath', '-ll'),
+ ])
+
+ @mock.patch.object(os.path, 'isfile', autospec=True)
+ def test_enable_multipath_mpathconf(self, mock_isfile, mocked_execute):
+ mock_isfile.side_effect = [True, False]
+ mocked_execute.side_effect = [
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ]
+ self.assertTrue(hardware._enable_multipath())
+ mocked_execute.assert_has_calls([
+ mock.call('/usr/sbin/mpathconf', '--enable',
+ '--find_multipaths', 'yes',
+ '--with_module', 'y',
+ '--with_multipathd', 'y'),
+ mock.call('multipathd'),
+ mock.call('multipath', '-ll'),
+ ])
+
+ @mock.patch.object(os.path, 'isfile', autospec=True)
+ def test_enable_multipath_no_multipath(self, mock_isfile, mocked_execute):
+ mock_isfile.return_value = False
+ mocked_execute.side_effect = [
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ('', ''),
+ ]
+ self.assertTrue(hardware._enable_multipath())
+ mocked_execute.assert_has_calls([
+ mock.call('modprobe', 'dm_multipath'),
+ mock.call('modprobe', 'multipath'),
+ 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,
+ 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,
+ 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 bb6a6f79..c99f7dca 100644
--- a/ironic_python_agent/tests/unit/test_utils.py
+++ b/ironic_python_agent/tests/unit/test_utils.py
@@ -428,8 +428,13 @@ class TestUtils(ironic_agent_base.IronicAgentTest):
mock.call(['lshw', '-quiet', '-json'])]
mock_outputs.assert_has_calls(calls, any_order=True)
mock_gzip_b64.assert_called_once_with(
- file_list=[],
- io_dict=mock.ANY)
+ io_dict={'journal': mock.ANY, 'ps': mock.ANY, 'df': mock.ANY,
+ 'iptables': mock.ANY, 'ip_addr': mock.ANY,
+ 'lshw': mock.ANY, 'lsblk': mock.ANY,
+ 'lsblk-full': mock.ANY, 'mdstat': mock.ANY,
+ 'mount': mock.ANY, 'parted': mock.ANY,
+ 'multipath': mock.ANY},
+ file_list=[])
@mock.patch.object(utils, 'gzip_and_b64encode', autospec=True)
@mock.patch.object(utils, 'is_journalctl_present', autospec=True)
@@ -453,8 +458,13 @@ class TestUtils(ironic_agent_base.IronicAgentTest):
mock.call(['lshw', '-quiet', '-json'])]
mock_outputs.assert_has_calls(calls, any_order=True)
mock_gzip_b64.assert_called_once_with(
- file_list=[tmp.name],
- io_dict=mock.ANY)
+ io_dict={'journal': mock.ANY, 'ps': mock.ANY, 'df': mock.ANY,
+ 'iptables': mock.ANY, 'ip_addr': mock.ANY,
+ 'lshw': mock.ANY, 'lsblk': mock.ANY,
+ 'lsblk-full': mock.ANY, 'mdstat': mock.ANY,
+ 'mount': mock.ANY, 'parted': mock.ANY,
+ 'multipath': mock.ANY},
+ file_list=[tmp.name])
@mock.patch.object(utils, 'gzip_and_b64encode', autospec=True)
@mock.patch.object(utils, 'is_journalctl_present', autospec=True)
@@ -473,8 +483,13 @@ class TestUtils(ironic_agent_base.IronicAgentTest):
mock.call(['lshw', '-quiet', '-json'])]
mock_outputs.assert_has_calls(calls, any_order=True)
mock_gzip_b64.assert_called_once_with(
- file_list=['/var/log'],
- io_dict=mock.ANY)
+ io_dict={'dmesg': mock.ANY, 'ps': mock.ANY, 'df': mock.ANY,
+ 'iptables': mock.ANY, 'ip_addr': mock.ANY,
+ 'lshw': mock.ANY, 'lsblk': mock.ANY,
+ 'lsblk-full': mock.ANY, 'mdstat': mock.ANY,
+ 'mount': mock.ANY, 'parted': mock.ANY,
+ 'multipath': mock.ANY},
+ file_list=['/var/log'])
@mock.patch.object(utils, 'gzip_and_b64encode', autospec=True)
@mock.patch.object(utils, 'is_journalctl_present', autospec=True)
@@ -497,8 +512,13 @@ class TestUtils(ironic_agent_base.IronicAgentTest):
mock.call(['lshw', '-quiet', '-json'])]
mock_outputs.assert_has_calls(calls, any_order=True)
mock_gzip_b64.assert_called_once_with(
- file_list=['/var/log', tmp.name],
- io_dict=mock.ANY)
+ io_dict={'dmesg': mock.ANY, 'ps': mock.ANY, 'df': mock.ANY,
+ 'iptables': mock.ANY, 'ip_addr': mock.ANY,
+ 'lshw': mock.ANY, 'lsblk': mock.ANY,
+ 'lsblk-full': mock.ANY, 'mdstat': mock.ANY,
+ 'mount': mock.ANY, 'parted': mock.ANY,
+ 'multipath': mock.ANY},
+ file_list=['/var/log', tmp.name])
def test_get_ssl_client_options(self):
# defaults
diff --git a/ironic_python_agent/utils.py b/ironic_python_agent/utils.py
index 3f27cb77..66f6819f 100644
--- a/ironic_python_agent/utils.py
+++ b/ironic_python_agent/utils.py
@@ -72,6 +72,7 @@ COLLECT_LOGS_COMMANDS = {
'mdstat': ['cat', '/proc/mdstat'],
'mount': ['mount'],
'parted': ['parted', '-l'],
+ '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.