diff options
| -rw-r--r-- | ironic_python_agent/extensions/image.py | 115 | ||||
| -rw-r--r-- | ironic_python_agent/hardware.py | 4 | ||||
| -rw-r--r-- | ironic_python_agent/tests/unit/extensions/test_image.py | 124 | ||||
| -rw-r--r-- | ironic_python_agent/tests/unit/samples/hardware_samples.py | 18 | ||||
| -rw-r--r-- | ironic_python_agent/tests/unit/test_hardware.py | 26 | ||||
| -rw-r--r-- | releasenotes/notes/efi-partuuid-5fe933a462eeede1.yaml | 5 | ||||
| -rw-r--r-- | releasenotes/notes/handle-partuuid-for-fstab-e0aadea20a056982.yaml | 10 |
7 files changed, 246 insertions, 56 deletions
diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py index 1f587dcd..d90edeaa 100644 --- a/ironic_python_agent/extensions/image.py +++ b/ironic_python_agent/extensions/image.py @@ -312,12 +312,43 @@ def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition, # Update the nvram using efibootmgr # 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', v_efi_bl_path) # Increment the ID in case the loop runs again. label_id += 1 +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 _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. @@ -333,6 +364,7 @@ def _manage_uefi(device, efi_system_part_uuid=None): """ efi_partition_mount_point = None efi_mounted = False + efi_partition = None try: # Force UEFI to rescan the device. Required if the deployment @@ -341,17 +373,27 @@ def _manage_uefi(device, efi_system_part_uuid=None): local_path = tempfile.mkdtemp() # Trust the contents on the disk in the event of a whole disk image. - efi_partition = utils.get_efi_part_on_device(device) + efi_part_num = utils.get_efi_part_on_device(device) + if efi_part_num is not None: + 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 - partition = _get_partition(device, uuid=efi_system_part_uuid) + efi_partition = _get_partition(device, uuid=efi_system_part_uuid) try: - efi_partition = int(partition.replace(device, "")) + 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 - efi_partition = int(partition.replace(device + 'p', "")) + try: + 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" % (device, exc)) if not efi_partition: # NOTE(dtantsur): we cannot have a valid EFI deployment without an @@ -368,20 +410,12 @@ def _manage_uefi(device, efi_system_part_uuid=None): 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) + utils.execute('mount', efi_partition, efi_partition_mount_point) efi_mounted = True valid_efi_bootloaders = _get_efi_bootloaders(efi_partition_mount_point) if valid_efi_bootloaders: - _run_efibootmgr(valid_efi_bootloaders, device, efi_partition, + _run_efibootmgr(valid_efi_bootloaders, device, efi_part_num, efi_partition_mount_point) return True else: @@ -891,17 +925,58 @@ def _try_preserve_efi_assets(device, path, def _append_uefi_to_fstab(fs_path, efi_system_part_uuid): """Append the efi partition id to the filesystem table. - :param fs_path: - :param efi_system_part_uuid: + :param fs_path: The path to the filesystem. + :param efi_system_part_uuid: uuid to use to try and find the + partition. Warning: this may be + a partition uuid or a actual uuid. """ fstab_file = os.path.join(fs_path, 'etc/fstab') if not os.path.exists(fstab_file): return try: - fstab_string = ("UUID=%s\t/boot/efi\tvfat\tumask=0077\t" - "0\t1\n") % efi_system_part_uuid + # Collect all of the block devices so we appropriately match UUID + # or PARTUUID into an fstab entry. + block_devs = hardware.list_all_block_devices(block_type='part') + + # Default to uuid, but if we find a partuuid instead, that is okay, + # we just need to know later on. + fstab_label = None + for bdev in block_devs: + # Check UUID first + if bdev.uuid and efi_system_part_uuid in bdev.uuid: + LOG.debug('Identified block device %(dev)s UUID %(uuid)s ' + 'for UEFI boot. Proceeding with fstab update using ' + 'a UUID.', + {'dev': bdev.name, + 'uuid': efi_system_part_uuid}) + # What we have works, and is correct, we can break the loop + fstab_label = 'UUID' + break + # Fallback to PARTUUID, since we don't know if the provided + # UUID matches a PARTUUID, or UUID field, and the fstab entry + # needs to match it. + if bdev.partuuid and efi_system_part_uuid in bdev.partuuid: + LOG.debug('Identified block device %(dev)s partition UUID ' + '%(uuid)s for UEFI boot. Proceeding with fstab ' + 'update using a PARTUUID.', + {'dev': bdev.name, + 'uuid': efi_system_part_uuid}) + fstab_label = 'PARTUUID' + break + + if not fstab_label: + # Fallback to prior behavior, which should generally be correct. + LOG.warning('Falling back to fstab entry addition label of UUID. ' + 'We could not identify which UUID or PARTUUID ' + 'identifier label should be used, thus UUID will be ' + 'used.') + fstab_label = 'UUID' + + fstab_string = ("%s=%s\t/boot/efi\tvfat\tumask=0077\t" + "0\t1\n") % (fstab_label, efi_system_part_uuid) with open(fstab_file, "r+") as fstab: - if efi_system_part_uuid not in fstab.read(): + already_present_string = fstab_label + '=' + efi_system_part_uuid + if already_present_string not in fstab.read(): fstab.writelines(fstab_string) except (OSError, EnvironmentError, IOError) as exc: LOG.debug('Failed to add entry to /etc/fstab. Error %s', exc) diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index f2156f84..c0a8e85a 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -530,6 +530,7 @@ def list_all_block_devices(block_type='disk', 'block', 'vendor'), by_path=by_path_name, uuid=device['UUID'], + partuuid=device['PARTUUID'], **extra)) return devices @@ -597,7 +598,7 @@ class BlockDevice(encoding.SerializableComparable): def __init__(self, name, model, size, rotational, wwn=None, serial=None, vendor=None, wwn_with_extension=None, wwn_vendor_extension=None, hctl=None, by_path=None, - uuid=None): + uuid=None, partuuid=None): self.name = name self.model = model self.size = size @@ -610,6 +611,7 @@ class BlockDevice(encoding.SerializableComparable): self.wwn_vendor_extension = wwn_vendor_extension self.hctl = hctl self.by_path = by_path + self.partuuid = partuuid class NetworkInterface(encoding.SerializableComparable): diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py index 3c5c69a6..6778527c 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -26,6 +26,7 @@ from ironic_python_agent.extensions import image from ironic_python_agent.extensions import iscsi from ironic_python_agent import hardware from ironic_python_agent.tests.unit import base +from ironic_python_agent.tests.unit.samples import hardware_samples from ironic_python_agent import utils @@ -239,14 +240,14 @@ class TestImageExtension(base.IronicAgentTest): @mock.patch.object(utils, 'get_efi_part_on_device', 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_iscsi_clean, 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 = '1' + mock_utils_efi_part.return_value = None mock_execute.side_effect = iter([('', ''), ('', ''), ('', ''), ('', ''), @@ -284,16 +285,16 @@ class TestImageExtension(base.IronicAgentTest): @mock.patch.object(os.path, 'exists', lambda *_: False) @mock.patch.object(iscsi, 'clean_up', autospec=True) @mock.patch.object(image, '_get_efi_bootloaders', autospec=True) - @mock.patch.object(image, '_get_partition', autospec=True) + @mock.patch.object(image, 'get_partition_path_by_number', autospec=True) @mock.patch.object(utils, 'get_efi_part_on_device', 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_iscsi_clean, 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 = '1' mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] mock_execute.side_effect = iter([('', ''), ('', ''), @@ -332,16 +333,16 @@ class TestImageExtension(base.IronicAgentTest): @mock.patch.object(os.path, 'exists', lambda *_: False) @mock.patch.object(iscsi, 'clean_up', autospec=True) @mock.patch.object(image, '_get_efi_bootloaders', autospec=True) - @mock.patch.object(image, '_get_partition', autospec=True) + @mock.patch.object(image, 'get_partition_path_by_number', autospec=True) @mock.patch.object(utils, 'get_efi_part_on_device', 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_iscsi_clean, 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 = '1' mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] stdout_msg = """ @@ -390,16 +391,16 @@ Boot0002 VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) @mock.patch.object(os.path, 'exists', lambda *_: False) @mock.patch.object(iscsi, 'clean_up', autospec=True) @mock.patch.object(image, '_get_efi_bootloaders', autospec=True) - @mock.patch.object(image, '_get_partition', autospec=True) + @mock.patch.object(image, 'get_partition_path_by_number', autospec=True) @mock.patch.object(utils, 'get_efi_part_on_device', 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_iscsi_clean, 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 = '1' mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] # NOTE(TheJulia): This test string was derived from a lenovo SR650 @@ -453,16 +454,16 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 @mock.patch.object(os.path, 'exists', lambda *_: False) @mock.patch.object(iscsi, 'clean_up', autospec=True) @mock.patch.object(image, '_get_efi_bootloaders', autospec=True) - @mock.patch.object(image, '_get_partition', autospec=True) + @mock.patch.object(image, 'get_partition_path_by_number', autospec=True) @mock.patch.object(utils, 'get_efi_part_on_device', 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_iscsi_clean, 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 = '1' mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI', 'WINDOWS/system32/winload.efi'] @@ -804,6 +805,18 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 mock_is_md_device.return_value = False mock_md_get_raid_devices.return_value = {} mock_exists.side_effect = iter([False, True, False, True, True]) + partuuid_device = ('KNAME="sda" MODEL="DRIVE 0" SIZE="10240000" ' + 'ROTA="1" TYPE="disk" UUID="987654-3210" ' + 'PARTUUID=""\n' + 'KNAME="sda0" MODEL="DRIVE 0" SIZE="102400" ' + 'ROTA="1" TYPE="part" ' + 'UUID="' + self.fake_efi_system_part_uuid + '" ' + 'PARTUUID="1234-2918"\n') + exec_side_effect = [('', '')] * 16 + exec_side_effect.append((partuuid_device, '')) + exec_side_effect.extend([('', '')] * 8) + mock_execute.side_effect = exec_side_effect + with mock.patch('builtins.open', mock.mock_open()) as mock_open: image._install_grub2( self.fake_dev, root_uuid=self.fake_root_uuid, @@ -858,6 +871,9 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 'GRUB_DISABLE_OS_PROBER': 'true', 'GRUB_SAVEDEFAULT': 'true'}, use_standard_locale=True), + mock.call('udevadm', 'settle'), + mock.call('lsblk', '-Pbia', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call(('chroot %s /bin/sh -c "umount -a -t vfat"' % @@ -902,8 +918,21 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 environ_mock.get.return_value = '/sbin' mock_is_md_device.return_value = False mock_md_get_raid_devices.return_value = {} + partuuid_device = ('KNAME="sda" MODEL="DRIVE 0" SIZE="10240000" ' + 'ROTA="1" TYPE="disk" UUID="987654-3210" ' + 'PARTUUID=""\n' + 'KNAME="sda0" MODEL="DRIVE 0" SIZE="102400" ' + 'ROTA="1" TYPE="part" UUID="987654-3210" ' + 'PARTUUID="' + self.fake_efi_system_part_uuid + + '"\n') + exec_side_effect = [('', '')] * 16 + exec_side_effect.append((partuuid_device, '')) + exec_side_effect.extend([('', '')] * 8) + mock_execute.side_effect = exec_side_effect + # Validates the complete opposite path *and* no-write behavior + # occurs if the entry already exists. fstab_data = ( - 'UUID=%s\tpath vfat option' % self.fake_efi_system_part_uuid) + 'PARTUUID=%s\tpath vfat option' % self.fake_efi_system_part_uuid) mock_exists.side_effect = [True, False, True, True, True, False, True, True] with mock.patch('builtins.open', @@ -969,6 +998,9 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 'GRUB_DISABLE_OS_PROBER': 'true', 'GRUB_SAVEDEFAULT': 'true'}, use_standard_locale=True), + mock.call('udevadm', 'settle'), + mock.call('lsblk', '-Pbia', + '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID'), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call(('chroot %s /bin/sh -c "umount -a -t vfat"' % @@ -1079,6 +1111,7 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 @mock.patch.object(os, 'listdir', lambda *_: ['file1', 'file2']) @mock.patch.object(image, '_is_bootloader_loaded', lambda *_: False) + @mock.patch.object(image, '_append_uefi_to_fstab', autospec=True) @mock.patch.object(shutil, 'copy2', autospec=True) @mock.patch.object(os.path, 'isfile', autospec=True) @mock.patch.object(image, '_efi_boot_setup', autospec=True) @@ -1095,7 +1128,8 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 mock_is_md_device, mock_exists, mock_copytree, mock_efi_setup, mock_isfile, mock_copy2, - mock_execute, mock_dispatch): + mock_fstab_append, mock_execute, + mock_dispatch): mock_exists.return_value = True mock_efi_setup.return_value = True mock_get_part_uuid.side_effect = [self.fake_root_part, @@ -1159,6 +1193,9 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 uuid=self.fake_root_uuid) mock_get_part_uuid.assert_any_call(self.fake_dev, uuid=self.fake_efi_system_part_uuid) + mock_fstab_append.assert_called_once_with( + self.fake_dir, + self.fake_efi_system_part_uuid) self.assertFalse(mock_dispatch.called) @mock.patch.object(os.path, 'ismount', lambda *_: False) @@ -1276,6 +1313,7 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 self.fake_efi_system_part_uuid) @mock.patch.object(image, '_is_bootloader_loaded', lambda *_: False) + @mock.patch.object(image, '_append_uefi_to_fstab', autospec=True) @mock.patch.object(os, 'listdir', autospec=True) @mock.patch.object(shutil, 'copy2', autospec=True) @mock.patch.object(os.path, 'isfile', autospec=True) @@ -1293,8 +1331,8 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 mock_is_md_device, mock_exists, mock_copytree, mock_efi_setup, mock_isfile, mock_copy2, - mock_oslistdir, mock_execute, - mock_dispatch): + mock_oslistdir, mock_append_to_fstab, + mock_execute, mock_dispatch): mock_exists.return_value = True mock_efi_setup.return_value = True mock_get_part_uuid.side_effect = [self.fake_root_part, @@ -1363,6 +1401,8 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 uuid=self.fake_efi_system_part_uuid) self.assertFalse(mock_dispatch.called) self.assertEqual(2, mock_oslistdir.call_count) + mock_append_to_fstab.assert_called_with(self.fake_dir, + self.fake_efi_system_part_uuid) @mock.patch.object(os.path, 'ismount', lambda *_: False) @mock.patch.object(image, '_is_bootloader_loaded', lambda *_: False) @@ -2187,15 +2227,14 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 @mock.patch.object(os.path, 'exists', lambda *_: False) @mock.patch.object(image, '_get_efi_bootloaders', autospec=True) - @mock.patch.object(image, '_get_partition', autospec=True) + @mock.patch.object(image, 'get_partition_path_by_number', autospec=True) @mock.patch.object(utils, 'get_efi_part_on_device', autospec=True) @mock.patch.object(os, 'makedirs', autospec=True) def test__manage_uefi(self, mkdir_mock, mock_utils_efi_part, - mock_get_part_uuid, mock_efi_bl, mock_execute, + mock_get_part_path, mock_efi_bl, mock_execute, mock_dispatch): mock_utils_efi_part.return_value = '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/BOOT/BOOTX64.EFI'] mock_execute.side_effect = iter([('', ''), ('', ''), @@ -2226,14 +2265,14 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 @mock.patch.object(os.path, 'exists', lambda *_: False) @mock.patch.object(image, '_get_efi_bootloaders', autospec=True) - @mock.patch.object(image, '_get_partition', autospec=True) + @mock.patch.object(image, 'get_partition_path_by_number', autospec=True) @mock.patch.object(utils, 'get_efi_part_on_device', autospec=True) @mock.patch.object(os, 'makedirs', autospec=True) def test__manage_uefi_found_csv(self, mkdir_mock, mock_utils_efi_part, - mock_get_part_uuid, mock_efi_bl, + mock_get_part_path, mock_efi_bl, mock_execute, mock_dispatch): mock_utils_efi_part.return_value = '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'] # Format is <file>,<entry_name>,<options>,humanfriendlytextnotused @@ -2284,15 +2323,14 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) @mock.patch.object(os.path, 'exists', lambda *_: False) @mock.patch.object(image, '_get_efi_bootloaders', autospec=True) - @mock.patch.object(image, '_get_partition', autospec=True) + @mock.patch.object(image, 'get_partition_path_by_number', autospec=True) @mock.patch.object(utils, 'get_efi_part_on_device', autospec=True) @mock.patch.object(os, 'makedirs', autospec=True) def test__manage_uefi_nvme_device(self, mkdir_mock, mock_utils_efi_part, - mock_get_part_uuid, mock_efi_bl, + mock_get_part_path, mock_efi_bl, mock_execute, mock_dispatch): mock_utils_efi_part.return_value = '1' - mock_get_part_uuid.return_value = '/dev/fakenvme0p1' - + mock_get_part_path.return_value = '/dev/fakenvme0p1' mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] mock_execute.side_effect = iter([('', ''), ('', ''), @@ -2323,16 +2361,14 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) @mock.patch.object(os.path, 'exists', lambda *_: False) @mock.patch.object(image, '_get_efi_bootloaders', autospec=True) - @mock.patch.object(image, '_get_partition', autospec=True) + @mock.patch.object(image, 'get_partition_path_by_number', autospec=True) @mock.patch.object(utils, 'get_efi_part_on_device', autospec=True) @mock.patch.object(os, 'makedirs', autospec=True) def test__manage_uefi_wholedisk( self, mkdir_mock, mock_utils_efi_part, - mock_get_part_uuid, mock_efi_bl, mock_execute, - mock_dispatch): + mock_get_part_path, mock_efi_bl, mock_execute, mock_dispatch): mock_utils_efi_part.return_value = '1' - mock_get_part_uuid.side_effect = Exception - + mock_get_part_path.return_value = self.fake_efi_system_part mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] mock_execute.side_effect = iter([('', ''), ('', ''), @@ -2468,3 +2504,21 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) mock_open.side_effect = OSError('boom') image._append_uefi_to_fstab( self.fake_dir, 'abcd-efgh') + + +@mock.patch.object(image, '_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 = image.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( + image.get_partition_path_by_number('/dev/fake', 1)) + mock_get_partition.assert_not_called() diff --git a/ironic_python_agent/tests/unit/samples/hardware_samples.py b/ironic_python_agent/tests/unit/samples/hardware_samples.py index 3ab54153..660c61b5 100644 --- a/ironic_python_agent/tests/unit/samples/hardware_samples.py +++ b/ironic_python_agent/tests/unit/samples/hardware_samples.py @@ -160,6 +160,13 @@ RAID_BLK_DEVICE_TEMPLATE = ( 'PARTUUID=""' ) +PARTUUID_DEVICE_TEMPLATE = ( + 'KNAME="sda" MODEL="DRIVE 0" SIZE="1765517033472" ' + 'ROTA="1" TYPE="disk" UUID="" PARTUUID=""\n' + 'KNAME="sda1" MODEL="DRIVE 0" SIZE="107373133824" ' + 'ROTA="1" TYPE="part" UUID="987654-3210" PARTUUID="1234-5678"\n' +) + SHRED_OUTPUT_0_ITERATIONS_ZERO_FALSE = () SHRED_OUTPUT_1_ITERATION_ZERO_TRUE = ( @@ -1493,3 +1500,14 @@ 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 diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index 573d28d5..01825d05 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -66,6 +66,13 @@ RAID_BLK_DEVICE_TEMPLATE_DEVICES = [ vendor="FooTastic", uuid=""), ] +BLK_DEVICE_TEMPLATE_PARTUUID_DEVICE = [ + hardware.BlockDevice(name='/dev/sda1', model='DRIVE 0', + size=107373133824, rotational=True, + vendor="FooTastic", uuid="987654-3210", + partuuid="1234-5678"), +] + class FakeHardwareManager(hardware.GenericHardwareManager): def __init__(self, hardware_support): @@ -4143,6 +4150,25 @@ class TestModuleFunctions(base.IronicAgentTest): self.assertEqual(RAID_BLK_DEVICE_TEMPLATE_DEVICES, result) mocked_udev.assert_called_once_with() + @mock.patch.object(os, 'readlink', autospec=True) + @mock.patch.object(hardware, '_get_device_info', + lambda x, y, z: 'FooTastic') + @mock.patch.object(hardware, '_udev_settle', autospec=True) + @mock.patch.object(hardware.pyudev.Devices, "from_device_file", + autospec=False) + def test_list_all_block_devices_partuuid_success( + self, mocked_fromdevfile, + mocked_udev, mocked_readlink, + mocked_execute): + mocked_readlink.return_value = '../../sda' + mocked_fromdevfile.return_value = {} + mocked_execute.return_value = (hws.PARTUUID_DEVICE_TEMPLATE, '') + result = hardware.list_all_block_devices(block_type='part') + mocked_execute.assert_called_once_with( + 'lsblk', '-Pbia', '-oKNAME,MODEL,SIZE,ROTA,TYPE,UUID,PARTUUID') + self.assertEqual(BLK_DEVICE_TEMPLATE_PARTUUID_DEVICE, result) + mocked_udev.assert_called_once_with() + @mock.patch.object(hardware, '_get_device_info', lambda x, y: "FooTastic") @mock.patch.object(hardware, '_udev_settle', autospec=True) 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/handle-partuuid-for-fstab-e0aadea20a056982.yaml b/releasenotes/notes/handle-partuuid-for-fstab-e0aadea20a056982.yaml new file mode 100644 index 00000000..f9b6bbc5 --- /dev/null +++ b/releasenotes/notes/handle-partuuid-for-fstab-e0aadea20a056982.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + Fixes handling of a Partition UUID being returned instead of a + Partition's UUID when the OS may not return the Partition's UUID in time. + These two fields are typically referred to as PARTUUID and UUID, + respectively. Often these sorts of issues arise under heavy IO load. + We now scan, and identify which "UUID" we identified, and update + a Linux fstab entry appropriately. For more information, please see + `story #2009881 <https://storyboard.openstack.org/#!/story/2009881>`_. |