From abe38a6a5fb8cbfda4064351103b0ec6971dfd3e Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Fri, 19 Nov 2021 14:48:38 +0100 Subject: [PATCH] Fix compatibility with disk_utils.find_efi_partition This function returns the complete block device record, not just number. Fixes regression in 89bc73aa0105850c6ae44428642e31802bba3b20. Also fix the incorrect job in the gate queue, which prevented us from catching this issue on merging. Change-Id: I4cbc359ceabfc193ce18fed14a1952359460e7d9 --- ironic_python_agent/efi_utils.py | 5 ++++- ironic_python_agent/extensions/image.py | 2 +- .../tests/unit/extensions/test_image.py | 12 ++++++------ ironic_python_agent/tests/unit/test_efi_utils.py | 8 ++++---- zuul.d/project.yaml | 2 +- 5 files changed, 16 insertions(+), 13 deletions(-) diff --git a/ironic_python_agent/efi_utils.py b/ironic_python_agent/efi_utils.py index 6f6b103cf..1f28f6352 100644 --- a/ironic_python_agent/efi_utils.py +++ b/ironic_python_agent/efi_utils.py @@ -50,6 +50,9 @@ def manage_uefi(device, efi_system_part_uuid=None): local_path = tempfile.mkdtemp() # Trust the contents on the disk in the event of a whole disk image. efi_partition = disk_utils.find_efi_partition(device) + if efi_partition: + efi_partition = efi_partition['number'] + if not efi_partition and efi_system_part_uuid: # _get_partition returns + and we only need the # partition number @@ -100,7 +103,7 @@ def manage_uefi(device, efi_system_part_uuid=None): return False except processutils.ProcessExecutionError as e: - error_msg = ('Could not verify uefi on device %(dev)s' + error_msg = ('Could not verify uefi on device %(dev)s, ' 'failed with %(err)s.' % {'dev': device, 'err': e}) LOG.error(error_msg) raise errors.CommandExecutionError(error_msg) diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py index cd86b91ba..11caf5859 100644 --- a/ironic_python_agent/extensions/image.py +++ b/ironic_python_agent/extensions/image.py @@ -139,7 +139,7 @@ def _prepare_boot_partitions_for_softraid(device, holders, efi_part, # let grub handle the magic. efi_part = disk_utils.find_efi_partition(device) if efi_part: - efi_part = '{}p{}'.format(device, efi_part) + efi_part = '{}p{}'.format(device, efi_part['number']) LOG.info("Creating EFI partitions on software RAID holder disks") # We know that we kept this space when configuring raid,see diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py index 0c2844a1d..4d389005b 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -225,7 +225,7 @@ class TestImageExtension(base.IronicAgentTest): ] mock_partition.side_effect = [self.fake_dev, self.fake_efi_system_part] mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] - mock_utils_efi_part.return_value = '1' + mock_utils_efi_part.return_value = {'number': '1'} mock_execute.side_effect = iter([('', ''), ('', ''), ('', ''), ('', ''), @@ -272,7 +272,7 @@ class TestImageExtension(base.IronicAgentTest): self.fake_dev, hardware.BootInfo(current_boot_mode='uefi') ] mock_partition.return_value = self.fake_dev - mock_utils_efi_part.return_value = '1' + mock_utils_efi_part.return_value = {'number': '1'} mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] mock_execute.side_effect = iter([('', ''), ('', ''), ('', ''), ('', ''), @@ -319,7 +319,7 @@ class TestImageExtension(base.IronicAgentTest): self.fake_dev, hardware.BootInfo(current_boot_mode='uefi') ] mock_partition.return_value = self.fake_dev - mock_utils_efi_part.return_value = '1' + mock_utils_efi_part.return_value = {'number': '1'} mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] stdout_msg = """ BootCurrent: 0001 @@ -376,7 +376,7 @@ Boot0002 VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) self.fake_dev, hardware.BootInfo(current_boot_mode='uefi') ] mock_partition.return_value = self.fake_dev - mock_utils_efi_part.return_value = '1' + mock_utils_efi_part.return_value = {'number': '1'} mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] # NOTE(TheJulia): This test string was derived from a lenovo SR650 # which does do some weird things with additional entries. @@ -438,7 +438,7 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 self.fake_dev, hardware.BootInfo(current_boot_mode='uefi') ] mock_partition.return_value = self.fake_dev - mock_utils_efi_part.return_value = '1' + mock_utils_efi_part.return_value = {'number': '1'} mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI', 'WINDOWS/system32/winload.efi'] @@ -1656,7 +1656,7 @@ Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x640 @mock.patch.object(disk_utils, 'find_efi_partition', autospec=True) def test__prepare_boot_partitions_for_softraid_uefi_gpt( self, mock_efi_part, mock_execute, mock_dispatch): - mock_efi_part.return_value = '12' + mock_efi_part.return_value = {'number': '12'} mock_execute.side_effect = [ ('451', None), # sgdisk -F (None, None), # sgdisk create part diff --git a/ironic_python_agent/tests/unit/test_efi_utils.py b/ironic_python_agent/tests/unit/test_efi_utils.py index da5122c26..0933cc32f 100644 --- a/ironic_python_agent/tests/unit/test_efi_utils.py +++ b/ironic_python_agent/tests/unit/test_efi_utils.py @@ -162,7 +162,7 @@ class TestManageUefi(base.IronicAgentTest): @mock.patch.object(os, 'makedirs', autospec=True) def test_ok(self, mkdir_mock, mock_efi_bl, mock_utils_efi_part, mock_get_part_uuid, mock_execute, mock_rescan): - mock_utils_efi_part.return_value = '1' + mock_utils_efi_part.return_value = {'number': '1'} mock_get_part_uuid.return_value = self.fake_dev mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] @@ -196,7 +196,7 @@ class TestManageUefi(base.IronicAgentTest): @mock.patch.object(os, 'makedirs', autospec=True) def test_found_csv(self, mkdir_mock, mock_efi_bl, mock_utils_efi_part, mock_get_part_uuid, mock_execute, mock_rescan): - mock_utils_efi_part.return_value = '1' + mock_utils_efi_part.return_value = {'number': '1'} mock_get_part_uuid.return_value = self.fake_dev mock_efi_bl.return_value = ['EFI/vendor/BOOTX64.CSV'] @@ -246,7 +246,7 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) @mock.patch.object(os, 'makedirs', autospec=True) def test_nvme_device(self, mkdir_mock, mock_efi_bl, mock_utils_efi_part, mock_get_part_uuid, mock_execute, mock_rescan): - mock_utils_efi_part.return_value = '1' + mock_utils_efi_part.return_value = {'number': '1'} mock_get_part_uuid.return_value = '/dev/fakenvme0p1' mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] @@ -278,7 +278,7 @@ Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) @mock.patch.object(os, 'makedirs', autospec=True) def test_wholedisk(self, mkdir_mock, mock_efi_bl, mock_utils_efi_part, mock_get_part_uuid, mock_execute, mock_rescan): - mock_utils_efi_part.return_value = '1' + mock_utils_efi_part.return_value = {'number': '1'} mock_get_part_uuid.side_effect = Exception mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] diff --git a/zuul.d/project.yaml b/zuul.d/project.yaml index fe9355b81..0983e938e 100644 --- a/zuul.d/project.yaml +++ b/zuul.d/project.yaml @@ -39,7 +39,7 @@ - openstack-tox-functional - ipa-tempest-bios-ipmi-direct-src - ipa-tempest-uefi-redfish-vmedia-src - - metalsmith-integration-ipa-src + - metalsmith-integration-ipa-src-uefi post: jobs: - ironic-python-agent-build-image-tinyipa