openstack/ironic-python-agent
Code Issues Proposed changes

Merge "A boot partition on a GPT disk should be considered an EFI partition"

Browse Source
This commit is contained in:
Zuul 2020-04-16 16:22:49 +00:00 committed by Gerrit Code Review
commit 43d2b2cbe0
3 changed files with 58 additions and 70 deletions
ironic_python_agent
tests/unit
test_utils.py
utils.py
releasenotes/notes
uefi-esp-660fc2c650e6af92.yaml

101
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'))

22
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'])

5
releasenotes/notes/uefi-esp-660fc2c650e6af92.yaml Normal file

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