summaryrefslogtreecommitdiff
path: root/ironic
diff options
context:
space:
mode:
authorKaifeng Wang <kaifeng.w@gmail.com>2020-08-03 15:38:58 +0000
committerKaifeng Wang <kaifeng.w@gmail.com>2020-08-05 22:46:24 +0800
commit07a7a269bb34702d1922fe58b98c209674c08e33 (patch)
treea17d3d12d8c1f3c951df90edd598521c41ed402e /ironic
parent35e76ad82ddc0fb0390a5f1b9bcc452558b782c4 (diff)
downloadironic-07a7a269bb34702d1922fe58b98c209674c08e33.tar.gz
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
Diffstat (limited to 'ironic')
-rw-r--r--ironic/drivers/modules/console_utils.py23
-rw-r--r--ironic/drivers/modules/ipmitool.py7
-rw-r--r--ironic/tests/unit/drivers/modules/test_console_utils.py43
-rw-r--r--ironic/tests/unit/drivers/modules/test_ipmitool.py5
4 files changed, 65 insertions, 13 deletions
diff --git a/ironic/drivers/modules/console_utils.py b/ironic/drivers/modules/console_utils.py
index b2f92ba3d..6e08b6712 100644
--- a/ironic/drivers/modules/console_utils.py
+++ b/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:
diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py
index 85beca183..a3b443b58 100644
--- a/ironic/drivers/modules/ipmitool.py
+++ b/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)
diff --git a/ironic/tests/unit/drivers/modules/test_console_utils.py b/ironic/tests/unit/drivers/modules/test_console_utils.py
index 752fa5fd1..3419abb4a 100644
--- a/ironic/tests/unit/drivers/modules/test_console_utils.py
+++ b/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))
diff --git a/ironic/tests/unit/drivers/modules/test_ipmitool.py b/ironic/tests/unit/drivers/modules/test_ipmitool.py
index e45aee26f..c13aae62a 100644
--- a/ironic/tests/unit/drivers/modules/test_ipmitool.py
+++ b/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',