Skip to content

Commit 42f2456

Browse files
markgoddardmarkgoddard
authored
Merge pull request #54 from stackhpc/upstream/wallaby-2023-07-17
Synchronise wallaby with upstream
2 parents 5c58878 + e1813e1 commit 42f2456

File tree

3 files changed

+52
-27
lines changed

3 files changed

+52
-27
lines changed

‎ironic_python_agent/extensions/image.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,15 @@ def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition,
275275

276276
# Before updating let's get information about the bootorder
277277
LOG.debug("Getting information about boot order.")
278-
original_efi_output = utils.execute('efibootmgr', '-v')
278+
# Invokes binary=True so we get a bytestream back.
279+
original_efi_output = utils.execute('efibootmgr', '-v', binary=True)
280+
# Bytes must be decoded before regex can be run and
281+
# matching to work as intended.
282+
# Also ignore errors on decoding, as we can basically get
283+
# garbage out of the nvram record, this way we don't fail
284+
# hard on unrelated records.
285+
original_efi_output = original_efi_output[0].decode(
286+
'utf-16', errors='ignore')
279287
# NOTE(TheJulia): regex used to identify entries in the efibootmgr
280288
# output on stdout.
281289
entry_label = re.compile(r'Boot([0-9a-f-A-F]+)\*?\s(.*).*$')
@@ -304,7 +312,7 @@ def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition,
304312
label = 'ironic' + str(label_id)
305313

306314
# Iterate through standard out, and look for duplicates
307-
for line in original_efi_output[0].split('\n'):
315+
for line in original_efi_output.split('\n'):
308316
match = entry_label.match(line)
309317
# Look for the base label in the string if a line match
310318
# occurs, so we can identify if we need to eliminate the

‎ironic_python_agent/tests/unit/extensions/test_image.py

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@
3030
from ironic_python_agent import utils
3131

3232

33+
EFI_RESULT = ''.encode('utf-16')
34+
35+
3336
@mock.patch.object(hardware, 'dispatch_to_managers', autospec=True)
3437
@mock.patch.object(utils, 'execute', autospec=True)
3538
@mock.patch.object(tempfile, 'mkdtemp', lambda *_: '/tmp/fake-dir')
@@ -251,7 +254,7 @@ def test__uefi_bootloader_given_partition(
251254

252255
mock_execute.side_effect = iter([('', ''), ('', ''),
253256
('', ''), ('', ''),
254-
('', ''), ('', ''),
257+
(EFI_RESULT, ''), (EFI_RESULT, ''),
255258
('', ''), ('', '')])
256259

257260
expected = [mock.call('efibootmgr', '--version'),
@@ -260,7 +263,7 @@ def test__uefi_bootloader_given_partition(
260263
mock.call('udevadm', 'settle'),
261264
mock.call('mount', self.fake_efi_system_part,
262265
self.fake_dir + '/boot/efi'),
263-
mock.call('efibootmgr', '-v'),
266+
mock.call('efibootmgr', '-v', binary=True),
264267
mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev,
265268
'-p', '1', '-w',
266269
'-L', 'ironic1', '-l',
@@ -299,7 +302,7 @@ def test__uefi_bootloader_find_partition(
299302
mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI']
300303
mock_execute.side_effect = iter([('', ''), ('', ''),
301304
('', ''), ('', ''),
302-
('', ''), ('', ''),
305+
(EFI_RESULT, ''), (EFI_RESULT, ''),
303306
('', ''), ('', '')])
304307

305308
expected = [mock.call('efibootmgr', '--version'),
@@ -308,7 +311,7 @@ def test__uefi_bootloader_find_partition(
308311
mock.call('udevadm', 'settle'),
309312
mock.call('mount', self.fake_efi_system_part,
310313
self.fake_dir + '/boot/efi'),
311-
mock.call('efibootmgr', '-v'),
314+
mock.call('efibootmgr', '-v', binary=True),
312315
mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev,
313316
'-p', '1', '-w',
314317
'-L', 'ironic1', '-l',
@@ -353,10 +356,11 @@ def test__uefi_bootloader_with_entry_removal(
353356
Boot0001 ironic2 HD(1,GPT,4f3c6294-bf9b-4208-9808-111111111112)File(\EFI\Boot\BOOTX64.EFI)
354357
Boot0002 VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51)
355358
""" # noqa This is a giant literal string for testing.
359+
stdout_msg = stdout_msg.encode('utf-16')
356360
mock_execute.side_effect = iter([('', ''), ('', ''),
357361
('', ''), ('', ''),
358-
(stdout_msg, ''), ('', ''),
359-
('', ''), ('', ''),
362+
(stdout_msg, ''), (EFI_RESULT, ''),
363+
(EFI_RESULT, ''), (EFI_RESULT, ''),
360364
('', ''), ('', '')])
361365

362366
expected = [mock.call('efibootmgr', '--version'),
@@ -365,7 +369,7 @@ def test__uefi_bootloader_with_entry_removal(
365369
mock.call('udevadm', 'settle'),
366370
mock.call('mount', self.fake_efi_system_part,
367371
self.fake_dir + '/boot/efi'),
368-
mock.call('efibootmgr', '-v'),
372+
mock.call('efibootmgr', '-v', binary=True),
369373
mock.call('efibootmgr', '-b', '0000', '-B'),
370374
mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev,
371375
'-p', '1', '-w',
@@ -416,6 +420,7 @@ def test__uefi_bootloader_with_entry_removal_lenovo(
416420
Boot0003* Network VenHw(1fad3248-0000-7950-2166-a1e506fdb83a,05000000)..GO..NO............U.E.F.I.:. . . .S.L.O.T.2. .(.2.F./.0./.0.). .P.X.E. .I.P.4. . .Q.L.o.g.i.c. .Q.L.4.1.2.6.2. .P.C.I.e. .2.5.G.b. .2.-.P.o.r.t. .S.F.P.2.8. .E.t.h.e.r.n.e.t. .A.d.a.p.t.e.r. .-. .P.X.E........A....................%.4..Z...............................................................Gd-.;.A..MQ..L.P.X.E. .I.P.4. .Q.L.o.g.i.c. .Q.L.4.1.2.6.2. .P.C.I.e. .2.5.G.b. .2.-.P.o.r.t. .S.F.P.2.8. .E.t.h.e.r.n.e.t. .A.d.a.p.t.e.r. .-. .P.X.E.......BO..NO............U.E.F.I.:. . . .S.L.O.T.1. .(.3.0./.0./.0.). .P.X.E. .I.P.4. . .Q.L.o.g.i.c. .Q.L.4.1.2.6.2. .P.C.I.e. .2.5.G.b. .2.-.P.o.r.t. .S.F.P.2.8. .E.t.h.e.r.n.e.t. .A.d.a.p.t.e.r. .-.
417421
Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x64000)/File(\EFI\boot\shimx64.efi)
418422
""" # noqa This is a giant literal string for testing.
423+
stdout_msg = stdout_msg.encode('utf-16')
419424
mock_execute.side_effect = iter([('', ''), ('', ''),
420425
('', ''), ('', ''),
421426
(stdout_msg, ''), ('', ''),
@@ -427,7 +432,7 @@ def test__uefi_bootloader_with_entry_removal_lenovo(
427432
mock.call('udevadm', 'settle'),
428433
mock.call('mount', self.fake_efi_system_part,
429434
self.fake_dir + '/boot/efi'),
430-
mock.call('efibootmgr', '-v'),
435+
mock.call('efibootmgr', '-v', binary=True),
431436
mock.call('efibootmgr', '-b', '0000', '-B'),
432437
mock.call('efibootmgr', '-b', '0004', '-B'),
433438
mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev,
@@ -470,8 +475,8 @@ def test__add_multi_bootloaders(
470475

471476
mock_execute.side_effect = iter([('', ''), ('', ''),
472477
('', ''), ('', ''),
473-
('', ''), ('', ''),
474-
('', ''), ('', ''),
478+
(EFI_RESULT, ''), (EFI_RESULT, ''),
479+
(EFI_RESULT, ''), ('', ''),
475480
('', '')])
476481

477482
expected = [mock.call('efibootmgr', '--version'),
@@ -480,7 +485,7 @@ def test__add_multi_bootloaders(
480485
mock.call('udevadm', 'settle'),
481486
mock.call('mount', self.fake_efi_system_part,
482487
self.fake_dir + '/boot/efi'),
483-
mock.call('efibootmgr', '-v'),
488+
mock.call('efibootmgr', '-v', binary=True),
484489
mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev,
485490
'-p', '1', '-w',
486491
'-L', 'ironic1', '-l',
@@ -2379,7 +2384,7 @@ def test__manage_uefi(self, mkdir_mock, mock_utils_efi_part,
23792384
mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI']
23802385

23812386
mock_execute.side_effect = iter([('', ''), ('', ''),
2382-
('', ''), ('', ''),
2387+
('', ''), (b'', ''),
23832388
('', ''), ('', ''),
23842389
('', '')])
23852390

@@ -2388,7 +2393,7 @@ def test__manage_uefi(self, mkdir_mock, mock_utils_efi_part,
23882393
mock.call('udevadm', 'settle'),
23892394
mock.call('mount', self.fake_efi_system_part,
23902395
self.fake_dir + '/boot/efi'),
2391-
mock.call('efibootmgr', '-v'),
2396+
mock.call('efibootmgr', '-v', binary=True),
23922397
mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev,
23932398
'-p', '1', '-w',
23942399
'-L', 'ironic1', '-l',
@@ -2432,18 +2437,19 @@ def test__manage_uefi_found_csv(self, mkdir_mock, mock_utils_efi_part,
24322437
Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51)
24332438
""" # noqa This is a giant literal string for testing.
24342439

2435-
mock_execute.side_effect = iter([('', ''), ('', ''),
2436-
('', ''), (dupe_entry, ''),
2437-
('', ''), ('', ''),
2438-
('', ''), ('', ''),
2439-
('', '')])
2440+
mock_execute.side_effect = iter([
2441+
('', ''), ('', ''),
2442+
('', ''), (dupe_entry.encode('utf-16'), ''),
2443+
('', ''), ('', ''),
2444+
('', ''), ('', ''),
2445+
('', '')])
24402446

24412447
expected = [mock.call('partx', '-a', '/dev/fake', attempts=3,
24422448
delay_on_retry=True),
24432449
mock.call('udevadm', 'settle'),
24442450
mock.call('mount', self.fake_efi_system_part,
24452451
self.fake_dir + '/boot/efi'),
2446-
mock.call('efibootmgr', '-v'),
2452+
mock.call('efibootmgr', '-v', binary=True),
24472453
mock.call('efibootmgr', '-b', '0000', '-B'),
24482454
mock.call('efibootmgr', '-b', '0001', '-B'),
24492455
mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev,
@@ -2475,7 +2481,7 @@ def test__manage_uefi_nvme_device(self, mkdir_mock, mock_utils_efi_part,
24752481
mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI']
24762482

24772483
mock_execute.side_effect = iter([('', ''), ('', ''),
2478-
('', ''), ('', ''),
2484+
('', ''), (b'', ''),
24792485
('', ''), ('', ''),
24802486
('', '')])
24812487

@@ -2484,7 +2490,7 @@ def test__manage_uefi_nvme_device(self, mkdir_mock, mock_utils_efi_part,
24842490
mock.call('udevadm', 'settle'),
24852491
mock.call('mount', '/dev/fakenvme0p1',
24862492
self.fake_dir + '/boot/efi'),
2487-
mock.call('efibootmgr', '-v'),
2493+
mock.call('efibootmgr', '-v', binary=True),
24882494
mock.call('efibootmgr', '-v', '-c', '-d', '/dev/fakenvme0',
24892495
'-p', '1', '-w',
24902496
'-L', 'ironic1', '-l',
@@ -2513,7 +2519,7 @@ def test__manage_uefi_wholedisk(
25132519
mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI']
25142520

25152521
mock_execute.side_effect = iter([('', ''), ('', ''),
2516-
('', ''), ('', ''),
2522+
('', ''), (b'', ''),
25172523
('', ''), ('', ''),
25182524
('', '')])
25192525

@@ -2522,7 +2528,7 @@ def test__manage_uefi_wholedisk(
25222528
mock.call('udevadm', 'settle'),
25232529
mock.call('mount', self.fake_efi_system_part,
25242530
self.fake_dir + '/boot/efi'),
2525-
mock.call('efibootmgr', '-v'),
2531+
mock.call('efibootmgr', '-v', binary=True),
25262532
mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev,
25272533
'-p', '1', '-w',
25282534
'-L', 'ironic1', '-l',
@@ -2625,12 +2631,14 @@ def test__run_efibootmgr_no_bootloaders(self, mock_execute, mock_dispatch):
26252631
mock_execute.assert_has_calls(expected)
26262632

26272633
def test__run_efibootmgr(self, mock_execute, mock_dispatch):
2628-
mock_execute.return_value = ('', '')
2634+
mock_execute.side_effect = [
2635+
(b'', ''),
2636+
('', '')]
26292637
result = image._run_efibootmgr(['EFI/BOOT/BOOTX64.EFI'],
26302638
self.fake_dev,
26312639
self.fake_efi_system_part,
26322640
self.fake_dir)
2633-
expected = [mock.call('efibootmgr', '-v'),
2641+
expected = [mock.call('efibootmgr', '-v', binary=True),
26342642
mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev,
26352643
'-p', self.fake_efi_system_part, '-w',
26362644
'-L', 'ironic1', '-l',

‎releasenotes/notes/fixes-efibootmgr-character-encoding-19e531ba694824c1.yaml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
fixes:
3+
- |
4+
Fixes UEFI NVRAM record handling with efibootmgr so we can accept and
5+
handle UTF-16 encoded data which is to be expected in UEFI NVRAM as
6+
the records are UTF-16 encoded.
7+
- |
8+
Fixes handling of UEFI NVRAM records to allow for unexpected characters
9+
in the response, so it is non-fatal to Ironic.

0 commit comments

Comments
 (0)