diff --git a/ironic_python_agent/tests/unit/test_utils.py b/ironic_python_agent/tests/unit/test_utils.py index 48948fc35..214072921 100644 --- a/ironic_python_agent/tests/unit/test_utils.py +++ b/ironic_python_agent/tests/unit/test_utils.py @@ -23,6 +23,7 @@ import tarfile import tempfile from unittest import mock +from ironic_lib import disk_utils from ironic_lib import utils as ironic_utils from oslo_concurrency import processutils from oslo_serialization import base64 @@ -45,30 +46,6 @@ Number Start End Size File system Name Flags 1 116MB 2361MB 2245MB ext4 ''' -PARTED_OUTPUT_UNFORMATTED_NOFS = '''Model: whatever -Disk /dev/sda: 480GB -Sector size (logical/physical): 512B/512B -Partition Table: gpt -Disk Flags: - -Number Start End Size File system Name Flags -1 1049kB 9437kB 8389kB ESP boot, esp -2 9437kB 17.8MB 8389kB BSP bios_grub -3 17.8MB 40.0GB 40.0GB -4 479GB 480GB 68.1MB -''' - -PARTED_OUTPUT_NO_EFI = '''Model: whatever -Disk /dev/sda: 450GB -Sector size (logical/physical): 512B/512B -Partition Table: gpt -Disk Flags: - -Number Start End Size File system Name Flags -14 1049kB 5243kB 4194kB bios_grub - 1 116MB 2361MB 2245MB ext4 -''' - class ExecuteTestCase(ironic_agent_base.IronicAgentTest): # This test case does call utils.execute(), so don't block access to the @@ -630,40 +607,6 @@ class TestUtils(testtools.TestCase): utils.extract_device('whatevernotmatchin12a') ) - @mock.patch.object(utils, 'execute', autospec=True) - def test_get_efi_part_on_device(self, mocked_execute): - parted_ret = PARTED_OUTPUT_UNFORMATTED.format('gpt') - mocked_execute.side_effect = [ - (parted_ret, None) - ] - ret = utils.get_efi_part_on_device('/dev/sda') - mocked_execute.assert_has_calls( - [mock.call('parted', '-s', '/dev/sda', '--', 'print')] - ) - self.assertEqual('15', ret) - - @mock.patch.object(utils, 'execute', autospec=True) - def test_get_efi_part_on_device_without_fs(self, mocked_execute): - parted_ret = PARTED_OUTPUT_UNFORMATTED_NOFS - mocked_execute.side_effect = [ - (parted_ret, None) - ] - ret = utils.get_efi_part_on_device('/dev/sda') - mocked_execute.assert_has_calls( - [mock.call('parted', '-s', '/dev/sda', '--', 'print')] - ) - self.assertEqual('1', ret) - - @mock.patch.object(utils, 'execute', autospec=True) - def test_get_efi_part_on_device_not_found(self, mocked_execute): - mocked_execute.side_effect = [ - (PARTED_OUTPUT_NO_EFI, None) - ] - self.assertIsNone(utils.get_efi_part_on_device('/dev/sda')) - mocked_execute.assert_has_calls( - [mock.call('parted', '-s', '/dev/sda', '--', 'print')] - ) - def test_extract_capability_from_dict(self): expected_dict = {"hello": "world"} root = {"capabilities": expected_dict} @@ -1040,3 +983,45 @@ class TestClockSyncUtils(ironic_agent_base.IronicAgentTest): mock_time_method.return_value = None utils.sync_clock() self.assertEqual(0, mock_execute.call_count) + + +@mock.patch.object(disk_utils, 'list_partitions', autospec=True) +@mock.patch.object(utils, 'scan_partition_table_type', autospec=True) +class TestGetEfiPart(testtools.TestCase): + + def test_get_efi_part_on_device(self, mocked_type, mocked_parts): + mocked_parts.return_value = [ + {'number': '1', 'flags': ''}, + {'number': '14', 'flags': 'bios_grub'}, + {'number': '15', 'flags': 'esp, boot'}, + ] + ret = utils.get_efi_part_on_device('/dev/sda') + self.assertEqual('15', ret) + + def test_get_efi_part_on_device_only_boot_flag_gpt(self, mocked_type, + mocked_parts): + mocked_type.return_value = 'gpt' + mocked_parts.return_value = [ + {'number': '1', 'flags': ''}, + {'number': '14', 'flags': 'bios_grub'}, + {'number': '15', 'flags': 'boot'}, + ] + ret = utils.get_efi_part_on_device('/dev/sda') + self.assertEqual('15', ret) + + def test_get_efi_part_on_device_only_boot_flag_mbr(self, mocked_type, + mocked_parts): + mocked_type.return_value = 'msdos' + mocked_parts.return_value = [ + {'number': '1', 'flags': ''}, + {'number': '14', 'flags': 'bios_grub'}, + {'number': '15', 'flags': 'boot'}, + ] + self.assertIsNone(utils.get_efi_part_on_device('/dev/sda')) + + def test_get_efi_part_on_device_not_found(self, mocked_type, mocked_parts): + mocked_parts.return_value = [ + {'number': '1', 'flags': ''}, + {'number': '14', 'flags': 'bios_grub'}, + ] + self.assertIsNone(utils.get_efi_part_on_device('/dev/sda')) diff --git a/ironic_python_agent/utils.py b/ironic_python_agent/utils.py index e74cc1f16..dca48d118 100644 --- a/ironic_python_agent/utils.py +++ b/ironic_python_agent/utils.py @@ -25,6 +25,7 @@ import tarfile import tempfile import time +from ironic_lib import disk_utils from ironic_lib import utils as ironic_utils from oslo_concurrency import processutils from oslo_config import cfg @@ -73,7 +74,6 @@ DEVICE_EXTRACTOR = re.compile(r'^(?:(.*\d)p|(.*\D))(?:\d+)$') PARTED_TABLE_TYPE_REGEX = re.compile(r'^.*partition\s+table\s*:\s*(gpt|msdos)', re.IGNORECASE) -PARTED_ESP_PATTERN = re.compile(r'^\s*(\d+)\s.*\s\s.*\s.*esp(,|\s|$).*$') def execute(*cmd, **kwargs): @@ -611,23 +611,21 @@ def scan_partition_table_type(device): def get_efi_part_on_device(device): - """Looks for the efi partition on a given device + """Looks for the efi partition on a given device. + + A boot partition on a GPT disk is assumed to be an EFI partition as well. :param device: lock device upon which to check for the efi partition :return: the efi partition or None """ - efi_part = None - out, _u = execute('parted', '-s', device, '--', 'print') - for line in out.splitlines(): - m = PARTED_ESP_PATTERN.match(line) - if m: - efi_part = m.group(1) - - LOG.debug("Found efi partition %s on device %s.", efi_part, device) - break + is_gpt = scan_partition_table_type(device) == 'gpt' + for part in disk_utils.list_partitions(device): + flags = {x.strip() for x in part['flags'].split(',')} + if 'esp' in flags or ('boot' in flags and is_gpt): + LOG.debug("Found EFI partition %s on device %s.", part, device) + return part['number'] else: LOG.debug("No efi partition found on device %s", device) - return efi_part _LARGE_KEYS = frozenset(['configdrive', 'system_logs']) diff --git a/releasenotes/notes/uefi-esp-660fc2c650e6af92.yaml b/releasenotes/notes/uefi-esp-660fc2c650e6af92.yaml new file mode 100644 index 000000000..8701019e4 --- /dev/null +++ b/releasenotes/notes/uefi-esp-660fc2c650e6af92.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + No longer tries to use GRUB2 for configuring boot for whole disk images + with an EFI partition present but only marked as ``boot`` (not ``esp``).