From e5f7e052ac3e84b7cfcbf821b0e5bc74ccaab6d0 Mon Sep 17 00:00:00 2001
From: likui <likui@yovole.com>
Date: Tue, 14 Nov 2023 10:02:42 +0800
Subject: [PATCH] Fix Reopen Web Console Duplicate Sol Session

Reopen web console may occasionally result in duplicated
sol session. get_console action open
one console process while another sol session remains.

This patch adds "sol deactivate" action before get
console. Make sure the current connection always a success.

Change-Id: Ie5d9c94a3e9e3561b6aa1a52462d6739662d4eb0
---
 ironic/drivers/modules/ipmitool.py               | 16 ++++++++++++++++
 .../tests/unit/drivers/modules/test_ipmitool.py  | 12 ++++++++++--
 ...ol-console-before-start-5cbb7be7816f3886.yaml |  6 ++++++
 3 files changed, 32 insertions(+), 2 deletions(-)
 create mode 100644 releasenotes/notes/stop-sol-console-before-start-5cbb7be7816f3886.yaml

diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py
index 7611403e60..de4df96a70 100644
--- a/ironic/drivers/modules/ipmitool.py
+++ b/ironic/drivers/modules/ipmitool.py
@@ -1624,6 +1624,14 @@ class IPMIShellinaboxConsole(IPMIConsole):
     def get_console(self, task):
         """Get the type and connection information about the console."""
         driver_info = _parse_driver_info(task.node)
+
+        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
         url = console_utils.get_shellinabox_console_url(driver_info['port'])
         return {'type': 'shellinabox', 'url': url}
 
@@ -1687,5 +1695,13 @@ class IPMISocatConsole(IPMIConsole):
         :param task: a task from TaskManager
         """
         driver_info = _parse_driver_info(task.node)
+
+        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
         url = console_utils.get_socat_console_url(driver_info['port'])
         return {'type': 'socat', 'url': url}
diff --git a/ironic/tests/unit/drivers/modules/test_ipmitool.py b/ironic/tests/unit/drivers/modules/test_ipmitool.py
index 1b63ebcdec..aa5da4697b 100644
--- a/ironic/tests/unit/drivers/modules/test_ipmitool.py
+++ b/ironic/tests/unit/drivers/modules/test_ipmitool.py
@@ -3418,7 +3418,9 @@ class IPMIToolShellinaboxTestCase(db_base.DbTestCase):
 
     @mock.patch.object(console_utils, 'get_shellinabox_console_url',
                        autospec=True)
-    def test_get_console(self, mock_get):
+    @mock.patch.object(ipmi.IPMIShellinaboxConsole, "_exec_stop_console",
+                       autospec=True)
+    def test_get_console(self, mock_exec_stop, mock_get):
         url = 'http://localhost:4201'
         mock_get.return_value = url
         expected = {'type': 'shellinabox', 'url': url}
@@ -3426,7 +3428,9 @@ class IPMIToolShellinaboxTestCase(db_base.DbTestCase):
         with task_manager.acquire(self.context,
                                   self.node.uuid) as task:
             console_info = self.console.get_console(task)
+            driver_info = ipmi._parse_driver_info(task.node)
 
+        mock_exec_stop.assert_called_once_with(self.console, driver_info)
         self.assertEqual(expected, console_info)
         mock_get.assert_called_once_with(self.info['port'])
 
@@ -3651,7 +3655,9 @@ class IPMIToolSocatDriverTestCase(IPMIToolShellinaboxTestCase):
 
     @mock.patch.object(console_utils, 'get_socat_console_url',
                        autospec=True)
-    def test_get_console(self, mock_get_url):
+    @mock.patch.object(ipmi.IPMISocatConsole, '_exec_stop_console',
+                       autospec=True)
+    def test_get_console(self, mock_exec_stop, mock_get_url):
         url = 'tcp://localhost:4201'
         mock_get_url.return_value = url
         expected = {'type': 'socat', 'url': url}
@@ -3659,6 +3665,8 @@ class IPMIToolSocatDriverTestCase(IPMIToolShellinaboxTestCase):
         with task_manager.acquire(self.context,
                                   self.node.uuid) as task:
             console_info = self.console.get_console(task)
+            driver_info = ipmi._parse_driver_info(task.node)
 
+        mock_exec_stop.assert_called_once_with(self.console, driver_info)
         self.assertEqual(expected, console_info)
         mock_get_url.assert_called_once_with(self.info['port'])
diff --git a/releasenotes/notes/stop-sol-console-before-start-5cbb7be7816f3886.yaml b/releasenotes/notes/stop-sol-console-before-start-5cbb7be7816f3886.yaml
new file mode 100644
index 0000000000..b60395b161
--- /dev/null
+++ b/releasenotes/notes/stop-sol-console-before-start-5cbb7be7816f3886.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Ironic now stops any active IPMI Serial-Over-LAN console sessions when
+    initializing a console session. This resolves and issue where console support
+    would fail if a previous console session was not properly disconnected.