openstack
/
ironic-python-agent
Code Issues Proposed changes
Browse Source

Delete EFI boot entry duplicate labels first 

Some firmware seems to take an objection with EFI nvram
entries being deleted after one is added, resulting in the
entire entry table being reset to the last known good state.

This is problematic, as ultimately deployments can time out
if we previously booted with Networking, and the machine, while
commanded to do other wise, reboots back to networking regardless.

We will now delete entries first, before proceeding.

Additionally, for general use, this pattern may serve the
community better by avoiding cases where we would have
previously just relied upon efibootmgr[0] to warn us of duplicate
entries.

[0]: 103aa22ece/src/efibootmgr.c (L228)

Change-Id: Ib61a7100a059e79a8b0901fd8f46b9bc41d657dc
Story: 2009649
Task: 43808
(cherry picked from commit 67eddfa7e3)
(cherry picked from commit 33b39705a5)
(cherry picked from commit 8fca145739)
changes/17/817017/1
Julia Kreger 7 months ago
parent
750934a207
commit
47ac40a7f9
3 changed files with 51 additions and 24 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 34
      ironic_python_agent/extensions/image.py
  2. 35
      ironic_python_agent/tests/unit/extensions/test_image.py
  3. 6
      releasenotes/notes/de-duplicate-by-label-baa090c5b1bff992.yaml

34
ironic_python_agent/extensions/image.py
Unescape View File

@ -270,12 +270,10 @@ def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition,
# Before updating let's get information about the bootorder
LOG.debug("Getting information about boot order.")
utils.execute('efibootmgr', '-v')
# NOTE(iurygregory): regex used to identify the Warning in the stderr after
# we add the new entry. Example:
# "efibootmgr: ** Warning ** : Boot0004 has same label ironic"
duplicated_label = re.compile(r'^.*:\s\*\*.*\*\*\s:\s.*'
r'Boot([0-9a-f-A-F]+)\s.*$')
original_efi_output = utils.execute('efibootmgr', '-v')
# NOTE(TheJulia): regex used to identify entries in the efibootmgr
# output on stdout.
entry_label = re.compile(r'Boot([0-9a-f-A-F]+):\s(.*).*$')
label_id = 1
for v_bl in valid_efi_bootloaders:
if 'csv' in v_bl.lower():
@ -292,20 +290,26 @@ def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition,
v_efi_bl_path = '\\' + v_bl.replace('/', '\\')
label = 'ironic' + str(label_id)
# Iterate through standard out, and look for duplicates
for line in original_efi_output[0].split('\n'):
match = entry_label.match(line)
# Look for the base label in the string if a line match
# occurs, so we can identify if we need to eliminate the
# entry.
if match and label in match.group(2):
boot_num = match.group(1)
LOG.debug("Found bootnum %s matching label", boot_num)
utils.execute('efibootmgr', '-b', boot_num, '-B')
LOG.debug("Adding loader %(path)s on partition %(part)s of device "
" %(dev)s", {'path': v_efi_bl_path, 'part': efi_partition,
'dev': device})
# Update the nvram using efibootmgr
# https://linux.die.net/man/8/efibootmgr
cmd = utils.execute('efibootmgr', '-v', '-c', '-d', device,
'-p', efi_partition, '-w', '-L', label,
'-l', v_efi_bl_path)
for line in cmd[1].split('\n'):
match = duplicated_label.match(line)
if match:
boot_num = match.group(1)
LOG.debug("Found bootnum %s matching label", boot_num)
utils.execute('efibootmgr', '-b', boot_num, '-B')
utils.execute('efibootmgr', '-v', '-c', '-d', device,
'-p', efi_partition, '-w', '-L', label,
'-l', v_efi_bl_path)
# Increment the ID in case the loop runs again.
label_id += 1

35
ironic_python_agent/tests/unit/extensions/test_image.py
Unescape View File

@ -344,13 +344,17 @@ class TestImageExtension(base.IronicAgentTest):
mock_partition.return_value = self.fake_dev
mock_utils_efi_part.return_value = '1'
mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI']
stdeer_msg = """
efibootmgr: ** Warning ** : Boot0004 has same label ironic1\n
efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
"""
stdout_msg = """
BootCurrent: 0001
Timeout: 0 seconds
BootOrder: 0000,00001
Boot0000: ironic1 HD(1, GPT,4f3c6294-bf9b-4208-9808-be45dfc34b5c)File(\EFI\Boot\BOOTX64.EFI)
Boot0001: ironic2 HD(1, GPT,4f3c6294-bf9b-4208-9808-111111111112)File(\EFI\Boot\BOOTX64.EFI)
Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51)
""" # noqa This is a giant literal string for testing.
mock_execute.side_effect = iter([('', ''), ('', ''),
('', ''), ('', ''),
('', ''), ('', stdeer_msg),
(stdout_msg, ''), ('', ''),
('', ''), ('', ''),
('', ''), ('', '')])
@ -361,12 +365,11 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
mock.call('mount', self.fake_efi_system_part,
self.fake_dir + '/boot/efi'),
mock.call('efibootmgr', '-v'),
mock.call('efibootmgr', '-b', '0000', '-B'),
mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev,
'-p', '1', '-w',
'-L', 'ironic1', '-l',
'\\EFI\\BOOT\\BOOTX64.EFI'),
mock.call('efibootmgr', '-b', '0004', '-B'),
mock.call('efibootmgr', '-b', '0005', '-B'),
mock.call('umount', self.fake_dir + '/boot/efi',
attempts=3, delay_on_retry=True),
mock.call('sync')]
@ -381,7 +384,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
mock_efi_bl.assert_called_once_with(self.fake_dir + '/boot/efi')
mock_execute.assert_has_calls(expected)
mock_utils_efi_part.assert_called_once_with(self.fake_dev)
self.assertEqual(10, mock_execute.call_count)
self.assertEqual(9, mock_execute.call_count)
@mock.patch.object(hardware, 'is_md_device', lambda *_: False)
@mock.patch.object(os.path, 'exists', lambda *_: False)
@ -2150,8 +2153,19 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
# Mild difference, Ubuntu ships a file without a 0xFEFF delimiter
# at the start of the file, where as Red Hat *does*
csv_file_data = u'shimx64.efi,Vendor String,,Grub2MadeUSDoThis\n'
# This test also handles deleting a pre-existing matching vendor
# string in advance.
dupe_entry = """
BootCurrent: 0001
Timeout: 0 seconds
BootOrder: 0000,00001
Boot0000: Vendor String HD(1, GPT,4f3c6294-bf9b-4208-9808-be45dfc34b5c)File(\EFI\Boot\BOOTX64.EFI)
Boot0001: Vendor String HD(2, GPT,4f3c6294-bf9b-4208-9808-be45dfc34b5c)File(\EFI\Boot\BOOTX64.EFI)
Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51)
""" # noqa This is a giant literal string for testing.
mock_execute.side_effect = iter([('', ''), ('', ''),
('', ''), (dupe_entry, ''),
('', ''), ('', ''),
('', ''), ('', ''),
('', '')])
@ -2162,6 +2176,8 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
mock.call('mount', self.fake_efi_system_part,
self.fake_dir + '/boot/efi'),
mock.call('efibootmgr', '-v'),
mock.call('efibootmgr', '-b', '0000', '-B'),
mock.call('efibootmgr', '-b', '0001', '-B'),
mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev,
'-p', '1', '-w',
'-L', 'Vendor String', '-l',
@ -2176,7 +2192,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
mkdir_mock.assert_called_once_with(self.fake_dir + '/boot/efi')
mock_efi_bl.assert_called_once_with(self.fake_dir + '/boot/efi')
mock_execute.assert_has_calls(expected)
self.assertEqual(7, mock_execute.call_count)
self.assertEqual(9, mock_execute.call_count)
@mock.patch.object(os.path, 'exists', lambda *_: False)
@mock.patch.object(image, '_get_efi_bootloaders', autospec=True)
@ -2344,6 +2360,7 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
mock_execute.assert_has_calls(expected)
def test__run_efibootmgr(self, mock_execute, mock_dispatch):
mock_execute.return_value = ('', '')
result = image._run_efibootmgr(['EFI/BOOT/BOOTX64.EFI'],
self.fake_dev,
self.fake_efi_system_part,

6
releasenotes/notes/de-duplicate-by-label-baa090c5b1bff992.yaml
Unescape View File

@ -0,0 +1,6 @@
---
fixes:
- |
Fixes cases where duplicates may not be found in the UEFI
firmware NVRAM boot entry table by explicitly looking for, and deleting
for matching labels in advance of creating the EFI boot loader entry.
Write Preview
Loading…
Cancel
Save