diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py index 7362ebe34..5b54df7d7 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 1e6298889..67a122a40 100644 --- a/ironic_python_agent/raid_utils.py +++ b/ironic_python_agent/raid_utils.py @@ -18,6 +18,11 @@ from ironic_python_agent import errors from ironic_python_agent import utils +# 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. @@ -84,11 +89,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 9dbec95e6..11ef29f15 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -649,7 +649,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'), @@ -657,7 +657,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'), @@ -693,14 +693,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'), @@ -737,7 +737,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'), @@ -745,7 +745,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 f993c6794..48bbd19b4 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -3082,10 +3082,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'), @@ -3697,10 +3697,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 000000000..9f75b8c64 --- /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 + `_ + as well as the `guidance from the author of gdisk + `_.