Increase the ESP partition size to 550 MiB when using software RAID

This has been a popular guidance, and diskimage-builder has recently
started following it.

Change-Id: I794c846fb191c15b0a30546bf64d624dfbde0fd4

5 files changed, 30 insertions, 16 deletions

diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py
index e0138a7c..75e3f320 100644
--- a/ironic_python_agent/extensions/image.py
+++ b/ironic_python_agent/extensions/image.py
@@ -28,6 +28,7 @@ from ironic_python_agent import errors
 from ironic_python_agent.extensions import base
 from ironic_python_agent.extensions import iscsi
 from ironic_python_agent import hardware
+from ironic_python_agent import raid_utils
 from ironic_python_agent import utils
 
 LOG = log.getLogger(__name__)
@@ -391,7 +392,7 @@ def _prepare_boot_partitions_for_softraid(device, holders, efi_part,
         # We know that we kept this space when configuring raid,see
         # hardware.GenericHardwareManager.create_configuration.
         # We could also directly get the EFI partition size.
-        partsize_mib = 128
+        partsize_mib = raid_utils.ESP_SIZE_MIB
         partlabel_prefix = 'uefi-holder-'
         for number, holder in enumerate(holders):
             # NOTE: see utils.get_partition_table_type_from_specs
diff --git a/ironic_python_agent/raid_utils.py b/ironic_python_agent/raid_utils.py
index 2ad49e0a..d62b4280 100644
--- a/ironic_python_agent/raid_utils.py
+++ b/ironic_python_agent/raid_utils.py
@@ -22,6 +22,11 @@ from ironic_python_agent import utils
 LOG = logging.getLogger(__name__)
 
 
+# NOTE(dtantsur): 550 MiB is used by DIB and seems a common guidance:
+# https://www.rodsbooks.com/efi-bootloaders/principles.html
+ESP_SIZE_MIB = 550
+
+
 def get_block_devices_for_raid(block_devices, logical_disks):
     """Get block devices that are involved in the RAID configuration.
 
@@ -88,11 +93,10 @@ def calculate_raid_start(target_boot_mode, partition_table_type, dev_name):
     # granularity is GiB, so you lose up to 1GiB just for a bios boot
     # partition...
     if target_boot_mode == 'uefi':
-        # Leave 129MiB - start_sector s for the esp (approx 128MiB)
-        # NOTE: any image efi partition is expected to be less
-        # than 128MiB
-        # TBD: 129MiB is a waste in most cases.
-        raid_start = '129MiB'
+        # Leave 551MiB - start_sector s for the esp (approx 550 MiB)
+        # TODO(dtantsur): 550 MiB is a waste in most cases, make it
+        # configurable?
+        raid_start = '%sMiB' % (ESP_SIZE_MIB + 1)
     else:
         if partition_table_type == 'gpt':
             # Leave 8MiB - start_sector s (approx 7MiB)
diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py
index d84307fb..154edbe0 100644
--- a/ironic_python_agent/tests/unit/extensions/test_image.py
+++ b/ironic_python_agent/tests/unit/extensions/test_image.py
@@ -682,7 +682,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
         mock_efi_part.assert_called_once_with('/dev/md0')
         expected = [
             mock.call('sgdisk', '-F', '/dev/sda'),
-            mock.call('sgdisk', '-n', '0:451s:+128MiB', '-t', '0:ef00', '-c',
+            mock.call('sgdisk', '-n', '0:451s:+550MiB', '-t', '0:ef00', '-c',
                       '0:uefi-holder-0', '/dev/sda'),
             mock.call('partprobe'),
             mock.call('blkid'),
@@ -690,7 +690,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
                       '/dev/sda'),
             mock.call('cp', '/dev/md0p12', '/dev/sda12'),
             mock.call('sgdisk', '-F', '/dev/sdb'),
-            mock.call('sgdisk', '-n', '0:452s:+128MiB', '-t', '0:ef00', '-c',
+            mock.call('sgdisk', '-n', '0:452s:+550MiB', '-t', '0:ef00', '-c',
                       '0:uefi-holder-1', '/dev/sdb'),
             mock.call('partprobe'),
             mock.call('blkid'),
@@ -726,14 +726,14 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
         mock_efi_part.assert_called_once_with('/dev/md0')
         expected = [
             mock.call('sgdisk', '-F', '/dev/sda'),
-            mock.call('sgdisk', '-n', '0:451s:+128MiB', '-t', '0:ef00', '-c',
+            mock.call('sgdisk', '-n', '0:451s:+550MiB', '-t', '0:ef00', '-c',
                       '0:uefi-holder-0', '/dev/sda'),
             mock.call('partprobe'),
             mock.call('blkid'),
             mock.call('blkid', '-l', '-t', 'PARTLABEL=uefi-holder-0',
                       '/dev/sda'),
             mock.call('sgdisk', '-F', '/dev/sdb'),
-            mock.call('sgdisk', '-n', '0:452s:+128MiB', '-t', '0:ef00', '-c',
+            mock.call('sgdisk', '-n', '0:452s:+550MiB', '-t', '0:ef00', '-c',
                       '0:uefi-holder-1', '/dev/sdb'),
             mock.call('partprobe'),
             mock.call('blkid'),
@@ -770,7 +770,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
 
         expected = [
             mock.call('sgdisk', '-F', '/dev/sda'),
-            mock.call('sgdisk', '-n', '0:451s:+128MiB', '-t', '0:ef00', '-c',
+            mock.call('sgdisk', '-n', '0:451s:+550MiB', '-t', '0:ef00', '-c',
                       '0:uefi-holder-0', '/dev/sda'),
             mock.call('partprobe'),
             mock.call('blkid'),
@@ -778,7 +778,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
                       '/dev/sda'),
             mock.call('cp', '/dev/md0p15', '/dev/sda12'),
             mock.call('sgdisk', '-F', '/dev/sdb'),
-            mock.call('sgdisk', '-n', '0:452s:+128MiB', '-t', '0:ef00', '-c',
+            mock.call('sgdisk', '-n', '0:452s:+550MiB', '-t', '0:ef00', '-c',
                       '0:uefi-holder-1', '/dev/sdb'),
             mock.call('partprobe'),
             mock.call('blkid'),
diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py
index 0be427e5..29a46939 100644
--- a/ironic_python_agent/tests/unit/test_hardware.py
+++ b/ironic_python_agent/tests/unit/test_hardware.py
@@ -3078,10 +3078,10 @@ class TestGenericHardwareManager(base.IronicAgentTest):
             mock.call('parted', '/dev/sda', '-s', '--', 'mklabel', 'gpt'),
             mock.call('parted', '/dev/sdb', '-s', '--', 'mklabel', 'gpt'),
             mock.call('parted', '/dev/sda', '-s', '-a', 'optimal', '--',
-                      'mkpart', 'primary', '129MiB', '10GiB'),
+                      'mkpart', 'primary', '551MiB', '10GiB'),
             mock.call('partx', '-u', '/dev/sda', check_exit_code=False),
             mock.call('parted', '/dev/sdb', '-s', '-a', 'optimal', '--',
-                      'mkpart', 'primary', '129MiB', '10GiB'),
+                      'mkpart', 'primary', '551MiB', '10GiB'),
             mock.call('partx', '-u', '/dev/sdb', check_exit_code=False),
             mock.call('parted', '/dev/sda', '-s', '-a', 'optimal', '--',
                       'mkpart', 'primary', '10GiB', '-1'),
@@ -3693,10 +3693,10 @@ class TestGenericHardwareManager(base.IronicAgentTest):
             mock.call('parted', '/dev/nvme1n1', '-s', '--', 'mklabel',
                       'gpt'),
             mock.call('parted', '/dev/nvme0n1', '-s', '-a', 'optimal', '--',
-                      'mkpart', 'primary', '129MiB', '10GiB'),
+                      'mkpart', 'primary', '551MiB', '10GiB'),
             mock.call('partx', '-u', '/dev/nvme0n1', check_exit_code=False),
             mock.call('parted', '/dev/nvme1n1', '-s', '-a', 'optimal', '--',
-                      'mkpart', 'primary', '129MiB', '10GiB'),
+                      'mkpart', 'primary', '551MiB', '10GiB'),
             mock.call('partx', '-u', '/dev/nvme1n1', check_exit_code=False),
             mock.call('parted', '/dev/nvme0n1', '-s', '-a', 'optimal', '--',
                       'mkpart', 'primary', '10GiB', '-1'),
diff --git a/releasenotes/notes/raid-esp-size-2c322adb2d3b9ce7.yaml b/releasenotes/notes/raid-esp-size-2c322adb2d3b9ce7.yaml
new file mode 100644
index 00000000..9f75b8c6
--- /dev/null
+++ b/releasenotes/notes/raid-esp-size-2c322adb2d3b9ce7.yaml
@@ -0,0 +1,9 @@
+---
+fixes:
+  - |
+    The size of the ESP partition created for software RAID has been increased
+    from 128 MiB to 550 MiB. This change is in line with the recent
+    `diskimage-builder change
+    <https://opendev.org/openstack/diskimage-builder/commit/7fd52ba84180b4e749ccf4c9db8c49eafff46ea8>`_
+    as well as the `guidance from the author of gdisk
+    <https://www.rodsbooks.com/efi-bootloaders/principles.html>`_.