From 2076df91e72ea140eb747550cc61af3a7dfdff5a Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Wed, 26 Aug 2020 08:35:33 -0700 Subject: [PATCH] Add fstab pointer to EFI partition Adds support for the EFI partition to be appended to fstab so the filesystem can be automounted and EFI loader updated should the deployed operating system need to do so. This should enable bootloaders to be upgraded by linux based operating systems after the instance has been deployed when a partition image was utilized for the initial deployment. Change-Id: Iec28a8841cc01ec8b01a3f5cca070c934c7a2531 Story: 2008070 Task: 40754 (cherry picked from commit a12a5744b66063816af17769f99ec3f03da0a2d5) --- ironic_python_agent/extensions/image.py | 25 ++ .../tests/unit/extensions/test_image.py | 245 +++++++++++++++++- ...i-partition-to-fstab-e9f945a4dd19bd7a.yaml | 8 + 3 files changed, 271 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/append-efi-partition-to-fstab-e9f945a4dd19bd7a.yaml 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.