diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py index 73af522c5..3fc059998 100644 --- a/ironic_python_agent/extensions/image.py +++ b/ironic_python_agent/extensions/image.py @@ -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) diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py index 52012867f..6ad365abe 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/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) @@ -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) diff --git a/releasenotes/notes/rescan-before-checking-uefi-64597c937880134d.yaml b/releasenotes/notes/rescan-before-checking-uefi-64597c937880134d.yaml new file mode 100644 index 000000000..90041f1c4 --- /dev/null +++ b/releasenotes/notes/rescan-before-checking-uefi-64597c937880134d.yaml @@ -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.