summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJulia Kreger <juliaashleykreger@gmail.com>2021-04-27 11:09:52 -0700
committerJulia Kreger <juliaashleykreger@gmail.com>2021-05-26 18:05:00 +0000
commite3da25329df5ade4f30da562f1b196ecae437636 (patch)
treefa84a9ead425fb37ee005f2f6312d9ff3834902c
parent9f2ef8ab02f81ce1a652f76e2c09689dd7d574a8 (diff)
downloadironic-python-agent-e3da25329df5ade4f30da562f1b196ecae437636.tar.gz
Fix NVMe Partition image on UEFI
The _manage_uefi code has a check where it attempts to just identify the precise partition number of the device, in order for configuration to be parsed and passed. However, the same code did not handle the existence of a `p1` partition instead of just a partition #1. This is because the device naming format is different with NVMe and Software RAID. Likely, this wasn't an issue with software raid due to how complex the code interaction is, but the docs also indicate to use only whole disk images in that case. This patch was pulled down my one RH's professional services folks who has confirmed it does indeed fix the issue at hand. This is noted as a public comment on the Red Hat bugzilla. https://bugzilla.redhat.com/show_bug.cgi?id=1954096 Story: 2008881 Task: 42426 Related: rhbz#1954096 Change-Id: Ie3bd49add9a57fabbcdcbae4b73309066b620d02 (cherry picked from commit fe825fa97ed1f3c9fa8b1461b63ab133fec20b72)
-rw-r--r--ironic_python_agent/extensions/image.py7
-rw-r--r--ironic_python_agent/tests/unit/extensions/test_image.py39
-rw-r--r--releasenotes/notes/fix-nvme-partition-image-handling-b8487133a188fd32.yaml6
3 files changed, 51 insertions, 1 deletions
diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py
index 879ae3b4..e0dfd20a 100644
--- a/ironic_python_agent/extensions/image.py
+++ b/ironic_python_agent/extensions/image.py
@@ -289,7 +289,12 @@ def _manage_uefi(device, efi_system_part_uuid=None):
# _get_partition returns <device>+<partition> and we only need the
# partition number
partition = _get_partition(device, uuid=efi_system_part_uuid)
- efi_partition = int(partition.replace(device, ""))
+ try:
+ efi_partition = int(partition.replace(device, ""))
+ except ValueError:
+ # NVMe Devices get a partitioning scheme that is different from
+ # traditional block devices like SCSI/SATA
+ efi_partition = int(partition.replace(device + 'p', ""))
if efi_partition:
efi_partition_mount_point = os.path.join(local_path, "boot/efi")
diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py
index 9f4ae88c..78085900 100644
--- a/ironic_python_agent/tests/unit/extensions/test_image.py
+++ b/ironic_python_agent/tests/unit/extensions/test_image.py
@@ -993,6 +993,45 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
@mock.patch.object(image, '_get_partition', autospec=True)
@mock.patch.object(utils, 'get_efi_part_on_device', autospec=True)
@mock.patch.object(os, 'makedirs', autospec=True)
+ def test__manage_uefi_nvme_device(self, mkdir_mock, mock_utils_efi_part,
+ mock_get_part_uuid, mock_efi_bl,
+ mock_execute, mock_dispatch):
+ mock_utils_efi_part.return_value = '1'
+ mock_get_part_uuid.return_value = '/dev/fakenvme0p1'
+
+ mock_efi_bl.return_value = ['\\EFI\\BOOT\\BOOTX64.EFI']
+
+ mock_execute.side_effect = iter([('', ''), ('', ''),
+ ('', ''), ('', ''),
+ ('', ''), ('', ''),
+ ('', '')])
+
+ expected = [mock.call('partx', '-u', '/dev/fakenvme0', attempts=3,
+ delay_on_retry=True),
+ mock.call('udevadm', 'settle'),
+ mock.call('mount', '/dev/fakenvme0p1',
+ self.fake_dir + '/boot/efi'),
+ mock.call('efibootmgr'),
+ mock.call('efibootmgr', '-c', '-d', '/dev/fakenvme0',
+ '-p', '1', '-w',
+ '-L', 'ironic1', '-l',
+ '\\EFI\\BOOT\\BOOTX64.EFI'),
+ mock.call('umount', self.fake_dir + '/boot/efi',
+ attempts=3, delay_on_retry=True),
+ mock.call('sync')]
+
+ result = image._manage_uefi('/dev/fakenvme0', self.fake_root_uuid)
+ self.assertTrue(result)
+ 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)
+
+ @mock.patch.object(os.path, 'exists', lambda *_: False)
+ @mock.patch.object(image, '_get_efi_bootloaders', autospec=True)
+ @mock.patch.object(image, '_get_partition', autospec=True)
+ @mock.patch.object(utils, 'get_efi_part_on_device', autospec=True)
+ @mock.patch.object(os, 'makedirs', autospec=True)
def test__manage_uefi_wholedisk(
self, mkdir_mock, mock_utils_efi_part,
mock_get_part_uuid, mock_efi_bl, mock_execute,
diff --git a/releasenotes/notes/fix-nvme-partition-image-handling-b8487133a188fd32.yaml b/releasenotes/notes/fix-nvme-partition-image-handling-b8487133a188fd32.yaml
new file mode 100644
index 00000000..ee1cf085
--- /dev/null
+++ b/releasenotes/notes/fix-nvme-partition-image-handling-b8487133a188fd32.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+ - |
+ Fixes an error with UEFI based deployments where using a partition image
+ a NVMe device was previously failing due to the different device name
+ pattern.