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
This commit is contained in:
Arne Wiebalck 2020-05-05 16:45:51 +02:00
parent e00fe3b85d
commit c5022790b3
2 changed files with 73 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
ironic_python_agent/extensions/image.py
View File

@ -497,6 +497,13 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None,
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()
if efi_system_part_uuid:
@ -533,12 +540,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:/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})
if efi_partitions:
if not os.path.exists(efi_partition_mount_point):
@ -658,6 +666,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,

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

@ -361,6 +361,10 @@ 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,
@ -372,6 +376,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
shell=True,
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',
@ -412,6 +420,10 @@ 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)),
@ -424,6 +436,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
shell=True,
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',
@ -472,6 +488,10 @@ 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('mount', self.fake_efi_system_part,
self.fake_dir + '/boot/efi'),
mock.call(('chroot %s /bin/sh -c "grub-install"' %
@ -496,6 +516,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
'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',
@ -549,6 +573,10 @@ 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('mount', self.fake_efi_system_part,
self.fake_dir + '/boot/efi'),
mock.call(('chroot %s /bin/sh -c "grub-install"' %
@ -582,6 +610,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):
@ -595,6 +624,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',
@ -848,6 +881,10 @@ 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('mount', '/dev/sda1',
self.fake_dir + '/boot/efi'),
mock.call(('chroot %s /bin/sh -c "grub-install"' %
@ -886,6 +923,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
'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',
@ -952,6 +993,10 @@ 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, '/dev/sda')), shell=True,
@ -968,6 +1013,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
shell=True,
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',