openstack
/
ironic-python-agent
Code Issues Proposed changes

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 c5022790b3)
This commit is contained in:
Arne Wiebalck 2020-05-05 16:45:51 +02:00 committed by Steve Baker
parent 83932952c1
commit 8419c6d3f1
2 changed files with 78 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
ironic_python_agent/extensions/image.py
View File

@ -382,6 +382,13 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None,
"as it is already marked bootable.", device) "as it is already marked bootable.", device)
return return
try: 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 # Mount the partition and binds
path = tempfile.mkdtemp() 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')): if os.path.exists(os.path.join(path, 'usr/sbin/grub2-install')):
binary_name = "grub2" binary_name = "grub2"
# Add /bin to PATH variable as grub requires it to find efibootmgr # Mount all vfat partitions listed in the fstab of the root partition.
# when running in uefi boot mode. # This is to make sure grub2 finds all files it needs, as some of them
# Add /usr/sbin to PATH variable to ensure it is there as we do # may not be inside the root partition but in the ESP (like grub2env).
# not use full path to grub binary anymore. LOG.debug("Mounting all partitions inside the image ...")
path_variable = os.environ.get('PATH', '') utils.execute('chroot %(path)s /bin/sh -c "mount -a -t vfat"' %
path_variable = '%s:/bin:/usr/sbin' % path_variable {'path': path}, shell=True,
env_variables={'PATH': path_variable})
# Install grub. Normally, grub goes to one disk only. In case of # Install grub. Normally, grub goes to one disk only. In case of
# md devices, grub goes to all underlying holder (RAID-1) disks. # 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) LOG.error(error_msg)
raise errors.CommandExecutionError(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: for fs in BIND_MOUNTS:
try: try:
utils.execute('umount', path + fs, attempts=3, utils.execute('umount', path + fs, attempts=3,

64
ironic_python_agent/tests/unit/extensions/test_image.py
View File

@ -18,6 +18,7 @@ import shutil
import tempfile import tempfile
import mock import mock
from ironic_lib import utils as ilib_utils
from oslo_concurrency import processutils from oslo_concurrency import processutils
from ironic_python_agent import errors from ironic_python_agent import errors
@ -356,15 +357,25 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
self.fake_dir + '/run'), self.fake_dir + '/run'),
mock.call('mount', '-t', 'sysfs', 'none', mock.call('mount', '-t', 'sysfs', 'none',
self.fake_dir + '/sys'), 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 ' mock.call(('chroot %s /bin/sh -c '
'"grub-install %s"' % '"grub-install %s"' %
(self.fake_dir, self.fake_dev)), shell=True, (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 ' mock.call(('chroot %s /bin/sh -c '
'"grub-mkconfig -o ' '"grub-mkconfig -o '
'/boot/grub/grub.cfg"' % self.fake_dir), '/boot/grub/grub.cfg"' % self.fake_dir),
shell=True, 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', mock.call('umount', self.fake_dir + '/dev',
attempts=3, delay_on_retry=True), attempts=3, delay_on_retry=True),
mock.call('umount', self.fake_dir + '/proc', mock.call('umount', self.fake_dir + '/proc',
@ -405,16 +416,26 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
self.fake_dir + '/run'), self.fake_dir + '/run'),
mock.call('mount', '-t', 'sysfs', 'none', mock.call('mount', '-t', 'sysfs', 'none',
self.fake_dir + '/sys'), 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 ' mock.call(('chroot %s /bin/sh -c '
'"grub-install %s"' % '"grub-install %s"' %
(self.fake_dir, self.fake_prep_boot_part)), (self.fake_dir, self.fake_prep_boot_part)),
shell=True, shell=True,
env_variables={'PATH': '/sbin:/bin:/usr/sbin'}), env_variables={'PATH':
'/sbin:/bin:/usr/sbin:/sbin'}),
mock.call(('chroot %s /bin/sh -c ' mock.call(('chroot %s /bin/sh -c '
'"grub-mkconfig -o ' '"grub-mkconfig -o '
'/boot/grub/grub.cfg"' % self.fake_dir), '/boot/grub/grub.cfg"' % self.fake_dir),
shell=True, 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', mock.call('umount', self.fake_dir + '/dev',
attempts=3, delay_on_retry=True), attempts=3, delay_on_retry=True),
mock.call('umount', self.fake_dir + '/proc', mock.call('umount', self.fake_dir + '/proc',
@ -464,21 +485,32 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
self.fake_dir + '/sys'), self.fake_dir + '/sys'),
mock.call('mount', self.fake_efi_system_part, mock.call('mount', self.fake_efi_system_part,
self.fake_dir + '/boot/efi'), 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 ' mock.call(('chroot %s /bin/sh -c '
'"grub-install %s"' % '"grub-install %s"' %
(self.fake_dir, self.fake_dev)), shell=True, (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 ' mock.call(('chroot %s /bin/sh -c '
'"grub-install %s --removable"' % '"grub-install %s --removable"' %
(self.fake_dir, self.fake_dev)), shell=True, (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 ' mock.call(('chroot %s /bin/sh -c '
'"grub-mkconfig -o ' '"grub-mkconfig -o '
'/boot/grub/grub.cfg"' % self.fake_dir), '/boot/grub/grub.cfg"' % self.fake_dir),
shell=True, 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', mock.call('umount', self.fake_dir + '/boot/efi',
attempts=3, delay_on_retry=True), 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', mock.call('umount', self.fake_dir + '/dev',
attempts=3, delay_on_retry=True), attempts=3, delay_on_retry=True),
mock.call('umount', self.fake_dir + '/proc', mock.call('umount', self.fake_dir + '/proc',
@ -534,19 +566,26 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
self.fake_dir + '/sys'), self.fake_dir + '/sys'),
mock.call('mount', self.fake_efi_system_part, mock.call('mount', self.fake_efi_system_part,
self.fake_dir + '/boot/efi'), 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 ' mock.call(('chroot %s /bin/sh -c '
'"grub-install %s"' % '"grub-install %s"' %
(self.fake_dir, self.fake_dev)), shell=True, (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 ' mock.call(('chroot %s /bin/sh -c '
'"grub-install %s --removable"' % '"grub-install %s --removable"' %
(self.fake_dir, self.fake_dev)), shell=True, (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 ' mock.call(('chroot %s /bin/sh -c '
'"grub-mkconfig -o ' '"grub-mkconfig -o '
'/boot/grub/grub.cfg"' % self.fake_dir), '/boot/grub/grub.cfg"' % self.fake_dir),
shell=True, 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', mock.call('umount', self.fake_dir + '/boot/efi',
attempts=3, delay_on_retry=True)] attempts=3, delay_on_retry=True)]
mock_execute.assert_has_calls(expected) 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, mock_get_part_uuid.side_effect = [self.fake_root_part,
self.fake_efi_system_part] self.fake_efi_system_part]
mock_is_md_device.side_effect = [False, False] mock_is_md_device.side_effect = [False, False]
environ_mock.get.return_value = '/sbin'
mock_md_get_raid_devices.return_value = {} mock_md_get_raid_devices.return_value = {}
def mount_raise_func(*args, **kwargs): 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) efi_system_part_uuid=self.fake_efi_system_part_uuid)
expected = [mock.call('mount', '/dev/fake2', self.fake_dir), 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', mock.call('umount', self.fake_dir + '/dev',
attempts=3, delay_on_retry=True), attempts=3, delay_on_retry=True),
mock.call('umount', self.fake_dir + '/proc', mock.call('umount', self.fake_dir + '/proc',