curtin

Merge lp:~blake-rouse/curtin/uefi-clear-reorder into lp:~curtin-dev/curtin/trunk

  1. uefi-clear-reorder
  2. Merge into trunk
Proposed by Blake Rouse on 2017-05-10
Status: Merged
Merged at revision: 503
Proposed branch: lp:~blake-rouse/curtin/uefi-clear-reorder
Merge into: lp:~curtin-dev/curtin/trunk
Diff against target: 544 lines (+450/-4)
4 files modified
curtin/commands/curthooks.py (+66/-1)
curtin/util.py (+53/-0)
tests/unittests/test_curthooks.py (+261/-3)
tests/unittests/test_util.py (+70/-0)
To merge this branch: bzr merge lp:~blake-rouse/curtin/uefi-clear-reorder
Reviewer Review Type Date Requested Status
Ryan Harper 2017-05-10 Approve on 2017-05-23
Server Team CI bot continuous-integration Approve on 2017-05-12
Review via email: mp+323875@code.launchpad.net

Commit Message

Clear and re-order UEFI boot methods during UEFI grub installation.

Previously when installing Ubuntu using curtin it was default to pass '--no-nvram' to the grub-install. This branch reverts that has passing '--no-nvram' will prevent the system from booting if MAAS is down because Ubuntu not be a loader in the EFI system for the system to fallback on. When updating the nvram grub places Ubuntu before the currently booted method, which prevents the ability for the machine to boot from the network anymore. This branch reorders to boot order of the EFI system to place the currently booted method before all others, but Ubuntu is still second in the list so if MAAS is down the machine will still boot from its local disk correctly.

Another issue is that older EFI loaders would be present in the EFI firmware even through curtin deletes and re-creates the entire EFI partition. This removes only those loaders before grub-install is performed to make sure that only the relevant loads for the current state of the system are present.

To post a comment you must log in.
lp:~blake-rouse/curtin/uefi-clear-reorder updated on 2017-05-10
497. By Blake Rouse on 2017-05-10

Link bug.

Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
lp:~blake-rouse/curtin/uefi-clear-reorder updated on 2017-05-11
498. By Blake Rouse on 2017-05-11

Link bug.

Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Ryan Harper (raharper) wrote :

Thanks for working on this.

Let's have a

uefi_get_boot_config(grubcfg, target)

which returns a dictionary(ordered, I suppose) with the key, value pairs of the current menu.
That's nicer than reparsing

uefi_remove_old_loaders(grubcfg, target)

which handles the removal of previously installed File loaders

uefi_set_boot_order(grugcfg, bootorder, target)

which sets the order.

If I understand the flow, we're looking to achieve:

1. removal of any FILE boot menu entries
2. installation of a new entry for what we just installed
3. placing the currently booted method first.

Instead of doing(1) which would end up removing entries that we may not have written
could we instead collect the current entries and order, perform a grub install, collect
the new menu; then set the new boot order with current first,
and pick out of the menu the *new* entry we created? I believe this is more robust
since (3) assumes that the first entry the boot order list is the newly created
boot entry; but does not verify that. There may be non-FILE based entries in the menu
that this code does not consider.

The FILE entry includes the part UUID, which curtin can find since it knowns
the partition that was used for grub install. I think we should ensure that the
second entry should be the newly installed partition.

Also noted in IRC from this morning, we should have unittests for the efibootmgr output manipulation.

Some questions in-line below

review: Needs Fixing
Blake Rouse (blake-rouse) wrote :

> Thanks for working on this.
>
> Let's have a
>
> uefi_get_boot_config(grubcfg, target)
>
> which returns a dictionary(ordered, I suppose) with the key, value pairs of
> the current menu.
> That's nicer than reparsing

What is being reparsed? It has to reparse after grub-install because it adds its entry.

>
>
> uefi_remove_old_loaders(grubcfg, target)
>
> which handles the removal of previously installed File loaders
>
> uefi_set_boot_order(grugcfg, bootorder, target)
>
> which sets the order.
>
>
> If I understand the flow, we're looking to achieve:
>
> 1. removal of any FILE boot menu entries
> 2. installation of a new entry for what we just installed
> 3. placing the currently booted method first.
>
> Instead of doing(1) which would end up removing entries that we may not have
> written
> could we instead collect the current entries and order, perform a grub
> install, collect
> the new menu; then set the new boot order with current first,
> and pick out of the menu the *new* entry we created? I believe this is more
> robust
> since (3) assumes that the first entry the boot order list is the newly
> created
> boot entry; but does not verify that. There may be non-FILE based entries in
> the menu
> that this code does not consider.

No the code only places the current boot loader first. Grub can place the boot entry in whatever order it prefers and I don't want to override that behavior. So I specifically did it this way to prevent the need for curtin to explicitly set the boot order, it just places the current boot loader first.

So I don't see the need to pickout the new entry.

>
>
> The FILE entry includes the part UUID, which curtin can find since it knowns
> the partition that was used for grub install. I think we should ensure that
> the
> second entry should be the newly installed partition.

This should not be needed. The reordering explicitly places the current boot method first that is it. Allowing grub to make the decision on the order and I do not want to override that.

>
> Also noted in IRC from this morning, we should have unittests for the
> efibootmgr output manipulation.

I have pushed a new branch that unit tests all of setup_grub, because sadly it had no unit tests at all. Seems like a critical code path with no unit tests, that is worrying.

>
> Some questions in-line below

Blake Rouse (blake-rouse) :
Blake Rouse (blake-rouse) wrote :

I have not split out the methods as I did the unit tests before I saw your review. I will split them out in my next push.

lp:~blake-rouse/curtin/uefi-clear-reorder updated on 2017-05-11
499. By Blake Rouse on 2017-05-11

Add unit tests for all of setup_grub.

Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
lp:~blake-rouse/curtin/uefi-clear-reorder updated on 2017-05-12
500. By Blake Rouse on 2017-05-12

Refactor from code review, add more tests.

501. By Blake Rouse on 2017-05-12

Capture output of subp.

Blake Rouse (blake-rouse) wrote :

I have split apart the code and add a new get_efibootmgr util that calls efibootmgr and parses it into a dictionary.

Server Team CI bot (server-team-bot) wrote :
review: Needs Fixing (continuous-integration)
lp:~blake-rouse/curtin/uefi-clear-reorder updated on 2017-05-12
502. By Blake Rouse on 2017-05-12

Fix unit tests on trusty-py27 and trusty-py35.

Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Ryan Harper (raharper) wrote :

This looks really good. We can switch the with Chrootable target to use of util.subp(... target=target) which will chroot as needed.

review: Needs Fixing
Ryan Harper (raharper) wrote :

See comment below re: ChrootableTarget; I can add the comment during merge.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'curtin/commands/curthooks.py'
2--- curtin/commands/curthooks.py 2017-04-20 19:21:35 +0000
3+++ curtin/commands/curthooks.py 2017-05-12 14:36:12 +0000
4@@ -18,6 +18,7 @@
5 import copy
6 import os
7 import platform
8+import re
9 import sys
10 import shutil
11 import textwrap
12@@ -240,6 +241,63 @@
13 " System may not boot.", package)
14
15
16+def uefi_remove_old_loaders(grubcfg, target):
17+ """Removes the old UEFI loaders from efibootmgr."""
18+ efi_output = util.get_efibootmgr(target)
19+ current_uefi_boot = efi_output.get('current', None)
20+ old_efi_entries = {
21+ entry: info
22+ for entry, info in efi_output['entries'].items()
23+ if re.match(r'^.*File\(\\EFI.*$', info['path'])
24+ }
25+ old_efi_entries.pop(current_uefi_boot, None)
26+ remove_old_loaders = grubcfg.get('remove_old_uefi_loaders', True)
27+ if old_efi_entries:
28+ if remove_old_loaders:
29+ with util.ChrootableTarget(target) as in_chroot:
30+ for entry, info in old_efi_entries.items():
31+ LOG.debug("removing old UEFI entry: %s" % info['name'])
32+ in_chroot.subp(
33+ ['efibootmgr', '-B', '-b', entry], capture=True)
34+ else:
35+ LOG.debug(
36+ "Skipped removing %d old UEFI entrie%s.",
37+ len(old_efi_entries),
38+ '' if len(old_efi_entries) == 1 else 's')
39+ for info in old_efi_entries.values():
40+ LOG.debug(
41+ "UEFI entry '%s' might no longer exist and "
42+ "should be removed.", info['name'])
43+
44+
45+def uefi_reorder_loaders(grubcfg, target):
46+ """Reorders the UEFI BootOrder to place BootCurrent first.
47+
48+ The specifically doesn't try to do to much. The order in which grub places
49+ a new EFI loader is up to grub. This only moves the BootCurrent to the
50+ front of the BootOrder.
51+ """
52+ if grubcfg.get('reorder_uefi', True):
53+ efi_output = util.get_efibootmgr(target)
54+ currently_booted = efi_output.get('current', None)
55+ boot_order = efi_output.get('order', [])
56+ if currently_booted:
57+ if currently_booted in boot_order:
58+ boot_order.remove(currently_booted)
59+ boot_order = [currently_booted] + boot_order
60+ new_boot_order = ','.join(boot_order)
61+ LOG.debug(
62+ "Setting currently booted %s as the first "
63+ "UEFI loader.", currently_booted)
64+ LOG.debug(
65+ "New UEFI boot order: %s", new_boot_order)
66+ with util.ChrootableTarget(target) as in_chroot:
67+ in_chroot.subp(['efibootmgr', '-o', new_boot_order])
68+ else:
69+ LOG.debug("Skipped reordering of UEFI boot methods.")
70+ LOG.debug("Currently booted UEFI loader might no longer boot.")
71+
72+
73 def setup_grub(cfg, target):
74 # target is the path to the mounted filesystem
75
76@@ -350,13 +408,17 @@
77 instdevs = [block.get_dev_name_entry(i)[1] for i in instdevs]
78 else:
79 instdevs = ["none"]
80+
81+ if util.is_uefi_bootable() and grubcfg.get('update_nvram', True):
82+ uefi_remove_old_loaders(grubcfg, target)
83+
84 LOG.debug("installing grub to %s [replace_default=%s]",
85 instdevs, replace_default)
86 with util.ChrootableTarget(target):
87 args = ['install-grub']
88 if util.is_uefi_bootable():
89 args.append("--uefi")
90- if grubcfg.get('update_nvram', False):
91+ if grubcfg.get('update_nvram', True):
92 LOG.debug("GRUB UEFI enabling NVRAM updates")
93 args.append("--update-nvram")
94 else:
95@@ -370,6 +432,9 @@
96 join_stdout_err + args + instdevs, env=env, capture=True)
97 LOG.debug("%s\n%s\n", args, out)
98
99+ if util.is_uefi_bootable() and grubcfg.get('update_nvram', True):
100+ uefi_reorder_loaders(grubcfg, target)
101+
102
103 def update_initramfs(target=None, all_kernels=False):
104 cmd = ['update-initramfs', '-u']
105
106=== modified file 'curtin/util.py'
107--- curtin/util.py 2017-04-21 15:24:13 +0000
108+++ curtin/util.py 2017-05-12 14:36:12 +0000
109@@ -836,6 +836,59 @@
110 return os.path.exists('/sys/firmware/efi') is True
111
112
113+def get_efibootmgr(target):
114+ """Return mapping of EFI information.
115+
116+ Calls `efibootmgr` inside the `target`.
117+
118+ Example output:
119+ {
120+ 'current': '0000',
121+ 'timeout': '1 seconds',
122+ 'order': ['0000', '0001'],
123+ 'entries': {
124+ '0000': {
125+ 'name': 'ubuntu',
126+ 'path': (
127+ 'HD(1,GPT,0,0x8,0x1)/File(\\EFI\\ubuntu\\shimx64.efi)'),
128+ },
129+ '0001': {
130+ 'name': 'UEFI:Network Device',
131+ 'path': 'BBS(131,,0x0)',
132+ }
133+ }
134+ }
135+ """
136+ efikey_to_dict_key = {
137+ 'BootCurrent': 'current',
138+ 'Timeout': 'timeout',
139+ 'BootOrder': 'order',
140+ }
141+ with ChrootableTarget(target) as in_chroot:
142+ stdout, _ = in_chroot.subp(['efibootmgr', '-v'], capture=True)
143+ output = {}
144+ for line in stdout.splitlines():
145+ split = line.split(':')
146+ if len(split) == 2:
147+ key = split[0].strip()
148+ output_key = efikey_to_dict_key.get(key, None)
149+ if output_key:
150+ output[output_key] = split[1].strip()
151+ if output_key == 'order':
152+ output[output_key] = output[output_key].split(',')
153+ output['entries'] = {
154+ entry: {
155+ 'name': name.strip(),
156+ 'path': path.strip(),
157+ }
158+ for entry, name, path in re.findall(
159+ r"^Boot(?P<entry>[0-9a-fA-F]{4})\*?\s(?P<name>.+)\t"
160+ r"(?P<path>.*)$",
161+ stdout, re.MULTILINE)
162+ }
163+ return output
164+
165+
166 def run_hook_if_exists(target, hook):
167 """
168 Look for "hook" in "target" and run it
169
170=== modified file 'tests/unittests/test_curthooks.py'
171--- tests/unittests/test_curthooks.py 2017-04-20 19:21:35 +0000
172+++ tests/unittests/test_curthooks.py 2017-05-12 14:36:12 +0000
173@@ -1,6 +1,6 @@
174 import os
175 from unittest import TestCase
176-from mock import call, patch
177+from mock import call, patch, MagicMock
178 import shutil
179 import tempfile
180
181@@ -14,10 +14,10 @@
182 def setUp(self):
183 super(CurthooksBase, self).setUp()
184
185- def add_patch(self, target, attr):
186+ def add_patch(self, target, attr, autospec=True):
187 """Patches specified target object and sets it as attr on test
188 instance also schedules cleanup"""
189- m = patch(target, autospec=True)
190+ m = patch(target, autospec=autospec)
191 p = m.start()
192 self.addCleanup(m.stop)
193 setattr(self, attr, p)
194@@ -176,6 +176,264 @@
195 self.assertEqual([], self.mock_install_packages.call_args_list)
196
197
198+class TestSetupGrub(CurthooksBase):
199+
200+ def setUp(self):
201+ super(TestSetupGrub, self).setUp()
202+ self.target = tempfile.mkdtemp()
203+ self.add_patch('curtin.util.lsb_release', 'mock_lsb_release')
204+ self.mock_lsb_release.return_value = {
205+ 'codename': 'xenial',
206+ }
207+ self.add_patch('curtin.util.is_uefi_bootable',
208+ 'mock_is_uefi_bootable')
209+ self.mock_is_uefi_bootable.return_value = False
210+ self.add_patch('curtin.util.subp', 'mock_subp')
211+ self.subp_output = []
212+ self.mock_subp.side_effect = iter(self.subp_output)
213+ self.add_patch('curtin.commands.block_meta.devsync', 'mock_devsync')
214+ self.add_patch('curtin.util.get_architecture', 'mock_arch')
215+ self.mock_arch.return_value = 'amd64'
216+ self.add_patch(
217+ 'curtin.util.ChrootableTarget', 'mock_chroot', autospec=False)
218+ self.mock_in_chroot = MagicMock()
219+ self.mock_in_chroot.__enter__.return_value = self.mock_in_chroot
220+ self.in_chroot_subp_output = []
221+ self.mock_in_chroot_subp = self.mock_in_chroot.subp
222+ self.mock_in_chroot_subp.side_effect = iter(self.in_chroot_subp_output)
223+ self.mock_chroot.return_value = self.mock_in_chroot
224+
225+ def tearDown(self):
226+ shutil.rmtree(self.target)
227+
228+ def test_uses_old_grub_install_devices_in_cfg(self):
229+ cfg = {
230+ 'grub_install_devices': ['/dev/vdb']
231+ }
232+ self.subp_output.append(('', ''))
233+ curthooks.setup_grub(cfg, self.target)
234+ self.assertEquals(
235+ ([
236+ 'sh', '-c', 'exec "$0" "$@" 2>&1',
237+ 'install-grub', self.target, '/dev/vdb'],),
238+ self.mock_subp.call_args_list[0][0])
239+
240+ def test_uses_install_devices_in_grubcfg(self):
241+ cfg = {
242+ 'grub': {
243+ 'install_devices': ['/dev/vdb'],
244+ },
245+ }
246+ self.subp_output.append(('', ''))
247+ curthooks.setup_grub(cfg, self.target)
248+ self.assertEquals(
249+ ([
250+ 'sh', '-c', 'exec "$0" "$@" 2>&1',
251+ 'install-grub', self.target, '/dev/vdb'],),
252+ self.mock_subp.call_args_list[0][0])
253+
254+ def test_uses_grub_install_on_storage_config(self):
255+ cfg = {
256+ 'storage': {
257+ 'version': 1,
258+ 'config': [
259+ {
260+ 'id': 'vdb',
261+ 'type': 'disk',
262+ 'grub_device': True,
263+ 'path': '/dev/vdb',
264+ }
265+ ]
266+ },
267+ }
268+ self.subp_output.append(('', ''))
269+ curthooks.setup_grub(cfg, self.target)
270+ self.assertEquals(
271+ ([
272+ 'sh', '-c', 'exec "$0" "$@" 2>&1',
273+ 'install-grub', self.target, '/dev/vdb'],),
274+ self.mock_subp.call_args_list[0][0])
275+
276+ def test_grub_install_installs_to_none_if_install_devices_None(self):
277+ cfg = {
278+ 'grub': {
279+ 'install_devices': None,
280+ },
281+ }
282+ self.subp_output.append(('', ''))
283+ curthooks.setup_grub(cfg, self.target)
284+ self.assertEquals(
285+ ([
286+ 'sh', '-c', 'exec "$0" "$@" 2>&1',
287+ 'install-grub', self.target, 'none'],),
288+ self.mock_subp.call_args_list[0][0])
289+
290+ def test_grub_install_uefi_installs_signed_packages_for_amd64(self):
291+ self.add_patch('curtin.util.install_packages', 'mock_install')
292+ self.add_patch('curtin.util.has_pkg_available', 'mock_haspkg')
293+ self.mock_is_uefi_bootable.return_value = True
294+ cfg = {
295+ 'grub': {
296+ 'install_devices': ['/dev/vdb'],
297+ 'update_nvram': False,
298+ },
299+ }
300+ self.subp_output.append(('', ''))
301+ self.mock_arch.return_value = 'amd64'
302+ self.mock_haspkg.return_value = True
303+ curthooks.setup_grub(cfg, self.target)
304+ self.assertEquals(
305+ (['grub-efi-amd64', 'grub-efi-amd64-signed', 'shim-signed'],),
306+ self.mock_install.call_args_list[0][0])
307+ self.assertEquals(
308+ ([
309+ 'sh', '-c', 'exec "$0" "$@" 2>&1',
310+ 'install-grub', '--uefi', self.target, '/dev/vdb'],),
311+ self.mock_subp.call_args_list[0][0])
312+
313+ def test_grub_install_uefi_installs_packages_for_arm64(self):
314+ self.add_patch('curtin.util.install_packages', 'mock_install')
315+ self.add_patch('curtin.util.has_pkg_available', 'mock_haspkg')
316+ self.mock_is_uefi_bootable.return_value = True
317+ cfg = {
318+ 'grub': {
319+ 'install_devices': ['/dev/vdb'],
320+ 'update_nvram': False,
321+ },
322+ }
323+ self.subp_output.append(('', ''))
324+ self.mock_arch.return_value = 'arm64'
325+ self.mock_haspkg.return_value = False
326+ curthooks.setup_grub(cfg, self.target)
327+ self.assertEquals(
328+ (['grub-efi-arm64'],),
329+ self.mock_install.call_args_list[0][0])
330+ self.assertEquals(
331+ ([
332+ 'sh', '-c', 'exec "$0" "$@" 2>&1',
333+ 'install-grub', '--uefi', self.target, '/dev/vdb'],),
334+ self.mock_subp.call_args_list[0][0])
335+
336+ def test_grub_install_uefi_updates_nvram_skips_remove_and_reorder(self):
337+ self.add_patch('curtin.util.install_packages', 'mock_install')
338+ self.add_patch('curtin.util.has_pkg_available', 'mock_haspkg')
339+ self.add_patch('curtin.util.get_efibootmgr', 'mock_efibootmgr')
340+ self.mock_is_uefi_bootable.return_value = True
341+ cfg = {
342+ 'grub': {
343+ 'install_devices': ['/dev/vdb'],
344+ 'update_nvram': True,
345+ 'remove_old_uefi_loaders': False,
346+ 'reorder_uefi': False,
347+ },
348+ }
349+ self.subp_output.append(('', ''))
350+ self.mock_haspkg.return_value = False
351+ self.mock_efibootmgr.return_value = {
352+ 'current': '0000',
353+ 'entries': {
354+ '0000': {
355+ 'name': 'ubuntu',
356+ 'path': (
357+ 'HD(1,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)'),
358+ }
359+ }
360+ }
361+ curthooks.setup_grub(cfg, self.target)
362+ self.assertEquals(
363+ ([
364+ 'sh', '-c', 'exec "$0" "$@" 2>&1',
365+ 'install-grub', '--uefi', '--update-nvram',
366+ self.target, '/dev/vdb'],),
367+ self.mock_subp.call_args_list[0][0])
368+
369+ def test_grub_install_uefi_updates_nvram_removes_old_loaders(self):
370+ self.add_patch('curtin.util.install_packages', 'mock_install')
371+ self.add_patch('curtin.util.has_pkg_available', 'mock_haspkg')
372+ self.add_patch('curtin.util.get_efibootmgr', 'mock_efibootmgr')
373+ self.mock_is_uefi_bootable.return_value = True
374+ cfg = {
375+ 'grub': {
376+ 'install_devices': ['/dev/vdb'],
377+ 'update_nvram': True,
378+ 'remove_old_uefi_loaders': True,
379+ 'reorder_uefi': False,
380+ },
381+ }
382+ self.subp_output.append(('', ''))
383+ self.mock_efibootmgr.return_value = {
384+ 'current': '0000',
385+ 'entries': {
386+ '0000': {
387+ 'name': 'ubuntu',
388+ 'path': (
389+ 'HD(1,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)'),
390+ },
391+ '0001': {
392+ 'name': 'centos',
393+ 'path': (
394+ 'HD(1,GPT)/File(\\EFI\\centos\\shimx64.efi)'),
395+ },
396+ '0002': {
397+ 'name': 'sles',
398+ 'path': (
399+ 'HD(1,GPT)/File(\\EFI\\sles\\shimx64.efi)'),
400+ },
401+ }
402+ }
403+ self.in_chroot_subp_output.append(('', ''))
404+ self.in_chroot_subp_output.append(('', ''))
405+ self.mock_haspkg.return_value = False
406+ curthooks.setup_grub(cfg, self.target)
407+ self.assertEquals(
408+ ['efibootmgr', '-B', '-b'],
409+ self.mock_in_chroot_subp.call_args_list[0][0][0][:3])
410+ self.assertEquals(
411+ ['efibootmgr', '-B', '-b'],
412+ self.mock_in_chroot_subp.call_args_list[1][0][0][:3])
413+ self.assertEquals(
414+ set(['0001', '0002']),
415+ set([
416+ self.mock_in_chroot_subp.call_args_list[0][0][0][3],
417+ self.mock_in_chroot_subp.call_args_list[1][0][0][3]]))
418+
419+ def test_grub_install_uefi_updates_nvram_reorders_loaders(self):
420+ self.add_patch('curtin.util.install_packages', 'mock_install')
421+ self.add_patch('curtin.util.has_pkg_available', 'mock_haspkg')
422+ self.add_patch('curtin.util.get_efibootmgr', 'mock_efibootmgr')
423+ self.mock_is_uefi_bootable.return_value = True
424+ cfg = {
425+ 'grub': {
426+ 'install_devices': ['/dev/vdb'],
427+ 'update_nvram': True,
428+ 'remove_old_uefi_loaders': False,
429+ 'reorder_uefi': True,
430+ },
431+ }
432+ self.subp_output.append(('', ''))
433+ self.mock_efibootmgr.return_value = {
434+ 'current': '0001',
435+ 'order': ['0000', '0001'],
436+ 'entries': {
437+ '0000': {
438+ 'name': 'ubuntu',
439+ 'path': (
440+ 'HD(1,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)'),
441+ },
442+ '0001': {
443+ 'name': 'UEFI:Network Device',
444+ 'path': 'BBS(131,,0x0)',
445+ },
446+ }
447+ }
448+ self.in_chroot_subp_output.append(('', ''))
449+ self.mock_haspkg.return_value = False
450+ curthooks.setup_grub(cfg, self.target)
451+ self.assertEquals(
452+ (['efibootmgr', '-o', '0001,0000'],),
453+ self.mock_in_chroot_subp.call_args_list[0][0])
454+
455+
456 class TestUbuntuCoreHooks(CurthooksBase):
457 def setUp(self):
458 super(TestUbuntuCoreHooks, self).setUp()
459
460=== modified file 'tests/unittests/test_util.py'
461--- tests/unittests/test_util.py 2017-04-10 17:41:51 +0000
462+++ tests/unittests/test_util.py 2017-05-12 14:36:12 +0000
463@@ -4,6 +4,7 @@
464 import stat
465 import shutil
466 import tempfile
467+from textwrap import dedent
468
469 from curtin import util
470 from .helpers import simple_mocked_open
471@@ -683,4 +684,73 @@
472 ])
473
474
475+class TestGetEFIBootMGR(TestCase):
476+
477+ def setUp(self):
478+ super(TestGetEFIBootMGR, self).setUp()
479+ mock_chroot = mock.patch(
480+ 'curtin.util.ChrootableTarget', autospec=False)
481+ self.mock_chroot = mock_chroot.start()
482+ self.addCleanup(mock_chroot.stop)
483+ self.mock_in_chroot = mock.MagicMock()
484+ self.mock_in_chroot.__enter__.return_value = self.mock_in_chroot
485+ self.in_chroot_subp_output = []
486+ self.mock_in_chroot_subp = self.mock_in_chroot.subp
487+ self.mock_in_chroot_subp.side_effect = self.in_chroot_subp_output
488+ self.mock_chroot.return_value = self.mock_in_chroot
489+
490+ def test_calls_efibootmgr_verbose(self):
491+ self.in_chroot_subp_output.append(('', ''))
492+ util.get_efibootmgr('target')
493+ self.assertEquals(
494+ (['efibootmgr', '-v'],),
495+ self.mock_in_chroot_subp.call_args_list[0][0])
496+
497+ def test_parses_output(self):
498+ self.in_chroot_subp_output.append((dedent(
499+ """\
500+ BootCurrent: 0000
501+ Timeout: 1 seconds
502+ BootOrder: 0000,0002,0001,0003,0004,0005
503+ Boot0000* ubuntu HD(1,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)
504+ Boot0001* CD/DVD Drive BBS(CDROM,,0x0)
505+ Boot0002* Hard Drive BBS(HD,,0x0)
506+ Boot0003* UEFI:CD/DVD Drive BBS(129,,0x0)
507+ Boot0004* UEFI:Removable Device BBS(130,,0x0)
508+ Boot0005* UEFI:Network Device BBS(131,,0x0)
509+ """), ''))
510+ observed = util.get_efibootmgr('target')
511+ self.assertEquals({
512+ 'current': '0000',
513+ 'timeout': '1 seconds',
514+ 'order': ['0000', '0002', '0001', '0003', '0004', '0005'],
515+ 'entries': {
516+ '0000': {
517+ 'name': 'ubuntu',
518+ 'path': 'HD(1,GPT)/File(\\EFI\\ubuntu\\shimx64.efi)',
519+ },
520+ '0001': {
521+ 'name': 'CD/DVD Drive',
522+ 'path': 'BBS(CDROM,,0x0)',
523+ },
524+ '0002': {
525+ 'name': 'Hard Drive',
526+ 'path': 'BBS(HD,,0x0)',
527+ },
528+ '0003': {
529+ 'name': 'UEFI:CD/DVD Drive',
530+ 'path': 'BBS(129,,0x0)',
531+ },
532+ '0004': {
533+ 'name': 'UEFI:Removable Device',
534+ 'path': 'BBS(130,,0x0)',
535+ },
536+ '0005': {
537+ 'name': 'UEFI:Network Device',
538+ 'path': 'BBS(131,,0x0)',
539+ },
540+ }
541+ }, observed)
542+
543+
544 # vi: ts=4 expandtab syntax=python

Subscribers

People subscribed via source and target branches