Merge "Fix Node Console Duplicate Sol Session"

2 files changed, 49 insertions, 16 deletions

diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py
index d5a699673..6c3dc678d 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 2507b431f..b982e0cb2 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)