From 8569b7b83051e802b2b248570ae9d720a2c4d1b5 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Mon, 31 Jan 2022 11:19:10 +0100 Subject: Use canonical device name for RAID device for ESP 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/ (cherry picked from commit 6ebf0417044d7266cdd8a6c7458f094bc29023fb) --- ironic_python_agent/extensions/image.py | 6 ++- ironic_python_agent/raid_utils.py | 14 +++++++ .../tests/unit/extensions/test_image.py | 45 ++++++++++++++-------- ironic_python_agent/tests/unit/test_raid_utils.py | 23 +++++++++++ 4 files changed, 71 insertions(+), 17 deletions(-) diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py index c917c111..6f20c8a8 100644 --- a/ironic_python_agent/extensions/image.py +++ b/ironic_python_agent/extensions/image.py @@ -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) diff --git a/ironic_python_agent/raid_utils.py b/ironic_python_agent/raid_utils.py index 962a6e9a..d4c338a8 100644 --- a/ironic_python_agent/raid_utils.py +++ b/ironic_python_agent/raid_utils.py @@ -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") diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py index c687e59e..b2506b9c 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -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') diff --git a/ironic_python_agent/tests/unit/test_raid_utils.py b/ironic_python_agent/tests/unit/test_raid_utils.py index 30a13065..a20bd13e 100644 --- a/ironic_python_agent/tests/unit/test_raid_utils.py +++ b/ironic_python_agent/tests/unit/test_raid_utils.py @@ -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) -- cgit v1.2.1