From 612e39087aee3b1242765e7c4f463f54a6ebd723 Mon Sep 17 00:00:00 2001 From: James Falcon Date: Fri, 17 Sep 2021 13:04:07 -0500 Subject: Add connectivity_url to Oracle's EphemeralDHCPv4 (#988) Add connectivity_url to Oracle's EphemeralDHCPv4 On bionic, when trying to bring up the EphemeralDHCPv4, it's possible that we already have a route defined, which will result in an error when trying to add the DHCP route. Use the connectivity_url to check if we can reach the metadata service, and if so, skip the EphemeralDHCPv4. The has_url_connectivity function has also been modified to take a dict of kwargs to send to readurl. LP: #1939603 --- cloudinit/net/__init__.py | 37 +++++++++++++++++++++++++--------- cloudinit/net/dhcp.py | 20 +++++++++++------- cloudinit/net/tests/test_dhcp.py | 8 ++++++-- cloudinit/net/tests/test_init.py | 20 +++++++++++------- cloudinit/sources/DataSourceOracle.py | 13 ++++++++---- cloudinit/sources/helpers/vultr.py | 2 +- cloudinit/sources/tests/test_oracle.py | 10 ++++++++- 7 files changed, 78 insertions(+), 32 deletions(-) (limited to 'cloudinit') diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index 017c50c5..7558745f 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -11,6 +11,7 @@ import ipaddress import logging import os import re +from typing import Any, Dict from cloudinit import subp from cloudinit import util @@ -971,18 +972,33 @@ def get_ib_hwaddrs_by_interface(): return ret -def has_url_connectivity(url): - """Return true when the instance has access to the provided URL +def has_url_connectivity(url_data: Dict[str, Any]) -> bool: + """Return true when the instance has access to the provided URL. Logs a warning if url is not the expected format. + + url_data is a dictionary of kwargs to send to readurl. E.g.: + + has_url_connectivity({ + "url": "http://example.invalid", + "headers": {"some": "header"}, + "timeout": 10 + }) """ + if 'url' not in url_data: + LOG.warning( + "Ignoring connectivity check. No 'url' to check in %s", url_data) + return False + url = url_data['url'] if not any([url.startswith('http://'), url.startswith('https://')]): LOG.warning( "Ignoring connectivity check. Expected URL beginning with http*://" " received '%s'", url) return False + if 'timeout' not in url_data: + url_data['timeout'] = 5 try: - readurl(url, timeout=5) + readurl(**url_data) except UrlError: return False return True @@ -1025,14 +1041,15 @@ class EphemeralIPv4Network(object): No operations are performed if the provided interface already has the specified configuration. - This can be verified with the connectivity_url. + This can be verified with the connectivity_url_data. If unconnected, bring up the interface with valid ip, prefix and broadcast. If router is provided setup a default route for that interface. Upon context exit, clean up the interface leaving no configuration behind. """ def __init__(self, interface, ip, prefix_or_mask, broadcast, router=None, - connectivity_url=None, static_routes=None): + connectivity_url_data: Dict[str, Any] = None, + static_routes=None): """Setup context manager and validate call signature. @param interface: Name of the network interface to bring up. @@ -1041,7 +1058,7 @@ class EphemeralIPv4Network(object): prefix. @param broadcast: Broadcast address for the IPv4 network. @param router: Optionally the default gateway IP. - @param connectivity_url: Optionally, a URL to verify if a usable + @param connectivity_url_data: Optionally, a URL to verify if a usable connection already exists. @param static_routes: Optionally a list of static routes from DHCP """ @@ -1056,7 +1073,7 @@ class EphemeralIPv4Network(object): 'Cannot setup network: {0}'.format(e) ) from e - self.connectivity_url = connectivity_url + self.connectivity_url_data = connectivity_url_data self.interface = interface self.ip = ip self.broadcast = broadcast @@ -1066,11 +1083,11 @@ class EphemeralIPv4Network(object): def __enter__(self): """Perform ephemeral network setup if interface is not connected.""" - if self.connectivity_url: - if has_url_connectivity(self.connectivity_url): + if self.connectivity_url_data: + if has_url_connectivity(self.connectivity_url_data): LOG.debug( 'Skip ephemeral network setup, instance has connectivity' - ' to %s', self.connectivity_url) + ' to %s', self.connectivity_url_data['url']) return self._bringup_device() diff --git a/cloudinit/net/dhcp.py b/cloudinit/net/dhcp.py index 9b94c9a0..3f4b0418 100644 --- a/cloudinit/net/dhcp.py +++ b/cloudinit/net/dhcp.py @@ -4,6 +4,7 @@ # # This file is part of cloud-init. See LICENSE file for license information. +from typing import Dict, Any import configobj import logging import os @@ -38,21 +39,26 @@ class NoDHCPLeaseError(Exception): class EphemeralDHCPv4(object): - def __init__(self, iface=None, connectivity_url=None, dhcp_log_func=None): + def __init__( + self, + iface=None, + connectivity_url_data: Dict[str, Any] = None, + dhcp_log_func=None + ): self.iface = iface self._ephipv4 = None self.lease = None self.dhcp_log_func = dhcp_log_func - self.connectivity_url = connectivity_url + self.connectivity_url_data = connectivity_url_data def __enter__(self): """Setup sandboxed dhcp context, unless connectivity_url can already be reached.""" - if self.connectivity_url: - if has_url_connectivity(self.connectivity_url): + if self.connectivity_url_data: + if has_url_connectivity(self.connectivity_url_data): LOG.debug( 'Skip ephemeral DHCP setup, instance has connectivity' - ' to %s', self.connectivity_url) + ' to %s', self.connectivity_url_data) return return self.obtain_lease() @@ -104,8 +110,8 @@ class EphemeralDHCPv4(object): if kwargs['static_routes']: kwargs['static_routes'] = ( parse_static_routes(kwargs['static_routes'])) - if self.connectivity_url: - kwargs['connectivity_url'] = self.connectivity_url + if self.connectivity_url_data: + kwargs['connectivity_url_data'] = self.connectivity_url_data ephipv4 = EphemeralIPv4Network(**kwargs) ephipv4.__enter__() self._ephipv4 = ephipv4 diff --git a/cloudinit/net/tests/test_dhcp.py b/cloudinit/net/tests/test_dhcp.py index 5ae048e2..28b4ecf7 100644 --- a/cloudinit/net/tests/test_dhcp.py +++ b/cloudinit/net/tests/test_dhcp.py @@ -617,7 +617,9 @@ class TestEphemeralDhcpNoNetworkSetup(HttprettyTestCase): url = 'http://example.org/index.html' httpretty.register_uri(httpretty.GET, url) - with net.dhcp.EphemeralDHCPv4(connectivity_url=url) as lease: + with net.dhcp.EphemeralDHCPv4( + connectivity_url_data={'url': url}, + ) as lease: self.assertIsNone(lease) # Ensure that no teardown happens: m_dhcp.assert_not_called() @@ -635,7 +637,9 @@ class TestEphemeralDhcpNoNetworkSetup(HttprettyTestCase): m_subp.return_value = ('', '') httpretty.register_uri(httpretty.GET, url, body={}, status=404) - with net.dhcp.EphemeralDHCPv4(connectivity_url=url) as lease: + with net.dhcp.EphemeralDHCPv4( + connectivity_url_data={'url': url}, + ) as lease: self.assertEqual(fake_lease, lease) # Ensure that dhcp discovery occurs m_dhcp.called_once_with() diff --git a/cloudinit/net/tests/test_init.py b/cloudinit/net/tests/test_init.py index ad9c90ff..f9102f7b 100644 --- a/cloudinit/net/tests/test_init.py +++ b/cloudinit/net/tests/test_init.py @@ -622,11 +622,14 @@ class TestEphemeralIPV4Network(CiTestCase): params = { 'interface': 'eth0', 'ip': '192.168.2.2', 'prefix_or_mask': '255.255.255.0', 'broadcast': '192.168.2.255', - 'connectivity_url': 'http://example.org/index.html'} + 'connectivity_url_data': {'url': 'http://example.org/index.html'} + } with net.EphemeralIPv4Network(**params): - self.assertEqual([mock.call('http://example.org/index.html', - timeout=5)], m_readurl.call_args_list) + self.assertEqual( + [mock.call(url='http://example.org/index.html', timeout=5)], + m_readurl.call_args_list + ) # Ensure that no teardown happens: m_subp.assert_has_calls([]) @@ -850,25 +853,28 @@ class TestHasURLConnectivity(HttprettyTestCase): def test_url_timeout_on_connectivity_check(self, m_readurl): """A timeout of 5 seconds is provided when reading a url.""" self.assertTrue( - net.has_url_connectivity(self.url), 'Expected True on url connect') + net.has_url_connectivity({'url': self.url}), + 'Expected True on url connect') def test_true_on_url_connectivity_success(self): httpretty.register_uri(httpretty.GET, self.url) self.assertTrue( - net.has_url_connectivity(self.url), 'Expected True on url connect') + net.has_url_connectivity({'url': self.url}), + 'Expected True on url connect') @mock.patch('requests.Session.request') def test_true_on_url_connectivity_timeout(self, m_request): """A timeout raised accessing the url will return False.""" m_request.side_effect = requests.Timeout('Fake Connection Timeout') self.assertFalse( - net.has_url_connectivity(self.url), + net.has_url_connectivity({'url': self.url}), 'Expected False on url timeout') def test_true_on_url_connectivity_failure(self): httpretty.register_uri(httpretty.GET, self.url, body={}, status=404) self.assertFalse( - net.has_url_connectivity(self.url), 'Expected False on url fail') + net.has_url_connectivity({'url': self.url}), + 'Expected False on url fail') def _mk_v1_phys(mac, name, driver, device_id): diff --git a/cloudinit/sources/DataSourceOracle.py b/cloudinit/sources/DataSourceOracle.py index bf81b10b..fbb5312a 100644 --- a/cloudinit/sources/DataSourceOracle.py +++ b/cloudinit/sources/DataSourceOracle.py @@ -40,6 +40,7 @@ METADATA_PATTERN = METADATA_ROOT + "{path}/" # https://docs.cloud.oracle.com/iaas/Content/Network/Troubleshoot/connectionhang.htm#Overview, # indicates that an MTU of 9000 is used within OCI MTU = 9000 +V2_HEADERS = {"Authorization": "Bearer Oracle"} OpcMetadata = namedtuple("OpcMetadata", "version instance_data vnics_data") @@ -134,7 +135,13 @@ class DataSourceOracle(sources.DataSource): ) network_context = noop() if not _is_iscsi_root(): - network_context = dhcp.EphemeralDHCPv4(net.find_fallback_nic()) + network_context = dhcp.EphemeralDHCPv4( + iface=net.find_fallback_nic(), + connectivity_url_data={ + "url": METADATA_PATTERN.format(version=2, path="instance"), + "headers": V2_HEADERS, + } + ) with network_context: fetched_metadata = read_opc_metadata( fetch_vnics_data=fetch_vnics_data @@ -304,11 +311,9 @@ def read_opc_metadata(*, fetch_vnics_data: bool = False): retries = 2 def _fetch(metadata_version: int, path: str) -> dict: - headers = { - "Authorization": "Bearer Oracle"} if metadata_version > 1 else None return readurl( url=METADATA_PATTERN.format(version=metadata_version, path=path), - headers=headers, + headers=V2_HEADERS if metadata_version > 1 else None, retries=retries, )._response.json() diff --git a/cloudinit/sources/helpers/vultr.py b/cloudinit/sources/helpers/vultr.py index c22cd0b1..2521ec2f 100644 --- a/cloudinit/sources/helpers/vultr.py +++ b/cloudinit/sources/helpers/vultr.py @@ -20,7 +20,7 @@ LOG = log.getLogger(__name__) def get_metadata(url, timeout, retries, sec_between): # Bring up interface try: - with EphemeralDHCPv4(connectivity_url=url): + with EphemeralDHCPv4(connectivity_url_data={"url": url}): # Fetch the metadata v1 = read_metadata(url, timeout, retries, sec_between) except (NoDHCPLeaseError) as exc: diff --git a/cloudinit/sources/tests/test_oracle.py b/cloudinit/sources/tests/test_oracle.py index dcf33b9b..5f608cbb 100644 --- a/cloudinit/sources/tests/test_oracle.py +++ b/cloudinit/sources/tests/test_oracle.py @@ -694,7 +694,15 @@ class TestNonIscsiRoot_GetDataBehaviour: assert oracle_ds._get_data() assert [ - mock.call(m_find_fallback_nic.return_value) + mock.call( + iface=m_find_fallback_nic.return_value, + connectivity_url_data={ + 'headers': { + 'Authorization': 'Bearer Oracle' + }, + 'url': 'http://169.254.169.254/opc/v2/instance/' + } + ) ] == m_EphemeralDHCPv4.call_args_list -- cgit v1.2.3