Catch ismount not being handled
While investigating another grub issue, I was confused by the path taken in the logs reported, and noticed that on a ramdisk, we might not actually have a valid response to os.path.ismount, I'm guessing depending on what in memory filesystem is in use while also coupled with attempting to check a filesystem. Adds a test to validate that exceptions raised on these commands where this issue can be encountered, are properly bypassed, and also adds additional logging to make it easier to figure out what is going on in the entire bootloader setup sequence. Change-Id: Ibd3060bef2e56468ada6b1a5c1cc1632a42803c3
This commit is contained in:
Julia Kreger
@@ -273,7 +273,7 @@ def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition,
|
||||
"""
|
||||
|
||||
# Before updating let's get information about the bootorder
|
||||
LOG.debug("Getting information about boot order")
|
||||
LOG.debug("Getting information about boot order.")
|
||||
utils.execute('efibootmgr')
|
||||
# NOTE(iurygregory): regex used to identify the Warning in the stderr after
|
||||
# we add the new entry. Example:
|
||||
@@ -283,6 +283,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:
|
||||
@@ -328,7 +330,7 @@ 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.
|
||||
_rescan_device(device)
|
||||
@@ -381,6 +383,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:
|
||||
@@ -551,6 +554,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."""
|
||||
@@ -638,11 +664,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')):
|
||||
@@ -662,9 +686,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 '
|
||||
|
||||
@@ -1154,6 +1154,127 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
|
||||
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(os, 'listdir', autospec=True)
|
||||
@mock.patch.object(shutil, 'copy2', autospec=True)
|
||||
|
||||
Reference in New Issue
Block a user