openstack/ironic-python-agent
Code Issues Proposed changes

Rescan devices before performing uefi checks

Browse Source 
With the fix to the uefi code path so secure boot works properly
and is not accidently stomped on, we forgot to rescan the device
and force the partition table to update, which is vital for iscsi
based deploys.

Depends-On: https://review.opendev.org/706960
Change-Id: Ic2f338be075e93a2ce8c76c706d37db9bf8792ea
Story: 2007276
Task: 38713
This commit is contained in:
Julia Kreger 2020-02-08 07:18:12 -08:00
parent af5f05a0ee
commit 6f1f9c7f6e
3 changed files with 59 additions and 17 deletions
ironic_python_agent
extensions
image.py
tests/unit/extensions
test_image.py
releasenotes/notes
rescan-before-checking-uefi-64597c937880134d.yaml

29
ironic_python_agent/extensions/image.py

@ -37,21 +37,28 @@ BIND_MOUNTS = ('/dev', '/proc', '/run')
BOOTLOADERS_EFI = ['bootx64.efi', 'grubaa64.efi', 'winload.efi']
def _rescan_device(device):
"""Force the device to be rescanned
:param device: device upon which to rescan and update
kernel partition records.
"""
try:
utils.execute('partx', '-u', device, attempts=3,
delay_on_retry=True)
utils.execute('udevadm', 'settle')
except processutils.ProcessExecutionError:
LOG.warning("Couldn't re-read the partition table "
"on device %s", device)
def _get_partition(device, uuid):
"""Find the partition of a given device."""
LOG.debug("Find the partition %(uuid)s on device %(dev)s",
{'dev': device, 'uuid': uuid})
try:
# Try to tell the kernel to re-read the partition table
try:
utils.execute('partx', '-u', device, attempts=3,
delay_on_retry=True)
utils.execute('udevadm', 'settle')
except processutils.ProcessExecutionError:
LOG.warning("Couldn't re-read the partition table "
"on device %s", device)
_rescan_device(device)
lsblk = utils.execute('lsblk', '-PbioKNAME,UUID,PARTUUID,TYPE', device)
report = lsblk[0]
for line in report.split('\n'):
@ -261,6 +268,10 @@ def _manage_uefi(device, efi_system_part_uuid=None):
efi_mounted = False
try:
# Force UEFI to rescan the device. Required if the deployment
# was over iscsi.
_rescan_device(device)
local_path = tempfile.mkdtemp()
# Trust the contents on the disk in the event of a whole disk image.
efi_partition = utils.get_efi_part_on_device(device)

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

@ -102,10 +102,14 @@ class TestImageExtension(base.IronicAgentTest):
mock_utils_efi_part.return_value = '1'
mock_execute.side_effect = iter([('', ''), ('', ''),
('', ''), ('', ''),
('', ''), ('', ''),
('', ''), ('', '')])
expected = [mock.call('efibootmgr', '--version'),
mock.call('partx', '-u', '/dev/fake', attempts=3,
delay_on_retry=True),
mock.call('udevadm', 'settle'),
mock.call('mount', self.fake_efi_system_part,
self.fake_dir + '/boot/efi'),
mock.call('efibootmgr'),
@ -127,7 +131,7 @@ class TestImageExtension(base.IronicAgentTest):
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(6, mock_execute.call_count)
self.assertEqual(8, mock_execute.call_count)
@mock.patch.object(os.path, 'exists', lambda *_: False)
@mock.patch.object(iscsi, 'clean_up', autospec=True)
@ -145,10 +149,14 @@ class TestImageExtension(base.IronicAgentTest):
mock_utils_efi_part.return_value = '1'
mock_efi_bl.return_value = ['\\EFI\\BOOT\\BOOTX64.EFI']
mock_execute.side_effect = iter([('', ''), ('', ''),
('', ''), ('', ''),
('', ''), ('', ''),
('', ''), ('', '')])
expected = [mock.call('efibootmgr', '--version'),
mock.call('partx', '-u', '/dev/fake', attempts=3,
delay_on_retry=True),
mock.call('udevadm', 'settle'),
mock.call('mount', self.fake_efi_system_part,
self.fake_dir + '/boot/efi'),
mock.call('efibootmgr'),
@ -170,7 +178,7 @@ class TestImageExtension(base.IronicAgentTest):
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(6, mock_execute.call_count)
self.assertEqual(8, mock_execute.call_count)
@mock.patch.object(os.path, 'exists', lambda *_: False)
@mock.patch.object(iscsi, 'clean_up', autospec=True)
@ -192,11 +200,15 @@ efibootmgr: ** Warning ** : Boot0004 has same label ironic1\n
efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
"""
mock_execute.side_effect = iter([('', ''), ('', ''),
('', ''), ('', ''),
('', ''), ('', stdeer_msg),
('', ''), ('', ''),
('', ''), ('', '')])
expected = [mock.call('efibootmgr', '--version'),
mock.call('partx', '-u', '/dev/fake', attempts=3,
delay_on_retry=True),
mock.call('udevadm', 'settle'),
mock.call('mount', self.fake_efi_system_part,
self.fake_dir + '/boot/efi'),
mock.call('efibootmgr'),
@ -220,7 +232,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(8, mock_execute.call_count)
self.assertEqual(10, mock_execute.call_count)
@mock.patch.object(os.path, 'exists', lambda *_: False)
@mock.patch.object(iscsi, 'clean_up', autospec=True)
@ -240,11 +252,15 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
'\\WINDOWS\\system32\\winload.efi']
mock_execute.side_effect = iter([('', ''), ('', ''),
('', ''), ('', ''),
('', ''), ('', ''),
('', ''), ('', ''),
('', '')])
expected = [mock.call('efibootmgr', '--version'),
mock.call('partx', '-u', '/dev/fake', attempts=3,
delay_on_retry=True),
mock.call('udevadm', 'settle'),
mock.call('mount', self.fake_efi_system_part,
self.fake_dir + '/boot/efi'),
mock.call('efibootmgr'),
@ -270,7 +286,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(7, mock_execute.call_count)
self.assertEqual(9, mock_execute.call_count)
@mock.patch.object(iscsi, 'clean_up', autospec=True)
@mock.patch.object(image, '_install_grub2', autospec=True)
@ -766,10 +782,14 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
mock_efi_bl.return_value = ['\\EFI\\BOOT\\BOOTX64.EFI']
mock_execute.side_effect = iter([('', ''), ('', ''),
('', ''), ('', ''),
('', ''), ('', ''),
('', '')])
expected = [mock.call('mount', self.fake_efi_system_part,
expected = [mock.call('partx', '-u', '/dev/fake', attempts=3,
delay_on_retry=True),
mock.call('udevadm', 'settle'),
mock.call('mount', self.fake_efi_system_part,
self.fake_dir + '/boot/efi'),
mock.call('efibootmgr'),
mock.call('efibootmgr', '-c', '-d', self.fake_dev,
@ -785,7 +805,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(5, mock_execute.call_count)
self.assertEqual(7, mock_execute.call_count)
@mock.patch.object(os.path, 'exists', lambda *_: False)
@mock.patch.object(image, '_get_efi_bootloaders', autospec=True)
@ -802,10 +822,14 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n
mock_efi_bl.return_value = ['\\EFI\\BOOT\\BOOTX64.EFI']
mock_execute.side_effect = iter([('', ''), ('', ''),
('', ''), ('', ''),
('', ''), ('', ''),
('', '')])
expected = [mock.call('mount', self.fake_efi_system_part,
expected = [mock.call('partx', '-u', '/dev/fake', attempts=3,
delay_on_retry=True),
mock.call('udevadm', 'settle'),
mock.call('mount', self.fake_efi_system_part,
self.fake_dir + '/boot/efi'),
mock.call('efibootmgr'),
mock.call('efibootmgr', '-c', '-d', self.fake_dev,
@ -821,7 +845,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(5, mock_execute.call_count)
self.assertEqual(7, mock_execute.call_count)
@mock.patch.object(os, 'walk', autospec=True)
@mock.patch.object(os, 'access', autospec=False)

7
releasenotes/notes/rescan-before-checking-uefi-64597c937880134d.yaml Normal file

@ -0,0 +1,7 @@
---
fixes:
- |
Fixes an issue where the agent was failing to rescan the device deployed
upon before checking uefi contents. This would occur with an iSCSI
based deployment, as partition management operations are performed by
the conductor, and not locally.