diff --git a/imagebuild/tinyipa/build_files/finalreqs.lst b/imagebuild/tinyipa/build_files/finalreqs.lst index e9356cb70..677bd084a 100644 --- a/imagebuild/tinyipa/build_files/finalreqs.lst +++ b/imagebuild/tinyipa/build_files/finalreqs.lst @@ -3,6 +3,7 @@ coreutils.tcz dmidecode.tcz gdisk.tcz hdparm.tcz +iproute2.tcz parted.tcz python.tcz raid-dm-3.16.6-tinycore64.tcz diff --git a/ironic_python_agent/agent.py b/ironic_python_agent/agent.py index 1716edac1..75a7e8aab 100644 --- a/ironic_python_agent/agent.py +++ b/ironic_python_agent/agent.py @@ -15,11 +15,14 @@ import os import random import select +import socket import threading import time +from oslo_concurrency import processutils from oslo_log import log import pkg_resources +from six.moves.urllib import parse as urlparse from stevedore import extension from wsgiref import simple_server @@ -30,6 +33,7 @@ from ironic_python_agent.extensions import base from ironic_python_agent import hardware from ironic_python_agent import inspector from ironic_python_agent import ironic_api_client +from ironic_python_agent import utils LOG = log.getLogger(__name__) @@ -183,6 +187,21 @@ class IronicPythonAgent(base.ExecuteCommandMixin): version=self.version ) + def _get_route_source(self, dest): + """Get the IP address to send packages to destination.""" + try: + out, _err = utils.execute('ip', 'route', 'get', dest) + except (EnvironmentError, processutils.ProcessExecutionError) as e: + LOG.warning('Cannot get route to host %(dest)s: %(err)s', + {'dest': dest, 'err': e}) + return + + try: + return out.strip().split('\n')[0].split('src')[1].strip() + except IndexError: + LOG.warning('No route to host %(dest)s, route record: %(rec)s', + {'dest': dest, 'rec': out}) + def set_agent_advertise_addr(self): """Set advertised IP address for the agent, if not already set. @@ -190,52 +209,38 @@ class IronicPythonAgent(base.ExecuteCommandMixin): find a better one. If the agent's network interface is None, replace that as well. - :raises: LookupAgentInterfaceError if a valid network interface cannot - be found. :raises: LookupAgentIPError if an IP address could not be found """ if self.advertise_address[0] is not None: return - if self.network_interface is None: - ifaces = self.get_agent_network_interfaces() + found_ip = None + if self.network_interface is not None: + # TODO(dtantsur): deprecate this + found_ip = hardware.dispatch_to_managers('get_ipv4_addr', + self.network_interface) else: - ifaces = [self.network_interface] + url = urlparse.urlparse(self.api_url) + ironic_host = url.hostname + # Try resolving it in case it's not an IP address + try: + ironic_host = socket.gethostbyname(ironic_host) + except socket.gaierror: + LOG.debug('Count not resolve %s, maybe no DNS', ironic_host) - attempts = 0 - while (attempts < self.ip_lookup_attempts): - for iface in ifaces: - found_ip = hardware.dispatch_to_managers('get_ipv4_addr', - iface) - if found_ip is not None: - self.advertise_address = (found_ip, - self.advertise_address[1]) - self.network_interface = iface - return - attempts += 1 - time.sleep(self.ip_lookup_sleep) + for attempt in range(self.ip_lookup_attempts): + found_ip = self._get_route_source(ironic_host) + if found_ip: + break - raise errors.LookupAgentIPError('Agent could not find a valid IP ' - 'address.') + time.sleep(self.ip_lookup_sleep) - def get_agent_network_interfaces(self): - """Get a list of all network interfaces available. - - Excludes loopback connections. - - :returns: list of network interfaces available. - :raises: LookupAgentInterfaceError if a valid interface could not - be found. - """ - iface_list = [iface.serialize()['name'] for iface in - hardware.dispatch_to_managers('list_network_interfaces')] - iface_list = [name for name in iface_list if 'lo' not in name] - - if len(iface_list) == 0: - raise errors.LookupAgentInterfaceError('Agent could not find a ' - 'valid network interface.') + if found_ip: + self.advertise_address = (found_ip, + self.advertise_address[1]) else: - return iface_list + raise errors.LookupAgentIPError('Agent could not find a valid IP ' + 'address.') def get_node_uuid(self): """Get UUID for Ironic node. diff --git a/ironic_python_agent/errors.py b/ironic_python_agent/errors.py index 6fae45857..59ec6400c 100644 --- a/ironic_python_agent/errors.py +++ b/ironic_python_agent/errors.py @@ -136,15 +136,6 @@ class LookupAgentIPError(IronicAPIError): super(LookupAgentIPError, self).__init__(details) -class LookupAgentInterfaceError(IronicAPIError): - """Error raised when agent interface lookup fails.""" - - message = 'Error finding network interface for Ironic Agent' - - def __init__(self, details): - super(LookupAgentInterfaceError, self).__init__(details) - - class ImageDownloadError(RESTError): """Error raised when an image cannot be downloaded.""" diff --git a/ironic_python_agent/tests/unit/test_agent.py b/ironic_python_agent/tests/unit/test_agent.py index 350028332..516e6d53f 100644 --- a/ironic_python_agent/tests/unit/test_agent.py +++ b/ironic_python_agent/tests/unit/test_agent.py @@ -13,9 +13,11 @@ # limitations under the License. import json +import socket import time import mock +from oslo_concurrency import processutils from oslo_config import cfg from oslotest import base as test_base import pkg_resources @@ -28,6 +30,7 @@ from ironic_python_agent import errors from ironic_python_agent.extensions import base from ironic_python_agent import hardware from ironic_python_agent import inspector +from ironic_python_agent import utils EXPECTED_ERROR = RuntimeError('command execution failed') @@ -228,56 +231,6 @@ class TestBaseAgent(test_base.BaseTestCase): self.agent.heartbeater.start.assert_called_once_with() - @mock.patch('os.read') - @mock.patch('select.poll') - @mock.patch('time.sleep', return_value=None) - @mock.patch.object(hardware.GenericHardwareManager, - 'list_network_interfaces') - @mock.patch.object(hardware.GenericHardwareManager, 'get_ipv4_addr') - def test_ipv4_lookup(self, - mock_get_ipv4, - mock_list_net, - mock_time_sleep, - mock_poll, - mock_read): - homeless_agent = agent.IronicPythonAgent('https://fake_api.example.' - 'org:8081/', - (None, 9990), - ('192.0.2.1', 9999), - 3, - 10, - None, - 300, - 1, - 'agent_ipmitool', - False) - - mock_poll.return_value.poll.return_value = True - mock_read.return_value = 'a' - - # Can't find network interfaces, and therefore can't find IP - mock_list_net.return_value = [] - mock_get_ipv4.return_value = None - self.assertRaises(errors.LookupAgentInterfaceError, - homeless_agent.set_agent_advertise_addr) - - # Can look up network interfaces, but not IP. Network interface not - # set, because no interface yields an IP. - mock_ifaces = [hardware.NetworkInterface('eth0', '00:00:00:00:00:00'), - hardware.NetworkInterface('eth1', '00:00:00:00:00:01')] - mock_list_net.return_value = mock_ifaces - - self.assertRaises(errors.LookupAgentIPError, - homeless_agent.set_agent_advertise_addr) - self.assertEqual(6, mock_get_ipv4.call_count) - self.assertIsNone(homeless_agent.network_interface) - - # First interface eth0 has no IP, second interface eth1 has an IP - mock_get_ipv4.side_effect = [None, '1.1.1.1'] - homeless_agent.heartbeater.run() - self.assertEqual(('1.1.1.1', 9990), homeless_agent.advertise_address) - self.assertEqual('eth1', homeless_agent.network_interface) - def test_async_command_success(self): result = base.AsyncCommandResult('foo_command', {'fail': False}, foo_execute) @@ -382,3 +335,129 @@ class TestAgentStandalone(test_base.BaseTestCase): self.assertFalse(self.agent.heartbeater.called) self.assertFalse(self.agent.api_client.lookup_node.called) + + +@mock.patch.object(socket, 'gethostbyname', autospec=True) +@mock.patch.object(utils, 'execute', autospec=True) +class TestAdvertiseAddress(test_base.BaseTestCase): + def setUp(self): + super(TestAdvertiseAddress, self).setUp() + + self.agent = agent.IronicPythonAgent( + api_url='https://fake_api.example.org:8081/', + advertise_address=(None, 9990), + listen_address=('0.0.0.0', 9999), + ip_lookup_attempts=5, + ip_lookup_sleep=10, + network_interface=None, + lookup_timeout=300, + lookup_interval=1, + driver_name='agent_ipmitool', + standalone=False) + + def test_advertise_address_provided(self, mock_exec, mock_gethostbyname): + self.agent.advertise_address = ('1.2.3.4', 9990) + + self.agent.set_agent_advertise_addr() + + self.assertEqual(('1.2.3.4', 9990), self.agent.advertise_address) + self.assertFalse(mock_exec.called) + self.assertFalse(mock_gethostbyname.called) + + @mock.patch.object(hardware.GenericHardwareManager, 'get_ipv4_addr', + autospec=True) + def test_with_network_interface(self, mock_get_ipv4, mock_exec, + mock_gethostbyname): + self.agent.network_interface = 'em1' + mock_get_ipv4.return_value = '1.2.3.4' + + self.agent.set_agent_advertise_addr() + + self.assertEqual(('1.2.3.4', 9990), self.agent.advertise_address) + mock_get_ipv4.assert_called_once_with(mock.ANY, 'em1') + self.assertFalse(mock_exec.called) + self.assertFalse(mock_gethostbyname.called) + + @mock.patch.object(hardware.GenericHardwareManager, 'get_ipv4_addr', + autospec=True) + def test_with_network_interface_failed(self, mock_get_ipv4, mock_exec, + mock_gethostbyname): + self.agent.network_interface = 'em1' + mock_get_ipv4.return_value = None + + self.assertRaises(errors.LookupAgentIPError, + self.agent.set_agent_advertise_addr) + + mock_get_ipv4.assert_called_once_with(mock.ANY, 'em1') + self.assertFalse(mock_exec.called) + self.assertFalse(mock_gethostbyname.called) + + def test_route_with_ip(self, mock_exec, mock_gethostbyname): + self.agent.api_url = 'http://1.2.1.2:8081/v1' + mock_gethostbyname.side_effect = socket.gaierror() + mock_exec.return_value = ( + """1.2.1.2 via 192.168.122.1 dev eth0 src 192.168.122.56 + cache """, + "" + ) + + self.agent.set_agent_advertise_addr() + + self.assertEqual(('192.168.122.56', 9990), + self.agent.advertise_address) + 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_host(self, mock_exec, mock_gethostbyname): + mock_gethostbyname.return_value = '1.2.1.2' + mock_exec.return_value = ( + """1.2.1.2 via 192.168.122.1 dev eth0 src 192.168.122.56 + cache """, + "" + ) + + self.agent.set_agent_advertise_addr() + + self.assertEqual(('192.168.122.56', 9990), + self.agent.advertise_address) + mock_exec.assert_called_once_with('ip', 'route', 'get', '1.2.1.2') + mock_gethostbyname.assert_called_once_with('fake_api.example.org') + + @mock.patch.object(time, 'sleep', autospec=True) + def test_route_retry(self, mock_sleep, mock_exec, mock_gethostbyname): + mock_gethostbyname.return_value = '1.2.1.2' + mock_exec.side_effect = [ + processutils.ProcessExecutionError('boom'), + ( + "Error: some error text", + "" + ), + ( + """1.2.1.2 via 192.168.122.1 dev eth0 src 192.168.122.56 + cache """, + "" + ) + ] + + self.agent.set_agent_advertise_addr() + + self.assertEqual(('192.168.122.56', 9990), + self.agent.advertise_address) + mock_exec.assert_called_with('ip', 'route', 'get', '1.2.1.2') + mock_gethostbyname.assert_called_once_with('fake_api.example.org') + mock_sleep.assert_called_with(10) + self.assertEqual(3, mock_exec.call_count) + self.assertEqual(2, mock_sleep.call_count) + + @mock.patch.object(time, 'sleep', autospec=True) + def test_route_failed(self, mock_sleep, mock_exec, mock_gethostbyname): + mock_gethostbyname.return_value = '1.2.1.2' + mock_exec.side_effect = processutils.ProcessExecutionError('boom') + + self.assertRaises(errors.LookupAgentIPError, + self.agent.set_agent_advertise_addr) + + mock_exec.assert_called_with('ip', 'route', 'get', '1.2.1.2') + mock_gethostbyname.assert_called_once_with('fake_api.example.org') + self.assertEqual(5, mock_exec.call_count) + self.assertEqual(5, mock_sleep.call_count) diff --git a/ironic_python_agent/tests/unit/test_errors.py b/ironic_python_agent/tests/unit/test_errors.py index 778f3a631..a904edb7f 100644 --- a/ironic_python_agent/tests/unit/test_errors.py +++ b/ironic_python_agent/tests/unit/test_errors.py @@ -103,7 +103,6 @@ class TestErrors(test_base.BaseTestCase): (errors.HeartbeatError(DETAILS), SAME_DETAILS), (errors.LookupNodeError(DETAILS), SAME_DETAILS), (errors.LookupAgentIPError(DETAILS), SAME_DETAILS), - (errors.LookupAgentInterfaceError(DETAILS), SAME_DETAILS), (errors.ImageDownloadError('image_id', DETAILS), DIFF_CL_DETAILS), (errors.ImageChecksumError( diff --git a/releasenotes/notes/advertise-address-c3b152fe475fb539.yaml b/releasenotes/notes/advertise-address-c3b152fe475fb539.yaml new file mode 100644 index 000000000..9ed81cf1e --- /dev/null +++ b/releasenotes/notes/advertise-address-c3b152fe475fb539.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - IPA will now advertise IP address via which the Ironic API is routed + instead of using the first available. + See https://bugs.launchpad.net/ironic-python-agent/+bug/1558956.