openstack/ironic-python-agent
Code Issues Proposed changes

Merge "Test advertised ip reachability before assigning it" into stable/2025.2

Browse Source
This commit is contained in:
Zuul
2025-11-21 19:55:33 +00:00
committed by Gerrit Code Review
parent 9711320ae0 eff0529e7d
commit 02ea3ad7e3
4 changed files with 228 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

67
ironic_python_agent/agent.py
View File

@@ -22,6 +22,8 @@ import threading
import time
from urllib import parse as urlparse
import requests
from oslo_concurrency import processutils
from oslo_config import cfg
from oslo_log import log
@@ -38,6 +40,7 @@ from ironic_python_agent import mdns
from ironic_python_agent import netutils
from ironic_python_agent import utils
CONF = cfg.CONF
LOG = log.getLogger(__name__)
# Time(in seconds) to wait for any of the interfaces to be up
@@ -305,19 +308,59 @@ class IronicPythonAgent(base.ExecuteCommandMixin):
return source
def _find_routable_addr(self):
ips = set()
for api_url in self.api_urls:
ironic_host = urlparse.urlparse(api_url).hostname
# Try resolving it in case it's not an IP address
try:
addrs = socket.getaddrinfo(ironic_host, 0)
except socket.gaierror:
LOG.debug('Could not resolve %s, maybe no DNS', ironic_host)
ips.add(ironic_host)
continue
ips.update(addr for _, _, _, _, (addr, *_) in addrs)
def _test_ip_reachability(self, api_url):
"""Test if an API URL is reachable via HTTP GET request.
:param api_url: The full API URL to test (including protocol and port)
:returns: True if the URL is reachable, False otherwise
"""
try:
# Disable SSL verification for reachability testing only
response = requests.get(
api_url, timeout=CONF.http_request_timeout, verify=False
) # nosec
# Any HTTP response (even 404, 500, etc.) indicates reachability
LOG.debug('API URL %s is reachable (status: %s)',
api_url, response.status_code)
return True
except requests.exceptions.RequestException as e:
LOG.debug('API URL %s not reachable: %s', api_url, e)
return False
def _find_routable_addr(self):
# Process API URLs: check reachability and collect IPs in one pass
reachable_api_urls = []
ips = set()
for api_url in self.api_urls:
parsed = urlparse.urlparse(api_url)
ironic_host = parsed.hostname
# Test reachability using the full URL (including port)
if self._test_ip_reachability(api_url):
reachable_api_urls.append(api_url)
LOG.debug('API URL %s is reachable', api_url)
# Collect IPs for reachable hosts
try:
addrs = socket.getaddrinfo(ironic_host, 0)
ips.update(addr for _, _, _, _, (addr, *_) in addrs)
except socket.gaierror:
LOG.debug('Could not resolve %s, maybe no DNS',
ironic_host)
ips.add(ironic_host)
else:
LOG.debug('API URL %s is not reachable, skipping', api_url)
# Update api_urls configuration to only include reachable endpoints
if reachable_api_urls:
LOG.info('Filtered API URLs from %d to %d reachable endpoints',
len(self.api_urls), len(reachable_api_urls))
self.api_urls = reachable_api_urls
else:
LOG.warning('No reachable Ironic API URLs found, keeping all URLs')
# Find routable address using collected IPs
for attempt in range(self.ip_lookup_attempts):
for ironic_host in ips:
found_ip = self._get_route_source(ironic_host)

165
ironic_python_agent/tests/unit/test_agent.py
View File

@@ -21,6 +21,7 @@ from unittest.mock import sentinel
from oslo_concurrency import processutils
from oslo_config import cfg
import requests
from stevedore import extension
from ironic_python_agent import agent
@@ -948,6 +949,123 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest):
self.assertIsNone(source)
mock_log.warning.assert_called_once()
@mock.patch('requests.get', autospec=True)
def test_test_ip_reachability_success(self, mock_get):
"""Test _test_ip_reachability with successful HTTP response."""
mock_response = mock.Mock()
mock_response.status_code = 200
mock_get.return_value = mock_response
result = self.agent._test_ip_reachability('http://192.168.1.1:6385/v1')
self.assertTrue(result)
mock_get.assert_called_once_with('http://192.168.1.1:6385/v1',
timeout=30, verify=False)
@mock.patch('requests.get', autospec=True)
def test_test_ip_reachability_https_success(self, mock_get):
"""Test _test_ip_reachability with HTTPS URL."""
mock_response = mock.Mock()
mock_response.status_code = 404 # Any status code is acceptable
mock_get.return_value = mock_response
result = self.agent._test_ip_reachability(
'https://192.168.1.1:6385/v1')
self.assertTrue(result)
mock_get.assert_called_once_with('https://192.168.1.1:6385/v1',
timeout=30, verify=False)
@mock.patch('requests.get', autospec=True)
def test_test_ip_reachability_failure(self, mock_get):
"""Test _test_ip_reachability with connection failure."""
mock_get.side_effect = requests.exceptions.ConnectionError(
'Connection failed')
result = self.agent._test_ip_reachability('http://192.168.1.1:6385/v1')
self.assertFalse(result)
mock_get.assert_called_once_with('http://192.168.1.1:6385/v1',
timeout=30, verify=False)
@mock.patch('requests.get', autospec=True)
@mock.patch.object(socket, 'getaddrinfo', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True)
def test_find_routable_addr_filters_api_urls(self, mock_exec,
mock_getaddrinfo,
mock_requests_get):
"""Test that _find_routable_addr filters unreachable API URLs."""
# Set up multiple API URLs - some reachable, some not
self.agent.api_urls = [
'http://reachable1.example.com:8081/v1',
'http://unreachable.example.com:8081/v1',
'http://reachable2.example.com:8081/v1'
]
# Mock reachability tests - only reachable1 and reachable2 are
# reachable
def mock_reachability_side_effect(url, **kwargs):
mock_response = mock.Mock()
mock_response.status_code = 200
if 'reachable1' in url or 'reachable2' in url:
return mock_response
else:
raise requests.exceptions.ConnectionError('Connection failed')
mock_requests_get.side_effect = mock_reachability_side_effect
# Mock DNS resolution for reachable hosts
mock_getaddrinfo.return_value = [
(sentinel.a, sentinel.b, sentinel.c, sentinel.d,
('192.168.1.1', sentinel.e)),
]
# Mock successful route lookup
mock_exec.return_value = (
"""192.168.1.1 via 192.168.122.1 dev eth0 src 192.168.122.56
cache """,
""
)
result = self.agent._find_routable_addr()
# Should return the found IP
self.assertEqual('192.168.122.56', result)
# Should have filtered API URLs to only include reachable ones
self.assertEqual(2, len(self.agent.api_urls))
self.assertIn('http://reachable1.example.com:8081/v1',
self.agent.api_urls)
self.assertIn('http://reachable2.example.com:8081/v1',
self.agent.api_urls)
self.assertNotIn('http://unreachable.example.com:8081/v1',
self.agent.api_urls)
@mock.patch('requests.get', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True)
def test_find_routable_addr_no_reachable_urls(self, mock_exec,
mock_requests_get):
"""Test _find_routable_addr when no API URLs are reachable."""
# Set up API URLs that are all unreachable
self.agent.api_urls = [
'http://unreachable1.example.com:8081/v1',
'http://unreachable2.example.com:8081/v1'
]
# Mock all reachability tests to fail
mock_requests_get.side_effect = requests.exceptions.ConnectionError(
'Connection failed')
# Mock execute to raise an exception (no route found)
mock_exec.side_effect = processutils.ProcessExecutionError('No route')
# Should keep original URLs with warning
original_urls = self.agent.api_urls.copy()
result = self.agent._find_routable_addr()
# Should return None (no routable address found)
self.assertIsNone(result)
# Should keep original URLs when none are reachable
self.assertEqual(original_urls, self.agent.api_urls)
@mock.patch.object(hardware, '_md_scan_and_assemble', lambda: None)
@mock.patch.object(hardware, '_check_for_iscsi', lambda: None)
@@ -1040,6 +1158,7 @@ class TestAgentStandalone(ironic_agent_base.IronicAgentTest):
@mock.patch.object(hardware, '_load_ipmi_modules', lambda: None)
@mock.patch.object(hardware.GenericHardwareManager, 'wait_for_disks',
lambda self: None)
@mock.patch('requests.get', autospec=True)
@mock.patch.object(socket, 'getaddrinfo', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True)
class TestAdvertiseAddress(ironic_agent_base.IronicAgentTest):
@@ -1058,7 +1177,8 @@ class TestAdvertiseAddress(ironic_agent_base.IronicAgentTest):
agent_token=None,
standalone=False)
def test_advertise_address_provided(self, mock_exec, mock_getaddrinfo):
def test_advertise_address_provided(self, mock_exec, mock_getaddrinfo,
mock_requests_get):
self.agent.advertise_address = agent.Host('1.2.3.4', 9990)
self.agent.set_agent_advertise_addr()
@@ -1079,7 +1199,8 @@ class TestAdvertiseAddress(ironic_agent_base.IronicAgentTest):
)
def test_with_network_interface(self, mock_hardware, mock_cna,
mock_get_ipv4, mock_mpath,
mock_exec, mock_getaddrinfo):
mock_exec, mock_getaddrinfo,
mock_requests_get):
self.agent.network_interface = 'em1'
mock_get_ipv4.return_value = '1.2.3.4'
mock_cna.return_value = False
@@ -1106,7 +1227,8 @@ class TestAdvertiseAddress(ironic_agent_base.IronicAgentTest):
def test_with_network_interface_failed(self, mock_hardware,
mock_cna, mock_get_ipv4,
mock_mpath, mock_exec,
mock_getaddrinfo):
mock_getaddrinfo,
mock_requests_get):
self.agent.network_interface = 'em1'
mock_get_ipv4.return_value = None
mock_cna.return_value = False
@@ -1119,7 +1241,8 @@ class TestAdvertiseAddress(ironic_agent_base.IronicAgentTest):
self.assertFalse(mock_exec.called)
self.assertFalse(mock_getaddrinfo.called)
def test_route_with_ip(self, mock_exec, mock_getaddrinfo):
def test_route_with_ip(self, mock_exec, mock_getaddrinfo,
mock_requests_get):
self.agent.api_urls = ['http://1.2.1.2:8081/v1']
mock_getaddrinfo.side_effect = socket.gaierror()
mock_exec.return_value = (
@@ -1127,6 +1250,10 @@ class TestAdvertiseAddress(ironic_agent_base.IronicAgentTest):
cache """,
""
)
# Mock successful HTTP reachability test
mock_response = mock.Mock()
mock_response.status_code = 200
mock_requests_get.return_value = mock_response
self.agent.set_agent_advertise_addr()
@@ -1135,7 +1262,8 @@ class TestAdvertiseAddress(ironic_agent_base.IronicAgentTest):
mock_exec.assert_called_once_with('ip', 'route', 'get', '1.2.1.2')
mock_getaddrinfo.assert_called_once_with('1.2.1.2', 0)
def test_route_with_ipv6(self, mock_exec, mock_getaddrinfo):
def test_route_with_ipv6(self, mock_exec, mock_getaddrinfo,
mock_requests_get):
self.agent.api_urls = ['http://[fc00:1111::1]:8081/v1']
mock_getaddrinfo.side_effect = socket.gaierror()
mock_exec.return_value = (
@@ -1143,6 +1271,10 @@ class TestAdvertiseAddress(ironic_agent_base.IronicAgentTest):
cache """,
""
)
# Mock successful HTTP reachability test
mock_response = mock.Mock()
mock_response.status_code = 200
mock_requests_get.return_value = mock_response
self.agent.set_agent_advertise_addr()
@@ -1151,7 +1283,8 @@ class TestAdvertiseAddress(ironic_agent_base.IronicAgentTest):
mock_exec.assert_called_once_with('ip', 'route', 'get', 'fc00:1111::1')
mock_getaddrinfo.assert_called_once_with('fc00:1111::1', 0)
def test_route_with_host(self, mock_exec, mock_getaddrinfo):
def test_route_with_host(self, mock_exec, mock_getaddrinfo,
mock_requests_get):
mock_getaddrinfo.return_value = [
(sentinel.a, sentinel.b, sentinel.c, sentinel.d,
('1.2.1.2', sentinel.e)),
@@ -1161,6 +1294,10 @@ class TestAdvertiseAddress(ironic_agent_base.IronicAgentTest):
cache """,
""
)
# Mock successful HTTP reachability test
mock_response = mock.Mock()
mock_response.status_code = 200
mock_requests_get.return_value = mock_response
self.agent.set_agent_advertise_addr()
@@ -1169,7 +1306,8 @@ class TestAdvertiseAddress(ironic_agent_base.IronicAgentTest):
mock_exec.assert_called_once_with('ip', 'route', 'get', '1.2.1.2')
mock_getaddrinfo.assert_called_once_with('fake_api.example.org', 0)
def test_route_with_host_v6(self, mock_exec, mock_getaddrinfo):
def test_route_with_host_v6(self, mock_exec, mock_getaddrinfo,
mock_requests_get):
mock_getaddrinfo.return_value = [
(sentinel.a, sentinel.b, sentinel.c, sentinel.d,
('fc00:1111::1', sentinel.e)),
@@ -1179,6 +1317,10 @@ class TestAdvertiseAddress(ironic_agent_base.IronicAgentTest):
cache """,
""
)
# Mock successful HTTP reachability test
mock_response = mock.Mock()
mock_response.status_code = 200
mock_requests_get.return_value = mock_response
self.agent.set_agent_advertise_addr()
@@ -1188,7 +1330,8 @@ class TestAdvertiseAddress(ironic_agent_base.IronicAgentTest):
mock_getaddrinfo.assert_called_once_with('fake_api.example.org', 0)
@mock.patch.object(time, 'sleep', autospec=True)
def test_route_retry(self, mock_sleep, mock_exec, mock_getaddrinfo):
def test_route_retry(self, mock_sleep, mock_exec, mock_getaddrinfo,
mock_requests_get):
mock_getaddrinfo.return_value = [
(sentinel.a, sentinel.b, sentinel.c, sentinel.d,
('1.2.1.2', sentinel.e)),
@@ -1218,7 +1361,8 @@ class TestAdvertiseAddress(ironic_agent_base.IronicAgentTest):
@mock.patch.object(time, 'sleep', autospec=True)
def test_route_several_urls_and_retries(self, mock_sleep, mock_exec,
mock_getaddrinfo):
mock_getaddrinfo,
mock_requests_get):
mock_getaddrinfo.side_effect = lambda addr, port: [
(sentinel.a, sentinel.b, sentinel.c, sentinel.d,
(addr, sentinel.e)),
@@ -1263,7 +1407,8 @@ class TestAdvertiseAddress(ironic_agent_base.IronicAgentTest):
self.assertEqual(2, mock_getaddrinfo.call_count)
@mock.patch.object(time, 'sleep', autospec=True)
def test_route_failed(self, mock_sleep, mock_exec, mock_getaddrinfo):
def test_route_failed(self, mock_sleep, mock_exec, mock_getaddrinfo,
mock_requests_get):
mock_getaddrinfo.return_value = [
(sentinel.a, sentinel.b, sentinel.c, sentinel.d,
('1.2.1.2', sentinel.e)),

8
releasenotes/notes/fix-reachability-test-7da1c27ad261bbc4.yaml Normal file
View File

@@ -0,0 +1,8 @@
---
fixes:
- |
Fixes an issue where the agent could not find a valid IP address when
the Ironic API was running on a non-standard port. The reachability test
now uses the full API URL including the port number, instead of only
using the hostname. This prevents ``LookupAgentIPError`` when connecting
to Ironic APIs on custom ports.

10
releasenotes/notes/test-ip-reachability-before-advertise-689f2d3b9276d399.yaml Normal file
View File

@@ -0,0 +1,10 @@
---
fixes:
- |
The agent now tests Ironic API URL reachability via HTTP during startup
before determining the advertised IP address. Unreachable API URLs are
filtered out, preventing the agent from advertising an IP address that
cannot actually reach the Ironic API. This improves reliability when
multiple API URLs are configured or in complex network topologies.
The IP reachability test uses the configured ``http_request_timeout``
(default 30 seconds) timeout for the HTTP request.