diff --git a/ironic_lib/mdns.py b/ironic_lib/mdns.py index e9bcc09b..a31283c8 100644 --- a/ironic_lib/mdns.py +++ b/ironic_lib/mdns.py @@ -17,6 +17,7 @@ https://review.opendev.org/651222. """ import collections +import ipaddress import socket import time @@ -28,6 +29,7 @@ import zeroconf from ironic_lib.common.i18n import _ from ironic_lib import exception +from ironic_lib import utils opts = [ @@ -59,7 +61,7 @@ LOG = logging.getLogger(__name__) _MDNS_DOMAIN = '_openstack._tcp.local.' _endpoint = collections.namedtuple('Endpoint', - ['ip', 'hostname', 'port', 'params']) + ['addresses', 'hostname', 'port', 'params']) class Zeroconf(object): @@ -91,26 +93,18 @@ class Zeroconf(object): :raises: :exc:`.ServiceRegistrationFailure` if the service cannot be registered, e.g. because of conflicts. """ - try: - parsed = _parse_endpoint(endpoint) - except socket.error as ex: - msg = (_("Cannot resolve the host name of %(endpoint)s: " - "%(error)s. Hint: only IPv4 is supported for now.") % - {'endpoint': endpoint, 'error': ex}) - raise exception.ServiceRegistrationFailure( - service=service_type, error=msg) + parsed = _parse_endpoint(endpoint, service_type) all_params = CONF.mdns.params.copy() if params: all_params.update(params) all_params.update(parsed.params) - # TODO(dtantsur): allow overriding TTL values via configuration when - # https://github.com/jstasiak/python-zeroconf/commit/ecc021b7a3cec863eed5a3f71a1f28e3026c25b0 - # is released. + # TODO(dtantsur): allow overriding TTL values via configuration info = zeroconf.ServiceInfo(_MDNS_DOMAIN, '%s.%s' % (service_type, _MDNS_DOMAIN), - parsed.ip, parsed.port, + addresses=parsed.addresses, + port=parsed.port, properties=all_params, server=parsed.hostname) @@ -138,13 +132,16 @@ class Zeroconf(object): self._registered.append(info) - def get_endpoint(self, service_type): + def get_endpoint(self, service_type, skip_loopback=True, + skip_link_local=False): """Get an endpoint and its properties from mDNS. If the requested endpoint is already in the built-in server cache, and its TTL is not exceeded, the cached value is returned. :param service_type: OpenStack service type. + :param skip_loopback: Whether to ignore loopback addresses. + :param skip_link_local: Whether to ignore link local V6 addresses. :returns: tuple (endpoint URL, properties as a dict). :raises: :exc:`.ServiceLookupFailure` if the service cannot be found. """ @@ -160,8 +157,28 @@ class Zeroconf(object): time.sleep(delay) delay *= 2 - # TODO(dtantsur): IPv6 support - address = socket.inet_ntoa(info.address) + all_addr = info.parsed_addresses() + + # Try to find the first routable address + for addr in all_addr: + try: + loopback = ipaddress.ip_address(addr).is_loopback + except ValueError: + LOG.debug('Skipping invalid IP address %s', addr) + continue + else: + if loopback and skip_loopback: + LOG.debug('Skipping loopback IP address %s', addr) + continue + + if utils.get_route_source(addr, skip_link_local): + address = addr + break + else: + LOG.warning('None of addresses %s seem routable, using ' + 'the first one', all_addr) + address = all_addr[0] + properties = {} for key, value in info.properties.items(): try: @@ -195,6 +212,8 @@ class Zeroconf(object): # Local hostname means that the catalog lists an IP address, # so use it host = address + if int(ipaddress.ip_address(host).version) == 6: + host = '[%s]' % host else: # Otherwise use the provided hostname. host = info.server.rstrip('.') @@ -240,7 +259,7 @@ def get_endpoint(service_type): return zc.get_endpoint(service_type) -def _parse_endpoint(endpoint): +def _parse_endpoint(endpoint, service_type=None): params = {} url = parse.urlparse(endpoint) port = url.port @@ -251,16 +270,30 @@ def _parse_endpoint(endpoint): else: port = 80 + addresses = [] hostname = url.hostname - # FIXME(dtantsur): the zeroconf library does not support IPv6, use IPv4 - # only resolving for now. - ip = socket.gethostbyname(hostname) - if ip == hostname: - # we need a host name for the service record. if what we have in - # the catalog is an IP address, use the local hostname instead - hostname = None - # zeroconf requires addresses in network format (and see above re IPv6) - ip = socket.inet_aton(ip) + try: + infos = socket.getaddrinfo(hostname, port, 0, socket.IPPROTO_TCP) + except socket.error as exc: + raise exception.ServiceRegistrationFailure( + service=service_type, + error=_('Could not resolve hostname %(host)s: %(exc)s') % + {'host': hostname, 'exc': exc}) + + for info in infos: + ip = info[4][0] + if ip == hostname: + # we need a host name for the service record. if what we have in + # the catalog is an IP address, use the local hostname instead + hostname = None + # zeroconf requires addresses in network format + ip = socket.inet_pton(info[0], ip) + if ip not in addresses: + addresses.append(ip) + if not addresses: + raise exception.ServiceRegistrationFailure( + service=service_type, + error=_('No suitable addresses found for %s') % url.hostname) # avoid storing information that can be derived from existing data if url.path not in ('', '/'): @@ -274,7 +307,7 @@ def _parse_endpoint(endpoint): if hostname is not None and not hostname.endswith('.'): hostname += '.' - return _endpoint(ip, hostname, port, params) + return _endpoint(addresses, hostname, port, params) def list_opts(): diff --git a/ironic_lib/tests/test_mdns.py b/ironic_lib/tests/test_mdns.py index 21c7656a..322a63c2 100644 --- a/ironic_lib/tests/test_mdns.py +++ b/ironic_lib/tests/test_mdns.py @@ -101,43 +101,55 @@ class ParseEndpointTestCase(base.IronicLibTestCase): def test_simple(self): endpoint = mdns._parse_endpoint('http://127.0.0.1') - self.assertEqual('127.0.0.1', socket.inet_ntoa(endpoint.ip)) + self.assertEqual(1, len(endpoint.addresses)) + self.assertEqual('127.0.0.1', socket.inet_ntoa(endpoint.addresses[0])) self.assertEqual(80, endpoint.port) self.assertEqual({}, endpoint.params) self.assertIsNone(endpoint.hostname) def test_simple_https(self): endpoint = mdns._parse_endpoint('https://127.0.0.1') - self.assertEqual('127.0.0.1', socket.inet_ntoa(endpoint.ip)) + self.assertEqual(1, len(endpoint.addresses)) + self.assertEqual('127.0.0.1', socket.inet_ntoa(endpoint.addresses[0])) self.assertEqual(443, endpoint.port) self.assertEqual({}, endpoint.params) self.assertIsNone(endpoint.hostname) def test_with_path_and_port(self): endpoint = mdns._parse_endpoint('http://127.0.0.1:8080/bm') - self.assertEqual('127.0.0.1', socket.inet_ntoa(endpoint.ip)) + self.assertEqual(1, len(endpoint.addresses)) + self.assertEqual('127.0.0.1', socket.inet_ntoa(endpoint.addresses[0])) self.assertEqual(8080, endpoint.port) self.assertEqual({'path': '/bm', 'protocol': 'http'}, endpoint.params) self.assertIsNone(endpoint.hostname) - @mock.patch.object(socket, 'gethostbyname', autospec=True) + @mock.patch.object(socket, 'getaddrinfo', autospec=True) def test_resolve(self, mock_resolve): - mock_resolve.return_value = '1.2.3.4' + mock_resolve.return_value = [ + (socket.AF_INET, None, None, None, ('1.2.3.4',)), + (socket.AF_INET6, None, None, None, ('::2', 'scope')), + ] endpoint = mdns._parse_endpoint('http://example.com') - self.assertEqual('1.2.3.4', socket.inet_ntoa(endpoint.ip)) + self.assertEqual(2, len(endpoint.addresses)) + self.assertEqual('1.2.3.4', socket.inet_ntoa(endpoint.addresses[0])) + self.assertEqual('::2', socket.inet_ntop(socket.AF_INET6, + endpoint.addresses[1])) self.assertEqual(80, endpoint.port) self.assertEqual({}, endpoint.params) self.assertEqual('example.com.', endpoint.hostname) - mock_resolve.assert_called_once_with('example.com') + mock_resolve.assert_called_once_with('example.com', 80, mock.ANY, + socket.IPPROTO_TCP) +@mock.patch('ironic_lib.utils.get_route_source', autospec=True) @mock.patch('zeroconf.Zeroconf', autospec=True) class GetEndpointTestCase(base.IronicLibTestCase): - def test_simple(self, mock_zc): + def test_simple(self, mock_zc, mock_route): mock_zc.return_value.get_service_info.return_value = mock.Mock( address=socket.inet_aton('192.168.1.1'), port=80, - properties={} + properties={}, + **{'parsed_addresses.return_value': ['192.168.1.1']} ) endp, params = mdns.get_endpoint('baremetal') @@ -149,11 +161,46 @@ class GetEndpointTestCase(base.IronicLibTestCase): ) mock_zc.return_value.close.assert_called_once_with() - def test_https(self, mock_zc): + def test_v6(self, mock_zc, mock_route): + mock_zc.return_value.get_service_info.return_value = mock.Mock( + port=80, + properties={}, + **{'parsed_addresses.return_value': ['::2']} + ) + + endp, params = mdns.get_endpoint('baremetal') + self.assertEqual('http://[::2]:80', endp) + self.assertEqual({}, params) + mock_zc.return_value.get_service_info.assert_called_once_with( + 'baremetal._openstack._tcp.local.', + 'baremetal._openstack._tcp.local.' + ) + mock_zc.return_value.close.assert_called_once_with() + + def test_skip_invalid(self, mock_zc, mock_route): + mock_zc.return_value.get_service_info.return_value = mock.Mock( + port=80, + properties={}, + **{'parsed_addresses.return_value': ['::1', '::2', '::3']} + ) + mock_route.side_effect = [None, '::4'] + + endp, params = mdns.get_endpoint('baremetal') + self.assertEqual('http://[::3]:80', endp) + self.assertEqual({}, params) + mock_zc.return_value.get_service_info.assert_called_once_with( + 'baremetal._openstack._tcp.local.', + 'baremetal._openstack._tcp.local.' + ) + mock_zc.return_value.close.assert_called_once_with() + self.assertEqual(2, mock_route.call_count) + + def test_https(self, mock_zc, mock_route): mock_zc.return_value.get_service_info.return_value = mock.Mock( address=socket.inet_aton('192.168.1.1'), port=443, - properties={} + properties={}, + **{'parsed_addresses.return_value': ['192.168.1.1']} ) endp, params = mdns.get_endpoint('baremetal') @@ -164,11 +211,12 @@ class GetEndpointTestCase(base.IronicLibTestCase): 'baremetal._openstack._tcp.local.' ) - def test_with_custom_port_and_path(self, mock_zc): + def test_with_custom_port_and_path(self, mock_zc, mock_route): mock_zc.return_value.get_service_info.return_value = mock.Mock( address=socket.inet_aton('192.168.1.1'), port=8080, - properties={b'path': b'/baremetal'} + properties={b'path': b'/baremetal'}, + **{'parsed_addresses.return_value': ['192.168.1.1']} ) endp, params = mdns.get_endpoint('baremetal') @@ -179,11 +227,12 @@ class GetEndpointTestCase(base.IronicLibTestCase): 'baremetal._openstack._tcp.local.' ) - def test_with_custom_port_path_and_protocol(self, mock_zc): + def test_with_custom_port_path_and_protocol(self, mock_zc, mock_route): mock_zc.return_value.get_service_info.return_value = mock.Mock( address=socket.inet_aton('192.168.1.1'), port=8080, - properties={b'path': b'/baremetal', b'protocol': b'http'} + properties={b'path': b'/baremetal', b'protocol': b'http'}, + **{'parsed_addresses.return_value': ['192.168.1.1']} ) endp, params = mdns.get_endpoint('baremetal') @@ -194,11 +243,12 @@ class GetEndpointTestCase(base.IronicLibTestCase): 'baremetal._openstack._tcp.local.' ) - def test_with_params(self, mock_zc): + def test_with_params(self, mock_zc, mock_route): mock_zc.return_value.get_service_info.return_value = mock.Mock( address=socket.inet_aton('192.168.1.1'), port=80, - properties={b'ipa_debug': True} + properties={b'ipa_debug': True}, + **{'parsed_addresses.return_value': ['192.168.1.1']} ) endp, params = mdns.get_endpoint('baremetal') @@ -209,11 +259,12 @@ class GetEndpointTestCase(base.IronicLibTestCase): 'baremetal._openstack._tcp.local.' ) - def test_binary_data(self, mock_zc): + def test_binary_data(self, mock_zc, mock_route): mock_zc.return_value.get_service_info.return_value = mock.Mock( address=socket.inet_aton('192.168.1.1'), port=80, - properties={b'ipa_debug': True, b'binary': b'\xe2\x28\xa1'} + properties={b'ipa_debug': True, b'binary': b'\xe2\x28\xa1'}, + **{'parsed_addresses.return_value': ['192.168.1.1']} ) endp, params = mdns.get_endpoint('baremetal') @@ -225,11 +276,12 @@ class GetEndpointTestCase(base.IronicLibTestCase): 'baremetal._openstack._tcp.local.' ) - def test_invalid_key(self, mock_zc): + def test_invalid_key(self, mock_zc, mock_route): mock_zc.return_value.get_service_info.return_value = mock.Mock( address=socket.inet_aton('192.168.1.1'), port=80, - properties={b'ipa_debug': True, b'\xc3\x28': b'value'} + properties={b'ipa_debug': True, b'\xc3\x28': b'value'}, + **{'parsed_addresses.return_value': ['192.168.1.1']} ) self.assertRaisesRegex(exception.ServiceLookupFailure, @@ -240,12 +292,13 @@ class GetEndpointTestCase(base.IronicLibTestCase): 'baremetal._openstack._tcp.local.' ) - def test_with_server(self, mock_zc): + def test_with_server(self, mock_zc, mock_route): mock_zc.return_value.get_service_info.return_value = mock.Mock( address=socket.inet_aton('192.168.1.1'), port=443, server='openstack.example.com.', - properties={} + properties={}, + **{'parsed_addresses.return_value': ['192.168.1.1']} ) endp, params = mdns.get_endpoint('baremetal') @@ -257,7 +310,7 @@ class GetEndpointTestCase(base.IronicLibTestCase): ) @mock.patch('time.sleep', autospec=True) - def test_not_found(self, mock_sleep, mock_zc): + def test_not_found(self, mock_sleep, mock_zc, mock_route): mock_zc.return_value.get_service_info.return_value = None self.assertRaisesRegex(exception.ServiceLookupFailure, diff --git a/ironic_lib/tests/test_utils.py b/ironic_lib/tests/test_utils.py index e8d49b4b..fae34836 100644 --- a/ironic_lib/tests/test_utils.py +++ b/ironic_lib/tests/test_utils.py @@ -605,3 +605,40 @@ class WaitForDisk(base.IronicLibTestCase): check_exit_code=[0, 1]) self.assertEqual(2, mock_exc.call_count) mock_exc.assert_has_calls([fuser_call, fuser_call]) + + +@mock.patch.object(utils, 'execute', autospec=True) +class GetRouteSourceTestCase(base.IronicLibTestCase): + + def test_get_route_source_ipv4(self, mock_execute): + mock_execute.return_value = ('XXX src 1.2.3.4 XXX\n cache', None) + + source = utils.get_route_source('XXX') + self.assertEqual('1.2.3.4', source) + + def test_get_route_source_ipv6(self, mock_execute): + mock_execute.return_value = ('XXX src 1:2::3:4 metric XXX\n cache', + None) + + source = utils.get_route_source('XXX') + self.assertEqual('1:2::3:4', source) + + def test_get_route_source_ipv6_linklocal(self, mock_execute): + mock_execute.return_value = ( + 'XXX src fe80::1234:1234:1234:1234 metric XXX\n cache', None) + + source = utils.get_route_source('XXX') + self.assertIsNone(source) + + def test_get_route_source_ipv6_linklocal_allowed(self, mock_execute): + mock_execute.return_value = ( + 'XXX src fe80::1234:1234:1234:1234 metric XXX\n cache', None) + + source = utils.get_route_source('XXX', ignore_link_local=False) + self.assertEqual('fe80::1234:1234:1234:1234', source) + + def test_get_route_source_indexerror(self, mock_execute): + mock_execute.return_value = ('XXX src \n cache', None) + + source = utils.get_route_source('XXX') + self.assertIsNone(source) diff --git a/ironic_lib/utils.py b/ironic_lib/utils.py index 023b680d..d6ff8e21 100644 --- a/ironic_lib/utils.py +++ b/ironic_lib/utils.py @@ -20,6 +20,7 @@ import copy import errno +import ipaddress import logging import os import re @@ -503,3 +504,25 @@ def wait_for_disk_to_become_available(device): 'locks for device %(device)s. Timed out waiting for ' 'completion.') % {'device': device, 'fuser_err': stderr[0]}) + + +def get_route_source(dest, ignore_link_local=True): + """Get the IP address to send packages to destination.""" + try: + out, _err = 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: + source = out.strip().split('\n')[0].split('src')[1].split()[0] + if (ipaddress.ip_address(six.u(source)).is_link_local + and ignore_link_local): + LOG.debug('Ignoring link-local source to %(dest)s: %(rec)s', + {'dest': dest, 'rec': out}) + return + return source + except (IndexError, ValueError): + LOG.debug('No route to host %(dest)s, route record: %(rec)s', + {'dest': dest, 'rec': out}) diff --git a/lower-constraints.txt b/lower-constraints.txt index 9c6f026c..c67f1377 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -73,4 +73,4 @@ traceback2==1.4.0 unittest2==1.1.0 WebOb==1.7.1 wrapt==1.7.0 -zeroconf==0.19.1 +zeroconf==0.24.0 diff --git a/requirements.txt b/requirements.txt index e1711431..313d2e29 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,4 +12,4 @@ oslo.utils>=3.33.0 # Apache-2.0 requests>=2.14.2 # Apache-2.0 six>=1.10.0 # MIT oslo.log>=3.36.0 # Apache-2.0 -zeroconf>=0.19.1;python_version>='3.0' # LGPL +zeroconf>=0.24.0;python_version>='3.0' # LGPL