openstack/ironic-python-agent
Code Issues Proposed changes

Merge "Mount all vfat partitions before calling grub2"

Browse Source
This commit is contained in:
Zuul 2020-07-02 10:37:04 +00:00 committed by Gerrit Code Review
commit de7d5affe7
2 changed files with 73 additions and 6 deletions
ironic_python_agent
extensions
image.py
tests/unit/extensions
test_image.py

30
ironic_python_agent/extensions/image.py

@ -497,6 +497,13 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None,
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()
if efi_system_part_uuid: 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')): 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:/sbin' % path_variable {'path': path}, shell=True,
env_variables={'PATH': path_variable})
if efi_partitions: if efi_partitions:
if not os.path.exists(efi_partition_mount_point): 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) 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,

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

@ -362,6 +362,10 @@ 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,
@ -373,6 +377,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
shell=True, shell=True,
env_variables={ env_variables={
'PATH': '/sbin:/bin:/usr/sbin:/sbin'}), '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',
@ -413,6 +421,10 @@ 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)),
@ -425,6 +437,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
shell=True, shell=True,
env_variables={ env_variables={
'PATH': '/sbin:/bin:/usr/sbin:/sbin'}), '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',
@ -473,6 +489,10 @@ 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('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 "grub-install"' % mock.call(('chroot %s /bin/sh -c "grub-install"' %
@ -497,6 +517,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
'PATH': '/sbin:/bin:/usr/sbin:/sbin'}), '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',
@ -550,6 +574,10 @@ 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('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 "grub-install"' % mock.call(('chroot %s /bin/sh -c "grub-install"' %
@ -583,6 +611,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):
@ -596,6 +625,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',
@ -849,6 +882,10 @@ 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('mount', '/dev/sda1', mock.call('mount', '/dev/sda1',
self.fake_dir + '/boot/efi'), self.fake_dir + '/boot/efi'),
mock.call(('chroot %s /bin/sh -c "grub-install"' % mock.call(('chroot %s /bin/sh -c "grub-install"' %
@ -887,6 +924,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
'PATH': '/sbin:/bin:/usr/sbin:/sbin'}), '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',
@ -953,6 +994,10 @@ 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, '/dev/sda')), shell=True, (self.fake_dir, '/dev/sda')), shell=True,
@ -969,6 +1014,10 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
shell=True, shell=True,
env_variables={ env_variables={
'PATH': '/sbin:/bin:/usr/sbin:/sbin'}), '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',