curtin-dev team mailing list archive

[Ryan Harper <rharper@xxxxxxxxxxx>]

Ryan Harper has proposed merging ~raharper/curtin:fix/refactor-uefi-duplicate-for-reuse into curtin:master.

Commit message:
Refactor uefi_remove_duplicates into find/remove functions for reuse

Splitting the function into find/remove functions allows reuse in the
vmtest which exercises the code and then verifies the duplicate
removals.

- Remove skip_by_date on Centos7 in test_reuse_uefi_esp
- Use uefi_find_duplicate_entries to verify vmtest results

Requested reviews:
  Server Team CI bot (server-team-bot): continuous-integration
  curtin developers (curtin-dev)

For more details, see:
https://code.launchpad.net/~raharper/curtin/+git/curtin/+merge/390563
-- 
Your team curtin developers is requested to review the proposed merge of ~raharper/curtin:fix/refactor-uefi-duplicate-for-reuse into curtin:master.

diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py
index eaaae5e..4cf7301 100644
--- a/curtin/commands/curthooks.py
+++ b/curtin/commands/curthooks.py
@@ -557,13 +557,28 @@ def uefi_reorder_loaders(grubcfg, target, efi_orig=None, variant=None):
         LOG.debug("Currently booted UEFI loader might no longer boot.")
 
 
-def uefi_remove_duplicate_entries(grubcfg, target):
+def uefi_remove_duplicate_entries(grubcfg, target, to_remove=None):
     if not grubcfg.get('remove_duplicate_entries', True):
         LOG.debug("Skipped removing duplicate UEFI boot entries per config.")
         return
+    if to_remove is None:
+        to_remove = uefi_find_duplicate_entries(grubcfg, target)
+
+    # check so we don't run ChrootableTarget code unless we have things to do
+    if to_remove:
+        with util.ChrootableTarget(target) as in_chroot:
+            for bootnum, entry in to_remove:
+                LOG.debug('Removing duplicate EFI entry (%s, %s)',
+                          bootnum, entry)
+                in_chroot.subp(['efibootmgr', '--bootnum=%s' % bootnum,
+                                '--delete-bootnum'])
+
+
+def uefi_find_duplicate_entries(grubcfg, target, efi_output=None):
     seen = set()
     to_remove = []
-    efi_output = util.get_efibootmgr(target=target)
+    if efi_output is None:
+        efi_output = util.get_efibootmgr(target=target)
     entries = efi_output.get('entries', {})
     current_bootnum = efi_output.get('current', None)
     # adding BootCurrent to seen first allows us to remove any other duplicate
@@ -579,13 +594,7 @@ def uefi_remove_duplicate_entries(grubcfg, target):
             seen.add(t)
         else:
             to_remove.append((bootnum, entry))
-    if to_remove:
-        with util.ChrootableTarget(target) as in_chroot:
-            for bootnum, entry in to_remove:
-                LOG.debug('Removing duplicate EFI entry (%s, %s)',
-                          bootnum, entry)
-                in_chroot.subp(['efibootmgr', '--bootnum=%s' % bootnum,
-                                '--delete-bootnum'])
+    return to_remove
 
 
 def _debconf_multiselect(package, variable, choices):
diff --git a/tests/vmtests/test_reuse_uefi_esp.py b/tests/vmtests/test_reuse_uefi_esp.py
index 91e17ce..46e7ac7 100644
--- a/tests/vmtests/test_reuse_uefi_esp.py
+++ b/tests/vmtests/test_reuse_uefi_esp.py
@@ -3,20 +3,22 @@
 from .test_uefi_basic import TestBasicAbs
 from .releases import base_vm_classes as relbase
 from .releases import centos_base_vm_classes as cent_rbase
+from curtin.commands.curthooks import uefi_find_duplicate_entries
+from curtin import util
 
 
 class TestUefiReuseEspAbs(TestBasicAbs):
     conf_file = "examples/tests/uefi_reuse_esp.yaml"
 
     def test_efiboot_menu_has_one_distro_entry(self):
-        efiboot_mgr_content = self.load_collect_file("efibootmgr.out")
-        distro_lines = [line for line in efiboot_mgr_content.splitlines()
-                        if self.target_distro in line]
-        print(distro_lines)
-        self.assertEqual(1, len(distro_lines))
+        efi_output = util.parse_efibootmgr(
+            self.load_collect_file("efibootmgr.out"))
+        duplicates = uefi_find_duplicate_entries(
+            grubcfg=None, target=None, efi_output=efi_output)
+        print(duplicates)
+        self.assertEqual(0, len(duplicates))
 
 
-@TestUefiReuseEspAbs.skip_by_date("1881030", fixby="2020-07-15")
 class Cent70TestUefiReuseEsp(cent_rbase.centos70_bionic, TestUefiReuseEspAbs):
     __test__ = True