Merge ~raharper/curtin:fix/refactor-uefi-duplicate-for-reuse into curtin:master

Proposed by Ryan Harper
Status: Merged
Approved by: Paride Legovini
Approved revision: 76199108979409c68c821a0b3044d3ffd5810138
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~raharper/curtin:fix/refactor-uefi-duplicate-for-reuse
Merge into: curtin:master
Diff against target: 82 lines (+26/-15)
2 files modified
curtin/commands/curthooks.py (+18/-9)
tests/vmtests/test_reuse_uefi_esp.py (+8/-6)
Reviewer Review Type Date Requested Status
Paride Legovini Approve
Server Team CI bot continuous-integration Approve
Review via email: mp+390563@code.launchpad.net

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

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Paride Legovini (paride) wrote :

Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py
2index eaaae5e..4cf7301 100644
3--- a/curtin/commands/curthooks.py
4+++ b/curtin/commands/curthooks.py
5@@ -557,13 +557,28 @@ def uefi_reorder_loaders(grubcfg, target, efi_orig=None, variant=None):
6 LOG.debug("Currently booted UEFI loader might no longer boot.")
7
8
9-def uefi_remove_duplicate_entries(grubcfg, target):
10+def uefi_remove_duplicate_entries(grubcfg, target, to_remove=None):
11 if not grubcfg.get('remove_duplicate_entries', True):
12 LOG.debug("Skipped removing duplicate UEFI boot entries per config.")
13 return
14+ if to_remove is None:
15+ to_remove = uefi_find_duplicate_entries(grubcfg, target)
16+
17+ # check so we don't run ChrootableTarget code unless we have things to do
18+ if to_remove:
19+ with util.ChrootableTarget(target) as in_chroot:
20+ for bootnum, entry in to_remove:
21+ LOG.debug('Removing duplicate EFI entry (%s, %s)',
22+ bootnum, entry)
23+ in_chroot.subp(['efibootmgr', '--bootnum=%s' % bootnum,
24+ '--delete-bootnum'])
25+
26+
27+def uefi_find_duplicate_entries(grubcfg, target, efi_output=None):
28 seen = set()
29 to_remove = []
30- efi_output = util.get_efibootmgr(target=target)
31+ if efi_output is None:
32+ efi_output = util.get_efibootmgr(target=target)
33 entries = efi_output.get('entries', {})
34 current_bootnum = efi_output.get('current', None)
35 # adding BootCurrent to seen first allows us to remove any other duplicate
36@@ -579,13 +594,7 @@ def uefi_remove_duplicate_entries(grubcfg, target):
37 seen.add(t)
38 else:
39 to_remove.append((bootnum, entry))
40- if to_remove:
41- with util.ChrootableTarget(target) as in_chroot:
42- for bootnum, entry in to_remove:
43- LOG.debug('Removing duplicate EFI entry (%s, %s)',
44- bootnum, entry)
45- in_chroot.subp(['efibootmgr', '--bootnum=%s' % bootnum,
46- '--delete-bootnum'])
47+ return to_remove
48
49
50 def _debconf_multiselect(package, variable, choices):
51diff --git a/tests/vmtests/test_reuse_uefi_esp.py b/tests/vmtests/test_reuse_uefi_esp.py
52index 91e17ce..46e7ac7 100644
53--- a/tests/vmtests/test_reuse_uefi_esp.py
54+++ b/tests/vmtests/test_reuse_uefi_esp.py
55@@ -3,20 +3,22 @@
56 from .test_uefi_basic import TestBasicAbs
57 from .releases import base_vm_classes as relbase
58 from .releases import centos_base_vm_classes as cent_rbase
59+from curtin.commands.curthooks import uefi_find_duplicate_entries
60+from curtin import util
61
62
63 class TestUefiReuseEspAbs(TestBasicAbs):
64 conf_file = "examples/tests/uefi_reuse_esp.yaml"
65
66 def test_efiboot_menu_has_one_distro_entry(self):
67- efiboot_mgr_content = self.load_collect_file("efibootmgr.out")
68- distro_lines = [line for line in efiboot_mgr_content.splitlines()
69- if self.target_distro in line]
70- print(distro_lines)
71- self.assertEqual(1, len(distro_lines))
72+ efi_output = util.parse_efibootmgr(
73+ self.load_collect_file("efibootmgr.out"))
74+ duplicates = uefi_find_duplicate_entries(
75+ grubcfg=None, target=None, efi_output=efi_output)
76+ print(duplicates)
77+ self.assertEqual(0, len(duplicates))
78
79
80-@TestUefiReuseEspAbs.skip_by_date("1881030", fixby="2020-07-15")
81 class Cent70TestUefiReuseEsp(cent_rbase.centos70_bionic, TestUefiReuseEspAbs):
82 __test__ = True
83

Subscribers

People subscribed via source and target branches