diff --git a/ironic_python_agent/extensions/iscsi.py b/ironic_python_agent/extensions/iscsi.py index 7fca41542..21dfba392 100644 --- a/ironic_python_agent/extensions/iscsi.py +++ b/ironic_python_agent/extensions/iscsi.py @@ -114,18 +114,28 @@ def clean_up(device): try: rts_root = rtslib_fb.RTSRoot() except (OSError, EnvironmentError, rtslib_fb.RTSLibError) as exc: - LOG.info('Linux-IO is not available, attemting to stop tgtd mapping. ' - 'Error: %s.', exc) - cmd = ['tgtadm', '--lld', 'iscsi', '--mode', 'target', '--op', - 'unbind', '--tid', '1', '--initiator-address', 'ALL'] - _execute(cmd, "Error when cleaning up iscsi binds.") - + try: + LOG.info('Linux-IO is not available, attemting to stop tgtd ' + 'mapping. Error: %s.', exc) + cmd = ['tgtadm', '--lld', 'iscsi', '--mode', 'target', '--op', + 'unbind', '--tid', '1', '--initiator-address', 'ALL'] + _execute(cmd, "Error when cleaning up iscsi binds.") + except errors.ISCSICommandError: + # This command may fail if the target was already torn down + # and that is okay, we just want to ensure it has been torn + # down so there should be no disk locks persisting. + pass cmd = ['sync'] _execute(cmd, "Error flushing buffers to disk.") - - cmd = ['tgtadm', '--lld', 'iscsi', '--mode', 'target', '--op', - 'delete', '--tid', '1'] - _execute(cmd, "Error deleting the iscsi target configuration.") + try: + cmd = ['tgtadm', '--lld', 'iscsi', '--mode', 'target', '--op', + 'delete', '--tid', '1'] + _execute(cmd, "Error deleting the iscsi target configuration.") + except errors.ISCSICommandError: + # This command should remove the target from being offered. + # It is just proper clean-up, and often previously the IPA + # side, or "target" was never really torn down in many cases. + pass return storage = None diff --git a/ironic_python_agent/tests/unit/extensions/test_iscsi.py b/ironic_python_agent/tests/unit/extensions/test_iscsi.py index f275c0fc3..36741683e 100644 --- a/ironic_python_agent/tests/unit/extensions/test_iscsi.py +++ b/ironic_python_agent/tests/unit/extensions/test_iscsi.py @@ -304,6 +304,19 @@ class TestISCSIExtensionCleanUpFallback(base.IronicAgentTest): iscsi.clean_up(self.fake_dev) mock_execute.assert_has_calls(expected) + def test_commands_fail(self, mock_execute): + mock_execute.side_effect = [processutils.ProcessExecutionError(), + ('', ''), + processutils.ProcessExecutionError()] + expected = [mock.call('tgtadm', '--lld', 'iscsi', '--mode', + 'target', '--op', 'unbind', '--tid', '1', + '--initiator-address', 'ALL'), + mock.call('sync'), + mock.call('tgtadm', '--lld', 'iscsi', '--mode', 'target', + '--op', 'delete', '--tid', '1')] + iscsi.clean_up(self.fake_dev) + mock_execute.assert_has_calls(expected) + @mock.patch.object(iscsi.rtslib_fb, 'RTSRoot', autospec=True) class TestISCSIExtensionCleanUp(base.IronicAgentTest): diff --git a/releasenotes/notes/fix-iscsi-teardown-handling-0df2345318d3c843.yaml b/releasenotes/notes/fix-iscsi-teardown-handling-0df2345318d3c843.yaml new file mode 100644 index 000000000..070f2cc0a --- /dev/null +++ b/releasenotes/notes/fix-iscsi-teardown-handling-0df2345318d3c843.yaml @@ -0,0 +1,12 @@ +--- +fixes: + - | + Fixes an issue in fallback error handling where native iSCSI controls + are unavailable due to the composition of the IPA ramdisk and where direct + ``tgtadm`` commands also fails. + + Before fallback error handling was added, the teardown was skipped + completely in the event of the native iSCSI controls being unavailable. + The end user behavior is now as it was previously prior to the fallback + error handling being added, but IPA will still continue to attempt to + clean up the iSCSI session.