curtin

Merge ~raharper/curtin:feature/grub-resilient-boot into curtin:master

  1. Git
  2. lp:~raharper/curtin
  3. feature/grub-resilient-boot
  4. Merge into master
Proposed by Ryan Harper
Status: Merged
Approved by: Ryan Harper
Approved revision: 3745e0c1d25703c5c2d652d2a9d0b776501a2134
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~raharper/curtin:feature/grub-resilient-boot
Merge into: curtin:master
Diff against target: 800 lines (+515/-27)
11 files modified
curtin/block/schemas.py (+5/-0)
curtin/commands/apt_config.py (+1/-5)
curtin/commands/curthooks.py (+122/-12)
examples/tests/bcache-ceph-nvme-simple.yaml (+2/-2)
examples/tests/bcache-ceph-nvme.yaml (+2/-2)
examples/tests/mirrorboot-uefi.yaml (+17/-1)
examples/tests/reuse-raid-member-wipe-partition.yaml (+2/-0)
examples/tests/reuse-raid-member-wipe.yaml (+2/-1)
helpers/common (+11/-3)
tests/unittests/test_curthooks.py (+317/-1)
tests/vmtests/test_mdadm_bcache.py (+34/-0)
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve
Michael Hudson-Doyle Approve
Dan Watkins Pending
Review via email: mp+382101@code.launchpad.net

Commit message

curthooks: support multiple ESP on UEFI bootable systems

On debian systems we generate and set debconf selections values
to tell grub the set of configured devices. Also refactor
setup_grub to extract all UEFI boot partitions and determine
the primary ESP (mounted at /boot/efi). We include all ESPs
in the debconf selection for grub-[multi]-install to use.

Non-UEFI:
  grub-pc grub-pc/install_devices multiselect <devices>

UEFI:
  grub-pc grub-efi/install_devices multiselect <devices>

The grub-efi/install_devices is a new template value used for
handling multiple ESP partitions for boot resilience.

Curtin will check for the grub-multi-install tool and if
present will invoke that tool to install grub to all ESPs.

- Add block.schema update to allow partitions to set
  grub_device: True
- Refactor finding ESPs to prefer partition with grub_device,
  ESPs on disks with grub_device and fallback to grubcfg
  install_devices if we don't find one in storage config.

To post a comment you must log in.
Revision history for this message
Michael Hudson-Doyle (mwhudson) :
Revision history for this message
Ryan Harper (raharper) wrote :

Thanks for the review Michael.

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:0f96a2c15e0be5fe9ed95b2f58427e5eac7c1173
https://jenkins.ubuntu.com/server/job/curtin-ci/33/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/33/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/33/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/33/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/33/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/33//rebuild

review: Approve (continuous-integration)
~raharper/curtin:feature/grub-resilient-boot updated
b2d5a34... by Ryan Harper

Add warning if grub: install_devices and storage_config provided

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:7e26e4f675ced9c74ca2e0027d7f5fdc7623b372
https://jenkins.ubuntu.com/server/job/curtin-ci/36/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/36/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/36/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/36/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/36/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/36//rebuild

review: Approve (continuous-integration)
Revision history for this message
Michael Hudson-Doyle (mwhudson) :
review: Disapprove
Revision history for this message
Dan Watkins (oddbloke) wrote :

Some nits, which we don't need to fix, but a couple of comments that I think do need addressing.

~raharper/curtin:feature/grub-resilient-boot updated
476c404... by Ryan Harper

_debconf: remove dict and pass params directly.

Revision history for this message
Michael Hudson-Doyle (mwhudson) :
Revision history for this message
Ryan Harper (raharper) :
Revision history for this message
Ryan Harper (raharper) wrote :

Thanks for the review folks

~raharper/curtin:feature/grub-resilient-boot updated
25cf373... by Ryan Harper

Refactor esp searching through all devices

ec6587c... by Ryan Harper

block.schema: Allow partitions to be marked grub_device

2d5f5fd... by Ryan Harper

Use command -v apt-get to match command to run.

564ca2c... by Ryan Harper

Update Testcase spelling to make others.

Revision history for this message
Michael Hudson-Doyle (mwhudson) :
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Forgot to reply to the important bit!

Revision history for this message
Dan Watkins (oddbloke) wrote :

On Thu, Apr 16, 2020 at 09:20:56PM -0000, Ryan Harper wrote:
> > + # insert the primary esp as first element
> > + storage_grub_devices = ([primary_esp] + [dev for dev in
>
> +1
>
> timeit shows for a small list; it's faster to remove than to iterate the whole list.

Yep, I would anticipate this is for a couple of reasons: firstly, remove
doesn't (necessarily) iterate the whole list, it is complete once it
finds the first matching item. And, secondly, it's written in C and
your list comprehension isn't. ;)

> (crispyboi) curtin % cat bench.py
> #!/usr/bin/python3
> import timeit
> benchmark1 = '''
> primary_esp = '/dev/vda1'
> efi_boot_parts = ['/dev/vda1', '/dev/vdb1', '/dev/vdc1']
> grub_devs = [primary_esp] + [dev for dev in efi_boot_parts if dev != primary_esp]
> '''
> benchmark2 = '''
> primary_esp = '/dev/vda1'
> efi_boot_parts = ['/dev/vda1', '/dev/vdb1', '/dev/vdc1']
> efi_boot_parts.remove(primary_esp)
> grub_devs = [primary_esp] + efi_boot_parts
> '''
> print('Bench1')
> print(timeit.timeit(stmt=benchmark1))
> print('Bench2')
> print(timeit.timeit(stmt=benchmark2))
> (crispyboi) curtin % python3 bench.py
> Bench1
> 0.3933144999900833
> Bench2
> 0.16941187201882713

~raharper/curtin:feature/grub-resilient-boot updated
330c78f... by Ryan Harper

Refactor how we look for and find uefi ESPs for grub install.

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:9fe3d494b32e7e5edef7dbedbd9a92d782391201
https://jenkins.ubuntu.com/server/job/curtin-ci/37/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/37/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/37/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/37/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/37/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/37//rebuild

review: Approve (continuous-integration)
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

I like this now, thanks :)

review: Approve
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

FWIW, I started on a UI for this: https://github.com/CanonicalLtd/subiquity/pull/718

~raharper/curtin:feature/grub-resilient-boot updated
858c6ce... by Ryan Harper

EPS -> ESP

0909b0b... by Ryan Harper

Fix spelling

db1b6e7... by Ryan Harper

Ensure we're looking at a type:mount when checking 'path'

e358e95... by Ryan Harper

'path' is required element for type:mount

7ff1f42... by Ryan Harper

Fix typo

Revision history for this message
Ryan Harper (raharper) wrote :

Thanks for the review!

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:8afc73f93d3f0a30383a2f2161621f85bf824a61
https://jenkins.ubuntu.com/server/job/curtin-ci/38/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/38/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/38/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/38/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/38/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/38//rebuild

review: Approve (continuous-integration)
~raharper/curtin:feature/grub-resilient-boot updated
c608651... by Ryan Harper

Handle byid returning None; return the original device name in that case

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:69408ea340da92a6e8bdcfa09e525ef9e0dc063e
https://jenkins.ubuntu.com/server/job/curtin-ci/40/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/40/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/40/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/40/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/40/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/40//rebuild

review: Approve (continuous-integration)
Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

On thinking about it, I think we really do want to follow the spec from day 1 and store the partuuid. Rather than just complaining about this, I implemented it:

https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+ref/feature/grub-resilient-boot

(block._get_dev_disk_by_prefix is a cute hack btw :))

I haven't run the vmtest yet because I haven't run make sync-images on this laptop yet and that takes about a million years. Hopefully I'll get to that today!

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

I managed to run at least some vmtests on my branch, and using my hacked up subiquity branch https://github.com/CanonicalLtd/subiquity/pull/718 managed to install a system with 3 ESPs!

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Although it turns out that the grub maintainer scripts expect a by-id link, so maybe your branch is better for now.

Revision history for this message
Ryan Harper (raharper) wrote :

>
> I haven't run the vmtest yet because I haven't run make sync-images on this
> laptop yet and that takes about a million years. Hopefully I'll get to that
> today!

If you don't want to sync everything you can do:

CURTIN_VMTEST_IMAGE_SYNC=1 ./tools/jenkins tests/vmtest/test_foo.py:FocalTestFoo

and curtin will sync *only* the files it needs for that testcase.

~raharper/curtin:feature/grub-resilient-boot updated
3745e0c... by Ryan Harper

Fix reuse-esp on ubuntu, mark ESP with grub_device, fix missing part flags

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:3745e0c1d25703c5c2d652d2a9d0b776501a2134
https://jenkins.ubuntu.com/server/job/curtin-ci/48/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/48/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/48/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/48/
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/48/

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/48//rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/curtin/block/schemas.py b/curtin/block/schemas.py
2index 4de1daa..9e2c41f 100644
3--- a/curtin/block/schemas.py
4+++ b/curtin/block/schemas.py
5@@ -291,6 +291,11 @@ PARTITION = {
6 'enum': ['bios_grub', 'boot', 'extended', 'home', 'linux',
7 'logical', 'lvm', 'mbr', 'prep', 'raid', 'swap',
8 '']},
9+ 'grub_device': {
10+ 'type': ['boolean', 'integer'],
11+ 'minimum': 0,
12+ 'maximum': 1
13+ },
14 }
15 }
16 RAID = {
17diff --git a/curtin/commands/apt_config.py b/curtin/commands/apt_config.py
18index 8bd6e79..f012ae0 100644
19--- a/curtin/commands/apt_config.py
20+++ b/curtin/commands/apt_config.py
21@@ -135,6 +135,7 @@ def apply_debconf_selections(cfg, target=None):
22 LOG.debug("debconf_selections was not set in config")
23 return
24
25+ LOG.debug('Applying debconf selections')
26 selections = '\n'.join(
27 [selsets[key] for key in sorted(selsets.keys())])
28 debconf_set_selections(selections.encode() + b"\n", target=target)
29@@ -149,13 +150,8 @@ def apply_debconf_selections(cfg, target=None):
30 pkgs_cfgd.add(pkg)
31
32 pkgs_installed = distro.get_installed_packages(target)
33-
34- LOG.debug("pkgs_cfgd: %s", pkgs_cfgd)
35- LOG.debug("pkgs_installed: %s", pkgs_installed)
36 need_reconfig = pkgs_cfgd.intersection(pkgs_installed)
37-
38 if len(need_reconfig) == 0:
39- LOG.debug("no need for reconfig")
40 return
41
42 dpkg_reconfigure(need_reconfig, target=target)
43diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py
44index 86aa7b2..4afe00c 100644
45--- a/curtin/commands/curthooks.py
46+++ b/curtin/commands/curthooks.py
47@@ -485,6 +485,112 @@ def uefi_remove_duplicate_entries(grubcfg, target):
48 '--delete-bootnum'])
49
50
51+def _debconf_multiselect(package, variable, choices):
52+ return "{package} {variable} multiselect {choices}".format(
53+ package=package, variable=variable, choices=", ".join(choices))
54+
55+
56+def configure_grub_debconf(boot_devices, target, uefi):
57+ """Configure grub debconf variables in target.
58+
59+ Non-UEFI:
60+ grub-pc grub-pc/install_devices multiselect d1, d2, d3
61+
62+ UEFI:
63+ grub-pc grub-efi/install_devices multiselect d1
64+
65+ """
66+ LOG.debug('Generating grub debconf_selections for devices=%s uefi=%s',
67+ boot_devices, uefi)
68+
69+ byid_links = []
70+ for dev in boot_devices:
71+ link = block.disk_to_byid_path(dev)
72+ byid_links.extend([link] if link else [dev])
73+
74+ selections = []
75+ if uefi:
76+ selections.append(_debconf_multiselect(
77+ 'grub-pc', 'grub-efi/install_devices', byid_links))
78+ else:
79+ selections.append(_debconf_multiselect(
80+ 'grub-pc', 'grub-pc/install_devices', byid_links))
81+
82+ cfg = {'debconf_selections': {'grub': "\n".join(selections)}}
83+ LOG.info('Applying grub debconf_selections config:\n%s', cfg)
84+ apt_config.apply_debconf_selections(cfg, target)
85+ return
86+
87+
88+def uefi_find_grub_device_ids(sconfig):
89+ """ Scan the provided storage config for device_ids on which we
90+ will install grub. An order of precendence is required due to
91+ legacy configurations which set grub_device on the disk but not
92+ on the ESP config itself. We prefer the latter as this allows
93+ a disk to contain more than on ESP and choose to install grub
94+ to a subset. We always look for the 'primary' ESP which is
95+ signified by being mounted at /boot/efi (only one can be mounted).
96+
97+ 1. ESPs with grub_device: true are the preferred way to find
98+ the specific set of devices on which to install grub
99+ 2. ESPs whose parent disk has grub_device: true
100+
101+ The primary ESP is the first element of the result if any
102+ devices are found.
103+
104+ returns a list of storage-config ids on which grub will be installed.
105+ """
106+ # Only one EFI system partition can be mounted, but backup EFI
107+ # partitions may exist. Find all EFI partitions and determine
108+ # the primary.
109+ grub_device_ids = []
110+ primary_esp = None
111+ grub_partitions = []
112+ esp_partitions = []
113+ for item_id, item in sconfig.items():
114+ if item['type'] == 'partition':
115+ if item.get('grub_device'):
116+ grub_partitions.append(item_id)
117+ continue
118+ elif item.get('flag') == 'boot':
119+ esp_partitions.append(item_id)
120+ continue
121+
122+ if item['type'] == 'mount' and item['path'] == '/boot/efi':
123+ if primary_esp:
124+ LOG.debug('Ignoring duplicate mounted primary ESP: %s',
125+ item_id)
126+ continue
127+ primary_esp = sconfig[item['device']]['volume']
128+ if sconfig[primary_esp]['type'] == 'partition':
129+ LOG.debug("Found primary UEFI ESP: %s", primary_esp)
130+ else:
131+ LOG.warn('Found primary ESP not on a partition: %s', item)
132+
133+ if primary_esp is None:
134+ raise RuntimeError('Failed to find primary ESP mounted at /boot/efi')
135+
136+ grub_device_ids = [primary_esp]
137+ # prefer grub_device: true partitions
138+ if len(grub_partitions):
139+ if primary_esp in grub_partitions:
140+ grub_partitions.remove(primary_esp)
141+ # insert the primary esp as first element
142+ grub_device_ids.extend(grub_partitions)
143+
144+ # look at all esp entries, check if parent disk is grub_device: true
145+ elif len(esp_partitions):
146+ if primary_esp in esp_partitions:
147+ esp_partitions.remove(primary_esp)
148+ for esp_id in esp_partitions:
149+ esp_disk = sconfig[sconfig[esp_id]['device']]
150+ if esp_disk.get('grub_device'):
151+ grub_device_ids.append(esp_id)
152+
153+ LOG.debug('Found UEFI ESP(s) for grub install: %s', grub_device_ids)
154+ return grub_device_ids
155+
156+
157 def setup_grub(cfg, target, osfamily=DISTROS.debian):
158 # target is the path to the mounted filesystem
159
160@@ -507,19 +613,13 @@ def setup_grub(cfg, target, osfamily=DISTROS.debian):
161 except ValueError:
162 pass
163
164+ uefi_bootable = util.is_uefi_bootable()
165 if storage_cfg_odict:
166 storage_grub_devices = []
167- if util.is_uefi_bootable():
168- # Curtin only supports creating one EFI system partition. Thus the
169- # grub_device can only be the default system partition mounted at
170- # /boot/efi.
171- for item_id, item in storage_cfg_odict.items():
172- if item.get('path') == '/boot/efi':
173- efi_dev_id = storage_cfg_odict[item['device']]['volume']
174- LOG.debug("checking: %s", item)
175- storage_grub_devices.append(get_path_to_storage_volume(
176- efi_dev_id, storage_cfg_odict))
177- break
178+ if uefi_bootable:
179+ storage_grub_devices.extend([
180+ get_path_to_storage_volume(dev_id, storage_cfg_odict)
181+ for dev_id in uefi_find_grub_device_ids(storage_cfg_odict)])
182 else:
183 for item_id, item in storage_cfg_odict.items():
184 if not item.get('grub_device'):
185@@ -529,6 +629,10 @@ def setup_grub(cfg, target, osfamily=DISTROS.debian):
186 get_path_to_storage_volume(item_id, storage_cfg_odict))
187
188 if len(storage_grub_devices) > 0:
189+ if len(grubcfg.get('install_devices', [])):
190+ LOG.warn("Storage Config grub device config takes precedence "
191+ "over grub 'install_devices' value, ignoring: %s",
192+ grubcfg['install_devices'])
193 grubcfg['install_devices'] = storage_grub_devices
194
195 LOG.debug("install_devices: %s", grubcfg.get('install_devices'))
196@@ -602,10 +706,11 @@ def setup_grub(cfg, target, osfamily=DISTROS.debian):
197
198 if instdevs:
199 instdevs = [block.get_dev_name_entry(i)[1] for i in instdevs]
200+ if osfamily == DISTROS.debian:
201+ configure_grub_debconf(instdevs, target, uefi_bootable)
202 else:
203 instdevs = ["none"]
204
205- uefi_bootable = util.is_uefi_bootable()
206 if uefi_bootable and grubcfg.get('update_nvram', True):
207 uefi_remove_old_loaders(grubcfg, target)
208
209@@ -623,6 +728,11 @@ def setup_grub(cfg, target, osfamily=DISTROS.debian):
210 else:
211 LOG.debug("NOT enabling UEFI nvram updates")
212 LOG.debug("Target system may not boot")
213+ if len(instdevs) > 1:
214+ instdevs = [instdevs[0]]
215+ LOG.debug("Selecting primary EFI boot device %s for install",
216+ instdevs[0])
217+
218 args.append('--os-family=%s' % osfamily)
219 args.append(target)
220
221diff --git a/examples/tests/bcache-ceph-nvme-simple.yaml b/examples/tests/bcache-ceph-nvme-simple.yaml
222index ffb4aec..42624e7 100644
223--- a/examples/tests/bcache-ceph-nvme-simple.yaml
224+++ b/examples/tests/bcache-ceph-nvme-simple.yaml
225@@ -1,7 +1,6 @@
226 storage:
227 config:
228- - grub_device: true
229- id: sda
230+ - id: sda
231 model: MM1000GBKAL
232 name: sda
233 ptable: gpt
234@@ -35,6 +34,7 @@ storage:
235 type: partition
236 uuid: 1e27e7af-26dc-4af4-9ef5-aea928204997
237 wipe: superblock
238+ grub_device: true
239 - device: sda
240 id: sda-part2
241 name: sda-part2
242diff --git a/examples/tests/bcache-ceph-nvme.yaml b/examples/tests/bcache-ceph-nvme.yaml
243index 507bc0e..e16e0c0 100644
244--- a/examples/tests/bcache-ceph-nvme.yaml
245+++ b/examples/tests/bcache-ceph-nvme.yaml
246@@ -3,8 +3,7 @@ install:
247 showtrace: true
248 storage:
249 config:
250- - grub_device: true
251- id: sda
252+ - id: sda
253 model: MG04SCA60EA
254 name: sda
255 ptable: gpt
256@@ -80,6 +79,7 @@ storage:
257 wipe: superblock
258 - device: sdf
259 id: sdf-part1
260+ grub_device: true
261 name: sdf-part1
262 number: 1
263 offset: 4194304B
264diff --git a/examples/tests/mirrorboot-uefi.yaml b/examples/tests/mirrorboot-uefi.yaml
265index ca55be9..95108f4 100644
266--- a/examples/tests/mirrorboot-uefi.yaml
267+++ b/examples/tests/mirrorboot-uefi.yaml
268@@ -1,4 +1,18 @@
269 showtrace: true
270+install:
271+ unmount: disabled
272+
273+_install_debconf_utils:
274+ - &install_debconf_utils |
275+ command -v apt-get && {
276+ apt-get -qy install debconf-utils &>/dev/null || { echo "No debconf-utils available"; }
277+ }
278+ exit 0
279+
280+late_commands:
281+ # used to collect debconf_selections
282+ 01_install_debconf_utils: ['curtin', 'in-target', '--', 'bash', '-c', *install_debconf_utils]
283+
284 storage:
285 config:
286 - grub_device: true
287@@ -9,7 +23,8 @@ storage:
288 wipe: superblock
289 serial: disk-a
290 name: main_disk
291- - id: sdb
292+ - grub_device: true
293+ id: sdb
294 name: sdb
295 ptable: gpt
296 type: disk
297@@ -123,4 +138,5 @@ storage:
298 options: ''
299 path: /var
300 type: mount
301+
302 version: 1
303diff --git a/examples/tests/reuse-raid-member-wipe-partition.yaml b/examples/tests/reuse-raid-member-wipe-partition.yaml
304index 3fe2d83..d20b79c 100644
305--- a/examples/tests/reuse-raid-member-wipe-partition.yaml
306+++ b/examples/tests/reuse-raid-member-wipe-partition.yaml
307@@ -14,6 +14,7 @@ bucket:
308 parted /dev/disk/by-id/virtio-disk-a --script -- \
309 mklabel gpt \
310 mkpart primary 1GiB 2GiB \
311+ set 1 boot on \
312 mkpart primary 2GiB 9GiB
313 parted /dev/disk/by-id/virtio-disk-b --script -- \
314 mklabel gpt \
315@@ -54,6 +55,7 @@ storage:
316 device: id_disk0
317 number: 2
318 size: 7G
319+ wipe: superblock
320 - type: format
321 id: id_efi_format
322 volume: id_disk0_part1
323diff --git a/examples/tests/reuse-raid-member-wipe.yaml b/examples/tests/reuse-raid-member-wipe.yaml
324index 84a2686..9abd5d4 100644
325--- a/examples/tests/reuse-raid-member-wipe.yaml
326+++ b/examples/tests/reuse-raid-member-wipe.yaml
327@@ -14,7 +14,8 @@ bucket:
328 - &setup |
329 parted /dev/disk/by-id/virtio-disk-a --script -- \
330 mklabel gpt \
331- mkpart primary 1GiB 9GiB
332+ mkpart primary 1GiB 9GiB \
333+ set 1 boot on
334 parted /dev/disk/by-id/virtio-disk-b --script -- \
335 mklabel gpt \
336 mkpart primary 1GiB 9GiB
337diff --git a/helpers/common b/helpers/common
338index 8230577..5638d39 100644
339--- a/helpers/common
340+++ b/helpers/common
341@@ -901,9 +901,13 @@ install_grub() {
342 efi_disk="$5"
343 efi_part_num="$6"
344 grubpost=""
345+ grubmulti="/usr/lib/grub/grub-multi-install"
346 case $bootid in
347 debian|ubuntu)
348 grubcmd="grub-install"
349+ if [ -e "${grubmulti}" ]; then
350+ grubcmd="${grubmulti}"
351+ fi
352 dpkg-reconfigure "$1"
353 update-grub
354 ;;
355@@ -953,7 +957,7 @@ install_grub() {
356 echo "Dumping /boot/efi contents"
357 find /boot/efi
358 echo "Checking for existing EFI grub entry on ESP"
359- if [ -f /boot/efi/EFI/$bootid/grubx64.efi ]; then
360+ if [ "$grubcmd" = "grub2-install" -a -f /boot/efi/EFI/$bootid/grubx64.efi ]; then
361 if [ -z "$no_nvram" ]; then
362 # UEFI firmware should be pointed to the shim if available to
363 # enable secure boot.
364@@ -975,8 +979,12 @@ install_grub() {
365 fi
366 else
367 echo "No previous EFI grub entry found on ESP, use $grubcmd"
368- $grubcmd $target $efi_dir \
369- --bootloader-id=$bootid --recheck $no_nvram
370+ if [ "${grubcmd}" = "${grubmulti}" ]; then
371+ $grubcmd
372+ else
373+ $grubcmd $target $efi_dir \
374+ --bootloader-id=$bootid --recheck $no_nvram
375+ fi
376 fi
377 [ -z "$grubpost" ] || $grubpost;' \
378 -- "$grub_name" "$grub_target" "$nvram" "$os_variant" "$efi_disk" "$efi_part_num" </dev/null ||
379diff --git a/tests/unittests/test_curthooks.py b/tests/unittests/test_curthooks.py
380index 155c3ba..a391c28 100644
381--- a/tests/unittests/test_curthooks.py
382+++ b/tests/unittests/test_curthooks.py
383@@ -5,6 +5,7 @@ from mock import call, patch, MagicMock
384 import textwrap
385
386 from curtin.commands import curthooks
387+from curtin.commands.block_meta import extract_storage_ordered_dict
388 from curtin import distro
389 from curtin import util
390 from curtin import config
391@@ -555,6 +556,8 @@ class TestSetupGrub(CiTestCase):
392 self.mock_in_chroot_subp = self.mock_in_chroot.subp
393 self.mock_in_chroot_subp.side_effect = iter(self.in_chroot_subp_output)
394 self.mock_chroot.return_value = self.mock_in_chroot
395+ self.add_patch('curtin.commands.curthooks.configure_grub_debconf',
396+ 'm_grub_debconf')
397
398 def test_uses_old_grub_install_devices_in_cfg(self):
399 cfg = {
400@@ -629,6 +632,7 @@ class TestSetupGrub(CiTestCase):
401 'id': 'vdb-part1',
402 'type': 'partition',
403 'device': 'vdb',
404+ 'flag': 'boot',
405 'number': 1,
406 },
407 {
408@@ -651,7 +655,7 @@ class TestSetupGrub(CiTestCase):
409 }
410 self.subp_output.append(('', ''))
411 m_exists.return_value = True
412- m_is_valid_device.side_effect = (False, True)
413+ m_is_valid_device.side_effect = (False, True, False, True)
414 curthooks.setup_grub(cfg, self.target, osfamily=distro.DISTROS.redhat)
415 self.assertEquals(
416 ([
417@@ -1641,4 +1645,316 @@ class TestCurthooksCopyZkey(CiTestCase):
418 self.assertEqual(self.zkey_content, found_files)
419
420
421+class TestCurthooksGrubDebconf(CiTestCase):
422+ def setUp(self):
423+ super(TestCurthooksGrubDebconf, self).setUp()
424+ base = 'curtin.commands.curthooks.'
425+ self.add_patch(
426+ base + 'apt_config.apply_debconf_selections', 'm_debconf')
427+ self.add_patch(base + 'block.disk_to_byid_path', 'm_byid')
428+
429+ def test_debconf_multiselect(self):
430+ package = self.random_string()
431+ variable = "%s/%s" % (self.random_string(), self.random_string())
432+ choices = [c for c in self.random_string()]
433+ expected = "%s %s multiselect %s" % (package, variable,
434+ ", ".join(choices))
435+ self.assertEqual(expected,
436+ curthooks._debconf_multiselect(package, variable,
437+ choices))
438+
439+ def test_configure_grub_debconf(self):
440+ target = self.random_string()
441+ boot_devs = [self.random_string()]
442+ byid_boot_devs = ["/dev/disk/by-id/" + dev for dev in boot_devs]
443+ uefi = False
444+ self.m_byid.side_effect = (lambda x: '/dev/disk/by-id/' + x)
445+ curthooks.configure_grub_debconf(boot_devs, target, uefi)
446+ expected_selection = [
447+ ('grub-pc grub-pc/install_devices '
448+ 'multiselect %s' % ", ".join(byid_boot_devs))
449+ ]
450+ expectedcfg = {
451+ 'debconf_selections': {'grub': "\n".join(expected_selection)}}
452+ self.m_debconf.assert_called_with(expectedcfg, target)
453+
454+ def test_configure_grub_debconf_uefi_enabled(self):
455+ target = self.random_string()
456+ boot_devs = [self.random_string()]
457+ byid_boot_devs = ["/dev/disk/by-id/" + dev for dev in boot_devs]
458+ uefi = True
459+ self.m_byid.side_effect = (lambda x: '/dev/disk/by-id/' + x)
460+ curthooks.configure_grub_debconf(boot_devs, target, uefi)
461+ expected_selection = [
462+ ('grub-pc grub-efi/install_devices '
463+ 'multiselect %s' % ", ".join(byid_boot_devs))
464+ ]
465+ expectedcfg = {
466+ 'debconf_selections': {'grub': "\n".join(expected_selection)}}
467+ self.m_debconf.assert_called_with(expectedcfg, target)
468+
469+ def test_configure_grub_debconf_handle_no_byid_result(self):
470+ target = self.random_string()
471+ boot_devs = ['aaaaa', 'bbbbb']
472+ uefi = True
473+ self.m_byid.side_effect = (
474+ lambda x: ('/dev/disk/by-id/' + x if 'a' in x else None))
475+ curthooks.configure_grub_debconf(boot_devs, target, uefi)
476+ expected_selection = [
477+ ('grub-pc grub-efi/install_devices '
478+ 'multiselect /dev/disk/by-id/aaaaa, bbbbb')
479+ ]
480+ expectedcfg = {
481+ 'debconf_selections': {'grub': "\n".join(expected_selection)}}
482+ self.m_debconf.assert_called_with(expectedcfg, target)
483+
484+
485+class TestUefiFindGrubDeviceIds(CiTestCase):
486+
487+ def _sconfig(self, cfg):
488+ return extract_storage_ordered_dict(cfg)
489+
490+ def test_missing_primary_esp_raises_exception(self):
491+ cfg = {
492+ 'storage': {
493+ 'version': 1,
494+ 'config': [
495+ {
496+ 'id': 'vdb-part1',
497+ 'type': 'partition',
498+ 'device': 'vdb',
499+ 'flag': 'boot',
500+ 'number': 1,
501+ 'grub_device': True,
502+ },
503+ {
504+ 'id': 'vdb-part1_format',
505+ 'type': 'format',
506+ 'volume': 'vdb-part1',
507+ 'fstype': 'fat32',
508+ },
509+ ]
510+ },
511+ }
512+ with self.assertRaises(RuntimeError):
513+ curthooks.uefi_find_grub_device_ids(self._sconfig(cfg))
514+
515+ def test_single_esp_grub_device_true(self):
516+ cfg = {
517+ 'storage': {
518+ 'version': 1,
519+ 'config': [
520+ {
521+ 'id': 'vdb-part1',
522+ 'type': 'partition',
523+ 'device': 'vdb',
524+ 'flag': 'boot',
525+ 'number': 1,
526+ 'grub_device': True,
527+ },
528+ {
529+ 'id': 'vdb-part1_format',
530+ 'type': 'format',
531+ 'volume': 'vdb-part1',
532+ 'fstype': 'fat32',
533+ },
534+ {
535+ 'id': 'vdb-part1_mount',
536+ 'type': 'mount',
537+ 'device': 'vdb-part1_format',
538+ 'path': '/boot/efi',
539+ },
540+ ]
541+ },
542+ }
543+ self.assertEqual(['vdb-part1'],
544+ curthooks.uefi_find_grub_device_ids(
545+ self._sconfig(cfg)))
546+
547+ def test_single_esp_grub_device_true_on_disk(self):
548+ cfg = {
549+ 'storage': {
550+ 'version': 1,
551+ 'config': [
552+ {
553+ 'id': 'vdb',
554+ 'type': 'disk',
555+ 'name': 'vdb',
556+ 'path': '/dev/vdb',
557+ 'ptable': 'gpt',
558+ 'grub_device': True,
559+ },
560+ {
561+ 'id': 'vdb-part1',
562+ 'type': 'partition',
563+ 'device': 'vdb',
564+ 'flag': 'boot',
565+ 'number': 1,
566+ },
567+ {
568+ 'id': 'vdb-part1_format',
569+ 'type': 'format',
570+ 'volume': 'vdb-part1',
571+ 'fstype': 'fat32',
572+ },
573+ {
574+ 'id': 'vdb-part1_mount',
575+ 'type': 'mount',
576+ 'device': 'vdb-part1_format',
577+ 'path': '/boot/efi',
578+ },
579+ ]
580+ },
581+ }
582+ self.assertEqual(['vdb-part1'],
583+ curthooks.uefi_find_grub_device_ids(
584+ self._sconfig(cfg)))
585+
586+ def test_single_esp_no_grub_device(self):
587+ cfg = {
588+ 'storage': {
589+ 'version': 1,
590+ 'config': [
591+ {
592+ 'id': 'vdb',
593+ 'type': 'disk',
594+ 'name': 'vdb',
595+ 'path': '/dev/vdb',
596+ 'ptable': 'gpt',
597+ },
598+ {
599+ 'id': 'vdb-part1',
600+ 'type': 'partition',
601+ 'device': 'vdb',
602+ 'flag': 'boot',
603+ 'number': 1,
604+ },
605+ {
606+ 'id': 'vdb-part1_format',
607+ 'type': 'format',
608+ 'volume': 'vdb-part1',
609+ 'fstype': 'fat32',
610+ },
611+ {
612+ 'id': 'vdb-part1_mount',
613+ 'type': 'mount',
614+ 'device': 'vdb-part1_format',
615+ 'path': '/boot/efi',
616+ },
617+ ]
618+ },
619+ }
620+ self.assertEqual(['vdb-part1'],
621+ curthooks.uefi_find_grub_device_ids(
622+ self._sconfig(cfg)))
623+
624+ def test_multiple_esp_grub_device_true(self):
625+ cfg = {
626+ 'storage': {
627+ 'version': 1,
628+ 'config': [
629+ {
630+ 'id': 'vda-part1',
631+ 'type': 'partition',
632+ 'device': 'vda',
633+ 'flag': 'boot',
634+ 'number': 1,
635+ 'grub_device': True,
636+ },
637+ {
638+ 'id': 'vdb-part1',
639+ 'type': 'partition',
640+ 'device': 'vdb',
641+ 'flag': 'boot',
642+ 'number': 1,
643+ 'grub_device': True,
644+ },
645+ {
646+ 'id': 'vda-part1_format',
647+ 'type': 'format',
648+ 'volume': 'vdb-part1',
649+ 'fstype': 'fat32',
650+ },
651+ {
652+ 'id': 'vdb-part1_format',
653+ 'type': 'format',
654+ 'volume': 'vdb-part1',
655+ 'fstype': 'fat32',
656+ },
657+ {
658+ 'id': 'vdb-part1_mount',
659+ 'type': 'mount',
660+ 'device': 'vdb-part1_format',
661+ 'path': '/boot/efi',
662+ },
663+
664+ ]
665+ },
666+ }
667+ self.assertEqual(['vdb-part1', 'vda-part1'],
668+ curthooks.uefi_find_grub_device_ids(
669+ self._sconfig(cfg)))
670+
671+ def test_multiple_esp_grub_device_true_on_disk(self):
672+ cfg = {
673+ 'storage': {
674+ 'version': 1,
675+ 'config': [
676+ {
677+ 'id': 'vda',
678+ 'type': 'disk',
679+ 'name': 'vda',
680+ 'path': '/dev/vda',
681+ 'ptable': 'gpt',
682+ 'grub_device': True,
683+ },
684+ {
685+ 'id': 'vdb',
686+ 'type': 'disk',
687+ 'name': 'vdb',
688+ 'path': '/dev/vdb',
689+ 'ptable': 'gpt',
690+ 'grub_device': True,
691+ },
692+ {
693+ 'id': 'vda-part1',
694+ 'type': 'partition',
695+ 'device': 'vda',
696+ 'flag': 'boot',
697+ 'number': 1,
698+ },
699+ {
700+ 'id': 'vdb-part1',
701+ 'type': 'partition',
702+ 'device': 'vdb',
703+ 'flag': 'boot',
704+ 'number': 1,
705+ },
706+ {
707+ 'id': 'vda-part1_format',
708+ 'type': 'format',
709+ 'volume': 'vdb-part1',
710+ 'fstype': 'fat32',
711+ },
712+ {
713+ 'id': 'vdb-part1_format',
714+ 'type': 'format',
715+ 'volume': 'vdb-part1',
716+ 'fstype': 'fat32',
717+ },
718+ {
719+ 'id': 'vdb-part1_mount',
720+ 'type': 'mount',
721+ 'device': 'vdb-part1_format',
722+ 'path': '/boot/efi',
723+ },
724+
725+ ]
726+ },
727+ }
728+ self.assertEqual(['vdb-part1', 'vda-part1'],
729+ curthooks.uefi_find_grub_device_ids(
730+ self._sconfig(cfg)))
731+
732+
733 # vi: ts=4 expandtab syntax=python
734diff --git a/tests/vmtests/test_mdadm_bcache.py b/tests/vmtests/test_mdadm_bcache.py
735index f8d3e6a..53637ae 100644
736--- a/tests/vmtests/test_mdadm_bcache.py
737+++ b/tests/vmtests/test_mdadm_bcache.py
738@@ -3,7 +3,9 @@
739 from . import VMBaseClass
740 from .releases import base_vm_classes as relbase
741 from .releases import centos_base_vm_classes as centos_relbase
742+import re
743 import textwrap
744+from unittest import SkipTest
745
746
747 class TestMdadmAbs(VMBaseClass):
748@@ -271,6 +273,19 @@ class TestMirrorbootPartitionsUEFIAbs(TestMdadmAbs):
749 uefi = True
750 nr_cpus = 2
751 dirty_disks = True
752+ GRUB_RE = r'(?P<pkg>grub-pc)\s(?P<var>\S+)\smultiselect\s(?P<cfg>.*$)'
753+
754+ extra_collect_scripts = TestMdadmAbs.extra_collect_scripts + [
755+ textwrap.dedent("""
756+ cd OUTPUT_COLLECT_D
757+ debconf-get-selections > debconf_selections.txt
758+ ls -al /usr/lib/grub/* > usr_lib_grub.txt
759+ (cd /boot/efi && find .) | sort > diska-part1-efi.out
760+ mount /dev/disk/by-id/virtio-disk-b-part1 /mnt
761+ (cd /mnt && find .) | sort > diskb-part1-efi.out
762+ umount /mnt
763+ exit 0
764+ """)]
765
766 def get_fstab_expected(self):
767 return [
768@@ -280,6 +295,20 @@ class TestMirrorbootPartitionsUEFIAbs(TestMdadmAbs):
769 '/var', 'defaults'),
770 ]
771
772+ def test_grub_debconf_selections(self):
773+ """Verify we have grub2/efi_install_devices set correctly."""
774+ if self.target_distro not in ["ubuntu", "debian"]:
775+ raise SkipTest("debconf-selections not present in distro "
776+ "%s" % self.target_release)
777+
778+ selections = self.load_collect_file("debconf_selections.txt")
779+ found_selections = re.findall(self.GRUB_RE, selections, re.MULTILINE)
780+ disks_byid = ['/dev/disk/by-id/virtio-disk-a-part1',
781+ '/dev/disk/by-id/virtio-disk-b-part1']
782+ choice = ", ".join(disks_byid)
783+ self.assertIn(
784+ ('grub-pc', 'grub-efi/install_devices', choice), found_selections)
785+
786
787 class Centos70TestMirrorbootPartitionsUEFI(centos_relbase.centos70_xenial,
788 TestMirrorbootPartitionsUEFIAbs):
789@@ -315,6 +344,11 @@ class FocalTestMirrorbootPartitionsUEFI(relbase.focal,
790 TestMirrorbootPartitionsUEFIAbs):
791 __test__ = True
792
793+ def test_backup_esp_matches_primary(self):
794+ primary_esp = self.load_collect_file("diska-part1-efi.out")
795+ backup_esp = self.load_collect_file("diskb-part1-efi.out")
796+ self.assertEqual(primary_esp, backup_esp)
797+
798
799 class TestRaid5bootAbs(TestMdadmAbs):
800 # alternative config for more complex setup

Subscribers

People subscribed via source and target branches