From 28ccbf79c470167d17ade94497546a8d193e3995 Mon Sep 17 00:00:00 2001 From: Vasyl Saienko Date: Tue, 18 Oct 2016 19:07:47 +0300 Subject: [PATCH] Cleanup hung iscsi session This patch ensure that there is no iscsi session left after failed deployment. Change-Id: I569933f27746da65b1d22ba9e9279439e2e792cf Closes-Bug: #1632649 --- ironic/drivers/modules/deploy_utils.py | 34 +++++-- .../unit/drivers/modules/test_deploy_utils.py | 92 +++++++++++++++---- ...-hung-iscsi-sessions-d3b55c4c65fa4c8b.yaml | 3 + 3 files changed, 101 insertions(+), 28 deletions(-) create mode 100644 releasenotes/notes/clear-hung-iscsi-sessions-d3b55c4c65fa4c8b.yaml diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index ce7da1971a..8813d4b7bd 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -155,14 +155,31 @@ def login_iscsi(portal_address, portal_port, target_iqn): check_exit_code=[0], attempts=5, delay_on_retry=True) - # Ensure the login complete - verify_iscsi_connection(target_iqn) - # force iSCSI initiator to re-read luns - force_iscsi_lun_update(target_iqn) - # ensure file system sees the block device - check_file_system_for_iscsi_device(portal_address, - portal_port, - target_iqn) + + error_occurred = False + try: + # Ensure the login complete + verify_iscsi_connection(target_iqn) + # force iSCSI initiator to re-read luns + force_iscsi_lun_update(target_iqn) + # ensure file system sees the block device + check_file_system_for_iscsi_device(portal_address, + portal_port, + target_iqn) + except (exception.InstanceDeployFailure, + processutils.ProcessExecutionError) as e: + with excutils.save_and_reraise_exception(): + error_occurred = True + LOG.error(_LE("Failed to login to an iSCSI target due to %s"), + e) + finally: + if error_occurred: + try: + logout_iscsi(portal_address, portal_port, target_iqn) + delete_iscsi(portal_address, portal_port, target_iqn) + except processutils.ProcessExecutionError as e: + LOG.warning(_LW("An error occurred when trying to cleanup " + "failed ISCSI session error %s"), e) def check_file_system_for_iscsi_device(portal_address, @@ -219,7 +236,6 @@ def verify_iscsi_connection(target_iqn): def force_iscsi_lun_update(target_iqn): """force iSCSI initiator to re-read luns.""" LOG.debug("Re-reading iSCSI luns.") - utils.execute('iscsiadm', '-m', 'node', '-T', target_iqn, diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index 96d4ceecdd..08825389dc 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py @@ -21,6 +21,7 @@ import types from ironic_lib import disk_utils import mock +from oslo_concurrency import processutils from oslo_config import cfg from oslo_utils import uuidutils import testtools @@ -642,36 +643,63 @@ class PhysicalWorkTestCase(tests_base.TestCase): delay_on_retry=True) mock_verify.assert_called_once_with(iqn) - mock_update.assert_called_once_with(iqn) - mock_check_dev.assert_called_once_with(address, port, iqn) + @mock.patch.object(utils, 'LOG', autospec=True) @mock.patch.object(common_utils, 'execute', autospec=True) @mock.patch.object(utils, 'verify_iscsi_connection', autospec=True) @mock.patch.object(utils, 'force_iscsi_lun_update', autospec=True) @mock.patch.object(utils, 'check_file_system_for_iscsi_device', autospec=True) - def test_ipv6_address_wrapped(self, - mock_check_dev, - mock_update, - mock_verify, - mock_exec): - address = '2001:DB8::1111' + @mock.patch.object(utils, 'delete_iscsi', autospec=True) + @mock.patch.object(utils, 'logout_iscsi', autospec=True) + def test_login_iscsi_calls_raises( + self, mock_loiscsi, mock_discsi, mock_check_dev, mock_update, + mock_verify, mock_exec, mock_log): + address = '127.0.0.1' port = 3306 iqn = 'iqn.xyz' mock_exec.return_value = ['iqn.xyz', ''] - utils.login_iscsi(address, port, iqn) - mock_exec.assert_called_once_with( - 'iscsiadm', - '-m', 'node', - '-p', '[%s]:%s' % (address, port), - '-T', iqn, - '--login', - run_as_root=True, - check_exit_code=[0], - attempts=5, - delay_on_retry=True) + mock_check_dev.side_effect = exception.InstanceDeployFailure('boom') + self.assertRaises(exception.InstanceDeployFailure, + utils.login_iscsi, + address, port, iqn) + mock_verify.assert_called_once_with(iqn) + mock_update.assert_called_once_with(iqn) + mock_loiscsi.assert_called_once_with(address, port, iqn) + mock_discsi.assert_called_once_with(address, port, iqn) + self.assertIsInstance(mock_log.error.call_args[0][1], + exception.InstanceDeployFailure) + + @mock.patch.object(utils, 'LOG', autospec=True) + @mock.patch.object(common_utils, 'execute', autospec=True) + @mock.patch.object(utils, 'verify_iscsi_connection', autospec=True) + @mock.patch.object(utils, 'force_iscsi_lun_update', autospec=True) + @mock.patch.object(utils, 'check_file_system_for_iscsi_device', + autospec=True) + @mock.patch.object(utils, 'delete_iscsi', autospec=True) + @mock.patch.object(utils, 'logout_iscsi', autospec=True) + def test_login_iscsi_calls_raises_during_cleanup( + self, mock_loiscsi, mock_discsi, mock_check_dev, mock_update, + mock_verify, mock_exec, mock_log): + address = '127.0.0.1' + port = 3306 + iqn = 'iqn.xyz' + mock_exec.return_value = ['iqn.xyz', ''] + mock_check_dev.side_effect = exception.InstanceDeployFailure('boom') + mock_discsi.side_effect = processutils.ProcessExecutionError('boom') + self.assertRaises(exception.InstanceDeployFailure, + utils.login_iscsi, + address, port, iqn) + mock_verify.assert_called_once_with(iqn) + mock_update.assert_called_once_with(iqn) + mock_loiscsi.assert_called_once_with(address, port, iqn) + mock_discsi.assert_called_once_with(address, port, iqn) + self.assertIsInstance(mock_log.error.call_args[0][1], + exception.InstanceDeployFailure) + self.assertIsInstance(mock_log.warning.call_args[0][1], + processutils.ProcessExecutionError) @mock.patch.object(disk_utils, 'is_block_device', lambda d: True) def test_always_logout_and_delete_iscsi(self): @@ -732,6 +760,32 @@ class PhysicalWorkTestCase(tests_base.TestCase): self.assertEqual(utils_calls_expected, utils_mock.mock_calls) self.assertEqual(disk_utils_calls_expected, disk_utils_mock.mock_calls) + @mock.patch.object(common_utils, 'execute', autospec=True) + @mock.patch.object(utils, 'verify_iscsi_connection', autospec=True) + @mock.patch.object(utils, 'force_iscsi_lun_update', autospec=True) + @mock.patch.object(utils, 'check_file_system_for_iscsi_device', + autospec=True) + def test_ipv6_address_wrapped(self, + mock_check_dev, + mock_update, + mock_verify, + mock_exec): + address = '2001:DB8::1111' + port = 3306 + iqn = 'iqn.xyz' + mock_exec.return_value = ['iqn.xyz', ''] + utils.login_iscsi(address, port, iqn) + mock_exec.assert_called_once_with( + 'iscsiadm', + '-m', 'node', + '-p', '[%s]:%s' % (address, port), + '-T', iqn, + '--login', + run_as_root=True, + check_exit_code=[0], + attempts=5, + delay_on_retry=True) + class SwitchPxeConfigTestCase(tests_base.TestCase): diff --git a/releasenotes/notes/clear-hung-iscsi-sessions-d3b55c4c65fa4c8b.yaml b/releasenotes/notes/clear-hung-iscsi-sessions-d3b55c4c65fa4c8b.yaml new file mode 100644 index 0000000000..25150404cb --- /dev/null +++ b/releasenotes/notes/clear-hung-iscsi-sessions-d3b55c4c65fa4c8b.yaml @@ -0,0 +1,3 @@ +--- +fixes: + - An issue with hung iscsi sessions not being cleaned up in case of deploy failure. \ No newline at end of file