openstack/ironic-python-agent
Code Issues Proposed changes

Use canonical device name for RAID device for ESP

Browse Source 
It seems like tinyIPA silently replaces /dev/md/esp with /dev/md127.
Find the next free /dev/md device and use it instead.

Also rescan the resulting device before copying files.

Change-Id: Ie04f530be434c4b1561e75f387b9da679e4607e0
Depends-On: https://review.opendev.org/c/openstack/ironic/+/827129/
This commit is contained in:
Dmitry Tantsur 2022-01-31 11:19:10 +01:00
parent 62c5674a60
commit 6ebf041704
4 changed files with 71 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
ironic_python_agent/extensions/image.py
View File

@ -175,15 +175,17 @@ def prepare_boot_partitions_for_softraid(device, holders, efi_part,
target_part, holder)
# RAID the ESPs, metadata=1.0 is mandatory to be able to boot
md_device = '/dev/md/esp'
md_device = raid_utils.get_next_free_raid_device()
LOG.debug("Creating md device %(md_device)s for the ESPs "
"on %(efi_partitions)s",
{'md_device': md_device, 'efi_partitions': efi_partitions})
utils.execute('mdadm', '--create', md_device, '--force',
'--run', '--metadata=1.0', '--level', '1',
'--raid-devices', len(efi_partitions),
'--name', 'esp', '--raid-devices', len(efi_partitions),
*efi_partitions)
disk_utils.trigger_device_rescan(md_device)
if efi_part:
# Blockdev copy the source ESP and erase it
LOG.debug("Relocating EFI %s to %s", efi_part, md_device)

14
ironic_python_agent/raid_utils.py
View File

@ -248,3 +248,17 @@ def create_raid_device(index, logical_disk):
msg = "Failed re-add {} to {}: {}".format(
dev, md_device, e)
raise errors.SoftwareRAIDError(msg)
def get_next_free_raid_device():
"""Get a device name that is still free."""
from ironic_python_agent import hardware
names = {dev.name for dev in
hardware.dispatch_to_managers('list_block_devices')}
for idx in range(128):
name = f'/dev/md{idx}'
if name not in names:
return name
raise errors.SoftwareRAIDError("No free md (RAID) devices are left")

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

@ -27,6 +27,7 @@ from ironic_python_agent import errors
from ironic_python_agent.extensions import image
from ironic_python_agent import hardware
from ironic_python_agent import partition_utils
from ironic_python_agent import raid_utils
from ironic_python_agent.tests.unit import base
@ -1653,9 +1654,13 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640
uuid=self.fake_root_uuid)
self.assertFalse(mock_dispatch.called)
@mock.patch.object(disk_utils, 'trigger_device_rescan', autospec=True)
@mock.patch.object(raid_utils, 'get_next_free_raid_device', autospec=True,
return_value='/dev/md42')
@mock.patch.object(disk_utils, 'find_efi_partition', autospec=True)
def test_prepare_boot_partitions_for_softraid_uefi_gpt(
self, mock_efi_part, mock_execute, mock_dispatch):
self, mock_efi_part, mock_free_raid_device, mock_rescan,
mock_execute, mock_dispatch):
mock_efi_part.return_value = {'number': '12'}
mock_execute.side_effect = [
('451', None), # sgdisk -F
@ -1693,19 +1698,24 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640
mock.call('blkid'),
mock.call('blkid', '-l', '-t', 'PARTLABEL=uefi-holder-1',
'/dev/sdb'),
mock.call('mdadm', '--create', '/dev/md/esp', '--force', '--run',
'--metadata=1.0', '--level', '1', '--raid-devices', 2,
'/dev/sda12', '/dev/sdb14'),
mock.call('cp', '/dev/md0p12', '/dev/md/esp'),
mock.call('mdadm', '--create', '/dev/md42', '--force', '--run',
'--metadata=1.0', '--level', '1', '--name', 'esp',
'--raid-devices', 2, '/dev/sda12', '/dev/sdb14'),
mock.call('cp', '/dev/md0p12', '/dev/md42'),
mock.call('wipefs', '-a', '/dev/md0p12')
]
mock_execute.assert_has_calls(expected, any_order=False)
self.assertEqual(efi_part, '/dev/md/esp')
self.assertEqual(efi_part, '/dev/md42')
mock_rescan.assert_called_once_with('/dev/md42')
@mock.patch.object(disk_utils, 'trigger_device_rescan', autospec=True)
@mock.patch.object(raid_utils, 'get_next_free_raid_device', autospec=True,
return_value='/dev/md42')
@mock.patch.object(disk_utils, 'find_efi_partition', autospec=True)
@mock.patch.object(ilib_utils, 'mkfs', autospec=True)
def test_prepare_boot_partitions_for_softraid_uefi_gpt_esp_not_found(
self, mock_mkfs, mock_efi_part, mock_execute, mock_dispatch):
self, mock_mkfs, mock_efi_part, mock_free_raid_device,
mock_rescan, mock_execute, mock_dispatch):
mock_efi_part.return_value = None
mock_execute.side_effect = [
('451', None), # sgdisk -F
@ -1744,12 +1754,17 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640
]
mock_execute.assert_has_calls(expected, any_order=False)
mock_mkfs.assert_has_calls([
mock.call(path='/dev/md/esp', label='efi-part', fs='vfat'),
mock.call(path='/dev/md42', label='efi-part', fs='vfat'),
], any_order=False)
self.assertEqual(efi_part, '/dev/md/esp')
self.assertEqual(efi_part, '/dev/md42')
mock_rescan.assert_called_once_with('/dev/md42')
@mock.patch.object(disk_utils, 'trigger_device_rescan', autospec=True)
@mock.patch.object(raid_utils, 'get_next_free_raid_device', autospec=True,
return_value='/dev/md42')
def test_prepare_boot_partitions_for_softraid_uefi_gpt_efi_provided(
self, mock_execute, mock_dispatch):
self, mock_free_raid_device, mock_rescan,
mock_execute, mock_dispatch):
mock_execute.side_effect = [
('451', None), # sgdisk -F
(None, None), # sgdisk create part
@ -1785,14 +1800,14 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640
mock.call('blkid'),
mock.call('blkid', '-l', '-t', 'PARTLABEL=uefi-holder-1',
'/dev/sdb'),
mock.call('mdadm', '--create', '/dev/md/esp', '--force', '--run',
'--metadata=1.0', '--level', '1', '--raid-devices', 2,
'/dev/sda12', '/dev/sdb14'),
mock.call('cp', '/dev/md0p15', '/dev/md/esp'),
mock.call('mdadm', '--create', '/dev/md42', '--force', '--run',
'--metadata=1.0', '--level', '1', '--name', 'esp',
'--raid-devices', 2, '/dev/sda12', '/dev/sdb14'),
mock.call('cp', '/dev/md0p15', '/dev/md42'),
mock.call('wipefs', '-a', '/dev/md0p15')
]
mock_execute.assert_has_calls(expected, any_order=False)
self.assertEqual(efi_part, '/dev/md/esp')
self.assertEqual(efi_part, '/dev/md42')
@mock.patch.object(disk_utils, 'get_partition_table_type', autospec=True,
return_value='msdos')

23
ironic_python_agent/tests/unit/test_raid_utils.py
View File

@ -15,9 +15,11 @@ from unittest import mock
from oslo_concurrency import processutils
from ironic_python_agent import errors
from ironic_python_agent import hardware
from ironic_python_agent import raid_utils
from ironic_python_agent.tests.unit import base
from ironic_python_agent.tests.unit.samples import hardware_samples as hws
from ironic_python_agent.tests.unit import test_hardware
from ironic_python_agent import utils
@ -112,3 +114,24 @@ class TestRaidUtils(base.IronicAgentTest):
"Failed re-add /dev/sdb1 to /dev/md0",
raid_utils.create_raid_device, 0,
logical_disk)
@mock.patch.object(hardware, 'dispatch_to_managers', autospec=True)
class TestGetNextFreeRaidDevice(base.IronicAgentTest):
def test_ok(self, mock_dispatch):
mock_dispatch.return_value = \
test_hardware.RAID_BLK_DEVICE_TEMPLATE_DEVICES
result = raid_utils.get_next_free_raid_device()
self.assertEqual('/dev/md2', result)
mock_dispatch.assert_called_once_with('list_block_devices')
def test_no_device(self, mock_dispatch):
mock_dispatch.return_value = [
hardware.BlockDevice(name=f'/dev/md{idx}', model='RAID',
size=1765517033470, rotational=False,
vendor="FooTastic", uuid="")
for idx in range(128)
]
self.assertRaises(errors.SoftwareRAIDError,
raid_utils.get_next_free_raid_device)