diff options
author | Zuul <zuul@review.opendev.org> | 2022-08-09 05:35:25 +0000 |
---|---|---|
committer | Gerrit Code Review <review@openstack.org> | 2022-08-09 05:35:25 +0000 |
commit | 59d8969dc4a447390cd2268babdfcabcae3a2724 (patch) | |
tree | 3a98798a83294e704b9a3e6f154bf273a99a49d6 | |
parent | c3081d74414d20f0e41efa9f53c291eb9f039d0b (diff) | |
parent | 8fafc57e7ca81678e5509e99b504967fc2e62870 (diff) | |
download | ironic-python-agent-59d8969dc4a447390cd2268babdfcabcae3a2724.tar.gz |
Merge "Catch ismount not being handled" into stable/wallaby
-rw-r--r-- | ironic_python_agent/extensions/image.py | 38 | ||||
-rw-r--r-- | ironic_python_agent/tests/unit/extensions/test_image.py | 121 |
2 files changed, 152 insertions, 7 deletions
diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py index 5224cb77..f5a0142d 100644 --- a/ironic_python_agent/extensions/image.py +++ b/ironic_python_agent/extensions/image.py @@ -282,6 +282,8 @@ def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition, label_id = 1 for v_bl in valid_efi_bootloaders: if 'csv' in v_bl.lower(): + LOG.debug('A CSV file has been identified as a bootloader hint. ' + 'File: %s', v_bl) # These files are always UTF-16 encoded, sometimes have a header. # Positive bonus is python silently drops the FEFF header. with open(mount_point + '/' + v_bl, 'r', encoding='utf-16') as csv: @@ -366,6 +368,7 @@ def _manage_uefi(device, efi_system_part_uuid=None): efi_mounted = False efi_partition = None + LOG.debug('Attempting UEFI loader autodetection and NVRAM record setup.') try: # Force UEFI to rescan the device. Required if the deployment # was over iscsi. @@ -421,6 +424,7 @@ def _manage_uefi(device, efi_system_part_uuid=None): else: # NOTE(dtantsur): if we have an empty EFI partition, try to use # grub-install to populate it. + LOG.warning('Empty EFI partition detected.') return False except processutils.ProcessExecutionError as e: @@ -591,6 +595,29 @@ def _umount_all_partitions(path, path_variable, umount_warn_msg): return umount_binds_success +def _mount_partition(partition, path): + if not os.path.ismount(path): + LOG.debug('Attempting to mount %(device)s to %(path)s to ' + 'partition.', + {'device': partition, + 'path': path}) + try: + utils.execute('mount', partition, path) + except processutils.ProcessExecutionError as e: + # NOTE(TheJulia): It seems in some cases, + # the python os.path.ismount can return False + # even *if* it is actually mounted. This appears + # to be becasue it tries to rely on inode on device + # logic, yet the rules are sometimes different inside + # ramdisks. So lets check the error first. + if 'already mounted' not in e: + # Raise the error, since this is not a known + # failure case + raise + else: + LOG.debug('Partition already mounted, proceeding.') + + def _install_grub2(device, root_uuid, efi_system_part_uuid=None, prep_boot_part_uuid=None, target_boot_mode='bios'): """Install GRUB2 bootloader on a given device.""" @@ -678,11 +705,9 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None, # remounted. LOG.debug('No EFI assets were preserved for setup or the ' 'ramdisk was unable to complete the setup. ' - 'falling back to bootloader installation from' + 'falling back to bootloader installation from ' 'deployed image.') - if not os.path.ismount(path): - LOG.debug('Re-mounting the root partition.') - utils.execute('mount', root_partition, path) + _mount_partition(root_partition, path) binary_name = "grub" if os.path.exists(os.path.join(path, 'usr/sbin/grub2-install')): @@ -702,9 +727,8 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None, LOG.warning("GRUB2 will be installed for UEFI on efi partition " "%s using the install command which does not place " "Secure Boot signed binaries.", efi_partition) - if not os.path.ismount(efi_partition_mount_point): - utils.execute('mount', efi_partition, - efi_partition_mount_point) + + _mount_partition(efi_partition, efi_partition_mount_point) efi_mounted = True try: utils.execute('chroot %(path)s /bin/sh -c ' diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py index 64798567..64681b8d 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -1314,6 +1314,127 @@ 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(os.path, 'ismount', lambda *_: False) + @mock.patch.object(image, '_is_bootloader_loaded', lambda *_: False) + @mock.patch.object(image, '_append_uefi_to_fstab', autospec=True) + @mock.patch.object(image, '_preserve_efi_assets', autospec=True) + @mock.patch.object(image, '_efi_boot_setup', autospec=True) + @mock.patch.object(os.path, 'exists', autospec=True) + @mock.patch.object(hardware, 'is_md_device', autospec=True) + @mock.patch.object(hardware, 'md_get_raid_devices', autospec=True) + @mock.patch.object(os, 'environ', autospec=True) + @mock.patch.object(os, 'makedirs', autospec=True) + @mock.patch.object(image, '_get_partition', autospec=True) + def test__install_grub2_uefi_partition_image_with_preserve_failure2( + self, mock_get_part_uuid, mkdir_mock, + environ_mock, mock_md_get_raid_devices, + mock_is_md_device, mock_exists, + mock_efi_setup, + mock_preserve_efi_assets, + mock_append_to_fstab, + mock_execute, mock_dispatch): + mock_exists.return_value = True + mock_efi_setup.side_effect = Exception('meow') + mock_get_part_uuid.side_effect = [self.fake_root_part, + self.fake_efi_system_part] + environ_mock.get.return_value = '/sbin' + mock_is_md_device.return_value = False + mock_md_get_raid_devices.return_value = {} + mock_preserve_efi_assets.return_value = None + exec_results = [('', '')] * 21 + already_exists = processutils.ProcessExecutionError( + '/dev is already mounted at /path') + # Mark mounts as already mounted, which is where os.path.ismount + # usage corresponds. + exec_results[6] = already_exists + exec_results[8] = already_exists + + image._install_grub2( + self.fake_dev, root_uuid=self.fake_root_uuid, + efi_system_part_uuid=self.fake_efi_system_part_uuid, + target_boot_mode='uefi') + self.assertFalse(mock_efi_setup.called) + + expected = [mock.call('mount', '/dev/fake2', self.fake_dir), + mock.call('mount', '-o', 'bind', '/dev', + self.fake_dir + '/dev'), + mock.call('mount', '-o', 'bind', '/proc', + self.fake_dir + '/proc'), + mock.call('mount', '-o', 'bind', '/run', + self.fake_dir + '/run'), + mock.call('mount', '-t', 'sysfs', 'none', + self.fake_dir + '/sys'), + mock.call(('chroot %s /bin/sh -c ' + '"grub2-mkconfig -o ' + '/boot/grub2/grub.cfg"' % self.fake_dir), + shell=True, + env_variables={ + 'PATH': '/sbin:/bin:/usr/sbin:/sbin', + 'GRUB_DISABLE_OS_PROBER': 'true', + 'GRUB_SAVEDEFAULT': 'true'}, + use_standard_locale=True), + mock.call('mount', '/dev/fake2', self.fake_dir), + mock.call(('chroot %s /bin/sh -c "mount -a -t vfat"' % + (self.fake_dir)), shell=True, + env_variables={ + 'PATH': '/sbin:/bin:/usr/sbin:/sbin'}), + mock.call('mount', self.fake_efi_system_part, + self.fake_dir + '/boot/efi'), + mock.call(('chroot %s /bin/sh -c "grub2-install"' % + self.fake_dir), shell=True, + env_variables={ + 'PATH': '/sbin:/bin:/usr/sbin:/sbin'}), + mock.call(('chroot %s /bin/sh -c ' + '"grub2-install --removable"' % + self.fake_dir), shell=True, + env_variables={ + 'PATH': '/sbin:/bin:/usr/sbin:/sbin'}), + mock.call( + 'umount', self.fake_dir + '/boot/efi', + attempts=3, delay_on_retry=True), + mock.call('mount', self.fake_efi_system_part, + '/tmp/fake-dir/boot/efi'), + mock.call(('chroot %s /bin/sh -c ' + '"grub2-mkconfig -o ' + '/boot/grub2/grub.cfg"' % self.fake_dir), + shell=True, + env_variables={ + 'PATH': '/sbin:/bin:/usr/sbin:/sbin', + 'GRUB_DISABLE_OS_PROBER': 'true', + 'GRUB_SAVEDEFAULT': 'true'}, + use_standard_locale=True), + 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"' % + (self.fake_dir)), shell=True, + env_variables={ + 'PATH': '/sbin:/bin:/usr/sbin:/sbin'}), + mock.call('umount', self.fake_dir + '/dev', + attempts=3, delay_on_retry=True), + mock.call('umount', self.fake_dir + '/proc', + attempts=3, delay_on_retry=True), + mock.call('umount', self.fake_dir + '/run', + attempts=3, delay_on_retry=True), + mock.call('umount', self.fake_dir + '/sys', + attempts=3, delay_on_retry=True), + mock.call('umount', self.fake_dir, attempts=3, + delay_on_retry=True)] + + mkdir_mock.assert_not_called() + mock_execute.assert_has_calls(expected) + mock_get_part_uuid.assert_any_call(self.fake_dev, + uuid=self.fake_root_uuid) + mock_get_part_uuid.assert_any_call(self.fake_dev, + uuid=self.fake_efi_system_part_uuid) + self.assertFalse(mock_dispatch.called) + mock_preserve_efi_assets.assert_called_with( + self.fake_dir, + self.fake_dir + '/boot/efi/EFI', + '/dev/fake1', + self.fake_dir + '/boot/efi') + mock_append_to_fstab.assert_called_with(self.fake_dir, + 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) |