diff options
27 files changed, 1149 insertions, 280 deletions
diff --git a/ironic_python_agent/api/app.py b/ironic_python_agent/api/app.py index 93a53bad..47f5dd85 100644 --- a/ironic_python_agent/api/app.py +++ b/ironic_python_agent/api/app.py @@ -20,7 +20,6 @@ from oslo_service import wsgi import werkzeug from werkzeug import exceptions as http_exc from werkzeug import routing -from werkzeug.wrappers import json as http_json from ironic_python_agent import encoding @@ -30,7 +29,7 @@ _CUSTOM_MEDIA_TYPE = 'application/vnd.openstack.ironic-python-agent.v1+json' _DOCS_URL = 'https://docs.openstack.org/ironic-python-agent' -class Request(werkzeug.Request, http_json.JSONMixin): +class Request(werkzeug.Request): """Custom request class with JSON support.""" diff --git a/ironic_python_agent/efi_utils.py b/ironic_python_agent/efi_utils.py index c4e9dcc7..48e643d3 100644 --- a/ironic_python_agent/efi_utils.py +++ b/ironic_python_agent/efi_utils.py @@ -12,7 +12,7 @@ import os import re -import shutil +import sys import tempfile from ironic_lib import disk_utils @@ -29,6 +29,37 @@ from ironic_python_agent import utils LOG = log.getLogger(__name__) +def get_partition_path_by_number(device, part_num): + """Get partition path (/dev/something) by a partition number on device. + + Only works for GPT partition table. + """ + uuid = None + partinfo, _ = utils.execute('sgdisk', '-i', str(part_num), device, + use_standard_locale=True) + for line in partinfo.splitlines(): + if not line.strip(): + continue + + try: + field, value = line.rsplit(':', 1) + except ValueError: + LOG.warning('Invalid sgdisk line: %s', line) + continue + + if 'partition unique guid' in field.lower(): + uuid = value.strip().lower() + LOG.debug('GPT partition number %s on device %s has UUID %s', + part_num, device, uuid) + break + + if uuid is not None: + return partition_utils.get_partition(device, uuid) + else: + LOG.warning('No UUID information provided in sgdisk output for ' + 'partition %s on device %s', part_num, device) + + def manage_uefi(device, efi_system_part_uuid=None): """Manage the device looking for valid efi bootloaders to update the nvram. @@ -45,52 +76,59 @@ def manage_uefi(device, efi_system_part_uuid=None): efi_partition_mount_point = None efi_mounted = False LOG.debug('Attempting UEFI loader autodetection and NVRAM record setup.') - try: - # Force UEFI to rescan the device. - utils.rescan_device(device) - - local_path = tempfile.mkdtemp() - # Trust the contents on the disk in the event of a whole disk image. - efi_partition = disk_utils.find_efi_partition(device) - if efi_partition: - efi_partition = efi_partition['number'] - - if not efi_partition and efi_system_part_uuid: - # _get_partition returns <device>+<partition> and we only need the - # partition number - partition = partition_utils.get_partition( - device, uuid=efi_system_part_uuid) + + # Force UEFI to rescan the device. + utils.rescan_device(device) + + # Trust the contents on the disk in the event of a whole disk image. + efi_partition = disk_utils.find_efi_partition(device) + if efi_partition: + efi_part_num = efi_partition['number'] + efi_partition = get_partition_path_by_number(device, efi_part_num) + + if not efi_partition and efi_system_part_uuid: + # get_partition returns <device>+<partition> and we only need the + # partition number + efi_partition = partition_utils.get_partition( + device, uuid=efi_system_part_uuid) + # FIXME(dtantsur): this procedure will not work for devicemapper + # devices. To fix that we need a way to convert a UUID to a partition + # number, which is surprisingly non-trivial and may involve looping + # over existing numbers and calling `sgdisk -i` for each of them. + # But I'm not sure we even need this logic: find_efi_partition should + # be sufficient for both whole disk and partition images. + try: + efi_part_num = int(efi_partition.replace(device, "")) + except ValueError: + # NVMe Devices get a partitioning scheme that is different from + # traditional block devices like SCSI/SATA try: - efi_partition = int(partition.replace(device, "")) - except ValueError: - # NVMe Devices get a partitioning scheme that is different from - # traditional block devices like SCSI/SATA - efi_partition = int(partition.replace(device + 'p', "")) - - if not efi_partition: - # NOTE(dtantsur): we cannot have a valid EFI deployment without an - # EFI partition at all. This code path is easily hit when using an - # image that is not UEFI compatible (which sadly applies to most - # cloud images out there, with a nice exception of Ubuntu). - raise errors.DeviceNotFound( - "No EFI partition could be detected on device %s and " - "EFI partition UUID has not been recorded during deployment " - "(which is often the case for whole disk images). " - "Are you using a UEFI-compatible image?" % device) - - efi_partition_mount_point = os.path.join(local_path, "boot/efi") - if not os.path.exists(efi_partition_mount_point): - os.makedirs(efi_partition_mount_point) - - # The mount needs the device with the partition, in case the - # device ends with a digit we add a `p` and the partition number we - # found, otherwise we just join the device and the partition number - if device[-1].isdigit(): - efi_device_part = '{}p{}'.format(device, efi_partition) - utils.execute('mount', efi_device_part, efi_partition_mount_point) - else: - efi_device_part = '{}{}'.format(device, efi_partition) - utils.execute('mount', efi_device_part, efi_partition_mount_point) + efi_part_num = int(efi_partition.replace(device + 'p', "")) + except ValueError as exc: + # At least provide a reasonable error message if the device + # does not follow this procedure. + raise errors.DeviceNotFound( + "Cannot detect the partition number of the device %s: %s" % + (efi_partition, exc)) + + if not efi_partition: + # NOTE(dtantsur): we cannot have a valid EFI deployment without an + # EFI partition at all. This code path is easily hit when using an + # image that is not UEFI compatible (which sadly applies to most + # cloud images out there, with a nice exception of Ubuntu). + raise errors.DeviceNotFound( + "No EFI partition could be detected on device %s and " + "EFI partition UUID has not been recorded during deployment " + "(which is often the case for whole disk images). " + "Are you using a UEFI-compatible image?" % device) + + local_path = tempfile.mkdtemp() + efi_partition_mount_point = os.path.join(local_path, "boot/efi") + if not os.path.exists(efi_partition_mount_point): + os.makedirs(efi_partition_mount_point) + + try: + utils.execute('mount', efi_partition, efi_partition_mount_point) efi_mounted = True valid_efi_bootloaders = _get_efi_bootloaders(efi_partition_mount_point) @@ -102,7 +140,7 @@ def manage_uefi(device, efi_system_part_uuid=None): if not hardware.is_md_device(device): efi_devices = [device] - efi_partition_numbers = [efi_partition] + efi_partition_numbers = [efi_part_num] efi_label_suffix = '' else: # umount to allow for signature removal (to avoid confusion about @@ -113,7 +151,7 @@ def manage_uefi(device, efi_system_part_uuid=None): holders = hardware.get_holder_disks(device) efi_md_device = raid_utils.prepare_boot_partitions_for_softraid( - device, holders, efi_device_part, target_boot_mode='uefi' + device, holders, efi_partition, target_boot_mode='uefi' ) efi_devices = hardware.get_component_devices(efi_md_device) efi_partition_numbers = [] @@ -130,12 +168,12 @@ def manage_uefi(device, efi_system_part_uuid=None): efi_label_suffix = "(RAID, part%s)" # remount for _run_efibootmgr - utils.execute('mount', efi_device_part, efi_partition_mount_point) + utils.execute('mount', efi_partition, efi_partition_mount_point) efi_mounted = True efi_dev_part = zip(efi_devices, efi_partition_numbers) for i, (efi_dev, efi_part) in enumerate(efi_dev_part): - LOG.debug("Calling efibootmgr with dev %s part %s", + LOG.debug("Calling efibootmgr with dev %s partition number %s", efi_dev, efi_part) if efi_label_suffix: # NOTE (arne_wiebalck): uniqify the labels to prevent @@ -149,34 +187,28 @@ def manage_uefi(device, efi_system_part_uuid=None): return True except processutils.ProcessExecutionError as e: - error_msg = ('Could not verify uefi on device %(dev)s, ' - 'failed with %(err)s.' % {'dev': device, 'err': e}) - LOG.error(error_msg) + error_msg = ('Could not configure UEFI boot on device %(dev)s: %(err)s' + % {'dev': device, 'err': e}) + LOG.exception(error_msg) raise errors.CommandExecutionError(error_msg) finally: - LOG.debug('Executing _manage_uefi clean-up.') - umount_warn_msg = "Unable to umount %(local_path)s. Error: %(error)s" - - try: - if efi_mounted: + if efi_mounted: + try: utils.execute('umount', efi_partition_mount_point, attempts=3, delay_on_retry=True) - except processutils.ProcessExecutionError as e: - error_msg = ('Umounting efi system partition failed. ' - 'Attempted 3 times. Error: %s' % e) - LOG.error(error_msg) - raise errors.CommandExecutionError(error_msg) - - else: - # If umounting the binds succeed then we can try to delete it - try: - utils.execute('sync') except processutils.ProcessExecutionError as e: - LOG.warning(umount_warn_msg, {'path': local_path, 'error': e}) + error_msg = ('Umounting efi system partition failed. ' + 'Attempted 3 times. Error: %s' % e) + LOG.error(error_msg) + # Do not mask the actual failure, if any + if sys.exc_info()[0] is None: + raise errors.CommandExecutionError(error_msg) + else: - # After everything is umounted we can then remove the - # temporary directory - shutil.rmtree(local_path) + try: + utils.execute('sync') + except processutils.ProcessExecutionError as e: + LOG.warning('Unable to sync the local disks: %s', e) # NOTE(TheJulia): Do not add bootia32.csv to this list. That is 32bit @@ -260,7 +292,7 @@ def add_boot_record(device, efi_partition, loader, label): """ # https://linux.die.net/man/8/efibootmgr utils.execute('efibootmgr', '-v', '-c', '-d', device, - '-p', efi_partition, '-w', '-L', label, + '-p', str(efi_partition), '-w', '-L', label, '-l', loader) diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index c697911f..79f4a426 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: " @@ -503,26 +638,34 @@ def list_all_block_devices(block_type='disk', name = os.path.join('/dev', device['KNAME']) + extra = {} try: udev = pyudev.Devices.from_device_file(context, name) except pyudev.DeviceNotFoundByFileError as e: LOG.warning("Device %(dev)s is inaccessible, skipping... " "Error: %(error)s", {'dev': name, 'error': e}) - extra = {} except pyudev.DeviceNotFoundByNumberError as e: LOG.warning("Device %(dev)s is not supported by pyudev, " "skipping... Error: %(error)s", {'dev': name, 'error': e}) - extra = {} else: # TODO(lucasagomes): Since lsblk only supports # returning the short serial we are using - # ID_SERIAL_SHORT here to keep compatibility with the + # ID_SERIAL_SHORT first to keep compatibility with the # bash deploy ramdisk - extra = {key: udev.get('ID_%s' % udev_key) for key, udev_key in - [('wwn', 'WWN'), ('serial', 'SERIAL_SHORT'), - ('wwn_with_extension', 'WWN_WITH_EXTENSION'), - ('wwn_vendor_extension', 'WWN_VENDOR_EXTENSION')]} + for key, udev_key in [ + ('serial', 'SERIAL_SHORT'), + ('serial', 'SERIAL'), + ('wwn', 'WWN'), + ('wwn_with_extension', 'WWN_WITH_EXTENSION'), + ('wwn_vendor_extension', 'WWN_VENDOR_EXTENSION') + ]: + if key in extra: + continue + value = (udev.get(f'ID_{udev_key}') + or udev.get(f'DM_{udev_key}')) # devicemapper + if value: + extra[key] = value # NOTE(lucasagomes): Newer versions of the lsblk tool supports # HCTL as a parameter but let's get it from sysfs to avoid breaking @@ -1001,6 +1144,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 @@ -2788,3 +2935,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/partition_utils.py b/ironic_python_agent/partition_utils.py index 42b90445..d82d0072 100644 --- a/ironic_python_agent/partition_utils.py +++ b/ironic_python_agent/partition_utils.py @@ -35,6 +35,7 @@ from oslo_config import cfg from oslo_log import log from oslo_utils import excutils from oslo_utils import units +from oslo_utils import uuidutils import requests from ironic_python_agent import errors @@ -377,14 +378,18 @@ def create_config_drive_partition(node_uuid, device, configdrive): "%(part)s", {'node': node_uuid, 'part': config_drive_part}) else: - cur_parts = set(part['number'] - for part in disk_utils.list_partitions(device)) - + part_uuid = None if disk_utils.get_partition_table_type(device) == 'gpt': + part_uuid = uuidutils.generate_uuid() create_option = '0:-%dMB:0' % MAX_CONFIG_DRIVE_SIZE_MB - utils.execute('sgdisk', '-n', create_option, device, + uuid_option = '0:%s' % part_uuid + utils.execute('sgdisk', '-n', create_option, + '-u', uuid_option, device, run_as_root=True) else: + cur_parts = set(part['number'] + for part in disk_utils.list_partitions(device)) + # Check if the disk has 4 partitions. The MBR based disk # cannot have more than 4 partitions. # TODO(stendulker): One can use logical partitions to create @@ -426,17 +431,29 @@ def create_config_drive_partition(node_uuid, device, configdrive): # Trigger device rescan disk_utils.trigger_device_rescan(device) - upd_parts = set(part['number'] - for part in disk_utils.list_partitions(device)) - new_part = set(upd_parts) - set(cur_parts) - if len(new_part) != 1: - raise exception.InstanceDeployFailure( - 'Disk partitioning failed on device %(device)s. ' - 'Unable to retrieve config drive partition information.' - % {'device': device}) + if part_uuid is None: + new_parts = {part['number']: part + for part in disk_utils.list_partitions(device)} + new_part = set(new_parts) - set(cur_parts) + if len(new_part) != 1: + raise exception.InstanceDeployFailure( + 'Disk partitioning failed on device %(device)s. ' + 'Unable to retrieve config drive partition ' + 'information.' % {'device': device}) - config_drive_part = disk_utils.partition_index_to_path( - device, new_part.pop()) + config_drive_part = disk_utils.partition_index_to_path( + device, new_part.pop()) + else: + try: + config_drive_part = get_partition(device, part_uuid) + except errors.DeviceNotFound: + msg = ('Failed to create config drive on disk %(disk)s ' + 'for node %(node)s. Partition with UUID %(uuid)s ' + 'has not been found after creation.') % { + 'disk': device, 'node': node_uuid, + 'uuid': part_uuid} + LOG.error(msg) + raise exception.InstanceDeployFailure(msg) disk_utils.udev_settle() diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py index 00aaf706..2444e30d 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -219,14 +219,14 @@ class TestImageExtension(base.IronicAgentTest): @mock.patch.object(disk_utils, 'find_efi_partition', autospec=False) @mock.patch.object(os, 'makedirs', autospec=True) def test__uefi_bootloader_given_partition( - self, mkdir_mock, mock_utils_efi_part, mock_partition, + self, mkdir_mock, mock_utils_efi_part, mock_get_partition, mock_efi_bl, mock_execute, mock_dispatch): mock_dispatch.side_effect = [ self.fake_dev, hardware.BootInfo(current_boot_mode='uefi') ] - mock_partition.side_effect = [self.fake_dev, self.fake_efi_system_part] + mock_get_partition.return_value = self.fake_efi_system_part mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] - mock_utils_efi_part.return_value = {'number': '1'} + mock_utils_efi_part.return_value = None mock_execute.side_effect = iter([('', ''), ('', ''), ('', ''), ('', ''), @@ -263,16 +263,17 @@ class TestImageExtension(base.IronicAgentTest): @mock.patch.object(hardware, 'is_md_device', lambda *_: False) @mock.patch.object(os.path, 'exists', lambda *_: False) @mock.patch.object(efi_utils, '_get_efi_bootloaders', autospec=True) - @mock.patch.object(partition_utils, 'get_partition', autospec=True) + @mock.patch.object(efi_utils, 'get_partition_path_by_number', + autospec=True) @mock.patch.object(disk_utils, 'find_efi_partition', autospec=True) @mock.patch.object(os, 'makedirs', autospec=True) def test__uefi_bootloader_find_partition( - self, mkdir_mock, mock_utils_efi_part, mock_partition, + self, mkdir_mock, mock_utils_efi_part, mock_get_part_path, mock_efi_bl, mock_execute, mock_dispatch): mock_dispatch.side_effect = [ self.fake_dev, hardware.BootInfo(current_boot_mode='uefi') ] - mock_partition.return_value = self.fake_dev + mock_get_part_path.return_value = self.fake_efi_system_part mock_utils_efi_part.return_value = {'number': '1'} mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] mock_execute.side_effect = iter([('', ''), ('', ''), @@ -310,16 +311,17 @@ class TestImageExtension(base.IronicAgentTest): @mock.patch.object(hardware, 'is_md_device', lambda *_: False) @mock.patch.object(os.path, 'exists', lambda *_: False) @mock.patch.object(efi_utils, '_get_efi_bootloaders', autospec=True) - @mock.patch.object(partition_utils, 'get_partition', autospec=True) + @mock.patch.object(efi_utils, 'get_partition_path_by_number', + autospec=True) @mock.patch.object(disk_utils, 'find_efi_partition', autospec=True) @mock.patch.object(os, 'makedirs', autospec=True) def test__uefi_bootloader_with_entry_removal( - self, mkdir_mock, mock_utils_efi_part, mock_partition, + self, mkdir_mock, mock_utils_efi_part, mock_get_part_path, mock_efi_bl, mock_execute, mock_dispatch): mock_dispatch.side_effect = [ self.fake_dev, hardware.BootInfo(current_boot_mode='uefi') ] - mock_partition.return_value = self.fake_dev + mock_get_part_path.return_value = self.fake_efi_system_part mock_utils_efi_part.return_value = {'number': '1'} mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] stdout_msg = """ @@ -367,16 +369,17 @@ Boot0002 VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) @mock.patch.object(hardware, 'is_md_device', lambda *_: False) @mock.patch.object(os.path, 'exists', lambda *_: False) @mock.patch.object(efi_utils, '_get_efi_bootloaders', autospec=True) - @mock.patch.object(partition_utils, 'get_partition', autospec=True) + @mock.patch.object(efi_utils, 'get_partition_path_by_number', + autospec=True) @mock.patch.object(disk_utils, 'find_efi_partition', autospec=True) @mock.patch.object(os, 'makedirs', autospec=True) def test__uefi_bootloader_with_entry_removal_lenovo( - self, mkdir_mock, mock_utils_efi_part, mock_partition, + self, mkdir_mock, mock_utils_efi_part, mock_get_part_path, mock_efi_bl, mock_execute, mock_dispatch): mock_dispatch.side_effect = [ self.fake_dev, hardware.BootInfo(current_boot_mode='uefi') ] - mock_partition.return_value = self.fake_dev + mock_get_part_path.return_value = self.fake_efi_system_part mock_utils_efi_part.return_value = {'number': '1'} mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] # NOTE(TheJulia): This test string was derived from a lenovo SR650 @@ -429,16 +432,17 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 @mock.patch.object(hardware, 'is_md_device', lambda *_: False) @mock.patch.object(os.path, 'exists', lambda *_: False) @mock.patch.object(efi_utils, '_get_efi_bootloaders', autospec=True) - @mock.patch.object(partition_utils, 'get_partition', autospec=True) + @mock.patch.object(efi_utils, 'get_partition_path_by_number', + autospec=True) @mock.patch.object(disk_utils, 'find_efi_partition', autospec=True) @mock.patch.object(os, 'makedirs', autospec=True) def test__add_multi_bootloaders( - self, mkdir_mock, mock_utils_efi_part, mock_partition, + self, mkdir_mock, mock_utils_efi_part, mock_get_part_path, mock_efi_bl, mock_execute, mock_dispatch): mock_dispatch.side_effect = [ self.fake_dev, hardware.BootInfo(current_boot_mode='uefi') ] - mock_partition.return_value = self.fake_dev + mock_get_part_path.return_value = self.fake_efi_system_part mock_utils_efi_part.return_value = {'number': '1'} mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI', 'WINDOWS/system32/winload.efi'] @@ -733,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) @@ -844,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 8cc9b1ea..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' @@ -1500,3 +1546,23 @@ 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 +First sector: 2048 (at 1024.0 KiB) +Last sector: 1050623 (at 513.0 MiB) +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_api.py b/ironic_python_agent/tests/unit/test_api.py index ff71e323..c9491493 100644 --- a/ironic_python_agent/tests/unit/test_api.py +++ b/ironic_python_agent/tests/unit/test_api.py @@ -18,7 +18,6 @@ from unittest import mock from oslo_config import cfg from werkzeug import test as http_test from werkzeug import wrappers -from werkzeug.wrappers import json as http_json from ironic_python_agent import agent from ironic_python_agent.api import app @@ -29,7 +28,7 @@ from ironic_python_agent.tests.unit import base as ironic_agent_base PATH_PREFIX = '/v1' -class Response(wrappers.Response, http_json.JSONMixin): +class Response(wrappers.Response): pass diff --git a/ironic_python_agent/tests/unit/test_efi_utils.py b/ironic_python_agent/tests/unit/test_efi_utils.py index 137ec8d4..64de61bc 100644 --- a/ironic_python_agent/tests/unit/test_efi_utils.py +++ b/ironic_python_agent/tests/unit/test_efi_utils.py @@ -16,6 +16,7 @@ import tempfile from unittest import mock from ironic_lib import disk_utils +from oslo_concurrency import processutils from ironic_python_agent import efi_utils from ironic_python_agent import errors @@ -23,6 +24,7 @@ from ironic_python_agent import hardware from ironic_python_agent import partition_utils from ironic_python_agent import raid_utils from ironic_python_agent.tests.unit import base +from ironic_python_agent.tests.unit.samples import hardware_samples from ironic_python_agent import utils @@ -132,6 +134,7 @@ class TestRunEfiBootmgr(base.IronicAgentTest): @mock.patch.object(utils, 'rescan_device', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(partition_utils, 'get_partition', autospec=True) +@mock.patch.object(efi_utils, 'get_partition_path_by_number', autospec=True) @mock.patch.object(disk_utils, 'find_efi_partition', autospec=True) class TestManageUefi(base.IronicAgentTest): @@ -142,6 +145,7 @@ class TestManageUefi(base.IronicAgentTest): fake_dir = '/tmp/fake-dir' def test_no_partition(self, mock_utils_efi_part, + mock_get_part_path, mock_get_part_uuid, mock_execute, mock_rescan): mock_utils_efi_part.return_value = None @@ -151,6 +155,7 @@ class TestManageUefi(base.IronicAgentTest): mock_rescan.assert_called_once_with(self.fake_dev) def test_empty_partition_by_uuid(self, mock_utils_efi_part, + mock_get_part_path, mock_get_part_uuid, mock_execute, mock_rescan): mock_utils_efi_part.return_value = None @@ -164,10 +169,10 @@ class TestManageUefi(base.IronicAgentTest): @mock.patch.object(efi_utils, '_get_efi_bootloaders', autospec=True) @mock.patch.object(os, 'makedirs', autospec=True) def test_ok(self, mkdir_mock, mock_efi_bl, mock_is_md_device, - mock_utils_efi_part, mock_get_part_uuid, mock_execute, - mock_rescan): + mock_utils_efi_part, mock_get_part_path, mock_get_part_uuid, + mock_execute, mock_rescan): mock_utils_efi_part.return_value = {'number': '1'} - mock_get_part_uuid.return_value = self.fake_dev + mock_get_part_path.return_value = self.fake_efi_system_part mock_is_md_device.return_value = False mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] @@ -201,10 +206,10 @@ class TestManageUefi(base.IronicAgentTest): @mock.patch.object(efi_utils, '_get_efi_bootloaders', autospec=True) @mock.patch.object(os, 'makedirs', autospec=True) def test_found_csv(self, mkdir_mock, mock_efi_bl, mock_is_md_device, - mock_utils_efi_part, mock_get_part_uuid, mock_execute, - mock_rescan): + mock_utils_efi_part, mock_get_part_path, + mock_get_part_uuid, mock_execute, mock_rescan): mock_utils_efi_part.return_value = {'number': '1'} - mock_get_part_uuid.return_value = self.fake_dev + mock_get_part_path.return_value = self.fake_efi_system_part mock_efi_bl.return_value = ['EFI/vendor/BOOTX64.CSV'] mock_is_md_device.return_value = False @@ -254,9 +259,9 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) @mock.patch.object(efi_utils, '_get_efi_bootloaders', autospec=True) @mock.patch.object(os, 'makedirs', autospec=True) def test_nvme_device(self, mkdir_mock, mock_efi_bl, mock_is_md_device, - mock_utils_efi_part, mock_get_part_uuid, - mock_execute, mock_rescan): - mock_utils_efi_part.return_value = {'number': '1'} + mock_utils_efi_part, mock_get_part_path, + mock_get_part_uuid, mock_execute, mock_rescan): + mock_utils_efi_part.return_value = None mock_get_part_uuid.return_value = '/dev/fakenvme0p1' mock_is_md_device.return_value = False @@ -289,10 +294,10 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) @mock.patch.object(efi_utils, '_get_efi_bootloaders', autospec=True) @mock.patch.object(os, 'makedirs', autospec=True) def test_wholedisk(self, mkdir_mock, mock_efi_bl, mock_is_md_device, - mock_utils_efi_part, mock_get_part_uuid, mock_execute, - mock_rescan): + mock_utils_efi_part, mock_get_part_path, + mock_get_part_uuid, mock_execute, mock_rescan): mock_utils_efi_part.return_value = {'number': '1'} - mock_get_part_uuid.side_effect = Exception + mock_get_part_path.return_value = self.fake_efi_system_part mock_is_md_device.return_value = False mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] @@ -318,6 +323,7 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) mkdir_mock.assert_called_once_with(self.fake_dir + '/boot/efi') mock_efi_bl.assert_called_once_with(self.fake_dir + '/boot/efi') mock_execute.assert_has_calls(expected) + mock_get_part_uuid.assert_not_called() @mock.patch.object(os.path, 'exists', lambda *_: False) @mock.patch.object(hardware, 'get_component_devices', autospec=True) @@ -331,10 +337,10 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) def test_software_raid(self, mkdir_mock, mock_efi_bl, mock_is_md_device, mock_get_holder_disks, mock_prepare, mock_get_component_devices, - mock_utils_efi_part, mock_get_part_uuid, - mock_execute, mock_rescan): + mock_utils_efi_part, mock_get_part_path, + mock_get_part_uuid, mock_execute, mock_rescan): mock_utils_efi_part.return_value = {'number': '1'} - mock_get_part_uuid.side_effect = Exception + mock_get_part_path.return_value = self.fake_efi_system_part mock_is_md_device.return_value = True mock_get_holder_disks.return_value = ['/dev/sda', '/dev/sdb'] mock_prepare.return_value = '/dev/md125' @@ -371,3 +377,122 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) mkdir_mock.assert_called_once_with(self.fake_dir + '/boot/efi') mock_efi_bl.assert_called_once_with(self.fake_dir + '/boot/efi') mock_execute.assert_has_calls(expected) + + @mock.patch.object(os.path, 'exists', lambda *_: False) + @mock.patch.object(hardware, 'is_md_device', autospec=True) + @mock.patch.object(efi_utils, '_get_efi_bootloaders', autospec=True) + @mock.patch.object(os, 'makedirs', autospec=True) + def test_failure(self, mkdir_mock, mock_efi_bl, mock_is_md_device, + mock_utils_efi_part, mock_get_part_path, + mock_get_part_uuid, mock_execute, mock_rescan): + mock_utils_efi_part.return_value = {'number': '1'} + mock_get_part_path.return_value = self.fake_efi_system_part + mock_is_md_device.return_value = False + + mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] + + mock_execute.side_effect = processutils.ProcessExecutionError('boom') + + self.assertRaisesRegex(errors.CommandExecutionError, 'boom', + efi_utils.manage_uefi, + self.fake_dev, self.fake_root_uuid) + mkdir_mock.assert_called_once_with(self.fake_dir + '/boot/efi') + mock_efi_bl.assert_not_called() + mock_execute.assert_called_once_with( + 'mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi') + mock_rescan.assert_called_once_with(self.fake_dev) + + @mock.patch.object(os.path, 'exists', lambda *_: False) + @mock.patch.object(hardware, 'is_md_device', autospec=True) + @mock.patch.object(efi_utils, '_get_efi_bootloaders', autospec=True) + @mock.patch.object(os, 'makedirs', autospec=True) + def test_failure_after_mount(self, mkdir_mock, mock_efi_bl, + mock_is_md_device, mock_utils_efi_part, + mock_get_part_path, mock_get_part_uuid, + mock_execute, mock_rescan): + mock_utils_efi_part.return_value = {'number': '1'} + mock_get_part_path.return_value = self.fake_efi_system_part + mock_is_md_device.return_value = False + + mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] + + mock_execute.side_effect = [ + ('', ''), + processutils.ProcessExecutionError('boom'), + ('', ''), + ('', ''), + ] + + expected = [mock.call('mount', self.fake_efi_system_part, + self.fake_dir + '/boot/efi'), + mock.call('efibootmgr', '-v'), + mock.call('umount', self.fake_dir + '/boot/efi', + attempts=3, delay_on_retry=True), + mock.call('sync')] + + self.assertRaisesRegex(errors.CommandExecutionError, 'boom', + efi_utils.manage_uefi, + self.fake_dev, self.fake_root_uuid) + mkdir_mock.assert_called_once_with(self.fake_dir + '/boot/efi') + mock_efi_bl.assert_called_once_with(self.fake_dir + '/boot/efi') + mock_execute.assert_has_calls(expected) + self.assertEqual(4, mock_execute.call_count) + mock_rescan.assert_called_once_with(self.fake_dev) + + @mock.patch.object(os.path, 'exists', lambda *_: False) + @mock.patch.object(hardware, 'is_md_device', autospec=True) + @mock.patch.object(efi_utils, '_get_efi_bootloaders', autospec=True) + @mock.patch.object(os, 'makedirs', autospec=True) + def test_failure_after_failure(self, mkdir_mock, mock_efi_bl, + mock_is_md_device, mock_utils_efi_part, + mock_get_part_path, mock_get_part_uuid, + mock_execute, mock_rescan): + mock_utils_efi_part.return_value = {'number': '1'} + mock_get_part_path.return_value = self.fake_efi_system_part + mock_is_md_device.return_value = False + + mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] + + mock_execute.side_effect = [ + ('', ''), + processutils.ProcessExecutionError('boom'), + processutils.ProcessExecutionError('no umount'), + ('', ''), + ] + + expected = [mock.call('mount', self.fake_efi_system_part, + self.fake_dir + '/boot/efi'), + mock.call('efibootmgr', '-v'), + mock.call('umount', self.fake_dir + '/boot/efi', + attempts=3, delay_on_retry=True)] + + self.assertRaisesRegex(errors.CommandExecutionError, 'boom', + efi_utils.manage_uefi, + self.fake_dev, self.fake_root_uuid) + mkdir_mock.assert_called_once_with(self.fake_dir + '/boot/efi') + mock_efi_bl.assert_called_once_with(self.fake_dir + '/boot/efi') + mock_execute.assert_has_calls(expected) + self.assertEqual(3, mock_execute.call_count) + mock_rescan.assert_called_once_with(self.fake_dev) + + +@mock.patch.object(partition_utils, 'get_partition', autospec=True) +@mock.patch.object(utils, 'execute', autospec=True) +class TestGetPartitionPathByNumber(base.IronicAgentTest): + + def test_ok(self, mock_execute, mock_get_partition): + mock_execute.return_value = (hardware_samples.SGDISK_INFO_TEMPLATE, '') + mock_get_partition.return_value = '/dev/fake1' + + result = efi_utils.get_partition_path_by_number('/dev/fake', 1) + self.assertEqual('/dev/fake1', result) + + mock_execute.assert_called_once_with('sgdisk', '-i', '1', '/dev/fake', + use_standard_locale=True) + + def test_broken(self, mock_execute, mock_get_partition): + mock_execute.return_value = ('I am a teaport', '') + + self.assertIsNone( + efi_utils.get_partition_path_by_number('/dev/fake', 1)) + mock_get_partition.assert_not_called() diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index 0db92354..af850471 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,16 +1539,66 @@ 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_udev.side_effect = iter([ + 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 = True + mocked_udev.side_effect = [ {'ID_WWN': 'wwn%d' % i, 'ID_SERIAL_SHORT': 'serial%d' % i, + 'ID_SERIAL': 'do not use me', '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(3) + ] + [ + {'DM_WWN': 'wwn3', 'DM_SERIAL': 'serial3'}, + ] mocked_dev_vendor.return_value = 'Super Vendor' devices = hardware.list_all_block_devices() expected_devices = [ @@ -1354,19 +1632,19 @@ class TestGenericHardwareManager(base.IronicAgentTest): wwn_vendor_extension='wwn-vendor-ext2', serial='serial2', hctl='1:0:0:0'), - hardware.BlockDevice(name='/dev/sdd', + hardware.BlockDevice(name='/dev/dm-0', model='NWD-BLP4-1600', size=1765517033472, rotational=False, vendor='Super Vendor', wwn='wwn3', - wwn_with_extension='wwn-ext3', - wwn_vendor_extension='wwn-vendor-ext3', + wwn_with_extension=None, + wwn_vendor_extension=None, serial='serial3', hctl='1:0:0:0') ] - self.assertEqual(4, len(expected_devices)) + self.assertEqual(4, len(devices)) for expected, device in zip(expected_devices, devices): # Compare all attrs of the objects for attr in ['name', 'model', 'size', 'rotational', @@ -1375,8 +1653,9 @@ 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')] 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 +4439,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 +4454,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 +4474,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 +4489,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 +4521,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 +4543,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 +4553,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 +4564,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 +4579,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 +4588,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 +4621,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 +4672,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 +4710,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 +4762,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_partition_utils.py b/ironic_python_agent/tests/unit/test_partition_utils.py index b4017c2b..000d98c4 100644 --- a/ironic_python_agent/tests/unit/test_partition_utils.py +++ b/ironic_python_agent/tests/unit/test_partition_utils.py @@ -656,6 +656,7 @@ class CreateConfigDriveTestCases(base.IronicAgentTest): mock_dd.assert_called_with(configdrive_file, configdrive_part) mock_unlink.assert_called_with(configdrive_file) + @mock.patch('oslo_utils.uuidutils.generate_uuid', lambda: 'fake-uuid') @mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(utils, 'unlink_without_raise', autospec=True) @@ -665,7 +666,7 @@ class CreateConfigDriveTestCases(base.IronicAgentTest): autospec=True) @mock.patch.object(disk_utils, 'get_partition_table_type', autospec=True) - @mock.patch.object(disk_utils, 'list_partitions', + @mock.patch.object(partition_utils, 'get_partition', autospec=True) @mock.patch.object(partition_utils, 'get_labelled_partition', autospec=True) @@ -673,42 +674,25 @@ class CreateConfigDriveTestCases(base.IronicAgentTest): autospec=True) def test_create_partition_gpt(self, mock_get_configdrive, mock_get_labelled_partition, - mock_list_partitions, mock_table_type, + mock_get_partition_by_uuid, + mock_table_type, mock_fix_gpt_partition, mock_dd, mock_unlink, mock_execute): config_url = 'http://1.2.3.4/cd' configdrive_file = '/tmp/xyz' configdrive_mb = 10 - initial_partitions = [{'end': 49152, 'number': 1, 'start': 1, - 'flags': 'boot', 'filesystem': 'ext4', - 'size': 49151}, - {'end': 51099, 'number': 3, 'start': 49153, - 'flags': '', 'filesystem': '', 'size': 2046}, - {'end': 51099, 'number': 5, 'start': 49153, - 'flags': '', 'filesystem': '', 'size': 2046}] - updated_partitions = [{'end': 49152, 'number': 1, 'start': 1, - 'flags': 'boot', 'filesystem': 'ext4', - 'size': 49151}, - {'end': 51099, 'number': 3, 'start': 49153, - 'flags': '', 'filesystem': '', 'size': 2046}, - {'end': 51099, 'number': 4, 'start': 49153, - 'flags': '', 'filesystem': '', 'size': 2046}, - {'end': 51099, 'number': 5, 'start': 49153, - 'flags': '', 'filesystem': '', 'size': 2046}] - mock_get_configdrive.return_value = (configdrive_mb, configdrive_file) mock_get_labelled_partition.return_value = None mock_table_type.return_value = 'gpt' - mock_list_partitions.side_effect = [initial_partitions, - updated_partitions] expected_part = '/dev/fake4' + mock_get_partition_by_uuid.return_value = expected_part partition_utils.create_config_drive_partition(self.node_uuid, self.dev, config_url) mock_execute.assert_has_calls([ - mock.call('sgdisk', '-n', '0:-64MB:0', self.dev, - run_as_root=True), + mock.call('sgdisk', '-n', '0:-64MB:0', '-u', '0:fake-uuid', + self.dev, run_as_root=True), mock.call('sync'), mock.call('udevadm', 'settle'), mock.call('partprobe', self.dev, attempts=10, run_as_root=True), @@ -719,7 +703,6 @@ class CreateConfigDriveTestCases(base.IronicAgentTest): delay_on_retry=True) ]) - self.assertEqual(2, mock_list_partitions.call_count) mock_table_type.assert_called_with(self.dev) mock_fix_gpt_partition.assert_called_with(self.dev, self.node_uuid) mock_dd.assert_called_with(configdrive_file, expected_part) diff --git a/ironic_python_agent/tests/unit/test_utils.py b/ironic_python_agent/tests/unit/test_utils.py index 1bb7c1fe..c99f7dca 100644 --- a/ironic_python_agent/tests/unit/test_utils.py +++ b/ironic_python_agent/tests/unit/test_utils.py @@ -428,11 +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={'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, - 'mount': mock.ANY, 'parted': 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) @@ -456,11 +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={'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, - 'mount': mock.ANY, 'parted': 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) @@ -479,11 +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={'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, - 'mount': mock.ANY, 'parted': 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) @@ -506,11 +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={'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, - 'mount': mock.ANY, 'parted': 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 ff0f8926..66f6819f 100644 --- a/ironic_python_agent/utils.py +++ b/ironic_python_agent/utils.py @@ -68,9 +68,11 @@ COLLECT_LOGS_COMMANDS = { 'ip_addr': ['ip', 'addr'], 'lshw': ['lshw', '-quiet', '-json'], 'lsblk': ['lsblk', '--all', '-o%s' % ','.join(LSBLK_COLUMNS)], + 'lsblk-full': ['lsblk', '--all', '--bytes', '--output-all', '--pairs'], 'mdstat': ['cat', '/proc/mdstat'], 'mount': ['mount'], 'parted': ['parted', '-l'], + 'multipath': ['multipath', '-ll'], } diff --git a/ironic_python_agent/version.py b/ironic_python_agent/version.py index 3c0bdb35..a7bf3f3f 100644 --- a/ironic_python_agent/version.py +++ b/ironic_python_agent/version.py @@ -13,11 +13,6 @@ # License for the specific language governing permissions and limitations # under the License. -try: - # For Python 3.8 and later - import importlib.metadata as importlib_metadata -except ImportError: - # For everyone else - import importlib_metadata +from importlib import metadata as importlib_metadata __version__ = importlib_metadata.version("ironic_python_agent") diff --git a/lower-constraints.txt b/lower-constraints.txt deleted file mode 100644 index 64365d59..00000000 --- a/lower-constraints.txt +++ /dev/null @@ -1,28 +0,0 @@ -Pint==0.5 -Werkzeug==1.0.1 -bandit==1.1.0 -coverage==4.0 -cryptography==2.3 -dogpile.cache==0.9.2 -eventlet==0.18.2 -importlib_metadata==1.7.0;python_version<'3.8' -ironic-lib==5.1.0 -kazoo==2.8.0 -netifaces==0.10.4 -openstacksdk==0.49.0 -oslo.concurrency==3.26.0 -oslo.config==5.2.0 -oslo.log==3.36.0 -oslo.serialization==2.18.0 -oslo.service==1.24.0 -oslo.utils==3.34.0 -oslotest==3.2.0 -pbr==2.0.0 -psutil==3.2.2 -pyudev==0.18 -requests==2.14.2 -stestr==1.0.0 -stevedore==1.20.0 -tenacity==6.2.0 -testtools==2.2.0 -tooz==2.7.2 diff --git a/releasenotes/notes/configdrive-partuuid-3259cfb7428c1483.yaml b/releasenotes/notes/configdrive-partuuid-3259cfb7428c1483.yaml new file mode 100644 index 00000000..95344273 --- /dev/null +++ b/releasenotes/notes/configdrive-partuuid-3259cfb7428c1483.yaml @@ -0,0 +1,17 @@ +--- +issues: + - | + Creating a configdrive partition on a devicemapper device (e.g. a multipath + storage device) with MBR partitioning may fail with the following error:: + + Command execution failed: Failed to create config drive on disk /dev/dm-0 + for node 168af30d-0fad-4d67-af99-b28b3238e977. Error: Unexpected error + while running command. + + Use GPT partitioning instead. +fixes: + - | + Fixes creating a configdrive partition on a devicemapper device (e.g. + a multipath storage device) with GPT partitioning. The newly created + partition is now detected by a pre-generated UUID rather than by comparing + partition numbers. diff --git a/releasenotes/notes/efi-partuuid-5fe933a462eeede1.yaml b/releasenotes/notes/efi-partuuid-5fe933a462eeede1.yaml new file mode 100644 index 00000000..f1a7924f --- /dev/null +++ b/releasenotes/notes/efi-partuuid-5fe933a462eeede1.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixes configuring UEFI boot when the EFI partition is located on a + devicemapper device. diff --git a/releasenotes/notes/lsblk-all-956c1df808a169bf.yaml b/releasenotes/notes/lsblk-all-956c1df808a169bf.yaml new file mode 100644 index 00000000..d237f834 --- /dev/null +++ b/releasenotes/notes/lsblk-all-956c1df808a169bf.yaml @@ -0,0 +1,5 @@ +--- +other: + - | + The ramdisk logs now contain an ``lsblk`` output with all pairs in the new + ``lsblk-full`` file. 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. diff --git a/releasenotes/notes/multipath-serial-615fc925984abbf7.yaml b/releasenotes/notes/multipath-serial-615fc925984abbf7.yaml new file mode 100644 index 00000000..4a13352b --- /dev/null +++ b/releasenotes/notes/multipath-serial-615fc925984abbf7.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - | + Fixes discovering WWN/serial numbers for devicemapper devices. diff --git a/releasenotes/source/index.rst b/releasenotes/source/index.rst index 4f9a0465..d3d198cb 100644 --- a/releasenotes/source/index.rst +++ b/releasenotes/source/index.rst @@ -6,6 +6,7 @@ Ironic Python Agent Release Notes :maxdepth: 1 unreleased + yoga xena wallaby victoria diff --git a/releasenotes/source/yoga.rst b/releasenotes/source/yoga.rst new file mode 100644 index 00000000..7cd5e908 --- /dev/null +++ b/releasenotes/source/yoga.rst @@ -0,0 +1,6 @@ +========================= +Yoga Series Release Notes +========================= + +.. release-notes:: + :branch: stable/yoga diff --git a/requirements.txt b/requirements.txt index 3badcd0e..55ec6650 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,12 +2,11 @@ # of appearance. Changing the order has an impact on the overall integration # process, which may cause wedges in the gate later. pbr!=2.1.0,>=2.0.0 # Apache-2.0 -importlib_metadata>=1.7.0;python_version<'3.8' # Apache-2.0 eventlet!=0.18.3,!=0.20.1,>=0.18.2 # MIT netifaces>=0.10.4 # MIT oslo.config>=5.2.0 # Apache-2.0 oslo.concurrency>=3.26.0 # Apache-2.0 -oslo.log>=3.36.0 # Apache-2.0 +oslo.log>=4.6.1 # Apache-2.0 oslo.serialization!=2.19.1,>=2.18.0 # Apache-2.0 oslo.service!=1.28.1,>=1.24.0 # Apache-2.0 oslo.utils>=3.34.0 # Apache-2.0 @@ -18,6 +17,6 @@ requests>=2.14.2 # Apache-2.0 stevedore>=1.20.0 # Apache-2.0 tenacity>=6.2.0 # Apache-2.0 ironic-lib>=5.1.0 # Apache-2.0 -Werkzeug>=1.0.1 # BSD License +Werkzeug>=2.0.0 # BSD License cryptography>=2.3 # BSD/Apache-2.0 tooz>=2.7.2 # Apache-2.0 @@ -7,7 +7,7 @@ author_email = openstack-discuss@lists.openstack.org home_page = https://docs.openstack.org/ironic-python-agent/ summary = Ironic Python Agent Ramdisk license = Apache-2 -python_requires = >=3.6 +python_requires = >=3.8 classifier = Environment :: OpenStack Intended Audience :: System Administrators @@ -18,8 +18,6 @@ classifier = Programming Language :: Python :: Implementation :: CPython Programming Language :: Python :: 3 :: Only Programming Language :: Python :: 3 - Programming Language :: Python :: 3.6 - Programming Language :: Python :: 3.7 Programming Language :: Python :: 3.8 Programming Language :: Python :: 3.9 @@ -137,9 +137,3 @@ commands = pip install -e {toxinidir}/examples/business-logic pip install -e {toxinidir}/examples/vendor-device python -c 'import example_business_logic; import example_device' - -[testenv:lower-constraints] -deps = - -c{toxinidir}/lower-constraints.txt - -r{toxinidir}/test-requirements.txt - -r{toxinidir}/requirements.txt diff --git a/zuul.d/ironic-python-agent-jobs.yaml b/zuul.d/ironic-python-agent-jobs.yaml index 4d3de185..05c8116b 100644 --- a/zuul.d/ironic-python-agent-jobs.yaml +++ b/zuul.d/ironic-python-agent-jobs.yaml @@ -137,10 +137,22 @@ # excluding it from the enabled list to save gate time. IRONIC_ENABLED_DEPLOY_INTERFACES: "direct" -# This job will superceed the above centos7 metalsmith integration job - job: name: metalsmith-integration-ipa-src-uefi - parent: metalsmith-integration-glance-localboot-centos8-uefi + parent: metalsmith-integration-glance-centos8-uefi + required-projects: + - openstack/ironic-python-agent + - openstack/ironic-python-agent-builder + - openstack/ironic-lib + vars: + devstack_localrc: + # Don't waste time on cleaning, it's checked everywhere else + IRONIC_AUTOMATED_CLEAN_ENABLED: False + IRONIC_BUILD_DEPLOY_RAMDISK: True + +- job: + name: metalsmith-integration-ipa-src-legacy + parent: metalsmith-integration-glance-centos8-legacy required-projects: - openstack/ironic-python-agent - openstack/ironic-python-agent-builder diff --git a/zuul.d/project.yaml b/zuul.d/project.yaml index c7448f4a..f3769f42 100644 --- a/zuul.d/project.yaml +++ b/zuul.d/project.yaml @@ -2,8 +2,7 @@ templates: - check-requirements - openstack-cover-jobs - - openstack-lower-constraints-master-branch-jobs - - openstack-python3-yoga-jobs + - openstack-python3-zed-jobs - publish-openstack-docs-pti - release-notes-jobs-python3 check: @@ -15,13 +14,12 @@ - ipa-tempest-bios-ipmi-direct-src - ipa-tempest-uefi-redfish-vmedia-src - metalsmith-integration-ipa-src-uefi + - metalsmith-integration-ipa-src-legacy - ironic-standalone-ipa-src # NOTE(dtantsur): non-voting because IPA source code is very unlikely # to break them. They rather serve as a canary for broken POST jobs. - ironic-python-agent-check-image-tinyipa: voting: false - - ironic-python-agent-check-image-dib-centos8: - voting: false - ironic-python-agent-check-image-dib-centos9: voting: false # Non-voting jobs @@ -33,11 +31,13 @@ queue: ironic jobs: - openstack-tox-functional + - ipa-tox-examples - ipa-tempest-bios-ipmi-direct-src - ipa-tempest-uefi-redfish-vmedia-src - metalsmith-integration-ipa-src-uefi + - metalsmith-integration-ipa-src-legacy - ironic-standalone-ipa-src post: jobs: - ironic-python-agent-build-image-tinyipa - - ironic-python-agent-build-image-dib-centos8 + - ironic-python-agent-build-image-dib-centos9 |