From a2160f13af9559bd0a5b350f9317c21dc907feec Mon Sep 17 00:00:00 2001 From: taoruizhe Date: Tue, 28 Dec 2021 14:38:36 +0800 Subject: [PATCH] Fix Node Console Duplicate Sol Session Restart node console may occasionally result in duplicated sol session. Especially, when a cluster deployed with multi ironic-conductor backends, stop_console action shutdown only one console process while another sol session remains. This patch adds "sol deactivate" action before start node console. Make sure the current connection always a success. Story: 2009762 Task: 44233 Change-Id: I5bc8666ff0b4ceab61ed6a8c794d6882783d6bce --- ironic/drivers/modules/ipmitool.py | 11 ++++ .../unit/drivers/modules/test_ipmitool.py | 54 +++++++++++++------ .../notes/bug-2009762-403eac24c4823d2d.yaml | 7 +++ 3 files changed, 56 insertions(+), 16 deletions(-) create mode 100644 releasenotes/notes/bug-2009762-403eac24c4823d2d.yaml diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py index d5a6996730..6c3dc678d7 100644 --- a/ironic/drivers/modules/ipmitool.py +++ b/ironic/drivers/modules/ipmitool.py @@ -1560,6 +1560,13 @@ class IPMIShellinaboxConsole(IPMIConsole): if not driver_info['port']: driver_info['port'] = _allocate_port(task) + try: + self._exec_stop_console(driver_info) + except OSError: + # We need to drop any existing sol sessions with sol deactivate. + # OSError is raised when sol session is already deactivated, + # so we can ignore it. + pass self._start_console(driver_info, console_utils.start_shellinabox_console) @@ -1577,6 +1584,10 @@ class IPMIShellinaboxConsole(IPMIConsole): _console_pwfile_path(task.node.uuid)) _release_allocated_port(task) + def _exec_stop_console(self, driver_info): + cmd = "sol deactivate" + _exec_ipmitool(driver_info, cmd, check_exit_code=[0, 1]) + @METRICS.timer('IPMIShellinaboxConsole.get_console') def get_console(self, task): """Get the type and connection information about the console.""" diff --git a/ironic/tests/unit/drivers/modules/test_ipmitool.py b/ironic/tests/unit/drivers/modules/test_ipmitool.py index 2507b431f0..b982e0cb2f 100644 --- a/ironic/tests/unit/drivers/modules/test_ipmitool.py +++ b/ironic/tests/unit/drivers/modules/test_ipmitool.py @@ -1464,8 +1464,8 @@ class IPMIToolPrivateMethodTestCase( @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_cipher_suite_error_noconfig( self, mock_exec, mock_check_cs, mock_support): - no_matching_error = 'Error in open session response message : '\ - 'no matching cipher suite\n\nError: '\ + no_matching_error = 'Error in open session response message : ' \ + 'no matching cipher suite\n\nError: ' \ 'Unable to establish IPMI v2 / RMCP+ session\n' self.config(min_command_interval=1, group='ipmi') self.config(command_retry_timeout=2, group='ipmi') @@ -1511,8 +1511,8 @@ class IPMIToolPrivateMethodTestCase( @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_cipher_suite_set_with_error_noconfig( self, mock_exec, mock_check_cs, mock_support): - no_matching_error = 'Error in open session response message : '\ - 'no matching cipher suite\n\nError: '\ + no_matching_error = 'Error in open session response message : ' \ + 'no matching cipher suite\n\nError: ' \ 'Unable to establish IPMI v2 / RMCP+ session\n' self.config(min_command_interval=1, group='ipmi') self.config(command_retry_timeout=2, group='ipmi') @@ -1560,8 +1560,8 @@ class IPMIToolPrivateMethodTestCase( @mock.patch.object(utils, 'execute', autospec=True) def test__exec_ipmitool_cipher_suite_set_with_error_config( self, mock_exec, mock_check_cs, mock_support): - no_matching_error = 'Error in open session response message : '\ - 'no matching cipher suite\n\nError: '\ + no_matching_error = 'Error in open session response message : ' \ + 'no matching cipher suite\n\nError: ' \ 'Unable to establish IPMI v2 / RMCP+ session\n' self.config(min_command_interval=1, group='ipmi') self.config(command_retry_timeout=2, group='ipmi') @@ -1615,10 +1615,10 @@ class IPMIToolPrivateMethodTestCase( self.config(use_ipmitool_retries=False, group='ipmi') cs_list = ['0', '1', '2', '3', '17'] self.config(cipher_suite_versions=cs_list, group='ipmi') - no_matching_error = 'Error in open session response message : '\ - 'no matching cipher suite\n\nError: '\ + no_matching_error = 'Error in open session response message : ' \ + 'no matching cipher suite\n\nError: ' \ 'Unable to establish IPMI v2 / RMCP+ session\n' - unsupported_error = 'Unsupported cipher suite ID : 17\n\n'\ + unsupported_error = 'Unsupported cipher suite ID : 17\n\n' \ 'Error: Unable to establish IPMI v2 / RMCP+ session\n' ipmi.LAST_CMD_TIME = {} args = [ @@ -1736,10 +1736,10 @@ class IPMIToolPrivateMethodTestCase( 'Problem\n\nError: Unable to establish IPMI v2 / RMCP+ session\n', 'UnsupportedciphersuiteID:17\n\n' ] - no_matching_error = 'Error in open session response message : '\ - 'no matching cipher suite\n\nError: '\ + no_matching_error = 'Error in open session response message : ' \ + 'no matching cipher suite\n\nError: ' \ 'Unable to establish IPMI v2 / RMCP+ session\n' - unsupported_error = 'Unsupported cipher suite ID : 17\n\n'\ + unsupported_error = 'Unsupported cipher suite ID : 17\n\n' \ 'Error: Unable to establish IPMI v2 / RMCP+ session\n' valid_errors_stderr = [no_matching_error, unsupported_error] for invalid_err in invalid_errors_stderr: @@ -3209,7 +3209,9 @@ class IPMIToolShellinaboxTestCase(db_base.DbTestCase): @mock.patch.object(ipmi, '_allocate_port', autospec=True) @mock.patch.object(ipmi.IPMIConsole, '_start_console', autospec=True) - def test_start_console(self, mock_start, mock_alloc): + @mock.patch.object(ipmi.IPMIShellinaboxConsole, "_exec_stop_console", + autospec=True) + def test_start_console(self, mock_exec_stop, mock_start, mock_alloc): mock_start.return_value = None mock_alloc.return_value = 10000 @@ -3218,29 +3220,38 @@ class IPMIToolShellinaboxTestCase(db_base.DbTestCase): self.console.start_console(task) driver_info = ipmi._parse_driver_info(task.node) driver_info.update(port=10000) + mock_exec_stop.assert_called_once_with(self.console, driver_info) mock_start.assert_called_once_with( self.console, driver_info, console_utils.start_shellinabox_console) @mock.patch.object(ipmi, '_allocate_port', autospec=True) @mock.patch.object(ipmi, '_parse_driver_info', autospec=True) + @mock.patch.object(ipmi.IPMIShellinaboxConsole, "_exec_stop_console", + autospec=True) @mock.patch.object(ipmi.IPMIConsole, '_start_console', autospec=True) - def test_start_console_with_port(self, mock_start, mock_info, mock_alloc): + def test_start_console_with_port(self, mock_start, mock_exec_stop, + mock_info, mock_alloc): mock_start.return_value = None mock_info.return_value = {'port': 10000} with task_manager.acquire(self.context, self.node.uuid) as task: self.console.start_console(task) + driver_info = ipmi._parse_driver_info(task.node) mock_start.assert_called_once_with( self.console, {'port': 10000}, console_utils.start_shellinabox_console) + mock_exec_stop.assert_called_once_with(self.console, driver_info) mock_alloc.assert_not_called() @mock.patch.object(ipmi, '_allocate_port', autospec=True) @mock.patch.object(ipmi, '_parse_driver_info', autospec=True) + @mock.patch.object(ipmi.IPMIShellinaboxConsole, "_exec_stop_console", + autospec=True) @mock.patch.object(ipmi.IPMIConsole, '_start_console', autospec=True) - def test_start_console_alloc_port(self, mock_start, mock_info, mock_alloc): + def test_start_console_alloc_port(self, mock_start, mock_exec_stop, + mock_info, mock_alloc): mock_start.return_value = None mock_info.return_value = {'port': None} mock_alloc.return_value = 1234 @@ -3248,11 +3259,23 @@ class IPMIToolShellinaboxTestCase(db_base.DbTestCase): with task_manager.acquire(self.context, self.node.uuid) as task: self.console.start_console(task) + driver_info = ipmi._parse_driver_info(task.node) mock_start.assert_called_once_with( self.console, {'port': 1234}, console_utils.start_shellinabox_console) + mock_exec_stop.assert_called_once_with(self.console, driver_info) mock_alloc.assert_called_once_with(mock.ANY) + @mock.patch.object(ipmi, '_exec_ipmitool', autospec=True) + def test__exec_stop_console(self, mock_exec): + with task_manager.acquire(self.context, + self.node.uuid) as task: + driver_info = ipmi._parse_driver_info(task.node) + self.console._exec_stop_console(driver_info) + + mock_exec.assert_called_once_with( + driver_info, 'sol deactivate', check_exit_code=[0, 1]) + @mock.patch.object(ipmi.IPMIConsole, '_get_ipmi_cmd', autospec=True) @mock.patch.object(console_utils, 'start_shellinabox_console', autospec=True) @@ -3568,7 +3591,6 @@ class IPMIToolSocatDriverTestCase(IPMIToolShellinaboxTestCase): def test__exec_stop_console(self, mock_exec): with task_manager.acquire(self.context, self.node.uuid) as task: - driver_info = ipmi._parse_driver_info(task.node) self.console._exec_stop_console(driver_info) diff --git a/releasenotes/notes/bug-2009762-403eac24c4823d2d.yaml b/releasenotes/notes/bug-2009762-403eac24c4823d2d.yaml new file mode 100644 index 0000000000..ef132b2a70 --- /dev/null +++ b/releasenotes/notes/bug-2009762-403eac24c4823d2d.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fix Node Console Duplicate Sol Session. + This patch adds "sol deactivate" action before start node console, to make + sure the current console connection always a success. + See `story 2009762 `_.