diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py index 9fd019277..af83e5c07 100644 --- a/ironic_python_agent/extensions/image.py +++ b/ironic_python_agent/extensions/image.py @@ -576,6 +576,7 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None, device, path, efi_system_part_uuid, efi_partitions, efi_partition_mount_point) if efi_preserved: + _append_uefi_to_fstab(path, efi_system_part_uuid) # Success preserving efi assets return else: @@ -678,6 +679,9 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None, # recommended path. _configure_grub(device, path) + if efi_mounted: + _append_uefi_to_fstab(path, efi_system_part_uuid) + LOG.info("GRUB2 successfully installed on %s", device) except processutils.ProcessExecutionError as e: @@ -825,6 +829,27 @@ def _try_preserve_efi_assets(device, path, 'filesystem. Error: %s', e) +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: + """ + 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 + with open(fstab_file, "r+") as fstab: + if efi_system_part_uuid 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) + LOG.debug('Added entry to /etc/fstab for EFI partition auto-mount ' + 'with uuid %s', efi_system_part_uuid) + + def _efi_boot_setup(device, efi_system_part_uuid=None, target_boot_mode=None): """Identify and setup an EFI bootloader from supplied partition/disk. diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py index 5bccd0ad4..d0aef11de 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -503,13 +503,15 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n mock_execute.assert_has_calls(expected) @mock.patch.object(image, '_is_bootloader_loaded', lambda *_: False) + @mock.patch.object(image, '_append_uefi_to_fstab', 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(image, '_get_partition', autospec=True) def test__install_grub2(self, mock_get_part_uuid, environ_mock, mock_md_get_raid_devices, mock_is_md_device, - mock_execute, mock_dispatch): + mock_append_to_fstab, mock_execute, + mock_dispatch): mock_get_part_uuid.return_value = self.fake_root_part environ_mock.get.return_value = '/sbin' mock_is_md_device.return_value = False @@ -561,6 +563,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n mock_get_part_uuid.assert_called_once_with(self.fake_dev, uuid=self.fake_root_uuid) self.assertFalse(mock_dispatch.called) + self.assertFalse(mock_append_to_fstab.called) @mock.patch.object(image, '_is_bootloader_loaded', lambda *_: False) @mock.patch.object(hardware, 'is_md_device', autospec=True) @@ -630,6 +633,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n @mock.patch.object(os.path, 'ismount', lambda *_: True) @mock.patch.object(os.path, 'exists', lambda *_: False) @mock.patch.object(image, '_is_bootloader_loaded', lambda *_: True) + @mock.patch.object(image, '_append_uefi_to_fstab', 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) @@ -637,8 +641,8 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n @mock.patch.object(image, '_get_partition', autospec=True) def test__install_grub2_uefi(self, mock_get_part_uuid, mkdir_mock, environ_mock, mock_md_get_raid_devices, - mock_is_md_device, mock_execute, - mock_dispatch): + mock_is_md_device, mock_append_to_fstab, + mock_execute, mock_dispatch): mock_get_part_uuid.side_effect = [self.fake_root_part, self.fake_efi_system_part] environ_mock.get.return_value = '/sbin' @@ -711,10 +715,218 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n mock_get_part_uuid.assert_any_call(self.fake_dev, uuid=self.fake_efi_system_part_uuid) self.assertFalse(mock_dispatch.called) + mock_append_to_fstab.assert_called_with(self.fake_dir, + self.fake_efi_system_part_uuid) + + @mock.patch.object(os.path, 'ismount', lambda *_: True) + @mock.patch.object(image, '_is_bootloader_loaded', lambda *_: 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_fstab(self, mock_get_part_uuid, mkdir_mock, + environ_mock, mock_md_get_raid_devices, + mock_is_md_device, mock_exists, + mock_execute, mock_dispatch): + 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_exists.side_effect = iter([False, True, False, True, True]) + with mock.patch('builtins.open', mock.mock_open()) as mock_open: + 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') + write_calls = [ + mock.call(self.fake_dir + '/etc/fstab', 'r+'), + mock.call().__enter__(), + mock.call().read(), + mock.call().writelines('UUID=%s\t/boot/efi\tvfat\t' + 'umask=0077\t0\t1' + '\n' % self.fake_efi_system_part_uuid), + mock.call().__exit__(None, None, None)] + mock_open.assert_has_calls(write_calls) + + 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 "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_called_once_with(self.fake_dir + '/boot/efi') + 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.patch.object(image, '_efi_boot_setup', lambda *_: False) + @mock.patch.object(os.path, 'ismount', lambda *_: True) + @mock.patch.object(image, '_is_bootloader_loaded', lambda *_: 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_no_fstab( + self, mock_get_part_uuid, + mkdir_mock, + environ_mock, mock_md_get_raid_devices, + mock_is_md_device, mock_exists, + mock_execute, mock_dispatch): + 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 = {} + fstab_data = ( + 'UUID=%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', + mock.mock_open(read_data=fstab_data)) as mock_open: + 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') + write_calls = [ + mock.call(self.fake_dir + '/etc/fstab', 'r+'), + mock.call().__enter__(), + mock.call().read(), + mock.call().__exit__(None, None, None)] + mock_open.assert_has_calls(write_calls) + + 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('umount', self.fake_dir + '/boot/efi'), + # NOTE(TheJulia): chroot mount is for whole disk images + 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_called_once_with(self.fake_dir + '/boot/efi') + 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.patch.object(os.path, 'ismount', lambda *_: False) @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(image, '_efi_boot_setup', autospec=True) @mock.patch.object(shutil, 'copytree', autospec=True) @mock.patch.object(os.path, 'exists', autospec=True) @@ -728,8 +940,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n environ_mock, mock_md_get_raid_devices, mock_is_md_device, mock_exists, mock_copytree, mock_efi_setup, - mock_execute, mock_dispatch): - mock_exists.return_value = True + mock_append_to_fstab, mock_execute, + mock_dispatch): + mock_exists.side_effect = [True, False, True, True, True, False, True, + False, False] mock_efi_setup.return_value = True mock_get_part_uuid.side_effect = [self.fake_root_part, self.fake_efi_system_part] @@ -790,6 +1004,8 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n mock_get_part_uuid.assert_any_call(self.fake_dev, uuid=self.fake_efi_system_part_uuid) self.assertFalse(mock_dispatch.called) + mock_append_to_fstab.assert_called_with(self.fake_dir, + self.fake_efi_system_part_uuid) @mock.patch.object(os, 'listdir', lambda *_: ['file1', 'file2']) @mock.patch.object(image, '_is_bootloader_loaded', lambda *_: False) @@ -877,6 +1093,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n @mock.patch.object(os.path, 'ismount', lambda *_: True) @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) @@ -891,6 +1108,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n 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') @@ -983,6 +1201,8 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n 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) @@ -1076,6 +1296,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n @mock.patch.object(os.path, 'ismount', lambda *_: True) @mock.patch.object(image, '_is_bootloader_loaded', lambda *_: False) @mock.patch.object(os, 'listdir', autospec=True) + @mock.patch.object(image, '_append_uefi_to_fstab', autospec=True) @mock.patch.object(image, '_efi_boot_setup', autospec=True) @mock.patch.object(shutil, 'copytree', autospec=True) @mock.patch.object(os.path, 'exists', autospec=True) @@ -1089,8 +1310,8 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n environ_mock, mock_md_get_raid_devices, mock_is_md_device, mock_exists, mock_copytree, mock_efi_setup, - mock_oslistdir, mock_execute, - mock_dispatch): + mock_append_to_fstab, mock_oslistdir, + mock_execute, mock_dispatch): mock_exists.side_effect = [True, False, False, True, True, True, True] mock_efi_setup.side_effect = Exception('meow') mock_oslistdir.return_value = ['file1'] @@ -1173,6 +1394,8 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n mock_get_part_uuid.assert_any_call(self.fake_dev, uuid=self.fake_efi_system_part_uuid) self.assertFalse(mock_dispatch.called) + 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(hardware, 'is_md_device', autospec=True) @@ -2031,3 +2254,11 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n '\\EFI\\BOOT\\BOOTX64.EFI')] self.assertIsNone(result) mock_execute.assert_has_calls(expected) + + @mock.patch.object(os.path, 'exists', lambda *_: True) + def test__append_uefi_to_fstab_handles_error(self, mock_execute, + mock_dispatch): + with mock.patch('builtins.open', mock.mock_open()) as mock_open: + mock_open.side_effect = OSError('boom') + image._append_uefi_to_fstab( + self.fake_dir, 'abcd-efgh') diff --git a/releasenotes/notes/append-efi-partition-to-fstab-e9f945a4dd19bd7a.yaml b/releasenotes/notes/append-efi-partition-to-fstab-e9f945a4dd19bd7a.yaml new file mode 100644 index 000000000..13f80f6d1 --- /dev/null +++ b/releasenotes/notes/append-efi-partition-to-fstab-e9f945a4dd19bd7a.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + The system file system configuration file for Linux machines, the + ``/etc/fstab`` file is now updated to include a reference to the + EFI partition in the case of a partition image base deployment. + Without this reference, images deployed using partition images + could end up in situations where upgrading the bootloader could fail.