openstack/ironic-python-agent
Code Issues Proposed changes

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

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

Change-Id: I794c846fb191c15b0a30546bf64d624dfbde0fd4
This commit is contained in:
Dmitry Tantsur 2020-07-02 17:27:49 +02:00
parent de7d5affe7
commit ba3caa6c64
5 changed files with 30 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
ironic_python_agent/extensions/image.py
View File

@ -28,6 +28,7 @@ from ironic_python_agent import errors
from ironic_python_agent.extensions import base from ironic_python_agent.extensions import base
from ironic_python_agent.extensions import iscsi from ironic_python_agent.extensions import iscsi
from ironic_python_agent import hardware from ironic_python_agent import hardware
from ironic_python_agent import raid_utils
from ironic_python_agent import utils from ironic_python_agent import utils
LOG = log.getLogger(__name__) 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 # We know that we kept this space when configuring raid,see
# hardware.GenericHardwareManager.create_configuration. # hardware.GenericHardwareManager.create_configuration.
# We could also directly get the EFI partition size. # We could also directly get the EFI partition size.
partsize_mib = 128 partsize_mib = raid_utils.ESP_SIZE_MIB
partlabel_prefix = 'uefi-holder-' partlabel_prefix = 'uefi-holder-'
for number, holder in enumerate(holders): for number, holder in enumerate(holders):
# NOTE: see utils.get_partition_table_type_from_specs # NOTE: see utils.get_partition_table_type_from_specs

14
ironic_python_agent/raid_utils.py
View File

@ -22,6 +22,11 @@ from ironic_python_agent import utils
LOG = logging.getLogger(__name__) 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): def get_block_devices_for_raid(block_devices, logical_disks):
"""Get block devices that are involved in the RAID configuration. """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 # granularity is GiB, so you lose up to 1GiB just for a bios boot
# partition... # partition...
if target_boot_mode == 'uefi': if target_boot_mode == 'uefi':
# Leave 129MiB - start_sector s for the esp (approx 128MiB) # Leave 551MiB - start_sector s for the esp (approx 550 MiB)
# NOTE: any image efi partition is expected to be less # TODO(dtantsur): 550 MiB is a waste in most cases, make it
# than 128MiB # configurable?
# TBD: 129MiB is a waste in most cases. raid_start = '%sMiB' % (ESP_SIZE_MIB + 1)
raid_start = '129MiB'
else: else:
if partition_table_type == 'gpt': if partition_table_type == 'gpt':
# Leave 8MiB - start_sector s (approx 7MiB) # Leave 8MiB - start_sector s (approx 7MiB)

12
ironic_python_agent/tests/unit/extensions/test_image.py
View File

@ -682,7 +682,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
mock_efi_part.assert_called_once_with('/dev/md0') mock_efi_part.assert_called_once_with('/dev/md0')
expected = [ expected = [
mock.call('sgdisk', '-F', '/dev/sda'), 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'), '0:uefi-holder-0', '/dev/sda'),
mock.call('partprobe'), mock.call('partprobe'),
mock.call('blkid'), mock.call('blkid'),
@ -690,7 +690,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
'/dev/sda'), '/dev/sda'),
mock.call('cp', '/dev/md0p12', '/dev/sda12'), mock.call('cp', '/dev/md0p12', '/dev/sda12'),
mock.call('sgdisk', '-F', '/dev/sdb'), 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'), '0:uefi-holder-1', '/dev/sdb'),
mock.call('partprobe'), mock.call('partprobe'),
mock.call('blkid'), mock.call('blkid'),
@ -726,14 +726,14 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
mock_efi_part.assert_called_once_with('/dev/md0') mock_efi_part.assert_called_once_with('/dev/md0')
expected = [ expected = [
mock.call('sgdisk', '-F', '/dev/sda'), 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'), '0:uefi-holder-0', '/dev/sda'),
mock.call('partprobe'), mock.call('partprobe'),
mock.call('blkid'), mock.call('blkid'),
mock.call('blkid', '-l', '-t', 'PARTLABEL=uefi-holder-0', mock.call('blkid', '-l', '-t', 'PARTLABEL=uefi-holder-0',
'/dev/sda'), '/dev/sda'),
mock.call('sgdisk', '-F', '/dev/sdb'), 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'), '0:uefi-holder-1', '/dev/sdb'),
mock.call('partprobe'), mock.call('partprobe'),
mock.call('blkid'), mock.call('blkid'),
@ -770,7 +770,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
expected = [ expected = [
mock.call('sgdisk', '-F', '/dev/sda'), 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'), '0:uefi-holder-0', '/dev/sda'),
mock.call('partprobe'), mock.call('partprobe'),
mock.call('blkid'), mock.call('blkid'),
@ -778,7 +778,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
'/dev/sda'), '/dev/sda'),
mock.call('cp', '/dev/md0p15', '/dev/sda12'), mock.call('cp', '/dev/md0p15', '/dev/sda12'),
mock.call('sgdisk', '-F', '/dev/sdb'), 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'), '0:uefi-holder-1', '/dev/sdb'),
mock.call('partprobe'), mock.call('partprobe'),
mock.call('blkid'), mock.call('blkid'),

8
ironic_python_agent/tests/unit/test_hardware.py
View File

@ -3078,10 +3078,10 @@ class TestGenericHardwareManager(base.IronicAgentTest):
mock.call('parted', '/dev/sda', '-s', '--', 'mklabel', 'gpt'), mock.call('parted', '/dev/sda', '-s', '--', 'mklabel', 'gpt'),
mock.call('parted', '/dev/sdb', '-s', '--', 'mklabel', 'gpt'), mock.call('parted', '/dev/sdb', '-s', '--', 'mklabel', 'gpt'),
mock.call('parted', '/dev/sda', '-s', '-a', 'optimal', '--', 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('partx', '-u', '/dev/sda', check_exit_code=False),
mock.call('parted', '/dev/sdb', '-s', '-a', 'optimal', '--', 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('partx', '-u', '/dev/sdb', check_exit_code=False),
mock.call('parted', '/dev/sda', '-s', '-a', 'optimal', '--', mock.call('parted', '/dev/sda', '-s', '-a', 'optimal', '--',
'mkpart', 'primary', '10GiB', '-1'), 'mkpart', 'primary', '10GiB', '-1'),
@ -3693,10 +3693,10 @@ class TestGenericHardwareManager(base.IronicAgentTest):
mock.call('parted', '/dev/nvme1n1', '-s', '--', 'mklabel', mock.call('parted', '/dev/nvme1n1', '-s', '--', 'mklabel',
'gpt'), 'gpt'),
mock.call('parted', '/dev/nvme0n1', '-s', '-a', 'optimal', '--', 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('partx', '-u', '/dev/nvme0n1', check_exit_code=False),
mock.call('parted', '/dev/nvme1n1', '-s', '-a', 'optimal', '--', 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('partx', '-u', '/dev/nvme1n1', check_exit_code=False),
mock.call('parted', '/dev/nvme0n1', '-s', '-a', 'optimal', '--', mock.call('parted', '/dev/nvme0n1', '-s', '-a', 'optimal', '--',
'mkpart', 'primary', '10GiB', '-1'), 'mkpart', 'primary', '10GiB', '-1'),

9
releasenotes/notes/raid-esp-size-2c322adb2d3b9ce7.yaml Normal file
View File

@ -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>`_.