Merge ~raharper/curtin:fix/verify-secondary-esp into curtin:master

Proposed by Ryan Harper
Status: Needs review
Proposed branch: ~raharper/curtin:fix/verify-secondary-esp
Merge into: curtin:master
Diff against target: 117 lines (+69/-2)
2 files modified
curtin/commands/curthooks.py (+9/-1)
tests/unittests/test_curthooks.py (+60/-1)
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve
curtin developers Pending
Review via email: mp+395030@code.launchpad.net

Commit message

curthooks: verify secondary ESP partitions have fat32 format

Ignore partitions with flag: boot which are not FAT32 formatted as
they are not actual ESPs.

LP: #1907280

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

This seems OK for what it is, but I would like to understand why MAAS is generating the config it is first...

Unmerged commits

fd2fbde... by Ryan Harper

curthooks: verify secondary ESP partitions have fat32 format

Ignore partitions with flag: boot which are not FAT32 formatted as
they are not actual ESPs.

LP: #1907280

7eee461... by Ryan Harper

unittest: fix typo in type:format structure

8c1f7f9... by Ryan Harper

Add unittest to demonstrate EPS on /boot partition returns both

Unittest to trigger error where the /boot partition which is NOT
an ESP is selected as a grub device.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py
2index 4cf7301..42da2ca 100644
3--- a/curtin/commands/curthooks.py
4+++ b/curtin/commands/curthooks.py
5@@ -659,6 +659,7 @@ def uefi_find_grub_device_ids(sconfig):
6 primary_esp = None
7 grub_partitions = []
8 esp_partitions = []
9+ fat32_partitions = []
10 for item_id, item in sconfig.items():
11 if item['type'] == 'partition':
12 if item.get('grub_device'):
13@@ -678,6 +679,8 @@ def uefi_find_grub_device_ids(sconfig):
14 LOG.debug("Found primary UEFI ESP: %s", primary_esp)
15 else:
16 LOG.warn('Found primary ESP not on a partition: %s', item)
17+ if item['type'] == 'format' and item.get('fstype') == 'fat32':
18+ fat32_partitions.append(sconfig[item['volume']]['id'])
19
20 if primary_esp is None:
21 raise RuntimeError('Failed to find primary ESP mounted at /boot/efi')
22@@ -690,11 +693,16 @@ def uefi_find_grub_device_ids(sconfig):
23 # insert the primary esp as first element
24 grub_device_ids.extend(grub_partitions)
25
26- # look at all esp entries, check if parent disk is grub_device: true
27+ # look at all esp entries, check if formatted with fat32 and
28+ # parent disk is grub_device: true
29 elif len(esp_partitions):
30 if primary_esp in esp_partitions:
31 esp_partitions.remove(primary_esp)
32 for esp_id in esp_partitions:
33+ if esp_id not in fat32_partitions:
34+ LOG.debug("Skipping partition %s, not formatted with fat32",
35+ esp_id)
36+ continue
37 esp_disk = sconfig[sconfig[esp_id]['device']]
38 if esp_disk.get('grub_device'):
39 grub_device_ids.append(esp_id)
40diff --git a/tests/unittests/test_curthooks.py b/tests/unittests/test_curthooks.py
41index e5fead3..61ae061 100644
42--- a/tests/unittests/test_curthooks.py
43+++ b/tests/unittests/test_curthooks.py
44@@ -2188,7 +2188,7 @@ class TestUefiFindGrubDeviceIds(CiTestCase):
45 {
46 'id': 'vda-part1_format',
47 'type': 'format',
48- 'volume': 'vdb-part1',
49+ 'volume': 'vda-part1',
50 'fstype': 'fat32',
51 },
52 {
53@@ -2211,5 +2211,64 @@ class TestUefiFindGrubDeviceIds(CiTestCase):
54 curthooks.uefi_find_grub_device_ids(
55 self._sconfig(cfg)))
56
57+ def test_esp_on_boot_partition(self):
58+ cfg = {
59+ 'storage': {
60+ 'version': 1,
61+ 'config': [
62+ {
63+ 'id': 'vda',
64+ 'type': 'disk',
65+ 'name': 'vda',
66+ 'path': '/dev/vda',
67+ 'ptable': 'gpt',
68+ 'grub_device': True,
69+ },
70+ {
71+ 'id': 'vda-part1',
72+ 'type': 'partition',
73+ 'device': 'vda',
74+ 'flag': 'boot',
75+ 'number': 1,
76+ },
77+ {
78+ 'id': 'vda-part2',
79+ 'type': 'partition',
80+ 'device': 'vda',
81+ 'flag': 'boot',
82+ 'number': 1,
83+ },
84+ {
85+ 'id': 'vda-part1_format',
86+ 'type': 'format',
87+ 'volume': 'vda-part1',
88+ 'fstype': 'fat32',
89+ },
90+ {
91+ 'id': 'vda-part2_format',
92+ 'type': 'format',
93+ 'volume': 'vda-part1',
94+ 'fstype': 'ext4',
95+ },
96+ {
97+ 'id': 'vda-part2_mount',
98+ 'type': 'mount',
99+ 'device': 'vda-part2_format',
100+ 'path': '/boot',
101+ },
102+ {
103+ 'id': 'vda-part1_mount',
104+ 'type': 'mount',
105+ 'device': 'vda-part1_format',
106+ 'path': '/boot/efi',
107+ },
108+
109+ ]
110+ },
111+ }
112+ self.assertEqual(['vda-part1'],
113+ curthooks.uefi_find_grub_device_ids(
114+ self._sconfig(cfg)))
115+
116
117 # vi: ts=4 expandtab syntax=python

Subscribers

People subscribed via source and target branches