From 53d53290c357ecace4872b4fd71f0516e82e700c Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Mon, 20 Jul 2020 14:24:06 -0700 Subject: [PATCH] Prevent un-needed iscsi cleanup When we added software raid support, we started calling bootloader installation. As time went on, we ehnanced that code path for non RAID cases in order to ensure that UEFI nvram was setup for the instance to boot properly. Somewhere in this process, we missed a possible failure case where the iscsi client tgtadm may return failures. Obviously, the correct path is to not call iscsi teardown if we don't need to. Since it was always semi-opportunistic teardown, we can't blindly catch any error, and if we started iSCSI and failed to tear the connection down, we might want to still fail, so this change moves the logic over to use a flag on the agent object which one extension to set the flag and the other to read it and take action based upon that. Change-Id: Id3b1ae5e59282f4109f6246d5614d44c93aefa7c Story: 2007937 Task: 40395 (cherry picked from commit 2a56ee03b6ffb2d8c8f5ff553e90ecf6ee07f9af) (cherry picked from commit 4746f09b3dfa765684e6e20cf4b9742e1bb3bd62) --- ironic_python_agent/agent.py | 1 + ironic_python_agent/extensions/image.py | 3 ++- ironic_python_agent/extensions/iscsi.py | 3 ++- ironic_python_agent/tests/unit/extensions/test_image.py | 2 ++ ironic_python_agent/tests/unit/extensions/test_iscsi.py | 6 ++++++ .../prevent-needless-iscsi-cleanup-f8d602c0abc7e8ba.yaml | 9 +++++++++ 6 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/prevent-needless-iscsi-cleanup-f8d602c0abc7e8ba.yaml diff --git a/ironic_python_agent/agent.py b/ironic_python_agent/agent.py index c4b42f76a..e6a8e214f 100644 --- a/ironic_python_agent/agent.py +++ b/ironic_python_agent/agent.py @@ -195,6 +195,7 @@ class IronicPythonAgent(base.ExecuteCommandMixin): self.hardware_initialization_delay = hardware_initialization_delay # IPA will stop serving requests and exit after this is set to False self.serve_api = True + 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 34b105017..c1e5fea82 100644 --- a/ironic_python_agent/extensions/image.py +++ b/ironic_python_agent/extensions/image.py @@ -384,7 +384,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') if boot.current_boot_mode == 'uefi': has_efibootmgr = True diff --git a/ironic_python_agent/extensions/iscsi.py b/ironic_python_agent/extensions/iscsi.py index bc7ef0d44..87afe90a3 100644 --- a/ironic_python_agent/extensions/iscsi.py +++ b/ironic_python_agent/extensions/iscsi.py @@ -189,7 +189,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 dd193a51a..1005aa54e 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -45,6 +45,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) diff --git a/ironic_python_agent/tests/unit/extensions/test_iscsi.py b/ironic_python_agent/tests/unit/extensions/test_iscsi.py index 3dc509054..901e4fa48 100644 --- a/ironic_python_agent/tests/unit/extensions/test_iscsi.py +++ b/ironic_python_agent/tests/unit/extensions/test_iscsi.py @@ -25,6 +25,9 @@ from ironic_python_agent import utils class FakeAgent(object): + + iscsi_started = False + def get_node_uuid(self): return 'my_node_uuid' @@ -47,8 +50,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 `_.