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

[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

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:fd2fbde5c8bfae968572aefad990cff30ac1e22a
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)

[Michael Hudson-Doyle (mwhudson)]

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 on 2020-12-08

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 on 2020-12-08

unittest: fix typo in type:format structure

8c1f7f9... by Ryan Harper on 2020-12-08

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.

diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py
index 4cf7301..42da2ca 100644
--- a/curtin/commands/curthooks.py
+++ b/curtin/commands/curthooks.py
@@ -659,6 +659,7 @@ def uefi_find_grub_device_ids(sconfig):
     primary_esp = None
     grub_partitions = []
     esp_partitions = []
+    fat32_partitions = []
     for item_id, item in sconfig.items():
         if item['type'] == 'partition':
             if item.get('grub_device'):
@@ -678,6 +679,8 @@ def uefi_find_grub_device_ids(sconfig):
                 LOG.debug("Found primary UEFI ESP: %s", primary_esp)
             else:
                 LOG.warn('Found primary ESP not on a partition: %s', item)
+        if item['type'] == 'format' and item.get('fstype') == 'fat32':
+            fat32_partitions.append(sconfig[item['volume']]['id'])
 
     if primary_esp is None:
         raise RuntimeError('Failed to find primary ESP mounted at /boot/efi')
@@ -690,11 +693,16 @@ def uefi_find_grub_device_ids(sconfig):
         # insert the primary esp as first element
         grub_device_ids.extend(grub_partitions)
 
-    # look at all esp entries, check if parent disk is grub_device: true
+    # look at all esp entries, check if formatted with fat32 and
+    # parent disk is grub_device: true
     elif len(esp_partitions):
         if primary_esp in esp_partitions:
             esp_partitions.remove(primary_esp)
         for esp_id in esp_partitions:
+            if esp_id not in fat32_partitions:
+                LOG.debug("Skipping partition %s, not formatted with fat32",
+                          esp_id)
+                continue
             esp_disk = sconfig[sconfig[esp_id]['device']]
             if esp_disk.get('grub_device'):
                 grub_device_ids.append(esp_id)
diff --git a/tests/unittests/test_curthooks.py b/tests/unittests/test_curthooks.py
index e5fead3..61ae061 100644
--- a/tests/unittests/test_curthooks.py
+++ b/tests/unittests/test_curthooks.py
@@ -2188,7 +2188,7 @@ class TestUefiFindGrubDeviceIds(CiTestCase):
                     {
                         'id': 'vda-part1_format',
                         'type': 'format',
-                        'volume': 'vdb-part1',
+                        'volume': 'vda-part1',
                         'fstype': 'fat32',
                     },
                     {
@@ -2211,5 +2211,64 @@ class TestUefiFindGrubDeviceIds(CiTestCase):
                          curthooks.uefi_find_grub_device_ids(
                              self._sconfig(cfg)))
 
+    def test_esp_on_boot_partition(self):
+        cfg = {
+            'storage': {
+                'version': 1,
+                'config': [
+                    {
+                        'id': 'vda',
+                        'type': 'disk',
+                        'name': 'vda',
+                        'path': '/dev/vda',
+                        'ptable': 'gpt',
+                        'grub_device': True,
+                    },
+                    {
+                        'id': 'vda-part1',
+                        'type': 'partition',
+                        'device': 'vda',
+                        'flag': 'boot',
+                        'number': 1,
+                    },
+                    {
+                        'id': 'vda-part2',
+                        'type': 'partition',
+                        'device': 'vda',
+                        'flag': 'boot',
+                        'number': 1,
+                    },
+                    {
+                        'id': 'vda-part1_format',
+                        'type': 'format',
+                        'volume': 'vda-part1',
+                        'fstype': 'fat32',
+                    },
+                    {
+                        'id': 'vda-part2_format',
+                        'type': 'format',
+                        'volume': 'vda-part1',
+                        'fstype': 'ext4',
+                    },
+                    {
+                        'id': 'vda-part2_mount',
+                        'type': 'mount',
+                        'device': 'vda-part2_format',
+                        'path': '/boot',
+                    },
+                    {
+                        'id': 'vda-part1_mount',
+                        'type': 'mount',
+                        'device': 'vda-part1_format',
+                        'path': '/boot/efi',
+                    },
+
+                ]
+            },
+        }
+        self.assertEqual(['vda-part1'],
+                         curthooks.uefi_find_grub_device_ids(
+                             self._sconfig(cfg)))
+
 
 # vi: ts=4 expandtab syntax=python