Merge "Prevent broken partition image UEFI deploys"

3 files changed, 764 insertions, 68 deletions

diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py
index de16e21e..33cb4a60 100644
--- a/ironic_python_agent/extensions/image.py
+++ b/ironic_python_agent/extensions/image.py
@@ -334,6 +334,7 @@ def _manage_uefi(device, efi_system_part_uuid=None):
         LOG.error(error_msg)
         raise errors.CommandExecutionError(error_msg)
     finally:
+        LOG.debug('Executing _manage_uefi clean-up.')
         umount_warn_msg = "Unable to umount %(local_path)s. Error: %(error)s"
 
         try:
@@ -502,7 +503,9 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None,
     efi_part = None
     efi_partition_mount_point = None
     efi_mounted = False
+    efi_preserved = False
     holders = None
+    path_variable = _get_path_variable()
 
     # NOTE(TheJulia): Seems we need to get this before ever possibly
     # restart the device in the case of multi-device RAID as pyudev
@@ -529,13 +532,6 @@ 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:
@@ -563,10 +559,33 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None,
             disks = [device]
 
         utils.execute('mount', root_partition, path)
-        for fs in BIND_MOUNTS:
-            utils.execute('mount', '-o', 'bind', fs, path + fs)
 
-        utils.execute('mount', '-t', 'sysfs', 'none', path + '/sys')
+        _mount_for_chroot(path)
+
+        # UEFI asset management for RAID is handled elsewhere
+        if not hardware.is_md_device(device) and efi_partition_mount_point:
+            # NOTE(TheJulia): It may make sense to retool all efi
+            # asset preservation logic at some point since the paths
+            # can be a little different, but largely this is JUST for
+            # partition images as there _should not_ be a mount
+            # point if we have no efi partitions at all.
+            efi_preserved = _try_preserve_efi_assets(
+                device, path, efi_system_part_uuid,
+                efi_partitions, efi_partition_mount_point)
+            if efi_preserved:
+                # Success preserving efi assets
+                return
+            else:
+                # Failure, either via exception or not found
+                # which in this case the partition needs to be
+                # remounted.
+                LOG.debug('No EFI assets were preserved for setup or the '
+                          'ramdisk was unable to complete the setup. '
+                          'falling back to bootloader installation from'
+                          'deployed image.')
+                if not os.path.ismount(root_partition):
+                    LOG.debug('Re-mounting the root partition.')
+                    utils.execute('mount', root_partition, path)
 
         binary_name = "grub"
         if os.path.exists(os.path.join(path, 'usr/sbin/grub2-install')):
@@ -583,8 +602,9 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None,
         if efi_partitions:
             if not os.path.exists(efi_partition_mount_point):
                 os.makedirs(efi_partition_mount_point)
-            LOG.info("GRUB2 will be installed for UEFI on efi partitions %s",
-                     efi_partitions)
+            LOG.warning("GRUB2 will be installed for UEFI on efi partitions "
+                        "%s using the install command which does not place "
+                        "Secure Boot signed binaries.", efi_partitions)
             for efi_partition in efi_partitions:
                 utils.execute(
                     'mount', efi_partition, efi_partition_mount_point)
@@ -650,28 +670,10 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None,
                 LOG.debug("GRUB2 successfully installed on device %s",
                           grub_disk)
 
-        # If the image has dracut installed, set the rd.md.uuid kernel
-        # parameter for discovered md devices.
-        if hardware.is_md_device(device) and _has_dracut(path):
-            rd_md_uuids = ["rd.md.uuid=%s" % x['UUID']
-                           for x in hardware.md_get_raid_devices().values()]
-
-            LOG.debug("Setting rd.md.uuid kernel parameters: %s", rd_md_uuids)
-            with open('%s/etc/default/grub' % path, 'r') as g:
-                contents = g.read()
-            with open('%s/etc/default/grub' % path, 'w') as g:
-                g.write(
-                    re.sub(r'GRUB_CMDLINE_LINUX="(.*)"',
-                           r'GRUB_CMDLINE_LINUX="\1 %s"'
-                           % " ".join(rd_md_uuids),
-                           contents))
-        utils.execute('chroot %(path)s /bin/sh -c '
-                      '"%(bin)s-mkconfig -o '
-                      '/boot/%(bin)s/grub.cfg"' %
-                      {'path': path, 'bin': binary_name}, shell=True,
-                      env_variables={'PATH': path_variable,
-                                     'GRUB_DISABLE_OS_PROBER': 'true'},
-                      use_standard_locale=True)
+        # NOTE(TheJulia): Setup grub configuration again since IF we reach
+        # this point, then we've manually installed grub which is not the
+        # recommended path.
+        _configure_grub(device, path)
 
         LOG.info("GRUB2 successfully installed on %s", device)
 
@@ -682,6 +684,7 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None,
         raise errors.CommandExecutionError(error_msg)
 
     finally:
+        LOG.debug('Executing _install_grub2 clean-up.')
         # Umount binds and partition
         umount_warn_msg = "Unable to umount %(path)s. Error: %(error)s"
 
@@ -698,7 +701,9 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None,
             raise errors.CommandExecutionError(error_msg)
 
         # If umounting the binds succeed then we can try to delete it
-        if _umount_all_partitions(path, path_variable, umount_warn_msg):
+        if _umount_all_partitions(path,
+                                  path_variable,
+                                  umount_warn_msg):
             try:
                 utils.execute('umount', path, attempts=3, delay_on_retry=True)
             except processutils.ProcessExecutionError as e:
@@ -709,6 +714,242 @@ def _install_grub2(device, root_uuid, efi_system_part_uuid=None,
                 shutil.rmtree(path)
 
 
+def _get_path_variable():
+    # 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', '')
+    return '%s:/bin:/usr/sbin:/sbin' % path_variable
+
+
+def _configure_grub(device, path):
+    """Make consolidated grub configuration as it is device aware.
+
+    :param device: The device for the filesystem.
+    :param path: The path in which the filesystem is mounted.
+    """
+    LOG.debug('Attempting to generate grub Configuration')
+    path_variable = _get_path_variable()
+    binary_name = "grub"
+    if os.path.exists(os.path.join(path, 'usr/sbin/grub2-install')):
+        binary_name = "grub2"
+    # If the image has dracut installed, set the rd.md.uuid kernel
+    # parameter for discovered md devices.
+    if hardware.is_md_device(device) and _has_dracut(path):
+        rd_md_uuids = ["rd.md.uuid=%s" % x['UUID']
+                       for x in hardware.md_get_raid_devices().values()]
+        LOG.debug("Setting rd.md.uuid kernel parameters: %s", rd_md_uuids)
+        with open('%s/etc/default/grub' % path, 'r') as g:
+            contents = g.read()
+        with open('%s/etc/default/grub' % path, 'w') as g:
+            g.write(
+                re.sub(r'GRUB_CMDLINE_LINUX="(.*)"',
+                       r'GRUB_CMDLINE_LINUX="\1 %s"'
+                       % " ".join(rd_md_uuids),
+                       contents))
+
+    utils.execute('chroot %(path)s /bin/sh -c '
+                  '"%(bin)s-mkconfig -o '
+                  '/boot/%(bin)s/grub.cfg"' %
+                  {'path': path, 'bin': binary_name}, shell=True,
+                  env_variables={'PATH': path_variable,
+                                 'GRUB_DISABLE_OS_PROBER': 'true',
+                                 'GRUB_SAVEDEFAULT': 'true'},
+                  use_standard_locale=True)
+    LOG.debug('Completed basic grub configuration.')
+
+
+def _mount_for_chroot(path):
+    """Mount items for grub-mkconfig to succeed."""
+    LOG.debug('Mounting Linux standard partitions for bootloader '
+              'configuration generation')
+    for fs in BIND_MOUNTS:
+        utils.execute('mount', '-o', 'bind', fs, path + fs)
+    utils.execute('mount', '-t', 'sysfs', 'none', path + '/sys')
+
+
+def _try_preserve_efi_assets(device, path,
+                             efi_system_part_uuid,
+                             efi_partitions,
+                             efi_partition_mount_point):
+    """Attempt to preserve UEFI boot assets.
+
+    :param device: The device upon which wich to try to preserve
+                   assets.
+    :param path: The path in which the filesystem is already mounted
+                 which we should examine to preserve assets from.
+    :param efi_system_part_uuid: The partition ID representing the
+                                 created EFI system partition.
+    :param efi_partitions: The list of partitions upon wich to
+                           write the preserved assets to.
+    :param efi_partition_mount_point: The folder at which to mount
+                                      the assets for the process of
+                                      preservation.
+
+    :returns: True if assets have been preserved, otherwise False.
+              None is the result of this method if a failure has
+              occured.
+    """
+    efi_assets_folder = efi_partition_mount_point + '/EFI'
+    if os.path.exists(efi_assets_folder):
+        # We appear to have EFI Assets, that need to be preserved
+        # and as such if we succeed preserving them, we will be returned
+        # True from _preserve_efi_assets to correspond with success or
+        # failure in this action.
+        # NOTE(TheJulia): Still makes sense to invoke grub-install as
+        # fragmentation of grub has occured.
+        if (os.path.exists(os.path.join(path, 'usr/sbin/grub2-install'))
+            or os.path.exists(os.path.join(path, 'usr/sbin/grub-install'))):
+            _configure_grub(device, path)
+        # But first, if we have grub, we should try to build a grub config!
+        LOG.debug('EFI asset folder detected, attempting to preserve assets.')
+        if _preserve_efi_assets(path, efi_assets_folder,
+                                efi_partitions,
+                                efi_partition_mount_point):
+            try:
+                # Since we have preserved the assets, we should be able
+                # to call the _efi_boot_setup method to scan the device
+                # and add loader entries
+                efi_preserved = _efi_boot_setup(device, efi_system_part_uuid)
+                # Executed before the return so we don't return and then begin
+                # execution.
+                return efi_preserved
+            except Exception as e:
+                # Remount the partition and proceed as we were.
+                LOG.debug('Exception encountered while attempting to '
+                          'setup the EFI loader from a root '
+                          'filesystem. Error: %s', e)
+
+
+def _efi_boot_setup(device, efi_system_part_uuid=None, target_boot_mode=None):
+    """Identify and setup an EFI bootloader from supplied partition/disk.
+
+    :param device: The device upon which to attempt the EFI bootloader setup.
+    :param efi_system_part_uuid: The partition UUID to utilize in searching
+                                 for an EFI bootloader.
+    :param target_boot_mode: The requested boot mode target for the
+                             machine. This is optional and is mainly used
+                             for the purposes of identifying a mismatch and
+                             reporting a warning accordingly.
+    :returns: True if we succeeded in setting up an EFI bootloader in the
+              EFI nvram table.
+              False if we were unable to set the machine to EFI boot,
+              due to inability to locate assets required OR the efibootmgr
+              tool not being present.
+              None is returned if the node is NOT in UEFI boot mode or
+              the system is deploying upon a software RAID device.
+    """
+    boot = hardware.dispatch_to_managers('get_boot_info')
+    # Explicitly only run if a target_boot_mode is set which prevents
+    # callers following-up from re-logging the same message
+    if target_boot_mode and boot.current_boot_mode != target_boot_mode:
+        LOG.warning('Boot mode mismatch: target boot mode is %(target)s, '
+                    'current boot mode is %(current)s. Installing boot '
+                    'loader may fail or work incorrectly.',
+                    {'target': target_boot_mode,
+                     'current': boot.current_boot_mode})
+
+    # FIXME(arne_wiebalck): make software RAID work with efibootmgr
+    if (boot.current_boot_mode == 'uefi'
+            and not hardware.is_md_device(device)):
+        try:
+            utils.execute('efibootmgr', '--version')
+        except FileNotFoundError:
+            LOG.warning("efibootmgr is not available in the ramdisk")
+        else:
+            if _manage_uefi(device,
+                            efi_system_part_uuid=efi_system_part_uuid):
+                return True
+        return False
+
+
+def _preserve_efi_assets(path, efi_assets_folder, efi_partitions,
+                         efi_partition_mount_point):
+    """Preserve the EFI assets in a partition image.
+
+    :param path: The path used for the mounted image filesystem.
+    :param efi_assets_folder: The folder where we can find the
+                              UEFI assets required for booting.
+    :param efi_partitions: The list of partitions upon which to
+                           write the perserved assets to.
+    :param efi_partition_mount_point: The folder at which to mount
+                                      the assets for the process of
+                                      preservation.
+    :returns: True if EFI assets were able to be located and preserved
+              to their appropriate locations based upon the supplied
+              efi_partitions list.
+              False if any error is encountered in this process.
+    """
+    try:
+        save_efi = os.path.join(tempfile.mkdtemp(), 'efi_loader')
+        LOG.debug('Copying EFI assets to %s.', save_efi)
+        shutil.copytree(efi_assets_folder, save_efi)
+
+        # Identify grub2 config file for EFI booting as grub may require it
+        # in the folder.
+
+        destlist = os.listdir(efi_assets_folder)
+        grub2_file = os.path.join(path, 'boot/grub2/grub.cfg')
+        if os.path.isfile(grub2_file):
+            LOG.debug('Local Grub2 configuration detected.')
+            # A grub2 config seems to be present, we should preserve it!
+            for dest in destlist:
+                grub_dest = os.path.join(save_efi, dest, 'grub.cfg')
+                if not os.path.isfile(grub_dest):
+                    LOG.debug('A grub.cfg file was not found in %s. %s'
+                              'will be copied to that location.',
+                              grub_dest, grub2_file)
+                    try:
+                        shutil.copy2(grub2_file, grub_dest)
+                    except (IOError, OSError, shutil.SameFileError) as e:
+                        LOG.warning('Failed to copy grub.cfg file for '
+                                    'EFI boot operation. Error %s', e)
+        grub2_env_file = os.path.join(path, 'boot/grub2/grubenv')
+        # NOTE(TheJulia): By saving the default, this file should be created.
+        # this appears to what diskimage-builder does.
+        # if the file is just a file, then we'll need to copy it. If it is
+        # anything else like a link, we're good. This behaivor is inconsistent
+        # depending on packager install scripts for grub.
+        if os.path.isfile(grub2_env_file):
+            LOG.debug('Detected grub environment file %s, will attempt '
+                      'to copy this file to align with apparent bootloaders',
+                      grub2_env_file)
+            for dest in destlist:
+                grub2env_dest = os.path.join(save_efi, dest, 'grubenv')
+                if not os.path.isfile(grub2env_dest):
+                    LOG.debug('A grubenv file was not found. Copying '
+                              'to %s along with the grub.cfg file as '
+                              'grub generally expects it is present.',
+                              grub2env_dest)
+                    try:
+                        shutil.copy2(grub2_env_file, grub2env_dest)
+                    except (IOError, OSError, shutil.SameFileError) as e:
+                        LOG.warning('Failed to copy grubenv file. '
+                                    'Error: %s', e)
+        # Loop through partitions because software RAID.
+        for efi_part in efi_partitions:
+            utils.execute('mount', '-t', 'vfat', efi_part,
+                          efi_partition_mount_point)
+            shutil.copytree(save_efi, efi_assets_folder)
+            LOG.debug('Files preserved to %(disk)s for %(part)s. '
+                      'Files: %(filelist)s From: %(from)s',
+                      {'disk': efi_part,
+                       'part': efi_partition_mount_point,
+                       'filelist': os.listdir(efi_assets_folder),
+                       'from': save_efi})
+            utils.execute('umount', efi_partition_mount_point)
+        return True
+    except Exception as e:
+        LOG.debug('Failed to preserve EFI assets. Error %s', e)
+        try:
+            utils.execute('umount', efi_partition_mount_point)
+        except Exception as e:
+            LOG.debug('Exception encountered while attempting unmount '
+                      'the EFI partition mount point. Error: %s', e)
+        return False
+
+
 class ImageExtension(base.BaseAgentExtension):
 
     @base.async_command('install_bootloader')
@@ -744,34 +985,13 @@ class ImageExtension(base.BaseAgentExtension):
         else:
             ignore_failure = ignore_bootloader_failure
 
-        boot = hardware.dispatch_to_managers('get_boot_info')
-        if boot.current_boot_mode != target_boot_mode:
-            LOG.warning('Boot mode mismatch: target boot mode is %(target)s, '
-                        'current boot mode is %(current)s. Installing boot '
-                        'loader may fail or work incorrectly.',
-                        {'target': target_boot_mode,
-                         'current': boot.current_boot_mode})
-
-        # FIXME(arne_wiebalck): make software RAID work with efibootmgr
-        if (boot.current_boot_mode == 'uefi'
-                and not hardware.is_md_device(device)):
-            has_efibootmgr = True
-            try:
-                utils.execute('efibootmgr', '--version')
-            except FileNotFoundError:
-                LOG.warning("efibootmgr is not available in the ramdisk")
-                has_efibootmgr = False
-
-            if has_efibootmgr:
-                try:
-                    if _manage_uefi(
-                            device,
-                            efi_system_part_uuid=efi_system_part_uuid):
-                        return
-                except Exception as e:
-                    LOG.error('Error setting up bootloader. Error %s', e)
-                    if not ignore_failure:
-                        raise
+        try:
+            if _efi_boot_setup(device, efi_system_part_uuid, target_boot_mode):
+                return
+        except Exception as e:
+            LOG.error('Error setting up bootloader. Error %s', e)
+            if not ignore_failure:
+                raise
 
         # We don't have a working root UUID detection for whole disk images.
         # Until we can do it, avoid a confusing traceback.
diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py
index 605120df..fc8dfb90 100644
--- a/ironic_python_agent/tests/unit/extensions/test_image.py
+++ b/ironic_python_agent/tests/unit/extensions/test_image.py
@@ -541,7 +541,8 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
                               shell=True,
                               env_variables={
                                   'PATH': '/sbin:/bin:/usr/sbin:/sbin',
-                                  'GRUB_DISABLE_OS_PROBER': 'true'},
+                                  'GRUB_DISABLE_OS_PROBER': 'true',
+                                  'GRUB_SAVEDEFAULT': 'true'},
                               use_standard_locale=True),
                     mock.call(('chroot %s /bin/sh -c "umount -a -t vfat"' %
                               (self.fake_dir)), shell=True,
@@ -603,7 +604,8 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
                               shell=True,
                               env_variables={
                                   'PATH': '/sbin:/bin:/usr/sbin:/sbin',
-                                  'GRUB_DISABLE_OS_PROBER': 'true'},
+                                  'GRUB_DISABLE_OS_PROBER': 'true',
+                                  'GRUB_SAVEDEFAULT': 'true'},
                               use_standard_locale=True),
                     mock.call(('chroot %s /bin/sh -c "umount -a -t vfat"' %
                               (self.fake_dir)), shell=True,
@@ -626,6 +628,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
                                            uuid=self.fake_prep_boot_part_uuid)
         self.assertFalse(mock_dispatch.called)
 
+    @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(hardware, 'is_md_device', autospec=True)
@@ -683,7 +686,8 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
                               shell=True,
                               env_variables={
                                   'PATH': '/sbin:/bin:/usr/sbin:/sbin',
-                                  'GRUB_DISABLE_OS_PROBER': 'true'},
+                                  '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),
@@ -709,6 +713,468 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
                                            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, '_efi_boot_setup', autospec=True)
+    @mock.patch.object(shutil, 'copytree', autospec=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_partition_image_with_loader(
+            self, mock_get_part_uuid, mkdir_mock,
+            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_efi_setup.return_value = True
+        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 = {}
+
+        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')
+        mock_efi_setup.assert_called_once_with(self.fake_dev,
+                                               self.fake_efi_system_part_uuid)
+        mock_copytree.assert_has_calls([
+            mock.call(self.fake_dir + '/boot/efi/EFI',
+                      self.fake_dir + '/efi_loader'),
+            mock.call(self.fake_dir + '/efi_loader',
+                      self.fake_dir + '/boot/efi/EFI')])
+
+        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('mount', '-t', 'vfat', '/dev/fake1',
+                              self.fake_dir + '/boot/efi'),
+                    mock.call('umount', self.fake_dir + '/boot/efi'),
+                    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_not_called()
+        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, 'listdir', lambda *_: ['file1', 'file2'])
+    @mock.patch.object(image, '_is_bootloader_loaded', lambda *_: False)
+    @mock.patch.object(shutil, 'copy2', autospec=True)
+    @mock.patch.object(os.path, 'isfile', 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)
+    @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_partition_image_with_loader_with_grubcfg(
+            self, mock_get_part_uuid, mkdir_mock,
+            environ_mock, mock_md_get_raid_devices,
+            mock_is_md_device, mock_exists,
+            mock_copytree, mock_efi_setup,
+            mock_isfile, mock_copy2,
+            mock_execute, mock_dispatch):
+        mock_exists.return_value = True
+        mock_efi_setup.return_value = True
+        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_isfile.side_effect = [True, False, False, True, True, False]
+
+        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')
+        mock_efi_setup.assert_called_once_with(self.fake_dev,
+                                               self.fake_efi_system_part_uuid)
+        mock_copytree.assert_has_calls([
+            mock.call(self.fake_dir + '/boot/efi/EFI',
+                      self.fake_dir + '/efi_loader'),
+            mock.call(self.fake_dir + '/efi_loader',
+                      self.fake_dir + '/boot/efi/EFI')])
+
+        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 ' + self.fake_dir + ' /bin/sh -c '
+                               '"grub2-mkconfig -o /boot/grub2/grub.cfg"'),
+                              shell=True,
+                              env_variables={
+                                  'PATH': '/sbin:/bin:/usr/sbin:/sbin',
+                                  'GRUB_DISABLE_OS_PROBER': 'true',
+                                  'GRUB_SAVEDEFAULT': 'true'},
+                              use_standard_locale=True),
+                    mock.call('mount', '-t', 'vfat', '/dev/fake1',
+                              self.fake_dir + '/boot/efi'),
+                    mock.call('umount', self.fake_dir + '/boot/efi'),
+                    mock.call(('chroot ' + self.fake_dir
+                               + ' /bin/sh -c "umount -a -t vfat"'),
+                              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_not_called()
+        mock_execute.assert_has_calls(expected)
+        mock_copy2.assert_has_calls([])
+        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 *_: True)
+    @mock.patch.object(image, '_is_bootloader_loaded', lambda *_: False)
+    @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)
+    @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_partition_image_with_preserve_failure(
+            self, mock_get_part_uuid, mkdir_mock,
+            environ_mock, mock_md_get_raid_devices,
+            mock_is_md_device, mock_exists,
+            mock_efi_setup,
+            mock_preserve_efi_assets,
+            mock_execute, mock_dispatch):
+        mock_exists.return_value = True
+        mock_efi_setup.side_effect = Exception('meow')
+        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_preserve_efi_assets.return_value = False
+
+        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')
+        self.assertFalse(mock_efi_setup.called)
+
+        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(('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_not_called()
+        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_preserve_efi_assets.assert_called_with(
+            self.fake_dir,
+            self.fake_dir + '/boot/efi/EFI',
+            ['/dev/fake1'],
+            self.fake_dir + '/boot/efi')
+
+    @mock.patch.object(image, '_is_bootloader_loaded', lambda *_: False)
+    @mock.patch.object(os, 'listdir', autospec=True)
+    @mock.patch.object(shutil, 'copy2', autospec=True)
+    @mock.patch.object(os.path, 'isfile', 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)
+    @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_partition_image_with_loader_grubcfg_fails(
+            self, mock_get_part_uuid, mkdir_mock,
+            environ_mock, mock_md_get_raid_devices,
+            mock_is_md_device, mock_exists,
+            mock_copytree, mock_efi_setup,
+            mock_isfile, mock_copy2,
+            mock_oslistdir, mock_execute,
+            mock_dispatch):
+        mock_exists.return_value = True
+        mock_efi_setup.return_value = True
+        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_isfile.side_effect = [True, False, False, True, False,
+                                   True, False]
+        mock_copy2.side_effect = OSError('copy failed')
+        mock_oslistdir.return_value = ['file1', 'file2']
+
+        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')
+        mock_efi_setup.assert_called_once_with(self.fake_dev,
+                                               self.fake_efi_system_part_uuid)
+        mock_copytree.assert_has_calls([
+            mock.call(self.fake_dir + '/boot/efi/EFI',
+                      self.fake_dir + '/efi_loader'),
+            mock.call(self.fake_dir + '/efi_loader',
+                      self.fake_dir + '/boot/efi/EFI')])
+
+        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 ' + self.fake_dir + ' /bin/sh -c '
+                               '"grub2-mkconfig -o /boot/grub2/grub.cfg"'),
+                              shell=True,
+                              env_variables={
+                                  'PATH': '/sbin:/bin:/usr/sbin:/sbin',
+                                  'GRUB_DISABLE_OS_PROBER': 'true',
+                                  'GRUB_SAVEDEFAULT': 'true'},
+                              use_standard_locale=True),
+                    mock.call('mount', '-t', 'vfat', '/dev/fake1',
+                              self.fake_dir + '/boot/efi'),
+                    mock.call('umount', self.fake_dir + '/boot/efi'),
+                    mock.call(('chroot ' + self.fake_dir
+                               + ' /bin/sh -c "umount -a -t vfat"'),
+                              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_not_called()
+        mock_execute.assert_has_calls(expected)
+        self.assertEqual(3, mock_copy2.call_count)
+        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)
+        self.assertEqual(2, mock_oslistdir.call_count)
+
+    @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, '_efi_boot_setup', autospec=True)
+    @mock.patch.object(shutil, 'copytree', autospec=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_partition_image_with_no_loader(
+            self, mock_get_part_uuid, mkdir_mock,
+            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_exists.side_effect = [True, False, False, True, True, True, True]
+        mock_efi_setup.side_effect = Exception('meow')
+        mock_oslistdir.return_value = ['file1']
+        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 = {}
+
+        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')
+
+        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('mount', '-t', 'vfat', '/dev/fake1',
+                              self.fake_dir + '/boot/efi'),
+                    mock.call('umount', 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('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_not_called()
+        mock_execute.assert_has_calls(expected)
+        self.assertEqual(2, mock_copytree.call_count)
+        self.assertTrue(mock_efi_setup.called)
+        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, '_is_bootloader_loaded', lambda *_: False)
     @mock.patch.object(hardware, 'is_md_device', autospec=True)
     @mock.patch.object(hardware, 'md_get_raid_devices', autospec=True)
@@ -744,6 +1210,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
                               self.fake_dir + '/run'),
                     mock.call('mount', '-t', 'sysfs', 'none',
                               self.fake_dir + '/sys'),
+                    mock.call('mount', '/dev/fake2', self.fake_dir),
                     mock.call(('chroot %s /bin/sh -c "mount -a -t vfat"' %
                               (self.fake_dir)), shell=True,
                               env_variables={
@@ -1092,7 +1559,8 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
                               shell=True,
                               env_variables={
                                   'PATH': '/sbin:/bin:/usr/sbin:/sbin',
-                                  'GRUB_DISABLE_OS_PROBER': 'true'},
+                                  '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),
@@ -1186,7 +1654,8 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
                       shell=True,
                       env_variables={
                           'PATH': '/sbin:/bin:/usr/sbin:/sbin',
-                          'GRUB_DISABLE_OS_PROBER': 'true'},
+                          'GRUB_DISABLE_OS_PROBER': 'true',
+                          'GRUB_SAVEDEFAULT': 'true'},
                       use_standard_locale=True),
             mock.call(('chroot %s /bin/sh -c "umount -a -t vfat"' %
                       (self.fake_dir)), shell=True,
diff --git a/releasenotes/notes/preserve-efi-folder-contents-ea1e278b3093ec55.yaml b/releasenotes/notes/preserve-efi-folder-contents-ea1e278b3093ec55.yaml
new file mode 100644
index 00000000..27254821
--- /dev/null
+++ b/releasenotes/notes/preserve-efi-folder-contents-ea1e278b3093ec55.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fixes the agent's EFI boot handling such that EFI assets from a partition
+    image are preserved and used instead of overridden. This should permit
+    operators to use Secure Boot with partition images IF the assets are
+    already present in the partition image.