diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py
index cf49889582..1d3e7eb0bf 100644
--- a/ironic/conductor/manager.py
+++ b/ironic/conductor/manager.py
@@ -1773,10 +1773,6 @@ class ConductorManager(base_manager.BaseConductorManager):
if task.node.console_enabled:
notify_utils.emit_console_notification(
task, 'console_restore', fields.NotificationStatus.START)
- # NOTE(kaifeng) Clear allocated_ipmi_terminal_port if exists,
- # so current conductor can allocate a new free port from local
- # resources.
- task.node.del_driver_internal_info('allocated_ipmi_terminal_port')
try:
task.driver.console.start_console(task)
except Exception as err:
diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py
index 87420f369d..218f90a5c0 100644
--- a/ironic/drivers/modules/ipmitool.py
+++ b/ironic/drivers/modules/ipmitool.py
@@ -1556,6 +1556,9 @@ class IPMIShellinaboxConsole(IPMIConsole):
created
:raises: ConsoleSubprocessFailed when invoking the subprocess failed
"""
+ # Dealloc allocated port if any, so the same host can never has
+ # duplicated port.
+ _release_allocated_port(task)
driver_info = _parse_driver_info(task.node)
if not driver_info['port']:
driver_info['port'] = _allocate_port(task)
@@ -1611,6 +1614,9 @@ class IPMISocatConsole(IPMIConsole):
created
:raises: ConsoleSubprocessFailed when invoking the subprocess failed
"""
+ # Dealloc allocated port if any, so the same host can never has
+ # duplicated port.
+ _release_allocated_port(task)
driver_info = _parse_driver_info(task.node)
if not driver_info['port']:
driver_info['port'] = _allocate_port(
diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py
index fd206e36d3..b4a18c8c2f 100644
--- a/ironic/tests/unit/conductor/test_manager.py
+++ b/ironic/tests/unit/conductor/test_manager.py
@@ -7244,44 +7244,6 @@ class DoNodeTakeOverTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
mock.call(task, 'console_restore',
obj_fields.NotificationStatus.ERROR)])
- @mock.patch.object(notification_utils, 'emit_console_notification',
- autospec=True)
- @mock.patch('ironic.drivers.modules.fake.FakeConsole.start_console',
- autospec=True)
- @mock.patch('ironic.drivers.modules.fake.FakeDeploy.take_over',
- autospec=True)
- @mock.patch('ironic.drivers.modules.fake.FakeDeploy.prepare',
- autospec=True)
- def test__do_takeover_with_console_port_cleaned(self, mock_prepare,
- mock_take_over,
- mock_start_console,
- mock_notify):
- self._start_service(start_consoles=False)
- node = obj_utils.create_test_node(self.context, driver='fake-hardware',
- console_enabled=True)
- di_info = node.driver_internal_info
- di_info['allocated_ipmi_terminal_port'] = 12345
- node.driver_internal_info = di_info
- node.save()
-
- task = task_manager.TaskManager(self.context, node.uuid)
-
- self.service._do_takeover(task)
- node.refresh()
- self.assertIsNone(node.last_error)
- self.assertTrue(node.console_enabled)
- self.assertIsNone(
- node.driver_internal_info.get('allocated_ipmi_terminal_port',
- None))
- mock_prepare.assert_called_once_with(task.driver.deploy, task)
- mock_take_over.assert_called_once_with(task.driver.deploy, task)
- mock_start_console.assert_called_once_with(task.driver.console, task)
- mock_notify.assert_has_calls(
- [mock.call(task, 'console_restore',
- obj_fields.NotificationStatus.START),
- mock.call(task, 'console_restore',
- obj_fields.NotificationStatus.END)])
-
@mgr_utils.mock_record_keepalive
class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
diff --git a/ironic/tests/unit/drivers/modules/test_ipmitool.py b/ironic/tests/unit/drivers/modules/test_ipmitool.py
index b982e0cb2f..016b9d6ed4 100644
--- a/ironic/tests/unit/drivers/modules/test_ipmitool.py
+++ b/ironic/tests/unit/drivers/modules/test_ipmitool.py
@@ -3255,6 +3255,11 @@ class IPMIToolShellinaboxTestCase(db_base.DbTestCase):
mock_start.return_value = None
mock_info.return_value = {'port': None}
mock_alloc.return_value = 1234
+ # Ensure allocated port is not re-used
+ dii = self.node.driver_internal_info
+ dii['allocated_ipmi_terminal_port'] = 4321
+ self.node.driver_internal_info = dii
+ self.node.save()
with task_manager.acquire(self.context,
self.node.uuid) as task:
@@ -3468,6 +3473,11 @@ class IPMIToolSocatDriverTestCase(IPMIToolShellinaboxTestCase):
mock_start.return_value = None
mock_info.return_value = {'port': None}
mock_alloc.return_value = 1234
+ # Ensure allocated port is not re-used
+ dii = self.node.driver_internal_info
+ dii['allocated_ipmi_terminal_port'] = 4321
+ self.node.driver_internal_info = dii
+ self.node.save()
with task_manager.acquire(self.context,
self.node.uuid) as task:
diff --git a/releasenotes/notes/fix-console-port-conflict-6dc19688079e2c7f.yaml b/releasenotes/notes/fix-console-port-conflict-6dc19688079e2c7f.yaml
new file mode 100644
index 0000000000..32b4199151
--- /dev/null
+++ b/releasenotes/notes/fix-console-port-conflict-6dc19688079e2c7f.yaml
@@ -0,0 +1,8 @@
+---
+fixes:
+ - |
+ Fixes issues that auto-allocated console port could conflict on the same
+ host under certain circumstances related to conductor takeover.
+
+ For more information, see `story 2010489
+ <https://storyboard.openstack.org/#!/story/2010489>`_.