diff --git a/ironic_python_agent/agent.py b/ironic_python_agent/agent.py index dfe77b04b..64b5009fe 100644 --- a/ironic_python_agent/agent.py +++ b/ironic_python_agent/agent.py @@ -219,6 +219,7 @@ class IronicPythonAgent(base.ExecuteCommandMixin): # in the event of long running ramdisks where the conductor # got upgraded somewhere along the way. self.agent_token_required = cfg.CONF.agent_token_required + self.iscsi_started = False def get_status(self): """Retrieve a serializable status. diff --git a/ironic_python_agent/extensions/image.py b/ironic_python_agent/extensions/image.py index 5b54df7d7..d25b8e620 100644 --- a/ironic_python_agent/extensions/image.py +++ b/ironic_python_agent/extensions/image.py @@ -710,7 +710,8 @@ class ImageExtension(base.BaseAgentExtension): """ device = hardware.dispatch_to_managers('get_os_install_device') - iscsi.clean_up(device) + if self.agent.iscsi_started: + iscsi.clean_up(device) boot = hardware.dispatch_to_managers('get_boot_info') # FIXME(arne_wiebalck): make software RAID work with efibootmgr if (boot.current_boot_mode == 'uefi' diff --git a/ironic_python_agent/extensions/iscsi.py b/ironic_python_agent/extensions/iscsi.py index 21dfba392..1b3b308cb 100644 --- a/ironic_python_agent/extensions/iscsi.py +++ b/ironic_python_agent/extensions/iscsi.py @@ -210,7 +210,8 @@ class ISCSIExtension(base.BaseAgentExtension): else: _start_lio(iqn, portal_port, device) LOG.debug('Linux-IO configuration: %s', rts_root.dump()) - + # Mark iscsi as previously started + self.agent.iscsi_started = True LOG.info('Created iSCSI target with iqn %(iqn)s, portal port %(port)d,' ' on device %(dev)s using %(method)s', {'iqn': iqn, 'port': portal_port, 'dev': device, diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py index 11ef29f15..e6db55543 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -46,6 +46,8 @@ class TestImageExtension(base.IronicAgentTest): self.fake_efi_system_part_uuid = '45AB-2312' self.fake_prep_boot_part_uuid = '76937797-3253-8843-999999999999' self.fake_dir = '/tmp/fake-dir' + self.agent_extension.agent = mock.Mock() + self.agent_extension.agent.iscsi_started = True @mock.patch.object(iscsi, 'clean_up', autospec=True) @mock.patch.object(image, '_install_grub2', autospec=True) @@ -323,6 +325,31 @@ efibootmgr: ** Warning ** : Boot0005 has same label ironic1\n ) mock_iscsi_clean.assert_called_once_with(self.fake_dev) + @mock.patch.object(iscsi, 'clean_up', autospec=True) + @mock.patch.object(image, '_install_grub2', autospec=True) + def test__install_bootloader_prep_no_iscsi( + self, mock_grub2, mock_iscsi_clean, + mock_execute, mock_dispatch): + self.agent_extension.agent.iscsi_started = False + mock_dispatch.side_effect = [ + self.fake_dev, hardware.BootInfo(current_boot_mode='bios') + ] + self.agent_extension.install_bootloader( + root_uuid=self.fake_root_uuid, + efi_system_part_uuid=None, + prep_boot_part_uuid=self.fake_prep_boot_part_uuid).join() + mock_dispatch.assert_any_call('get_os_install_device') + mock_dispatch.assert_any_call('get_boot_info') + self.assertEqual(2, mock_dispatch.call_count) + mock_grub2.assert_called_once_with( + self.fake_dev, + root_uuid=self.fake_root_uuid, + efi_system_part_uuid=None, + prep_boot_part_uuid=self.fake_prep_boot_part_uuid, + target_boot_mode='bios' + ) + mock_iscsi_clean.assert_not_called() + @mock.patch.object(hardware, 'is_md_device', lambda *_: False) @mock.patch.object(os.path, 'exists', lambda *_: False) @mock.patch.object(iscsi, 'clean_up', autospec=True) diff --git a/ironic_python_agent/tests/unit/extensions/test_iscsi.py b/ironic_python_agent/tests/unit/extensions/test_iscsi.py index e914d4aa7..131ad0751 100644 --- a/ironic_python_agent/tests/unit/extensions/test_iscsi.py +++ b/ironic_python_agent/tests/unit/extensions/test_iscsi.py @@ -26,6 +26,9 @@ from ironic_python_agent import utils class FakeAgent(object): + + iscsi_started = False + def get_node_uuid(self): return 'my_node_uuid' @@ -48,8 +51,11 @@ class TestISCSIExtensionTgt(base.IronicAgentTest): mock_destroy): mock_dispatch.return_value = self.fake_dev mock_execute.return_value = ('', '') + self.assertFalse(self.agent_extension.agent.iscsi_started) + result = self.agent_extension.start_iscsi_target(iqn=self.fake_iqn) + self.assertTrue(self.agent_extension.agent.iscsi_started) expected = [mock.call('tgtd'), mock.call('tgtadm', '--lld', 'iscsi', '--mode', 'target', '--op', 'show', attempts=10), diff --git a/releasenotes/notes/prevent-needless-iscsi-cleanup-f8d602c0abc7e8ba.yaml b/releasenotes/notes/prevent-needless-iscsi-cleanup-f8d602c0abc7e8ba.yaml new file mode 100644 index 000000000..25f6c443d --- /dev/null +++ b/releasenotes/notes/prevent-needless-iscsi-cleanup-f8d602c0abc7e8ba.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + Fixes an issue with the ironic-python-agent where we would call to + setup the bootloader, which is necessary with software raid, but also + attempt to clean up iSCSI. This can cause issues when using the ``direct`` + ``deploy_interface``. Now the agent will only clean up iSCSI connections + if iSCSI was explicitly started. For more information, please see + `story 2007937 `_.