Advertise the correct address when using IPv6

Parse the output of "ip route get $IP" taking
IPv6 into consideration. Also wrap the IP address
in square brackets if it is IPv6.

Change-Id: Ifc44e5aa3c5b814b6ceba04461bb68fe1d75c22b
Closes-Bug: #1650533

6 files changed, 79 insertions, 2 deletions

diff --git a/ironic_python_agent/agent.py b/ironic_python_agent/agent.py
index 6ac11c97..281c4803 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 1fd8caf1..60c635ca 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 55c7a3eb..4d482b69 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 62bd509e..71ee80cc 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 3fe1807c..7046f5f9 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 e3fa823f..14c8b82e 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)