curtin-dev team mailing list archive

[Michael Hudson-Doyle <mp+444530@xxxxxxxxxxxxxxxxxx>]

Michael Hudson-Doyle has proposed merging ~mwhudson/curtin:grub-config-object into curtin:master with ~mwhudson/curtin:modernize-uefi-code as a prerequisite.

Commit message:
use a config object in uefi boot entry code

a very small start!



Requested reviews:
  curtin developers (curtin-dev)

For more details, see:
https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/444530
-- 
Your team curtin developers is requested to review the proposed merge of ~mwhudson/curtin:grub-config-object into curtin:master.

diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py
index ee71f41..d011974 100644
--- a/curtin/commands/curthooks.py
+++ b/curtin/commands/curthooks.py
@@ -423,8 +423,9 @@ def install_kernel(cfg, target):
                      " System may not boot.", package)
 
 
-def uefi_remove_old_loaders(grubcfg, target):
+def uefi_remove_old_loaders(grubcfg: dict, target):
     """Removes the old UEFI loaders from efibootmgr."""
+    grubcfg = config.fromdict(config.GrubConfig, grubcfg)
     efi_state = util.get_efibootmgr(target)
 
     LOG.debug('UEFI remove old olders efi state:\n%s', efi_state)
@@ -441,7 +442,7 @@ def uefi_remove_old_loaders(grubcfg, target):
     if not old_efi_entries:
         return
 
-    if grubcfg.get('remove_old_uefi_loaders', True):
+    if grubcfg.remove_old_uefi_loaders:
         with util.ChrootableTarget(target) as in_chroot:
             for number, entry in old_efi_entries.items():
                 LOG.debug("removing old UEFI entry: %s", entry.name)
@@ -532,7 +533,9 @@ def uefi_reorder_loaders(
     is installed after the the previous first entry (before we installed grub).
 
     """
-    if not grubcfg.get('reorder_uefi', True):
+    grubcfg = config.fromdict(config.GrubConfig, grubcfg)
+
+    if not grubcfg.reorder_uefi:
         LOG.debug("Skipped reordering of UEFI boot methods.")
         LOG.debug("Currently booted UEFI loader might no longer boot.")
         return
@@ -541,10 +544,7 @@ def uefi_reorder_loaders(
     LOG.debug('UEFI efibootmgr output after install:\n%s', efi_state)
     new_boot_order = None
 
-    force_fallback_reorder = config.value_as_boolean(
-        grubcfg.get('reorder_uefi_force_fallback', False))
-
-    if efi_state.current and not force_fallback_reorder:
+    if efi_state.current and not grubcfg.reorder_uefi_force_fallback:
         boot_order = list(efi_state.order)
         if efi_state.current in boot_order:
             boot_order.remove(efi_state.current)
@@ -553,7 +553,7 @@ def uefi_reorder_loaders(
             "Setting currently booted %s as the first UEFI loader.",
             efi_state.current)
     else:
-        if force_fallback_reorder:
+        if grubcfg.reorder_uefi_force_fallback:
             reason = "config 'reorder_uefi_force_fallback' is True"
         else:
             reason = "missing 'BootCurrent' value"
@@ -579,11 +579,13 @@ def uefi_reorder_loaders(
 
 
 def uefi_remove_duplicate_entries(grubcfg: dict, target: str) -> None:
-    if not grubcfg.get('remove_duplicate_entries', True):
+    grubcfg = config.fromdict(config.GrubConfig, grubcfg)
+
+    if not grubcfg.remove_duplicate_entries:
         LOG.debug("Skipped removing duplicate UEFI boot entries per config.")
         return
 
-    to_remove = uefi_find_duplicate_entries(grubcfg, target)
+    to_remove = uefi_find_duplicate_entries(target)
 
     # check so we don't run ChrootableTarget code unless we have things to do
     if not to_remove:
@@ -596,7 +598,7 @@ def uefi_remove_duplicate_entries(grubcfg: dict, target: str) -> None:
                 ['efibootmgr', '--bootnum=%s' % bootnum, '--delete-bootnum'])
 
 
-def uefi_find_duplicate_entries(grubcfg: dict, target: str) \
+def uefi_find_duplicate_entries(target: str) \
       -> List[Tuple[str, util.EFIBootEntry]]:
     seen = set()
     to_remove = []
diff --git a/curtin/config.py b/curtin/config.py
index 2106b23..9b992fc 100644
--- a/curtin/config.py
+++ b/curtin/config.py
@@ -1,8 +1,10 @@
 # This file is part of curtin. See LICENSE file for copyright and license info.
 
-import yaml
 import json
 
+import attr
+import yaml
+
 ARCHIVE_HEADER = "#curtin-config-archive"
 ARCHIVE_TYPE = "text/curtin-config-archive"
 CONFIG_HEADER = "#curtin-config"
@@ -126,4 +128,22 @@ def value_as_boolean(value):
     false_values = (False, None, 0, '0', 'False', 'false', 'None', 'none', '')
     return value not in false_values
 
+
+@attr.s(auto_attribs=True)
+class GrubConfig:
+    remove_old_uefi_loaders: bool = True
+    reorder_uefi: bool = True
+    reorder_uefi_force_fallback: bool = attr.ib(
+        default=False, converter=value_as_boolean)
+    remove_duplicate_entries: bool = True
+
+
+def fromdict(cls, d):
+    kw = {}
+    for field in attr.fields(cls):
+        if field.name in d:
+            kw[field.name] = d[field.name]
+    return cls(**kw)
+
+
 # vi: ts=4 expandtab syntax=python
diff --git a/tests/unittests/test_curthooks.py b/tests/unittests/test_curthooks.py
index 86e588e..6067ea4 100644
--- a/tests/unittests/test_curthooks.py
+++ b/tests/unittests/test_curthooks.py
@@ -1103,7 +1103,7 @@ class TestUefiRemoveDuplicateEntries(CiTestCase):
     def test_uefi_remove_duplicate_entries(self):
         grubcfg = {}
         curthooks.uefi_remove_duplicate_entries(grubcfg, self.target)
-        self.assertEquals([
+        self.assertEqual([
             call(['efibootmgr', '--bootnum=0001', '--delete-bootnum'],
                  target=self.target),
             call(['efibootmgr', '--bootnum=0003', '--delete-bootnum'],
@@ -1117,7 +1117,7 @@ class TestUefiRemoveDuplicateEntries(CiTestCase):
         efiout.current = ''
         self.m_efibootmgr.return_value = efiout
         curthooks.uefi_remove_duplicate_entries(grubcfg, self.target)
-        self.assertEquals([
+        self.assertEqual([
             call(['efibootmgr', '--bootnum=0001', '--delete-bootnum'],
                  target=self.target),
             call(['efibootmgr', '--bootnum=0003', '--delete-bootnum'],
@@ -1139,7 +1139,7 @@ class TestUefiRemoveDuplicateEntries(CiTestCase):
         efiout.current = '0003'
         self.m_efibootmgr.return_value = efiout
         curthooks.uefi_remove_duplicate_entries(grubcfg, self.target)
-        self.assertEquals([
+        self.assertEqual([
             call(['efibootmgr', '--bootnum=0000', '--delete-bootnum'],
                  target=self.target),
             call(['efibootmgr', '--bootnum=0001', '--delete-bootnum'],
@@ -1168,7 +1168,7 @@ class TestUefiRemoveDuplicateEntries(CiTestCase):
                 ),
             })
         curthooks.uefi_remove_duplicate_entries(grubcfg, self.target)
-        self.assertEquals([], self.m_subp.call_args_list)
+        self.assertEqual([], self.m_subp.call_args_list)
 
 
 class TestUbuntuCoreHooks(CiTestCase):