A boot partition on a GPT disk should be considered an EFI partition
DIB builds instance images with EFI partitions that only have the boot flag, but not esp. According to parted documentation, boot is an alias for esp on GPT, so accept it as well. To avoid complexities when parsing parted output, the implementation is switched to existing utils and ironic-lib functions. Change-Id: I5f57535e5a89528c38d0879177b59db6c0f5c06e Story: #2007455 Task: #39423
This commit is contained in:
Dmitry Tantsur
parent
f6668f94c9
commit
ff49b04e28
@ -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'))
|
||||
|
||||
@ -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
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``).
|
||||
Loading…
Reference in New Issue
Block a user