From 7d7735a21622ded09e1064fcdecb981009b3927b Mon Sep 17 00:00:00 2001 From: Riccardo Pittau Date: Tue, 18 Nov 2025 14:40:47 +0100 Subject: [PATCH] Fix API URL reachability test to use full URL with port The _test_ip_reachability method was only using the hostname/IP address when testing reachability, ignoring the port number from the API URL. This caused LookupAgentIPError when the Ironic API was running on a non-standard port (e.g., 6385). This change modifies _test_ip_reachability to: - Accept the full API URL instead of just an IP address - Use the complete URL (including protocol and port) when testing The _find_routable_addr method now passes the full api_url to _test_ip_reachability instead of just the hostname, ensuring the port is included in reachability tests. Assisted-By: Claude Sonnet 4.5 Change-Id: Ibb407255cfcd5cf9617f040338561fd494e8b41f Signed-off-by: Riccardo Pittau --- ironic_python_agent/agent.py | 49 ++++++++----------- ironic_python_agent/tests/unit/test_agent.py | 28 +++++------ ...ix-reachability-test-7da1c27ad261bbc4.yaml | 8 +++ 3 files changed, 42 insertions(+), 43 deletions(-) create mode 100644 releasenotes/notes/fix-reachability-test-7da1c27ad261bbc4.yaml diff --git a/ironic_python_agent/agent.py b/ironic_python_agent/agent.py index 686a5788b..670a3a61e 100644 --- a/ironic_python_agent/agent.py +++ b/ironic_python_agent/agent.py @@ -308,34 +308,24 @@ class IronicPythonAgent(base.ExecuteCommandMixin): return source - def _test_ip_reachability(self, ip_address): - """Test if an IP address is reachable via HTTP GET request. + def _test_ip_reachability(self, api_url): + """Test if an API URL is reachable via HTTP GET request. - :param ip_address: The IP address to test - :returns: True if the IP is reachable, False otherwise + :param api_url: The full API URL to test (including protocol and port) + :returns: True if the URL is reachable, False otherwise """ - test_urls = [ - 'http://{}'.format(ip_address), - 'https://{}'.format(ip_address), - ] - - for url in test_urls: - try: - # Disable SSL verification for reachability testing only - response = requests.get( - url, timeout=CONF.http_request_timeout, verify=False - ) # nosec - # Any HTTP response (even 404, 500, etc.) indicates - # reachability - LOG.debug('IP %s is reachable via %s (status: %s)', - ip_address, url, response.status_code) - return True - except requests.exceptions.RequestException as e: - LOG.debug('IP %s not reachable via %s: %s', - ip_address, url, e) - continue - - return False + 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 @@ -343,10 +333,11 @@ class IronicPythonAgent(base.ExecuteCommandMixin): ips = set() for api_url in self.api_urls: - ironic_host = urlparse.urlparse(api_url).hostname + parsed = urlparse.urlparse(api_url) + ironic_host = parsed.hostname - # Test reachability once per hostname - if self._test_ip_reachability(ironic_host): + # 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) diff --git a/ironic_python_agent/tests/unit/test_agent.py b/ironic_python_agent/tests/unit/test_agent.py index 5d643b827..a9be41855 100644 --- a/ironic_python_agent/tests/unit/test_agent.py +++ b/ironic_python_agent/tests/unit/test_agent.py @@ -956,24 +956,23 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest): mock_response.status_code = 200 mock_get.return_value = mock_response - result = self.agent._test_ip_reachability('192.168.1.1') + 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', timeout=30, - verify=False) + 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 fallback.""" - # First call (HTTP) fails, second call (HTTPS) succeeds - mock_get.side_effect = [ - requests.exceptions.ConnectionError('Connection failed'), - mock.Mock(status_code=404) # Any status code indicates - # reachability - ] + """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('192.168.1.1') + result = self.agent._test_ip_reachability( + 'https://192.168.1.1:6385/v1') self.assertTrue(result) - self.assertEqual(mock_get.call_count, 2) + 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): @@ -981,9 +980,10 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest): mock_get.side_effect = requests.exceptions.ConnectionError( 'Connection failed') - result = self.agent._test_ip_reachability('192.168.1.1') + result = self.agent._test_ip_reachability('http://192.168.1.1:6385/v1') self.assertFalse(result) - self.assertEqual(mock_get.call_count, 2) # Tries both HTTP and HTTPS + 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) diff --git a/releasenotes/notes/fix-reachability-test-7da1c27ad261bbc4.yaml b/releasenotes/notes/fix-reachability-test-7da1c27ad261bbc4.yaml new file mode 100644 index 000000000..b448ce6fb --- /dev/null +++ b/releasenotes/notes/fix-reachability-test-7da1c27ad261bbc4.yaml @@ -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.