Michael Hudson-Doyle has proposed merging ~mwhudson/curtin:modernize-uefi-code
into curtin:master.
Commit message:
modernize efi boot entry handling code
"modernize" means: type annotations and passing attr.s-based types
rather than dictionaries around.
I also made some changes to reduce indentation and removed a few
defaults for arguments that were always passed and a few other things I
couldn't restrain myself from.
Requested reviews:
curtin developers (curtin-dev)
For more details, see:
https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/444527
--
Your team curtin developers is requested to review the proposed merge of
~mwhudson/curtin:modernize-uefi-code into curtin:master.
diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py
index 74a67e8..ee71f41 100644
--- a/curtin/commands/curthooks.py
+++ b/curtin/commands/curthooks.py
@@ -8,6 +8,7 @@ import re
import sys
import shutil
import textwrap
+from typing import List, Tuple
from curtin import config
from curtin import block
@@ -424,32 +425,37 @@ def install_kernel(cfg, target):
def uefi_remove_old_loaders(grubcfg, target):
"""Removes the old UEFI loaders from efibootmgr."""
- efi_output = util.get_efibootmgr(target)
- LOG.debug('UEFI remove old olders efi output:\n%s', efi_output)
- current_uefi_boot = efi_output.get('current', None)
+ efi_state = util.get_efibootmgr(target)
+
+ LOG.debug('UEFI remove old olders efi state:\n%s', efi_state)
+
old_efi_entries = {
- entry: info
- for entry, info in efi_output['entries'].items()
- if re.match(r'^.*File\(\\EFI.*$', info['path'])
+ number: entry
+ for number, entry in efi_state.entries.items()
+ if 'File(\\EFI' in entry.path
}
- old_efi_entries.pop(current_uefi_boot, None)
- remove_old_loaders = grubcfg.get('remove_old_uefi_loaders', True)
- if old_efi_entries:
- if remove_old_loaders:
- with util.ChrootableTarget(target) as in_chroot:
- for entry, info in old_efi_entries.items():
- LOG.debug("removing old UEFI entry: %s" % info['name'])
- in_chroot.subp(
- ['efibootmgr', '-B', '-b', entry], capture=True)
- else:
+
+ if efi_state.current in old_efi_entries:
+ del old_efi_entries[efi_state.current]
+
+ if not old_efi_entries:
+ return
+
+ if grubcfg.get('remove_old_uefi_loaders', True):
+ with util.ChrootableTarget(target) as in_chroot:
+ for number, entry in old_efi_entries.items():
+ LOG.debug("removing old UEFI entry: %s", entry.name)
+ in_chroot.subp(
+ ['efibootmgr', '-B', '-b', number], capture=True)
+ else:
+ LOG.debug(
+ "Skipped removing %d old UEFI entrie%s.",
+ len(old_efi_entries),
+ '' if len(old_efi_entries) == 1 else 's')
+ for entry in old_efi_entries.values():
LOG.debug(
- "Skipped removing %d old UEFI entrie%s.",
- len(old_efi_entries),
- '' if len(old_efi_entries) == 1 else 's')
- for info in old_efi_entries.values():
- LOG.debug(
- "UEFI entry '%s' might no longer exist and "
- "should be removed.", info['name'])
+ "UEFI entry '%s' might no longer exist and "
+ "should be removed.", entry.name)
def uefi_boot_entry_is_network(boot_entry_name):
@@ -460,7 +466,11 @@ def uefi_boot_entry_is_network(boot_entry_name):
boot_entry_name, re.IGNORECASE) is not None
-def _reorder_new_entry(boot_order, efi_output, efi_orig=None, variant=None):
+def _reorder_new_entry(
+ efi_state: util.EFIBootState,
+ efi_orig: util.EFIBootState,
+ variant: str,
+ ) -> List[str]:
"""
Reorder the EFI boot menu as follows
@@ -470,32 +480,22 @@ def _reorder_new_entry(boot_order, efi_output, efi_orig=None, variant=None):
returns a list of bootnum strings
"""
-
- if not boot_order:
- raise RuntimeError('boot_order is not a list')
-
- if efi_orig is None:
- raise RuntimeError('Missing efi_orig boot dictionary')
-
- if variant is None:
- variant = ""
-
net_boot = []
other = []
target = []
- LOG.debug("UEFI previous boot order: %s", efi_orig['order'])
- LOG.debug("UEFI current boot order: %s", boot_order)
- new_entries = list(set(boot_order).difference(set(efi_orig['order'])))
+ LOG.debug("UEFI previous boot order: %s", efi_orig.order)
+ LOG.debug("UEFI current boot order: %s", efi_state.order)
+ new_entries = list(set(efi_state.order).difference(set(efi_orig.order)))
if new_entries:
LOG.debug("UEFI Found new boot entries: %s", new_entries)
LOG.debug('UEFI Looking for installed entry variant=%s', variant.lower())
- for bootnum in boot_order:
- entry = efi_output['entries'][bootnum]
- if uefi_boot_entry_is_network(entry['name']):
+ for bootnum in efi_state.order:
+ entry = efi_state.entries[bootnum]
+ if uefi_boot_entry_is_network(entry.name):
net_boot.append(bootnum)
else:
- if entry['name'].lower() == variant.lower():
+ if entry.name.lower() == variant.lower():
target.append(bootnum)
else:
other.append(bootnum)
@@ -505,17 +505,22 @@ def _reorder_new_entry(boot_order, efi_output, efi_orig=None, variant=None):
if other:
LOG.debug("UEFI found other entries: %s", other)
if target:
- LOG.debug("UEFI found target entry: %s", target)
+ LOG.debug("UEFI found target entries: %s", target)
else:
LOG.debug("UEFI Did not find an entry with variant=%s",
variant.lower())
new_order = net_boot + target + other
- if boot_order == new_order:
+ if efi_state.order == new_order:
LOG.debug("UEFI Current and Previous bootorders match")
return new_order
-def uefi_reorder_loaders(grubcfg, target, efi_orig=None, variant=None):
+def uefi_reorder_loaders(
+ grubcfg: dict,
+ target: str,
+ efi_orig_state: util.EFIBootState,
+ variant: str,
+ ) -> None:
"""Reorders the UEFI BootOrder to place BootCurrent first.
The specifically doesn't try to do to much. The order in which grub places
@@ -527,85 +532,88 @@ def uefi_reorder_loaders(grubcfg, target, efi_orig=None, variant=None):
is installed after the the previous first entry (before we installed grub).
"""
- if grubcfg.get('reorder_uefi', True):
- efi_output = util.get_efibootmgr(target=target)
- LOG.debug('UEFI efibootmgr output after install:\n%s', efi_output)
- currently_booted = efi_output.get('current', None)
- boot_order = efi_output.get('order', [])
- new_boot_order = None
- force_fallback_reorder = config.value_as_boolean(
- grubcfg.get('reorder_uefi_force_fallback', False))
- if currently_booted and force_fallback_reorder is False:
- if currently_booted in boot_order:
- boot_order.remove(currently_booted)
- boot_order = [currently_booted] + boot_order
- new_boot_order = ','.join(boot_order)
- LOG.debug(
- "Setting currently booted %s as the first "
- "UEFI loader.", currently_booted)
- else:
- reason = (
- "config 'reorder_uefi_force_fallback' is True" if
- force_fallback_reorder else "missing 'BootCurrent' value")
- LOG.debug("Using fallback UEFI reordering: " + reason)
- if len(boot_order) < 2:
- LOG.debug(
- 'UEFI BootOrder has less than 2 entries, cannot reorder')
- return
- # look at efi entries before we added one to find the new addition
- new_order = _reorder_new_entry(
- copy.deepcopy(boot_order), efi_output, efi_orig, variant)
- if new_order != boot_order:
- new_boot_order = ','.join(new_order)
- else:
- LOG.debug("UEFI No changes to boot order.")
- if new_boot_order:
- LOG.debug(
- "New UEFI boot order: %s", new_boot_order)
- with util.ChrootableTarget(target) as in_chroot:
- in_chroot.subp(['efibootmgr', '-o', new_boot_order])
- else:
+ if not grubcfg.get('reorder_uefi', True):
LOG.debug("Skipped reordering of UEFI boot methods.")
LOG.debug("Currently booted UEFI loader might no longer boot.")
+ return
+
+ efi_state = util.get_efibootmgr(target=target)
+ 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:
+ boot_order = list(efi_state.order)
+ if efi_state.current in boot_order:
+ boot_order.remove(efi_state.current)
+ new_boot_order = ','.join([efi_state.current] + boot_order)
+ LOG.debug(
+ "Setting currently booted %s as the first UEFI loader.",
+ efi_state.current)
+ else:
+ if force_fallback_reorder:
+ reason = "config 'reorder_uefi_force_fallback' is True"
+ else:
+ reason = "missing 'BootCurrent' value"
+ LOG.debug("Using fallback UEFI reordering: " + reason)
+
+ if len(efi_state.order) < 2:
+ LOG.debug(
+ 'UEFI BootOrder has less than 2 entries, cannot reorder')
+ return
+
+ # look at efi entries before we added one to find the new addition
+ new_order = _reorder_new_entry(efi_state, efi_orig_state, variant)
+
+ if new_order != efi_state.order:
+ new_boot_order = ','.join(new_order)
+ else:
+ LOG.debug("UEFI No changes to boot order.")
+
+ if new_boot_order:
+ LOG.debug("New UEFI boot order: %s", new_boot_order)
+ with util.ChrootableTarget(target) as in_chroot:
+ in_chroot.subp(['efibootmgr', '-o', new_boot_order])
-def uefi_remove_duplicate_entries(grubcfg, target, to_remove=None):
+def uefi_remove_duplicate_entries(grubcfg: dict, target: str) -> 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)
+
+ 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'])
+ if not to_remove:
+ return
+
+ 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):
+def uefi_find_duplicate_entries(grubcfg: dict, target: str) \
+ -> List[Tuple[str, util.EFIBootEntry]]:
seen = set()
to_remove = []
- if efi_output is None:
- efi_output = util.get_efibootmgr(target=target)
- entries = efi_output.get('entries', {})
- current_bootnum = efi_output.get('current', None)
+ efi_state = util.get_efibootmgr(target=target)
# adding BootCurrent to seen first allows us to remove any other duplicate
# entry of BootCurrent.
- if current_bootnum:
- seen.add(tuple(entries[current_bootnum].items()))
- for bootnum in sorted(entries):
- if bootnum == current_bootnum:
+ if efi_state.current in efi_state.entries:
+ current = efi_state.entries[efi_state.current]
+ seen.add((current.name, current.path))
+ for bootnum, entry in sorted(efi_state.entries.items()):
+ if bootnum == efi_state.current:
continue
- entry = entries[bootnum]
- t = tuple(entry.items())
- if t not in seen:
- seen.add(t)
- else:
+ t = (entry.name, entry.path)
+ if t in seen:
to_remove.append((bootnum, entry))
+ seen.add(t)
+
return to_remove
@@ -715,7 +723,7 @@ def uefi_find_grub_device_ids(sconfig):
return grub_device_ids
-def setup_grub(cfg, target, osfamily=DISTROS.debian, variant=None):
+def setup_grub(cfg, target, osfamily, variant):
# target is the path to the mounted filesystem
# FIXME: these methods need moving to curtin.block
@@ -815,13 +823,13 @@ def setup_grub(cfg, target, osfamily=DISTROS.debian, variant=None):
update_nvram = grubcfg.get('update_nvram', True)
if uefi_bootable and update_nvram:
- efi_orig_output = util.get_efibootmgr(target)
+ efi_orig_state = util.get_efibootmgr(target)
uefi_remove_old_loaders(grubcfg, target)
install_grub(instdevs, target, uefi=uefi_bootable, grubcfg=grubcfg)
if uefi_bootable and update_nvram:
- uefi_reorder_loaders(grubcfg, target, efi_orig_output, variant)
+ uefi_reorder_loaders(grubcfg, target, efi_orig_state, variant)
uefi_remove_duplicate_entries(grubcfg, target)
diff --git a/curtin/util.py b/curtin/util.py
index 8a9e8f4..08c1fd5 100644
--- a/curtin/util.py
+++ b/curtin/util.py
@@ -16,6 +16,9 @@ import stat
import sys
import tempfile
import time
+from typing import Dict, List, Optional
+
+import attr
# avoid the dependency to python3-six as used in cloud-init
try:
@@ -907,63 +910,62 @@ def is_uefi_bootable():
return os.path.exists('/sys/firmware/efi') is True
-def parse_efibootmgr(content):
- efikey_to_dict_key = {
+@attr.s(auto_attribs=True)
+class EFIBootEntry:
+ name: str
+ path: str
+
+
+@attr.s(auto_attribs=True)
+class EFIBootState:
+ current: str
+ timeout: str
+ order: List[str]
+ entries: Dict[str, EFIBootEntry] = attr.Factory(dict)
+
+
+EFI_BOOT_ENTRY_LINE_REGEXP = \
+ r"^Boot(?P<entry>[0-9a-fA-F]{4})\*?\s(?P<name>.+)\t(?P<path>.*)$"
+
+
+def parse_efibootmgr(content: str) -> EFIBootState:
+ efikey_to_attr = {
'BootCurrent': 'current',
'Timeout': 'timeout',
'BootOrder': 'order',
}
- output = {}
+ args = {'current': '', 'timeout': '', 'order': ''}
+
for line in content.splitlines():
split = line.split(':')
- if len(split) == 2:
- key = split[0].strip()
- output_key = efikey_to_dict_key.get(key, None)
- if output_key:
- output[output_key] = split[1].strip()
- if output_key == 'order':
- output[output_key] = output[output_key].split(',')
- output['entries'] = {
- entry: {
- 'name': name.strip(),
- 'path': path.strip(),
- }
- for entry, name, path in re.findall(
- r"^Boot(?P<entry>[0-9a-fA-F]{4})\*?\s(?P<name>.+)\t"
- r"(?P<path>.*)$",
- content, re.MULTILINE)
- }
- if 'order' in output:
- new_order = [item for item in output['order']
- if item in output['entries']]
- output['order'] = new_order
- return output
-
-
-def get_efibootmgr(target=None):
- """Return mapping of EFI information.
-
- Calls `efibootmgr` inside the `target`.
-
- Example output:
- {
- 'current': '0000',
- 'timeout': '1 seconds',
- 'order': ['0000', '0001'],
- 'entries': {
- '0000': {
- 'name': 'ubuntu',
- 'path': (
- 'HD(1,GPT,0,0x8,0x1)/File(\\EFI\\ubuntu\\shimx64.efi)'),
- },
- '0001': {
- 'name': 'UEFI:Network Device',
- 'path': 'BBS(131,,0x0)',
- }
- }
- }
- """
+ if len(split) != 2:
+ continue
+ key, val = split
+ attr = efikey_to_attr.get(key.strip())
+ if not attr:
+ continue
+ args[attr] = val.strip()
+
+ print(args)
+ args['order'] = args['order'].split(',')
+
+ state = EFIBootState(**args)
+
+ for line in content.splitlines():
+ match = re.match(EFI_BOOT_ENTRY_LINE_REGEXP, line)
+ if match is None:
+ continue
+ state.entries[match['entry']] = EFIBootEntry(
+ name=match['name'].strip(), path=match['path'].strip())
+
+ state.order = [item for item in state.order if item in state.entries]
+
+ return state
+
+
+def get_efibootmgr(target: Optional[str] = None) -> EFIBootState:
+ """Return information about current EFI boot state."""
with ChrootableTarget(target=target) as in_chroot:
stdout, _ = in_chroot.subp(['efibootmgr', '-v'], capture=True)
output = parse_efibootmgr(stdout)
diff --git a/tests/unittests/test_curthooks.py b/tests/unittests/test_curthooks.py
index 9e4fa87..86e588e 100644
--- a/tests/unittests/test_curthooks.py
+++ b/tests/unittests/test_curthooks.py
@@ -1,9 +1,11 @@
# This file is part of curtin. See LICENSE file for copyright and license info.
-import copy
import os
from mock import call, patch
import textwrap
+from typing import Optional
+
+import attr
from curtin.commands import curthooks
from curtin.commands.block_meta import extract_storage_ordered_dict
@@ -576,44 +578,37 @@ class TestSetupZipl(CiTestCase):
self.assertIn('root={}'.format(root_dev), content)
-class EfiOutput(object):
-
- def __init__(self, current=None, order=None, entries=None):
- self.entries = {}
- if entries:
- for entry in entries:
- self.entries.update(entry)
- self.current = current
- self.order = order
- if not order and self.entries:
- self.order = sorted(self.entries.keys())
-
- def add_entry(self, bootnum=None, name=None, path=None, current=False):
- if not bootnum:
- bootnum = "%04x" % random.randint(0, 1000)
- if not name:
- name = CiTestCase.random_string()
- if not path:
- path = ''
- if bootnum not in self.entries:
- self.entries[bootnum] = {'name': name, 'path': path}
- if not self.order:
- self.order = []
- self.order.append(bootnum)
- if current:
- self.current = bootnum
-
- def set_order(self, new_order):
- self.order = new_order
-
- def as_dict(self):
- output = {}
- if self.current:
- output['current'] = self.current
- if self.order:
- output['order'] = self.order
- output['entries'] = self.entries
- return output
+def make_efi_state() -> util.EFIBootState:
+ return util.EFIBootState(current='', timeout='', order=[])
+
+
+def copy_efi_state(orig: util.EFIBootState) -> util.EFIBootState:
+ kw = attr.asdict(orig)
+ kw['entries'] = {
+ bootnum: util.EFIBootEntry(**d)
+ for bootnum, d in kw['entries'].items()
+ }
+ return util.EFIBootState(**kw)
+
+
+def add_efi_entry(
+ state: util.EFIBootState,
+ bootnum: Optional[str] = None,
+ name: Optional[str] = None,
+ path: Optional[str] = None,
+ current: bool = False) -> None:
+ if not bootnum:
+ bootnum = "%04x" % random.randint(0, 1000)
+ if not name:
+ name = CiTestCase.random_string()
+ if not path:
+ path = ''
+ if bootnum not in state.entries:
+ state.entries[bootnum] = util.EFIBootEntry(
+ name=name, path=path)
+ state.order.append(bootnum)
+ if current:
+ state.current = bootnum
class TestSetupGrub(CiTestCase):
@@ -655,7 +650,9 @@ class TestSetupGrub(CiTestCase):
'install_devices': ['/dev/vdb'],
},
}
- curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family)
+ curthooks.setup_grub(
+ cfg, self.target,
+ osfamily=self.distro_family, variant=self.variant)
self.m_install_grub.assert_called_with(
['/dev/vdb'], self.target, uefi=False, grubcfg=cfg.get('grub'))
@@ -763,16 +760,15 @@ class TestSetupGrub(CiTestCase):
},
}
self.mock_haspkg.return_value = False
- self.mock_efibootmgr.return_value = {
- 'current': '0000',
- 'entries': {
- '0000': {
- 'name': 'ubuntu',
- 'path': (
- 'HD(1,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)'),
- }
- }
- }
+ self.mock_efibootmgr.return_value = util.EFIBootState(
+ current='0000',
+ timeout='',
+ order=[],
+ entries={
+ '0000': util.EFIBootEntry(
+ name='ubuntu',
+ path='HD(1,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)'),
+ })
curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family,
variant=self.variant)
self.m_install_grub.assert_called_with(
@@ -793,26 +789,21 @@ class TestSetupGrub(CiTestCase):
'reorder_uefi': False,
},
}
- self.mock_efibootmgr.return_value = {
- 'current': '0000',
- 'entries': {
- '0000': {
- 'name': 'ubuntu',
- 'path': (
- 'HD(1,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)'),
- },
- '0001': {
- 'name': 'centos',
- 'path': (
- 'HD(1,GPT)/File(\\EFI\\centos\\shimx64.efi)'),
- },
- '0002': {
- 'name': 'sles',
- 'path': (
- 'HD(1,GPT)/File(\\EFI\\sles\\shimx64.efi)'),
- },
- }
- }
+ self.mock_efibootmgr.return_value = util.EFIBootState(
+ current='0000',
+ timeout='',
+ order=[],
+ entries={
+ '0000': util.EFIBootEntry(
+ name='ubuntu',
+ path='HD(1,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)'),
+ '0001': util.EFIBootEntry(
+ name='centos',
+ path='HD(1,GPT)/File(\\EFI\\centos\\shimx64.efi)'),
+ '0002': util.EFIBootEntry(
+ name='sles',
+ path='HD(1,GPT)/File(\\EFI\\sles\\shimx64.efi)'),
+ })
self.mock_haspkg.return_value = False
curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family,
variant=self.variant)
@@ -840,21 +831,21 @@ class TestSetupGrub(CiTestCase):
'reorder_uefi': True,
},
}
- self.mock_efibootmgr.return_value = {
- 'current': '0001',
- 'order': ['0000', '0001'],
- 'entries': {
- '0000': {
- 'name': 'ubuntu',
- 'path': (
- 'HD(1,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)'),
- },
- '0001': {
- 'name': 'UEFI:Network Device',
- 'path': 'BBS(131,,0x0)',
- },
- }
- }
+ self.mock_efibootmgr.return_value = util.EFIBootState(
+ current='0001',
+ timeout='',
+ order=['0000', '0001'],
+ entries={
+ '0000': util.EFIBootEntry(
+ name='ubuntu',
+ path='HD(1,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)'),
+ '0001': util.EFIBootEntry(
+ name='UEFI:Network Device',
+ path='BBS(131,,0x0)'),
+ '0002': util.EFIBootEntry(
+ name='sles',
+ path='HD(1,GPT)/File(\\EFI\\sles\\shimx64.efi)'),
+ })
self.mock_haspkg.return_value = False
curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family,
variant=self.variant)
@@ -878,29 +869,29 @@ class TestSetupGrub(CiTestCase):
}
# Single existing entry 0001
- efi_orig = EfiOutput()
- efi_orig.add_entry(bootnum='0001', name='centos')
+ orig_state = make_efi_state()
+ add_efi_entry(orig_state, bootnum='0001', name='centos')
# After install add a second entry, 0000 to the front of order
- efi_post = copy.deepcopy(efi_orig)
- efi_post.add_entry(bootnum='0000', name='ubuntu')
- efi_post.set_order(['0000', '0001'])
+ post_state = copy_efi_state(orig_state)
+ add_efi_entry(post_state, bootnum='0000', name='ubuntu')
+ post_state.order = ['0000', '0001']
- # After reorder we should have the target install first
- efi_final = copy.deepcopy(efi_post)
+ final_state = copy_efi_state(post_state)
self.mock_efibootmgr.side_effect = iter([
- efi_orig.as_dict(), # collect original order before install
- efi_orig.as_dict(), # remove_old_loaders query (no change)
- efi_post.as_dict(), # efi table after grub install, (changed)
- efi_final.as_dict(), # remove duplicates checks and finds reorder
- # has changed
+ orig_state, # collect original order before install
+ orig_state, # remove_old_loaders query (no change)
+ post_state, # efi table after grub install, (changed)
+ final_state, # remove duplicates checks and finds reorder has
+ # changed
])
self.mock_haspkg.return_value = False
curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family,
variant=self.variant)
logs = self.logs.getvalue()
print(logs)
+ print(self.mock_subp.call_args_list)
self.assertEquals([], self.mock_subp.call_args_list)
self.assertIn("Using fallback UEFI reordering:", logs)
self.assertIn("missing 'BootCurrent' value", logs)
@@ -924,25 +915,26 @@ class TestSetupGrub(CiTestCase):
}
# Existing Custom Ubuntu, usb and cd/dvd entry, booting Ubuntu
- efi_orig = EfiOutput()
- efi_orig.add_entry(bootnum='0001', name='Ubuntu Deluxe Edition')
- efi_orig.add_entry(bootnum='0002', name='USB Device')
- efi_orig.add_entry(bootnum='0000', name='CD/DVD')
- efi_orig.set_order(['0001', '0002', '0000'])
+ orig_state = make_efi_state()
+ add_efi_entry(orig_state, bootnum='0001', name='Ubuntu Deluxe Edition')
+ add_efi_entry(orig_state, bootnum='0002', name='USB Device')
+ add_efi_entry(orig_state, bootnum='0000', name='CD/DVD')
+ orig_state.order = ['0001', '0002', '0000']
# after install existing ubuntu entry is reused, no change in order
- efi_post = efi_orig
+ post_state = copy_efi_state(orig_state)
# after reorder, no change is made due to the installed distro variant
# string 'ubuntu' is not found in the boot entries so we retain the
# original efi order.
- efi_final = efi_post
+ final_state = copy_efi_state(post_state)
self.mock_efibootmgr.side_effect = iter([
- efi_orig.as_dict(), # collect original order before install
- efi_orig.as_dict(), # remove_old_loaders query
- efi_post.as_dict(), # reorder entries queries post install
- efi_final.as_dict(), # remove duplicates checks and finds reorder
+ orig_state, # collect original order before install
+ post_state, # remove_old_loaders query (no change)
+ post_state, # efi table after grub install, (changed)
+ final_state, # remove duplicates checks and finds reorder has
+ # changed
])
self.mock_haspkg.return_value = False
@@ -976,26 +968,30 @@ class TestSetupGrub(CiTestCase):
}
# Single existing entry 0001 and set as current, which should avoid
# any fallback logic, but we're forcing fallback pack via config
- efi_orig = EfiOutput()
- efi_orig.add_entry(bootnum='0001', name='PXE', current=True)
- print(efi_orig.as_dict())
+ orig_state = make_efi_state()
+ add_efi_entry(orig_state, bootnum='0001', name='PXE', current=True)
# After install add a second entry, 0000 to the front of order
- efi_post = copy.deepcopy(efi_orig)
- efi_post.add_entry(bootnum='0000', name='ubuntu')
- efi_post.set_order(['0000', '0001'])
- print(efi_orig.as_dict())
+ post_state = copy_efi_state(orig_state)
+ add_efi_entry(post_state, bootnum='0000', name='ubuntu')
+
+ final_state = copy_efi_state(post_state)
+
+ # After install add a second entry, 0000 to the front of order
+ post_state = copy_efi_state(orig_state)
+ add_efi_entry(post_state, bootnum='0000', name='ubuntu')
+ post_state.order = ['0000', '0001']
# After reorder we should have the original boot entry 0001 as first
- efi_final = copy.deepcopy(efi_post)
- efi_final.set_order(['0001', '0000'])
+ final_state = copy_efi_state(post_state)
+ final_state.order = ['0001', '0000']
self.mock_efibootmgr.side_effect = iter([
- efi_orig.as_dict(), # collect original order before install
- efi_orig.as_dict(), # remove_old_loaders query (no change)
- efi_post.as_dict(), # efi table after grub install, (changed)
- efi_final.as_dict(), # remove duplicates checks and finds reorder
- # has changed
+ orig_state, # collect original order before install
+ post_state, # remove_old_loaders query (no change)
+ post_state, # efi table after grub install, (changed)
+ final_state, # remove duplicates checks and finds reorder has
+ # changed
])
self.mock_haspkg.return_value = False
@@ -1003,7 +999,8 @@ class TestSetupGrub(CiTestCase):
variant=self.variant)
logs = self.logs.getvalue()
print(logs)
- self.assertEquals([
+ print(self.mock_subp.call_args_list)
+ self.assertEqual([
call(['efibootmgr', '-o', '0001,0000'], target=self.target)],
self.mock_subp.call_args_list)
self.assertIn("Using fallback UEFI reordering:", logs)
@@ -1025,31 +1022,29 @@ class TestSetupGrub(CiTestCase):
}
# Existing ubuntu, usb and cd/dvd entry, booting ubuntu
- efi_orig = EfiOutput()
- efi_orig.add_entry(bootnum='0001', name='centos')
- efi_orig.add_entry(bootnum='0002', name='Network')
- efi_orig.add_entry(bootnum='0003', name='PXE')
- efi_orig.add_entry(bootnum='0004', name='LAN')
- efi_orig.add_entry(bootnum='0000', name='CD/DVD')
- efi_orig.set_order(['0001', '0002', '0003', '0004', '0000'])
- print(efi_orig.as_dict())
+ orig_state = make_efi_state()
+ add_efi_entry(orig_state, bootnum='0001', name='centos')
+ add_efi_entry(orig_state, bootnum='0002', name='Network')
+ add_efi_entry(orig_state, bootnum='0003', name='PXE')
+ add_efi_entry(orig_state, bootnum='0004', name='LAN')
+ add_efi_entry(orig_state, bootnum='0000', name='CD/DVD')
+ orig_state.order = ['0001', '0002', '0003', '0004', '0000']
# after install we add an ubuntu entry, and grub puts it first
- efi_post = copy.deepcopy(efi_orig)
- efi_post.add_entry(bootnum='0007', name='ubuntu')
- efi_post.set_order(['0007'] + efi_orig.order)
- print(efi_post.as_dict())
+ post_state = copy_efi_state(orig_state)
+ add_efi_entry(post_state, bootnum='0007', name='ubuntu')
+ post_state.order = ['0007'] + orig_state.order
# reorder must place all network devices first, then ubuntu, and others
- efi_final = copy.deepcopy(efi_post)
- expected_order = ['0002', '0003', '0004', '0007', '0001', '0000']
- efi_final.set_order(expected_order)
+ final_state = copy_efi_state(post_state)
+ final_state.order = ['0002', '0003', '0004', '0007', '0001', '0000']
self.mock_efibootmgr.side_effect = iter([
- efi_orig.as_dict(), # collect original order before install
- efi_orig.as_dict(), # remove_old_loaders query
- efi_post.as_dict(), # reorder entries queries post install
- efi_final.as_dict(), # remove duplicates checks and finds reorder
+ orig_state, # collect original order before install
+ post_state, # remove_old_loaders query (no change)
+ post_state, # efi table after grub install, (changed)
+ final_state, # remove duplicates checks and finds reorder has
+ # changed
])
self.mock_haspkg.return_value = False
curthooks.setup_grub(cfg, self.target, osfamily=self.distro_family,
@@ -1058,7 +1053,7 @@ class TestSetupGrub(CiTestCase):
print(logs)
print('Number of bootmgr calls: %s' % self.mock_efibootmgr.call_count)
self.assertEquals([
- call(['efibootmgr', '-o', '%s' % (",".join(expected_order))],
+ call(['efibootmgr', '-o', '%s' % (",".join(final_state.order))],
target=self.target)],
self.mock_subp.call_args_list)
self.assertIn("Using fallback UEFI reordering:", logs)
@@ -1066,43 +1061,43 @@ class TestSetupGrub(CiTestCase):
self.assertIn("Looking for installed entry variant=", logs)
self.assertIn("found netboot entries: ['0002', '0003', '0004']", logs)
self.assertIn("found other entries: ['0001', '0000']", logs)
- self.assertIn("found target entry: ['0007']", logs)
+ self.assertIn("found target entries: ['0007']", logs)
class TestUefiRemoveDuplicateEntries(CiTestCase):
- efibootmgr_output = {
- 'current': '0000',
- 'entries': {
- '0000': {
- 'name': 'ubuntu',
- 'path': (
- 'HD(1,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)'),
- },
- '0001': { # Is duplicate of 0000
- 'name': 'ubuntu',
- 'path': (
- 'HD(1,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)'),
- },
- '0002': { # Is not a duplicate because of unique path
- 'name': 'ubuntu',
- 'path': (
- 'HD(2,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)'),
- },
- '0003': { # Is duplicate of 0000
- 'name': 'ubuntu',
- 'path': (
- 'HD(1,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)'),
- },
- }
- }
+ efibootmgr_output = util.EFIBootState(
+ current='0000',
+ order='',
+ timeout='',
+ entries={
+ '0000': util.EFIBootEntry(
+ name='ubuntu',
+ path='HD(1,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)',
+ ),
+ '0001': util.EFIBootEntry(
+ # Is duplicate of 0000
+ name='ubuntu',
+ path='HD(1,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)',
+ ),
+ '0002': util.EFIBootEntry(
+ # Is not a duplicate because of unique path
+ name='ubuntu',
+ path='HD(2,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)',
+ ),
+ '0003': util.EFIBootEntry(
+ # Is duplicate of 0000
+ name='ubuntu',
+ path='HD(1,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)',
+ ),
+ })
def setUp(self):
super(TestUefiRemoveDuplicateEntries, self).setUp()
self.target = self.tmp_dir()
self.add_patch('curtin.util.get_efibootmgr', 'm_efibootmgr')
self.add_patch('curtin.util.subp', 'm_subp')
- self.m_efibootmgr.return_value = copy.deepcopy(self.efibootmgr_output)
+ self.m_efibootmgr.return_value = copy_efi_state(self.efibootmgr_output)
@patch.object(util.ChrootableTarget, "__enter__", new=lambda a: a)
def test_uefi_remove_duplicate_entries(self):
@@ -1118,8 +1113,8 @@ class TestUefiRemoveDuplicateEntries(CiTestCase):
@patch.object(util.ChrootableTarget, "__enter__", new=lambda a: a)
def test_uefi_remove_duplicate_entries_no_bootcurrent(self):
grubcfg = {}
- efiout = copy.deepcopy(self.efibootmgr_output)
- del efiout['current']
+ efiout = copy_efi_state(self.efibootmgr_output)
+ efiout.current = ''
self.m_efibootmgr.return_value = efiout
curthooks.uefi_remove_duplicate_entries(grubcfg, self.target)
self.assertEquals([
@@ -1140,8 +1135,8 @@ class TestUefiRemoveDuplicateEntries(CiTestCase):
@patch.object(util.ChrootableTarget, "__enter__", new=lambda a: a)
def test_uefi_remove_duplicate_entries_skip_bootcurrent(self):
grubcfg = {}
- efiout = copy.deepcopy(self.efibootmgr_output)
- efiout['current'] = '0003'
+ efiout = copy_efi_state(self.efibootmgr_output)
+ efiout.current = '0003'
self.m_efibootmgr.return_value = efiout
curthooks.uefi_remove_duplicate_entries(grubcfg, self.target)
self.assertEquals([
@@ -1154,26 +1149,24 @@ class TestUefiRemoveDuplicateEntries(CiTestCase):
@patch.object(util.ChrootableTarget, "__enter__", new=lambda a: a)
def test_uefi_remove_duplicate_entries_no_change(self):
grubcfg = {}
- self.m_efibootmgr.return_value = {
- 'current': '0000',
- 'entries': {
- '0000': {
- 'name': 'ubuntu',
- 'path': (
- 'HD(1,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)'),
- },
- '0001': {
- 'name': 'centos',
- 'path': (
- 'HD(1,GPT)/File(\\EFI\\centos\\shimx64.efi)'),
- },
- '0002': {
- 'name': 'sles',
- 'path': (
- 'HD(1,GPT)/File(\\EFI\\sles\\shimx64.efi)'),
- },
- }
- }
+ self.m_efibootmgr.return_value = util.EFIBootState(
+ order=[],
+ timeout='',
+ current='0000',
+ entries={
+ '0000': util.EFIBootEntry(
+ name='ubuntu',
+ path='HD(1,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)',
+ ),
+ '0001': util.EFIBootEntry(
+ name='centos',
+ path='HD(1,GPT)/File(\\EFI\\centos\\shimx64.efi)',
+ ),
+ '0002': util.EFIBootEntry(
+ name='sles',
+ path='HD(1,GPT)/File(\\EFI\\sles\\shimx64.efi)',
+ ),
+ })
curthooks.uefi_remove_duplicate_entries(grubcfg, self.target)
self.assertEquals([], self.m_subp.call_args_list)
diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py
index 6a0c951..f259da4 100644
--- a/tests/unittests/test_util.py
+++ b/tests/unittests/test_util.py
@@ -893,7 +893,7 @@ class TestGetEFIBootMGR(CiTestCase):
def test_calls_efibootmgr_verbose(self):
self.in_chroot_subp_output.append(('', ''))
util.get_efibootmgr('target')
- self.assertEquals(
+ self.assertEqual(
(['efibootmgr', '-v'],),
self.mock_in_chroot_subp.call_args_list[0][0])
@@ -911,37 +911,38 @@ class TestGetEFIBootMGR(CiTestCase):
Boot0005* UEFI:Network Device BBS(131,,0x0)
"""), ''))
observed = util.get_efibootmgr('target')
- self.assertEquals({
- 'current': '0000',
- 'timeout': '1 seconds',
- 'order': ['0000', '0002', '0001', '0003', '0004', '0005'],
- 'entries': {
- '0000': {
- 'name': 'ubuntu',
- 'path': 'HD(1,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)',
- },
- '0001': {
- 'name': 'CD/DVD Drive',
- 'path': 'BBS(CDROM,,0x0)',
- },
- '0002': {
- 'name': 'Hard Drive',
- 'path': 'BBS(HD,,0x0)',
- },
- '0003': {
- 'name': 'UEFI:CD/DVD Drive',
- 'path': 'BBS(129,,0x0)',
- },
- '0004': {
- 'name': 'UEFI:Removable Device',
- 'path': 'BBS(130,,0x0)',
- },
- '0005': {
- 'name': 'UEFI:Network Device',
- 'path': 'BBS(131,,0x0)',
- },
- }
- }, observed)
+ expected = util.EFIBootState(
+ current='0000',
+ timeout='1 seconds',
+ order=['0000', '0002', '0001', '0003', '0004', '0005'],
+ entries={
+ '0000': util.EFIBootEntry(
+ name='ubuntu',
+ path='HD(1,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)',
+ ),
+ '0001': util.EFIBootEntry(
+ name='CD/DVD Drive',
+ path='BBS(CDROM,,0x0)',
+ ),
+ '0002': util.EFIBootEntry(
+ name='Hard Drive',
+ path='BBS(HD,,0x0)',
+ ),
+ '0003': util.EFIBootEntry(
+ name='UEFI:CD/DVD Drive',
+ path='BBS(129,,0x0)',
+ ),
+ '0004': util.EFIBootEntry(
+ name='UEFI:Removable Device',
+ path='BBS(130,,0x0)',
+ ),
+ '0005': util.EFIBootEntry(
+ name='UEFI:Network Device',
+ path='BBS(131,,0x0)',
+ ),
+ })
+
+ self.assertEqual(expected, observed)
def test_parses_output_filter_missing(self):
"""ensure parsing ignores items in order that don't have entries"""
@@ -958,37 +959,37 @@ class TestGetEFIBootMGR(CiTestCase):
Boot0005* UEFI:Network Device BBS(131,,0x0)
"""), ''))
observed = util.get_efibootmgr('target')
- self.assertEquals({
- 'current': '0000',
- 'timeout': '1 seconds',
- 'order': ['0000', '0002', '0001', '0003', '0004', '0005'],
- 'entries': {
- '0000': {
- 'name': 'ubuntu',
- 'path': 'HD(1,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)',
- },
- '0001': {
- 'name': 'CD/DVD Drive',
- 'path': 'BBS(CDROM,,0x0)',
- },
- '0002': {
- 'name': 'Hard Drive',
- 'path': 'BBS(HD,,0x0)',
- },
- '0003': {
- 'name': 'UEFI:CD/DVD Drive',
- 'path': 'BBS(129,,0x0)',
- },
- '0004': {
- 'name': 'UEFI:Removable Device',
- 'path': 'BBS(130,,0x0)',
- },
- '0005': {
- 'name': 'UEFI:Network Device',
- 'path': 'BBS(131,,0x0)',
- },
- }
- }, observed)
+ expected = util.EFIBootState(
+ current='0000',
+ timeout='1 seconds',
+ order=['0000', '0002', '0001', '0003', '0004', '0005'],
+ entries={
+ '0000': util.EFIBootEntry(
+ name='ubuntu',
+ path='HD(1,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)',
+ ),
+ '0001': util.EFIBootEntry(
+ name='CD/DVD Drive',
+ path='BBS(CDROM,,0x0)',
+ ),
+ '0002': util.EFIBootEntry(
+ name='Hard Drive',
+ path='BBS(HD,,0x0)',
+ ),
+ '0003': util.EFIBootEntry(
+ name='UEFI:CD/DVD Drive',
+ path='BBS(129,,0x0)',
+ ),
+ '0004': util.EFIBootEntry(
+ name='UEFI:Removable Device',
+ path='BBS(130,,0x0)',
+ ),
+ '0005': util.EFIBootEntry(
+ name='UEFI:Network Device',
+ path='BBS(131,,0x0)',
+ ),
+ })
+ self.assertEqual(expected, observed)
class TestUsesSystemd(CiTestCase):
--
Mailing list: https://launchpad.net/~curtin-dev
Post to : curtin-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~curtin-dev
More help : https://help.launchpad.net/ListHelp
