Fix console auto port allocation under IPv6

By default _verify_port() only works for IPv4 network, the same port can be
allocated to multiple nodes in a IPv6 network because the port checking
passed and be used for other nodes.

This fix passes the socat_address to the port validation and use the
correct address family to do the socket binding.

Story: 2007946
Task: 40412

Change-Id: I1355afaa551baee7b9fd7883d2d29342d059c5a0

ironic/drivers/modules/console_utils.py

@@ -162,11 +162,24 @@ def _get_port_range():
     return start, stop
 
 
-def _verify_port(port):
+def _verify_port(port, host=None):
     """Check whether specified port is in use."""
-    s = socket.socket()
+    ip_version = None
+    if host is not None:
+        try:
+            ip_version = ipaddress.ip_address(host).version
+        except ValueError:
+            # Assume it's a hostname
+            pass
+    else:
+        host = CONF.host
+    if ip_version == 6:
+        s = socket.socket(socket.AF_INET6)
+    else:
+        s = socket.socket()
+
     try:
-        s.bind((CONF.host, port))
+        s.bind((host, port))
     except socket.error:
         raise exception.Conflict()
     finally:
@@ -174,7 +187,7 @@ def _verify_port(port):
 
 
 @lockutils.synchronized(SERIAL_LOCK)
-def acquire_port():
+def acquire_port(host=None):
     """Returns a free TCP port on current host.
 
     Find and returns a free TCP port in the range
@@ -187,7 +200,7 @@ def acquire_port():
         if port in ALLOCATED_PORTS:
             continue
         try:
-            _verify_port(port)
+            _verify_port(port, host=host)
             ALLOCATED_PORTS.add(port)
             return port
         except exception.Conflict:

ironic/drivers/modules/ipmitool.py

@@ -807,10 +807,10 @@ def _constructor_checks(driver):
     _check_temp_dir()
 
 
-def _allocate_port(task):
+def _allocate_port(task, host=None):
     node = task.node
     dii = node.driver_internal_info or {}
-    allocated_port = console_utils.acquire_port()
+    allocated_port = console_utils.acquire_port(host=host)
     dii['allocated_ipmi_terminal_port'] = allocated_port
     node.driver_internal_info = dii
     node.save()
@@ -1411,7 +1411,8 @@ class IPMISocatConsole(IPMIConsole):
         """
         driver_info = _parse_driver_info(task.node)
         if not driver_info['port']:
-            driver_info['port'] = _allocate_port(task)
+            driver_info['port'] = _allocate_port(
+                task, host=CONF.console.socat_address)
 
         try:
             self._exec_stop_console(driver_info)

ironic/tests/unit/drivers/modules/test_console_utils.py

@@ -23,6 +23,7 @@ import ipaddress
 import os
 import random
 import signal
+import socket
 import string
 import subprocess
 import tempfile
@@ -668,7 +669,7 @@ class ConsoleUtilsTestCase(db_base.DbTestCase):
     def test_allocate_port_success(self, mock_verify, mock_ports):
         self.config(port_range='10000:10001', group='console')
         port = console_utils.acquire_port()
-        mock_verify.assert_called_once_with(10000)
+        mock_verify.assert_called_once_with(10000, host=None)
         self.assertEqual(port, 10000)
         mock_ports.add.assert_called_once_with(10000)
 
@@ -679,7 +680,9 @@ class ConsoleUtilsTestCase(db_base.DbTestCase):
         mock_verify.side_effect = (exception.Conflict, exception.Conflict,
                                    None)
         port = console_utils.acquire_port()
-        verify_calls = [mock.call(10000), mock.call(10001), mock.call(10002)]
+        verify_calls = [mock.call(10000, host=None),
+                        mock.call(10001, host=None),
+                        mock.call(10002, host=None)]
         mock_verify.assert_has_calls(verify_calls)
         self.assertEqual(port, 10002)
         mock_ports.add.assert_called_once_with(10002)
@@ -691,5 +694,39 @@ class ConsoleUtilsTestCase(db_base.DbTestCase):
         mock_verify.side_effect = exception.Conflict
         self.assertRaises(exception.NoFreeIPMITerminalPorts,
                           console_utils.acquire_port)
-        verify_calls = [mock.call(p) for p in range(10000, 10005)]
+        verify_calls = [mock.call(p, host=None) for p in range(10000, 10005)]
         mock_verify.assert_has_calls(verify_calls)
+
+    @mock.patch.object(socket, 'socket', autospec=True)
+    def test__verify_port_default(self, mock_socket):
+        self.config(host='localhost.localdomain')
+        mock_sock = mock.MagicMock()
+        mock_socket.return_value = mock_sock
+        console_utils._verify_port(10000)
+        mock_sock.bind.assert_called_once_with(('localhost.localdomain',
+                                                10000))
+
+    @mock.patch.object(socket, 'socket', autospec=True)
+    def test__verify_port_hostname(self, mock_socket):
+        mock_sock = mock.MagicMock()
+        mock_socket.return_value = mock_sock
+        console_utils._verify_port(10000, host='localhost.localdomain')
+        mock_socket.assert_called_once_with()
+        mock_sock.bind.assert_called_once_with(('localhost.localdomain',
+                                                10000))
+
+    @mock.patch.object(socket, 'socket', autospec=True)
+    def test__verify_port_ipv4(self, mock_socket):
+        mock_sock = mock.MagicMock()
+        mock_socket.return_value = mock_sock
+        console_utils._verify_port(10000, host='1.2.3.4')
+        mock_socket.assert_called_once_with()
+        mock_sock.bind.assert_called_once_with(('1.2.3.4', 10000))
+
+    @mock.patch.object(socket, 'socket', autospec=True)
+    def test__verify_port_ipv6(self, mock_socket):
+        mock_sock = mock.MagicMock()
+        mock_socket.return_value = mock_sock
+        console_utils._verify_port(10000, host='2001:dead:beef::1')
+        mock_socket.assert_called_once_with(socket.AF_INET6)
+        mock_sock.bind.assert_called_once_with(('2001:dead:beef::1', 10000))

ironic/tests/unit/drivers/modules/test_ipmitool.py

@@ -2629,7 +2629,7 @@ class IPMIToolDriverTestCase(Base):
         with task_manager.acquire(self.context,
                                   self.node.uuid) as task:
             port = ipmi._allocate_port(task)
-            mock_acquire.assert_called_once_with()
+            mock_acquire.assert_called_once_with(host=None)
             self.assertEqual(port, 1234)
             info = task.node.driver_internal_info
             self.assertEqual(info['allocated_ipmi_terminal_port'], 1234)
@@ -2959,6 +2959,7 @@ class IPMIToolSocatDriverTestCase(IPMIToolShellinaboxTestCase):
                        autospec=True)
     def test_start_console_alloc_port(self, mock_stop, mock_start, mock_info,
                                       mock_alloc):
+        self.config(socat_address='2001:dead:beef::1', group='console')
         mock_start.return_value = None
         mock_info.return_value = {'port': None}
         mock_alloc.return_value = 1234
@@ -2970,7 +2971,7 @@ class IPMIToolSocatDriverTestCase(IPMIToolShellinaboxTestCase):
         mock_start.assert_called_once_with(
             self.console, {'port': 1234},
             console_utils.start_socat_console)
-        mock_alloc.assert_called_once_with(mock.ANY)
+        mock_alloc.assert_called_once_with(mock.ANY, host='2001:dead:beef::1')
 
     @mock.patch.object(ipmi.IPMISocatConsole, '_get_ipmi_cmd', autospec=True)
     @mock.patch.object(console_utils, 'start_socat_console',

releasenotes/notes/socat-console-port-alloc-ipv6-26760f53f86209d0.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Fixes the issue that port auto allocation for the socat console failed to
+    correctly identify the availablility of ports under IPv6 networks.