diff --git a/ironic_python_agent/agent.py b/ironic_python_agent/agent.py index 6ac11c97e..281c48032 100644 --- a/ironic_python_agent/agent.py +++ b/ironic_python_agent/agent.py @@ -210,7 +210,7 @@ class IronicPythonAgent(base.ExecuteCommandMixin): return try: - return out.strip().split('\n')[0].split('src')[1].strip() + return out.strip().split('\n')[0].split('src')[1].split()[0] except IndexError: LOG.warning('No route to host %(dest)s, route record: %(rec)s', {'dest': dest, 'rec': out}) diff --git a/ironic_python_agent/ironic_api_client.py b/ironic_python_agent/ironic_api_client.py index 1fd8caf12..60c635cad 100644 --- a/ironic_python_agent/ironic_api_client.py +++ b/ironic_python_agent/ironic_api_client.py @@ -20,6 +20,7 @@ import requests from ironic_python_agent import encoding from ironic_python_agent import errors +from ironic_python_agent import netutils LOG = log.getLogger(__name__) @@ -141,5 +142,5 @@ class APIClient(object): raise loopingcall.LoopingCallDone(retvalue=content) def _get_agent_url(self, advertise_address): - return 'http://{}:{}'.format(advertise_address[0], + return 'http://{}:{}'.format(netutils.wrap_ipv6(advertise_address[0]), advertise_address[1]) diff --git a/ironic_python_agent/netutils.py b/ironic_python_agent/netutils.py index 55c7a3eb3..4d482b69d 100644 --- a/ironic_python_agent/netutils.py +++ b/ironic_python_agent/netutils.py @@ -22,6 +22,7 @@ import sys import netifaces from oslo_config import cfg from oslo_log import log as logging +from oslo_utils import netutils LOG = logging.getLogger(__name__) CONF = cfg.CONF @@ -213,3 +214,9 @@ def interface_has_carrier(interface_name): LOG.debug('No carrier information for interface %s', interface_name) return False + + +def wrap_ipv6(ip): + if netutils.is_valid_ipv6(ip): + return "[%s]" % ip + return ip diff --git a/ironic_python_agent/tests/unit/test_agent.py b/ironic_python_agent/tests/unit/test_agent.py index 62bd509e5..71ee80cc9 100644 --- a/ironic_python_agent/tests/unit/test_agent.py +++ b/ironic_python_agent/tests/unit/test_agent.py @@ -466,6 +466,22 @@ class TestBaseAgent(test_base.BaseTestCase): self.assertRaises(errors.UnknownNodeError, self.agent.get_node_uuid) + @mock.patch.object(utils, 'execute', autospec=True) + def test_get_route_source(self, mock_execute): + mock_execute.return_value = ('XXX src 1.2.3.4 XXX\n cache', None) + + source = self.agent._get_route_source('XXX') + self.assertEqual('1.2.3.4', source) + + @mock.patch.object(agent, 'LOG', autospec=True) + @mock.patch.object(utils, 'execute', autospec=True) + def test_get_route_source_indexerror(self, mock_execute, mock_log): + mock_execute.return_value = ('XXX src \n cache', None) + + source = self.agent._get_route_source('XXX') + self.assertIsNone(source) + mock_log.warning.assert_called_once() + @mock.patch.object(hardware.GenericHardwareManager, '_wait_for_disks', lambda self: None) @@ -597,6 +613,22 @@ class TestAdvertiseAddress(test_base.BaseTestCase): mock_exec.assert_called_once_with('ip', 'route', 'get', '1.2.1.2') mock_gethostbyname.assert_called_once_with('1.2.1.2') + def test_route_with_ipv6(self, mock_exec, mock_gethostbyname): + self.agent.api_url = 'http://[fc00:1111::1]:8081/v1' + mock_gethostbyname.side_effect = socket.gaierror() + mock_exec.return_value = ( + """fc00:101::1 dev br-ctlplane src fc00:101::4 metric 0 + cache """, + "" + ) + + self.agent.set_agent_advertise_addr() + + self.assertEqual(('fc00:101::4', 9990), + self.agent.advertise_address) + mock_exec.assert_called_once_with('ip', 'route', 'get', 'fc00:1111::1') + mock_gethostbyname.assert_called_once_with('fc00:1111::1') + def test_route_with_host(self, mock_exec, mock_gethostbyname): mock_gethostbyname.return_value = '1.2.1.2' mock_exec.return_value = ( diff --git a/ironic_python_agent/tests/unit/test_ironic_api_client.py b/ironic_python_agent/tests/unit/test_ironic_api_client.py index 3fe1807c0..7046f5f9b 100644 --- a/ironic_python_agent/tests/unit/test_ironic_api_client.py +++ b/ironic_python_agent/tests/unit/test_ironic_api_client.py @@ -70,8 +70,29 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase): heartbeat_path = 'v1/heartbeat/deadbeef-dabb-ad00-b105-f00d00bab10c' request_args = self.api_client.session.request.call_args[0] + data = self.api_client.session.request.call_args[1]['data'] self.assertEqual('POST', request_args[0]) self.assertEqual(API_URL + heartbeat_path, request_args[1]) + self.assertEqual('{"callback_url": "http://192.0.2.1:9999"}', data) + + def test_successful_heartbeat_ip6(self): + response = FakeResponse(status_code=202) + + self.api_client.session.request = mock.Mock() + self.api_client.session.request.return_value = response + + self.api_client.heartbeat( + uuid='deadbeef-dabb-ad00-b105-f00d00bab10c', + advertise_address=('fc00:1111::4', '9999') + ) + + heartbeat_path = 'v1/heartbeat/deadbeef-dabb-ad00-b105-f00d00bab10c' + request_args = self.api_client.session.request.call_args[0] + data = self.api_client.session.request.call_args[1]['data'] + self.assertEqual('POST', request_args[0]) + self.assertEqual(API_URL + heartbeat_path, request_args[1]) + self.assertEqual('{"callback_url": "http://[fc00:1111::4]:9999"}', + data) def test_heartbeat_requests_exception(self): self.api_client.session.request = mock.Mock() @@ -240,3 +261,11 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase): node_uuid=None) self.assertFalse(error) + + def test_get_agent_url_ipv4(self): + url = self.api_client._get_agent_url(('1.2.3.4', '9999')) + self.assertEqual('http://1.2.3.4:9999', url) + + def test_get_agent_url_ipv6(self): + url = self.api_client._get_agent_url(('1:2::3:4', '9999')) + self.assertEqual('http://[1:2::3:4]:9999', url) diff --git a/ironic_python_agent/tests/unit/test_netutils.py b/ironic_python_agent/tests/unit/test_netutils.py index e3fa823f5..14c8b82eb 100644 --- a/ironic_python_agent/tests/unit/test_netutils.py +++ b/ironic_python_agent/tests/unit/test_netutils.py @@ -336,3 +336,11 @@ class TestNetutils(test_base.BaseTestCase): sock1.close.assert_called_once_with() sock2.close.assert_called_once_with() + + def test_wrap_ipv6(self): + res = netutils.wrap_ipv6('1:2::3:4') + self.assertEqual('[1:2::3:4]', res) + + def test_wrap_ipv6_with_ipv4(self): + res = netutils.wrap_ipv6('1.2.3.4') + self.assertEqual('1.2.3.4', res) diff --git a/releasenotes/notes/ip6-addresses-1c2b9bcd9a124de7.yaml b/releasenotes/notes/ip6-addresses-1c2b9bcd9a124de7.yaml new file mode 100644 index 000000000..62c8fe682 --- /dev/null +++ b/releasenotes/notes/ip6-addresses-1c2b9bcd9a124de7.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - Ironic Python Agent now correctly detects its IPv6 address + if communicating with ironic over IPv6. This address is also + wrapped in square brackets if the address being advertised + is IPv6.