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.

Conflicts:
	ironic_python_agent/extensions/image.py

Depends-On: https://review.opendev.org/706960
Change-Id: Ic2f338be075e93a2ce8c76c706d37db9bf8792ea
Story: 2007276
Task: 38713
(cherry picked from commit 6f1f9c7f6e)
This commit is contained in:
Julia Kreger 2020-02-08 07:18:12 -08:00 committed by Dmitry Tantsur
parent f30dcb671f
commit 8b2ace33a2
3 changed files with 59 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
ironic_python_agent/extensions/image.py
View File

@ -37,20 +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)
# If the deploy device is an md device, we want to install on
# the first partition. We clearly take a shortcut here for now.
@ -259,6 +267,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
View File

@ -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)
@ -773,10 +789,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,
@ -792,7 +812,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)
@ -809,10 +829,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,
@ -828,7 +852,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
View 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.