diff options
author | Arne Wiebalck <Arne.Wiebalck@cern.ch> | 2020-05-05 16:45:51 +0200 |
---|---|---|
committer | Steve Baker <sbaker@redhat.com> | 2021-03-24 14:28:23 +1300 |
commit | 8419c6d3f10942a076a3d287f01977dd6129ffff (patch) | |
tree | d83c23b391a6ca7c4a4b3dd7b0ceb9bd4bb74f5e | |
parent | 83932952c18dbe347c81e91d5525d13ca567a419 (diff) | |
download | ironic-python-agent-8419c6d3f10942a076a3d287f01977dd6129ffff.tar.gz |
Mount all vfat partitions before calling grub2
In order to ensure grub2 finds all files it needs, mount all
vfat partitions specified in the deployed image.
Story: #2007618
Task: #39629
Change-Id: Ie5b6e0abc3f266409562f9ecb26538126b667056
(cherry picked from commit c5022790b3dca80c009e914aa07f3f879cd159b4)
-rw-r--r-- | ironic_python_agent/extensions/image.py | 30 | ||||
-rw-r--r-- | ironic_python_agent/tests/unit/extensions/test_image.py | 64 |
2 files changed, 78 insertions, 16 deletions
diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py index 576404fc..c1a32aa1 100644 --- a/ironic_python_agent/extensions/image.py +++ b/ironic_python_agent/extensions/image.py @@ -382,6 +382,13 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None, "as it is already marked bootable.", device) return try: + # Add /bin to PATH variable as grub requires it to find efibootmgr + # when running in uefi boot mode. + # Add /usr/sbin to PATH variable to ensure it is there as we do + # not use full path to grub binary anymore. + path_variable = os.environ.get('PATH', '') + path_variable = '%s:/bin:/usr/sbin:/sbin' % path_variable + # Mount the partition and binds path = tempfile.mkdtemp() @@ -416,12 +423,13 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None, if os.path.exists(os.path.join(path, 'usr/sbin/grub2-install')): binary_name = "grub2" - # Add /bin to PATH variable as grub requires it to find efibootmgr - # when running in uefi boot mode. - # Add /usr/sbin to PATH variable to ensure it is there as we do - # not use full path to grub binary anymore. - path_variable = os.environ.get('PATH', '') - path_variable = '%s:/bin:/usr/sbin' % path_variable + # Mount all vfat partitions listed in the fstab of the root partition. + # This is to make sure grub2 finds all files it needs, as some of them + # may not be inside the root partition but in the ESP (like grub2env). + LOG.debug("Mounting all partitions inside the image ...") + utils.execute('chroot %(path)s /bin/sh -c "mount -a -t vfat"' % + {'path': path}, shell=True, + env_variables={'PATH': path_variable}) # Install grub. Normally, grub goes to one disk only. In case of # md devices, grub goes to all underlying holder (RAID-1) disks. @@ -498,6 +506,16 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None, LOG.error(error_msg) raise errors.CommandExecutionError(error_msg) + # Umount the vfat partitions we may have mounted + LOG.debug("Unmounting all partitions inside the image ...") + try: + utils.execute('chroot %(path)s /bin/sh -c "umount -a -t vfat"' % + {'path': path}, shell=True, + env_variables={'PATH': path_variable}) + except processutils.ProcessExecutionError as e: + LOG.warning("Unable to umount vfat partitions. Error: %(error)s", + {'error': e}) + for fs in BIND_MOUNTS: try: utils.execute('umount', path + fs, attempts=3, diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py index 7334a7d8..b3369dbf 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -18,6 +18,7 @@ import shutil import tempfile import mock +from ironic_lib import utils as ilib_utils from oslo_concurrency import processutils from ironic_python_agent import errors @@ -356,15 +357,25 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n 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(('chroot %s /bin/sh -c ' '"grub-install %s"' % (self.fake_dir, self.fake_dev)), shell=True, - env_variables={'PATH': '/sbin:/bin:/usr/sbin'}), + env_variables={'PATH': + '/sbin:/bin:/usr/sbin:/sbin'}), mock.call(('chroot %s /bin/sh -c ' '"grub-mkconfig -o ' '/boot/grub/grub.cfg"' % self.fake_dir), shell=True, - env_variables={'PATH': '/sbin:/bin:/usr/sbin'}), + env_variables={'PATH': + '/sbin:/bin:/usr/sbin:/sbin'}), + 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', @@ -405,16 +416,26 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n 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(('chroot %s /bin/sh -c ' '"grub-install %s"' % (self.fake_dir, self.fake_prep_boot_part)), shell=True, - env_variables={'PATH': '/sbin:/bin:/usr/sbin'}), + env_variables={'PATH': + '/sbin:/bin:/usr/sbin:/sbin'}), mock.call(('chroot %s /bin/sh -c ' '"grub-mkconfig -o ' '/boot/grub/grub.cfg"' % self.fake_dir), shell=True, - env_variables={'PATH': '/sbin:/bin:/usr/sbin'}), + env_variables={'PATH': + '/sbin:/bin:/usr/sbin:/sbin'}), + 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', @@ -464,21 +485,32 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n self.fake_dir + '/sys'), mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), + 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(('chroot %s /bin/sh -c ' '"grub-install %s"' % (self.fake_dir, self.fake_dev)), shell=True, - env_variables={'PATH': '/sbin:/bin:/usr/sbin'}), + env_variables={ + 'PATH': '/sbin:/bin:/usr/sbin:/sbin'}), mock.call(('chroot %s /bin/sh -c ' '"grub-install %s --removable"' % (self.fake_dir, self.fake_dev)), shell=True, - env_variables={'PATH': '/sbin:/bin:/usr/sbin'}), + env_variables={ + 'PATH': '/sbin:/bin:/usr/sbin:/sbin'}), mock.call(('chroot %s /bin/sh -c ' '"grub-mkconfig -o ' '/boot/grub/grub.cfg"' % self.fake_dir), shell=True, - env_variables={'PATH': '/sbin:/bin:/usr/sbin'}), + env_variables={ + 'PATH': '/sbin:/bin:/usr/sbin:/sbin'}), 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', @@ -534,19 +566,26 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n self.fake_dir + '/sys'), mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), + 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(('chroot %s /bin/sh -c ' '"grub-install %s"' % (self.fake_dir, self.fake_dev)), shell=True, - env_variables={'PATH': '/sbin:/bin:/usr/sbin'}), + env_variables={'PATH': + '/sbin:/bin:/usr/sbin:/sbin'}), mock.call(('chroot %s /bin/sh -c ' '"grub-install %s --removable"' % (self.fake_dir, self.fake_dev)), shell=True, - env_variables={'PATH': '/sbin:/bin:/usr/sbin'}), + env_variables={'PATH': + '/sbin:/bin:/usr/sbin:/sbin'}), mock.call(('chroot %s /bin/sh -c ' '"grub-mkconfig -o ' '/boot/grub/grub.cfg"' % self.fake_dir), shell=True, - env_variables={'PATH': '/sbin:/bin:/usr/sbin'}), + env_variables={'PATH': + '/sbin:/bin:/usr/sbin:/sbin'}), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True)] mock_execute.assert_has_calls(expected) @@ -564,6 +603,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n mock_get_part_uuid.side_effect = [self.fake_root_part, self.fake_efi_system_part] mock_is_md_device.side_effect = [False, False] + environ_mock.get.return_value = '/sbin' mock_md_get_raid_devices.return_value = {} def mount_raise_func(*args, **kwargs): @@ -577,6 +617,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n efi_system_part_uuid=self.fake_efi_system_part_uuid) expected = [mock.call('mount', '/dev/fake2', self.fake_dir), + 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', |