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.

Conflicts:
	ironic_python_agent/tests/unit/test_utils.py
	ironic_python_agent/utils.py

Change-Id: I5f57535e5a89528c38d0879177b59db6c0f5c06e
Story: #2007455
Task: #39423
(cherry picked from commit ff49b04e28)
This commit is contained in:
Dmitry Tantsur 2020-04-15 12:44:37 +02:00
parent 1d418816d1
commit 53bab6ce44
3 changed files with 98 additions and 58 deletions

View File

@ -22,6 +22,7 @@ import subprocess
import tarfile
import tempfile
from ironic_lib import disk_utils
from ironic_lib import utils as ironic_utils
import mock
from oslo_concurrency import processutils
@ -46,30 +47,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,35 +607,73 @@ class TestUtils(testtools.TestCase):
)
@mock.patch.object(utils, 'execute', autospec=True)
def test_get_efi_part_on_device(self, mocked_execute):
parted_ret = PARTED_OUTPUT_UNFORMATTED.format('gpt')
def test_scan_partition_table_type_gpt(self, mocked_execute):
self._test_scan_partition_table_by_type(mocked_execute, 'gpt', 'gpt')
@mock.patch.object(utils, 'execute', autospec=True)
def test_scan_partition_table_type_msdos(self, mocked_execute):
self._test_scan_partition_table_by_type(mocked_execute, 'msdos',
'msdos')
@mock.patch.object(utils, 'execute', autospec=True)
def test_scan_partition_table_type_unknown(self, mocked_execute):
self._test_scan_partition_table_by_type(mocked_execute, 'whatever',
'unknown')
def _test_scan_partition_table_by_type(self, mocked_execute,
table_type_output,
expected_table_type):
parted_ret = PARTED_OUTPUT_UNFORMATTED.format(table_type_output)
mocked_execute.side_effect = [
(parted_ret, None)
(parted_ret, None),
]
ret = utils.scan_partition_table_type('hello')
mocked_execute.assert_has_calls(
[mock.call('parted', '-s', 'hello', '--', 'print')]
)
self.assertEqual(expected_table_type, ret)
@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')
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.format('gpt')
mocked_execute.side_effect = [
(parted_ret, None)
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')
mocked_execute.assert_has_calls(
[mock.call('parted', '-s', '/dev/sda', '--', 'print')]
)
self.assertEqual('1', ret)
self.assertEqual('15', 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)
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'))
mocked_execute.assert_has_calls(
[mock.call('parted', '-s', '/dev/sda', '--', 'print')]
)

View File

@ -24,6 +24,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_log import log as logging
@ -63,7 +64,8 @@ COLLECT_LOGS_COMMANDS = {
DEVICE_EXTRACTOR = re.compile(r'^(?:(.*\d)p|(.*\D))(?:\d+)$')
PARTED_ESP_PATTERN = re.compile(r'^\s*(\d+)\s.*\s\s.*\s.*esp(,|\s|$).*$')
PARTED_TABLE_TYPE_REGEX = re.compile(r'^.*partition\s+table\s*:\s*(gpt|msdos)',
re.IGNORECASE)
def execute(*cmd, **kwargs):
@ -457,21 +459,39 @@ def extract_device(part):
return (m.group(1) or m.group(2))
def scan_partition_table_type(device):
"""Get partition table type, msdos or gpt.
:param device_name: the name of the device
:return: msdos, gpt or unknown
"""
out, _u = execute('parted', '-s', device, '--', 'print')
out = out.splitlines()
for line in out:
m = PARTED_TABLE_TYPE_REGEX.match(line)
if m:
return m.group(1)
LOG.warning("Unable to get partition table type for device %s.",
device)
return 'unknown'
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

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