From fdadcb5fae51f4e6799314ab98e3aec56c79b17c Mon Sep 17 00:00:00 2001 From: Jason Zions Date: Tue, 15 Jan 2019 21:37:17 +0000 Subject: net: Wait for dhclient to daemonize before reading lease file cloud-init uses dhclient to fetch the DHCP lease so it can extract DHCP options. dhclient creates the leasefile, then writes to it; simply waiting for the leasefile to appear creates a race between dhclient and cloud-init. Instead, wait for dhclient to be parented by init. At that point, we know it has written to the leasefile, so it's safe to copy the file and kill the process. cloud-init creates a temporary directory in which to execute dhclient, and deletes that directory after it has killed the process. If cloud-init abandons waiting for dhclient to daemonize, it will still attempt to delete the temporary directory, but will not report an exception should that attempt fail. LP: #1794399 --- cloudinit/net/tests/test_dhcp.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) (limited to 'cloudinit/net/tests/test_dhcp.py') diff --git a/cloudinit/net/tests/test_dhcp.py b/cloudinit/net/tests/test_dhcp.py index cd3e7328..79e8842f 100644 --- a/cloudinit/net/tests/test_dhcp.py +++ b/cloudinit/net/tests/test_dhcp.py @@ -145,16 +145,20 @@ class TestDHCPDiscoveryClean(CiTestCase): 'subnet-mask': '255.255.255.0', 'routers': '192.168.2.1'}], dhcp_discovery(dhclient_script, 'eth9', tmpdir)) self.assertIn( - "pid file contains non-integer content ''", self.logs.getvalue()) + "dhclient(pid=, parentpid=unknown) failed " + "to daemonize after 10.0 seconds", + self.logs.getvalue()) m_kill.assert_not_called() + @mock.patch('cloudinit.net.dhcp.util.get_proc_ppid') @mock.patch('cloudinit.net.dhcp.os.kill') @mock.patch('cloudinit.net.dhcp.util.wait_for_files') @mock.patch('cloudinit.net.dhcp.util.subp') def test_dhcp_discovery_run_in_sandbox_waits_on_lease_and_pid(self, m_subp, m_wait, - m_kill): + m_kill, + m_getppid): """dhcp_discovery waits for the presence of pidfile and dhcp.leases.""" tmpdir = self.tmp_dir() dhclient_script = os.path.join(tmpdir, 'dhclient.orig') @@ -164,6 +168,7 @@ class TestDHCPDiscoveryClean(CiTestCase): pidfile = self.tmp_path('dhclient.pid', tmpdir) leasefile = self.tmp_path('dhcp.leases', tmpdir) m_wait.return_value = [pidfile] # Return the missing pidfile wait for + m_getppid.return_value = 1 # Indicate that dhclient has daemonized self.assertEqual([], dhcp_discovery(dhclient_script, 'eth9', tmpdir)) self.assertEqual( mock.call([pidfile, leasefile], maxwait=5, naplen=0.01), @@ -173,9 +178,10 @@ class TestDHCPDiscoveryClean(CiTestCase): self.logs.getvalue()) m_kill.assert_not_called() + @mock.patch('cloudinit.net.dhcp.util.get_proc_ppid') @mock.patch('cloudinit.net.dhcp.os.kill') @mock.patch('cloudinit.net.dhcp.util.subp') - def test_dhcp_discovery_run_in_sandbox(self, m_subp, m_kill): + def test_dhcp_discovery_run_in_sandbox(self, m_subp, m_kill, m_getppid): """dhcp_discovery brings up the interface and runs dhclient. It also returns the parsed dhcp.leases file generated in the sandbox. @@ -197,6 +203,7 @@ class TestDHCPDiscoveryClean(CiTestCase): pid_file = os.path.join(tmpdir, 'dhclient.pid') my_pid = 1 write_file(pid_file, "%d\n" % my_pid) + m_getppid.return_value = 1 # Indicate that dhclient has daemonized self.assertItemsEqual( [{'interface': 'eth9', 'fixed-address': '192.168.2.74', @@ -355,3 +362,5 @@ class TestEphemeralDhcpNoNetworkSetup(HttprettyTestCase): self.assertEqual(fake_lease, lease) # Ensure that dhcp discovery occurs m_dhcp.called_once_with() + +# vi: ts=4 expandtab -- cgit v1.2.3 From 1182ad5f9362e1570c622345a3ac996c07eb2eeb Mon Sep 17 00:00:00 2001 From: Daniel Watkins Date: Tue, 26 Feb 2019 15:37:36 +0000 Subject: tests: fix some slow tests and some leaking state In test_ds_identify, don't mutate otherwise-static test data. When running tests in a random order, this was causing failures due to breaking preconditions for other tests. In tests/helpers, reset logging level in tearDown. Some of the CLI tests set the level of the root logger in a way that isn't correctly reset. For test_poll_imds_re_dhcp_on_timeout and test_dhcp_discovery_run_in_sandbox_warns_invalid_pid, mock out time.sleep; this saves ~11 seconds (or ~40% of previous test time!). --- cloudinit/net/tests/test_dhcp.py | 1 + cloudinit/tests/helpers.py | 1 + tests/unittests/test_datasource/test_azure.py | 1 + tests/unittests/test_ds_identify.py | 2 +- 4 files changed, 4 insertions(+), 1 deletion(-) (limited to 'cloudinit/net/tests/test_dhcp.py') diff --git a/cloudinit/net/tests/test_dhcp.py b/cloudinit/net/tests/test_dhcp.py index 79e8842f..51390249 100644 --- a/cloudinit/net/tests/test_dhcp.py +++ b/cloudinit/net/tests/test_dhcp.py @@ -117,6 +117,7 @@ class TestDHCPDiscoveryClean(CiTestCase): self.assertEqual('eth9', call[0][1]) self.assertIn('/var/tmp/cloud-init/cloud-init-dhcp-', call[0][2]) + @mock.patch('time.sleep', mock.MagicMock()) @mock.patch('cloudinit.net.dhcp.os.kill') @mock.patch('cloudinit.net.dhcp.util.subp') def test_dhcp_discovery_run_in_sandbox_warns_invalid_pid(self, m_subp, diff --git a/cloudinit/tests/helpers.py b/cloudinit/tests/helpers.py index 2eb7b0cd..46a49416 100644 --- a/cloudinit/tests/helpers.py +++ b/cloudinit/tests/helpers.py @@ -207,6 +207,7 @@ class CiTestCase(TestCase): if self.with_logs: # Remove the handler we setup logging.getLogger().handlers = self.old_handlers + logging.getLogger().level = None util.subp = _real_subp super(CiTestCase, self).tearDown() diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index 417d86a9..5edf36e8 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -1692,6 +1692,7 @@ class TestPreprovisioningPollIMDS(CiTestCase): self.paths = helpers.Paths({'cloud_dir': self.tmp}) dsaz.BUILTIN_DS_CONFIG['data_dir'] = self.waagent_d + @mock.patch('time.sleep', mock.MagicMock()) @mock.patch(MOCKPATH + 'EphemeralDHCPv4') def test_poll_imds_re_dhcp_on_timeout(self, m_dhcpv4, report_ready_func, fake_resp, m_media_switch, m_dhcp, diff --git a/tests/unittests/test_ds_identify.py b/tests/unittests/test_ds_identify.py index 756b4fb4..d00c1b4b 100644 --- a/tests/unittests/test_ds_identify.py +++ b/tests/unittests/test_ds_identify.py @@ -441,7 +441,7 @@ class TestDsIdentify(DsIdentifyBase): nova does not identify itself on platforms other than intel. https://bugs.launchpad.net/cloud-init/+bugs?field.tag=dsid-nova""" - data = VALID_CFG['OpenStack'].copy() + data = copy.deepcopy(VALID_CFG['OpenStack']) del data['files'][P_PRODUCT_NAME] data.update({'policy_dmi': POLICY_FOUND_OR_MAYBE, 'policy_no_dmi': POLICY_FOUND_OR_MAYBE}) -- cgit v1.2.3 From 07b17236be5665bb552c7460102bcd07bf8f2be8 Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Tue, 16 Jul 2019 22:40:15 +0000 Subject: net: add rfc3442 (classless static routes) to EphemeralDHCP The EphemeralDHCP context manager did not parse or handle rfc3442 classless static routes which prevented reading datasource metadata in some clouds. This branch adds support for extracting the field from the leases output, parsing the format and then adding the required iproute2 ip commands to apply (and teardown) the static routes. LP: #1821102 --- cloudinit/net/__init__.py | 34 +++++++- cloudinit/net/dhcp.py | 90 +++++++++++++++++++ cloudinit/net/tests/test_dhcp.py | 120 +++++++++++++++++++++++++- cloudinit/net/tests/test_init.py | 39 +++++++++ tests/unittests/test_datasource/test_azure.py | 6 +- tests/unittests/test_datasource/test_ec2.py | 3 +- 6 files changed, 286 insertions(+), 6 deletions(-) (limited to 'cloudinit/net/tests/test_dhcp.py') diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index e758006f..624c9b42 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -679,7 +679,7 @@ class EphemeralIPv4Network(object): """ def __init__(self, interface, ip, prefix_or_mask, broadcast, router=None, - connectivity_url=None): + connectivity_url=None, static_routes=None): """Setup context manager and validate call signature. @param interface: Name of the network interface to bring up. @@ -690,6 +690,7 @@ class EphemeralIPv4Network(object): @param router: Optionally the default gateway IP. @param connectivity_url: Optionally, a URL to verify if a usable connection already exists. + @param static_routes: Optionally a list of static routes from DHCP """ if not all([interface, ip, prefix_or_mask, broadcast]): raise ValueError( @@ -706,6 +707,7 @@ class EphemeralIPv4Network(object): self.ip = ip self.broadcast = broadcast self.router = router + self.static_routes = static_routes self.cleanup_cmds = [] # List of commands to run to cleanup state. def __enter__(self): @@ -718,7 +720,21 @@ class EphemeralIPv4Network(object): return self._bringup_device() - if self.router: + + # rfc3442 requires us to ignore the router config *if* classless static + # routes are provided. + # + # https://tools.ietf.org/html/rfc3442 + # + # If the DHCP server returns both a Classless Static Routes option and + # a Router option, the DHCP client MUST ignore the Router option. + # + # Similarly, if the DHCP server returns both a Classless Static Routes + # option and a Static Routes option, the DHCP client MUST ignore the + # Static Routes option. + if self.static_routes: + self._bringup_static_routes() + elif self.router: self._bringup_router() def __exit__(self, excp_type, excp_value, excp_traceback): @@ -762,6 +778,20 @@ class EphemeralIPv4Network(object): ['ip', '-family', 'inet', 'addr', 'del', cidr, 'dev', self.interface]) + def _bringup_static_routes(self): + # static_routes = [("169.254.169.254/32", "130.56.248.255"), + # ("0.0.0.0/0", "130.56.240.1")] + for net_address, gateway in self.static_routes: + via_arg = [] + if gateway != "0.0.0.0/0": + via_arg = ['via', gateway] + util.subp( + ['ip', '-4', 'route', 'add', net_address] + via_arg + + ['dev', self.interface], capture=True) + self.cleanup_cmds.insert( + 0, ['ip', '-4', 'route', 'del', net_address] + via_arg + + ['dev', self.interface]) + def _bringup_router(self): """Perform the ip commands to fully setup the router if needed.""" # Check if a default route exists and exit if it does diff --git a/cloudinit/net/dhcp.py b/cloudinit/net/dhcp.py index c98a97cd..17379918 100644 --- a/cloudinit/net/dhcp.py +++ b/cloudinit/net/dhcp.py @@ -92,10 +92,14 @@ class EphemeralDHCPv4(object): nmap = {'interface': 'interface', 'ip': 'fixed-address', 'prefix_or_mask': 'subnet-mask', 'broadcast': 'broadcast-address', + 'static_routes': 'rfc3442-classless-static-routes', 'router': 'routers'} kwargs = dict([(k, self.lease.get(v)) for k, v in nmap.items()]) if not kwargs['broadcast']: kwargs['broadcast'] = bcip(kwargs['prefix_or_mask'], kwargs['ip']) + if kwargs['static_routes']: + kwargs['static_routes'] = ( + parse_static_routes(kwargs['static_routes'])) if self.connectivity_url: kwargs['connectivity_url'] = self.connectivity_url ephipv4 = EphemeralIPv4Network(**kwargs) @@ -272,4 +276,90 @@ def networkd_get_option_from_leases(keyname, leases_d=None): return data[keyname] return None + +def parse_static_routes(rfc3442): + """ parse rfc3442 format and return a list containing tuple of strings. + + The tuple is composed of the network_address (including net length) and + gateway for a parsed static route. + + @param rfc3442: string in rfc3442 format + @returns: list of tuple(str, str) for all valid parsed routes until the + first parsing error. + + E.g. + sr = parse_state_routes("32,169,254,169,254,130,56,248,255,0,130,56,240,1") + sr = [ + ("169.254.169.254/32", "130.56.248.255"), ("0.0.0.0/0", "130.56.240.1") + ] + + Python version of isc-dhclient's hooks: + /etc/dhcp/dhclient-exit-hooks.d/rfc3442-classless-routes + """ + # raw strings from dhcp lease may end in semi-colon + rfc3442 = rfc3442.rstrip(";") + tokens = rfc3442.split(',') + static_routes = [] + + def _trunc_error(cidr, required, remain): + msg = ("RFC3442 string malformed. Current route has CIDR of %s " + "and requires %s significant octets, but only %s remain. " + "Verify DHCP rfc3442-classless-static-routes value: %s" + % (cidr, required, remain, rfc3442)) + LOG.error(msg) + + current_idx = 0 + for idx, tok in enumerate(tokens): + if idx < current_idx: + continue + net_length = int(tok) + if net_length in range(25, 33): + req_toks = 9 + if len(tokens[idx:]) < req_toks: + _trunc_error(net_length, req_toks, len(tokens[idx:])) + return static_routes + net_address = ".".join(tokens[idx+1:idx+5]) + gateway = ".".join(tokens[idx+5:idx+req_toks]) + current_idx = idx + req_toks + elif net_length in range(17, 25): + req_toks = 8 + if len(tokens[idx:]) < req_toks: + _trunc_error(net_length, req_toks, len(tokens[idx:])) + return static_routes + net_address = ".".join(tokens[idx+1:idx+4] + ["0"]) + gateway = ".".join(tokens[idx+4:idx+req_toks]) + current_idx = idx + req_toks + elif net_length in range(9, 17): + req_toks = 7 + if len(tokens[idx:]) < req_toks: + _trunc_error(net_length, req_toks, len(tokens[idx:])) + return static_routes + net_address = ".".join(tokens[idx+1:idx+3] + ["0", "0"]) + gateway = ".".join(tokens[idx+3:idx+req_toks]) + current_idx = idx + req_toks + elif net_length in range(1, 9): + req_toks = 6 + if len(tokens[idx:]) < req_toks: + _trunc_error(net_length, req_toks, len(tokens[idx:])) + return static_routes + net_address = ".".join(tokens[idx+1:idx+2] + ["0", "0", "0"]) + gateway = ".".join(tokens[idx+2:idx+req_toks]) + current_idx = idx + req_toks + elif net_length == 0: + req_toks = 5 + if len(tokens[idx:]) < req_toks: + _trunc_error(net_length, req_toks, len(tokens[idx:])) + return static_routes + net_address = "0.0.0.0" + gateway = ".".join(tokens[idx+1:idx+req_toks]) + current_idx = idx + req_toks + else: + LOG.error('Parsed invalid net length "%s". Verify DHCP ' + 'rfc3442-classless-static-routes value.', net_length) + return static_routes + + static_routes.append(("%s/%s" % (net_address, net_length), gateway)) + + return static_routes + # vi: ts=4 expandtab diff --git a/cloudinit/net/tests/test_dhcp.py b/cloudinit/net/tests/test_dhcp.py index 51390249..91f503c9 100644 --- a/cloudinit/net/tests/test_dhcp.py +++ b/cloudinit/net/tests/test_dhcp.py @@ -8,7 +8,8 @@ from textwrap import dedent import cloudinit.net as net from cloudinit.net.dhcp import ( InvalidDHCPLeaseFileError, maybe_perform_dhcp_discovery, - parse_dhcp_lease_file, dhcp_discovery, networkd_load_leases) + parse_dhcp_lease_file, dhcp_discovery, networkd_load_leases, + parse_static_routes) from cloudinit.util import ensure_file, write_file from cloudinit.tests.helpers import ( CiTestCase, HttprettyTestCase, mock, populate_dir, wrap_and_call) @@ -64,6 +65,123 @@ class TestParseDHCPLeasesFile(CiTestCase): self.assertItemsEqual(expected, parse_dhcp_lease_file(lease_file)) +class TestDHCPRFC3442(CiTestCase): + + def test_parse_lease_finds_rfc3442_classless_static_routes(self): + """parse_dhcp_lease_file returns rfc3442-classless-static-routes.""" + lease_file = self.tmp_path('leases') + content = dedent(""" + lease { + interface "wlp3s0"; + fixed-address 192.168.2.74; + option subnet-mask 255.255.255.0; + option routers 192.168.2.1; + option rfc3442-classless-static-routes 0,130,56,240,1; + renew 4 2017/07/27 18:02:30; + expire 5 2017/07/28 07:08:15; + } + """) + expected = [ + {'interface': 'wlp3s0', 'fixed-address': '192.168.2.74', + 'subnet-mask': '255.255.255.0', 'routers': '192.168.2.1', + 'rfc3442-classless-static-routes': '0,130,56,240,1', + 'renew': '4 2017/07/27 18:02:30', + 'expire': '5 2017/07/28 07:08:15'}] + write_file(lease_file, content) + self.assertItemsEqual(expected, parse_dhcp_lease_file(lease_file)) + + @mock.patch('cloudinit.net.dhcp.EphemeralIPv4Network') + @mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery') + def test_obtain_lease_parses_static_routes(self, m_maybe, m_ipv4): + """EphemeralDHPCv4 parses rfc3442 routes for EphemeralIPv4Network""" + lease = [ + {'interface': 'wlp3s0', 'fixed-address': '192.168.2.74', + 'subnet-mask': '255.255.255.0', 'routers': '192.168.2.1', + 'rfc3442-classless-static-routes': '0,130,56,240,1', + 'renew': '4 2017/07/27 18:02:30', + 'expire': '5 2017/07/28 07:08:15'}] + m_maybe.return_value = lease + eph = net.dhcp.EphemeralDHCPv4() + eph.obtain_lease() + expected_kwargs = { + 'interface': 'wlp3s0', + 'ip': '192.168.2.74', + 'prefix_or_mask': '255.255.255.0', + 'broadcast': '192.168.2.255', + 'static_routes': [('0.0.0.0/0', '130.56.240.1')], + 'router': '192.168.2.1'} + m_ipv4.assert_called_with(**expected_kwargs) + + +class TestDHCPParseStaticRoutes(CiTestCase): + + with_logs = True + + def parse_static_routes_empty_string(self): + self.assertEqual([], parse_static_routes("")) + + def test_parse_static_routes_invalid_input_returns_empty_list(self): + rfc3442 = "32,169,254,169,254,130,56,248" + self.assertEqual([], parse_static_routes(rfc3442)) + + def test_parse_static_routes_bogus_width_returns_empty_list(self): + rfc3442 = "33,169,254,169,254,130,56,248" + self.assertEqual([], parse_static_routes(rfc3442)) + + def test_parse_static_routes_single_ip(self): + rfc3442 = "32,169,254,169,254,130,56,248,255" + self.assertEqual([('169.254.169.254/32', '130.56.248.255')], + parse_static_routes(rfc3442)) + + def test_parse_static_routes_single_ip_handles_trailing_semicolon(self): + rfc3442 = "32,169,254,169,254,130,56,248,255;" + self.assertEqual([('169.254.169.254/32', '130.56.248.255')], + parse_static_routes(rfc3442)) + + def test_parse_static_routes_default_route(self): + rfc3442 = "0,130,56,240,1" + self.assertEqual([('0.0.0.0/0', '130.56.240.1')], + parse_static_routes(rfc3442)) + + def test_parse_static_routes_class_c_b_a(self): + class_c = "24,192,168,74,192,168,0,4" + class_b = "16,172,16,172,16,0,4" + class_a = "8,10,10,0,0,4" + rfc3442 = ",".join([class_c, class_b, class_a]) + self.assertEqual(sorted([ + ("192.168.74.0/24", "192.168.0.4"), + ("172.16.0.0/16", "172.16.0.4"), + ("10.0.0.0/8", "10.0.0.4") + ]), sorted(parse_static_routes(rfc3442))) + + def test_parse_static_routes_logs_error_truncated(self): + bad_rfc3442 = { + "class_c": "24,169,254,169,10", + "class_b": "16,172,16,10", + "class_a": "8,10,10", + "gateway": "0,0", + "netlen": "33,0", + } + for rfc3442 in bad_rfc3442.values(): + self.assertEqual([], parse_static_routes(rfc3442)) + + logs = self.logs.getvalue() + self.assertEqual(len(bad_rfc3442.keys()), len(logs.splitlines())) + + def test_parse_static_routes_returns_valid_routes_until_parse_err(self): + class_c = "24,192,168,74,192,168,0,4" + class_b = "16,172,16,172,16,0,4" + class_a_error = "8,10,10,0,0" + rfc3442 = ",".join([class_c, class_b, class_a_error]) + self.assertEqual(sorted([ + ("192.168.74.0/24", "192.168.0.4"), + ("172.16.0.0/16", "172.16.0.4"), + ]), sorted(parse_static_routes(rfc3442))) + + logs = self.logs.getvalue() + self.assertIn(rfc3442, logs.splitlines()[0]) + + class TestDHCPDiscoveryClean(CiTestCase): with_logs = True diff --git a/cloudinit/net/tests/test_init.py b/cloudinit/net/tests/test_init.py index 6d2affe7..d393e6ad 100644 --- a/cloudinit/net/tests/test_init.py +++ b/cloudinit/net/tests/test_init.py @@ -549,6 +549,45 @@ class TestEphemeralIPV4Network(CiTestCase): self.assertEqual(expected_setup_calls, m_subp.call_args_list) m_subp.assert_has_calls(expected_teardown_calls) + def test_ephemeral_ipv4_network_with_rfc3442_static_routes(self, m_subp): + params = { + 'interface': 'eth0', 'ip': '192.168.2.2', + 'prefix_or_mask': '255.255.255.0', 'broadcast': '192.168.2.255', + 'static_routes': [('169.254.169.254/32', '192.168.2.1'), + ('0.0.0.0/0', '192.168.2.1')], + 'router': '192.168.2.1'} + expected_setup_calls = [ + mock.call( + ['ip', '-family', 'inet', 'addr', 'add', '192.168.2.2/24', + 'broadcast', '192.168.2.255', 'dev', 'eth0'], + capture=True, update_env={'LANG': 'C'}), + mock.call( + ['ip', '-family', 'inet', 'link', 'set', 'dev', 'eth0', 'up'], + capture=True), + mock.call( + ['ip', '-4', 'route', 'add', '169.254.169.254/32', + 'via', '192.168.2.1', 'dev', 'eth0'], capture=True), + mock.call( + ['ip', '-4', 'route', 'add', '0.0.0.0/0', + 'via', '192.168.2.1', 'dev', 'eth0'], capture=True)] + expected_teardown_calls = [ + mock.call( + ['ip', '-4', 'route', 'del', '0.0.0.0/0', + 'via', '192.168.2.1', 'dev', 'eth0'], capture=True), + mock.call( + ['ip', '-4', 'route', 'del', '169.254.169.254/32', + 'via', '192.168.2.1', 'dev', 'eth0'], capture=True), + mock.call( + ['ip', '-family', 'inet', 'link', 'set', 'dev', + 'eth0', 'down'], capture=True), + mock.call( + ['ip', '-family', 'inet', 'addr', 'del', + '192.168.2.2/24', 'dev', 'eth0'], capture=True) + ] + with net.EphemeralIPv4Network(**params): + self.assertEqual(expected_setup_calls, m_subp.call_args_list) + m_subp.assert_has_calls(expected_setup_calls + expected_teardown_calls) + class TestApplyNetworkCfgNames(CiTestCase): V1_CONFIG = textwrap.dedent("""\ diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index f27ef21b..2de2aea2 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -1807,7 +1807,8 @@ class TestAzureDataSourcePreprovisioning(CiTestCase): self.assertEqual(m_dhcp.call_count, 2) m_net.assert_any_call( broadcast='192.168.2.255', interface='eth9', ip='192.168.2.9', - prefix_or_mask='255.255.255.0', router='192.168.2.1') + prefix_or_mask='255.255.255.0', router='192.168.2.1', + static_routes=None) self.assertEqual(m_net.call_count, 2) def test__reprovision_calls__poll_imds(self, fake_resp, @@ -1845,7 +1846,8 @@ class TestAzureDataSourcePreprovisioning(CiTestCase): self.assertEqual(m_dhcp.call_count, 2) m_net.assert_any_call( broadcast='192.168.2.255', interface='eth9', ip='192.168.2.9', - prefix_or_mask='255.255.255.0', router='192.168.2.1') + prefix_or_mask='255.255.255.0', router='192.168.2.1', + static_routes=None) self.assertEqual(m_net.call_count, 2) diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py index 20d59bfd..1ec8e009 100644 --- a/tests/unittests/test_datasource/test_ec2.py +++ b/tests/unittests/test_datasource/test_ec2.py @@ -538,7 +538,8 @@ class TestEc2(test_helpers.HttprettyTestCase): m_dhcp.assert_called_once_with('eth9') m_net.assert_called_once_with( broadcast='192.168.2.255', interface='eth9', ip='192.168.2.9', - prefix_or_mask='255.255.255.0', router='192.168.2.1') + prefix_or_mask='255.255.255.0', router='192.168.2.1', + static_routes=None) self.assertIn('Crawl of metadata service took', self.logs.getvalue()) -- cgit v1.2.3 From a47405af306219d804ed262d6f7e2039a75883f2 Mon Sep 17 00:00:00 2001 From: Eric Lafontaine Date: Mon, 9 Dec 2019 19:48:37 -0500 Subject: dhcp: Support RedHat dhcp rfc3442 lease format for option 121 (#76) RedHat dhcp client writes out rfc3442 classless-static-routes in a different format[1] than what is found in isc-dhcp clients. This patch adds support for the RedHat format. 1. Background details on the format https://bugzilla.redhat.com/show_bug.cgi?id=516325 https://github.com/vaijab/fedora-dhcp/blob/e83fb19c51765442d77fa60596bfdb2b3b9fbe2e/dhcp-rfc3442-classless-static-routes.patch#L252 https://github.com/heftig/NetworkManager/blob/f56c82d86122fc45304fc829b5f1e4766ed51589/src/dhcp-manager/nm-dhcp-client.c#L978 LP: #1850642 --- cloudinit/net/dhcp.py | 42 +++++++++++++++++++++----- cloudinit/net/tests/test_dhcp.py | 65 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 7 deletions(-) (limited to 'cloudinit/net/tests/test_dhcp.py') diff --git a/cloudinit/net/dhcp.py b/cloudinit/net/dhcp.py index 17379918..c033cc8e 100644 --- a/cloudinit/net/dhcp.py +++ b/cloudinit/net/dhcp.py @@ -92,9 +92,12 @@ class EphemeralDHCPv4(object): nmap = {'interface': 'interface', 'ip': 'fixed-address', 'prefix_or_mask': 'subnet-mask', 'broadcast': 'broadcast-address', - 'static_routes': 'rfc3442-classless-static-routes', + 'static_routes': [ + 'rfc3442-classless-static-routes', + 'classless-static-routes' + ], 'router': 'routers'} - kwargs = dict([(k, self.lease.get(v)) for k, v in nmap.items()]) + kwargs = self.extract_dhcp_options_mapping(nmap) if not kwargs['broadcast']: kwargs['broadcast'] = bcip(kwargs['prefix_or_mask'], kwargs['ip']) if kwargs['static_routes']: @@ -107,6 +110,25 @@ class EphemeralDHCPv4(object): self._ephipv4 = ephipv4 return self.lease + def extract_dhcp_options_mapping(self, nmap): + result = {} + for internal_reference, lease_option_names in nmap.items(): + if isinstance(lease_option_names, list): + self.get_first_option_value( + internal_reference, + lease_option_names, + result + ) + else: + result[internal_reference] = self.lease.get(lease_option_names) + return result + + def get_first_option_value(self, internal_mapping, + lease_option_names, result): + for different_names in lease_option_names: + if not result.get(internal_mapping): + result[internal_mapping] = self.lease.get(different_names) + def maybe_perform_dhcp_discovery(nic=None): """Perform dhcp discovery if nic valid and dhclient command exists. @@ -281,24 +303,30 @@ def parse_static_routes(rfc3442): """ parse rfc3442 format and return a list containing tuple of strings. The tuple is composed of the network_address (including net length) and - gateway for a parsed static route. + gateway for a parsed static route. It can parse two formats of rfc3442, + one from dhcpcd and one from dhclient (isc). - @param rfc3442: string in rfc3442 format + @param rfc3442: string in rfc3442 format (isc or dhcpd) @returns: list of tuple(str, str) for all valid parsed routes until the first parsing error. E.g. - sr = parse_state_routes("32,169,254,169,254,130,56,248,255,0,130,56,240,1") - sr = [ + sr=parse_static_routes("32,169,254,169,254,130,56,248,255,0,130,56,240,1") + sr=[ ("169.254.169.254/32", "130.56.248.255"), ("0.0.0.0/0", "130.56.240.1") ] + sr2 = parse_static_routes("24.191.168.128 192.168.128.1,0 192.168.128.1") + sr2 = [ + ("191.168.128.0/24", "192.168.128.1"), ("0.0.0.0/0", "192.168.128.1") + ] + Python version of isc-dhclient's hooks: /etc/dhcp/dhclient-exit-hooks.d/rfc3442-classless-routes """ # raw strings from dhcp lease may end in semi-colon rfc3442 = rfc3442.rstrip(";") - tokens = rfc3442.split(',') + tokens = [tok for tok in re.split(r"[, .]", rfc3442) if tok] static_routes = [] def _trunc_error(cidr, required, remain): diff --git a/cloudinit/net/tests/test_dhcp.py b/cloudinit/net/tests/test_dhcp.py index 91f503c9..c3fa1e04 100644 --- a/cloudinit/net/tests/test_dhcp.py +++ b/cloudinit/net/tests/test_dhcp.py @@ -90,6 +90,32 @@ class TestDHCPRFC3442(CiTestCase): write_file(lease_file, content) self.assertItemsEqual(expected, parse_dhcp_lease_file(lease_file)) + def test_parse_lease_finds_classless_static_routes(self): + """ + parse_dhcp_lease_file returns classless-static-routes + for Centos lease format. + """ + lease_file = self.tmp_path('leases') + content = dedent(""" + lease { + interface "wlp3s0"; + fixed-address 192.168.2.74; + option subnet-mask 255.255.255.0; + option routers 192.168.2.1; + option classless-static-routes 0 130.56.240.1; + renew 4 2017/07/27 18:02:30; + expire 5 2017/07/28 07:08:15; + } + """) + expected = [ + {'interface': 'wlp3s0', 'fixed-address': '192.168.2.74', + 'subnet-mask': '255.255.255.0', 'routers': '192.168.2.1', + 'classless-static-routes': '0 130.56.240.1', + 'renew': '4 2017/07/27 18:02:30', + 'expire': '5 2017/07/28 07:08:15'}] + write_file(lease_file, content) + self.assertItemsEqual(expected, parse_dhcp_lease_file(lease_file)) + @mock.patch('cloudinit.net.dhcp.EphemeralIPv4Network') @mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery') def test_obtain_lease_parses_static_routes(self, m_maybe, m_ipv4): @@ -112,6 +138,31 @@ class TestDHCPRFC3442(CiTestCase): 'router': '192.168.2.1'} m_ipv4.assert_called_with(**expected_kwargs) + @mock.patch('cloudinit.net.dhcp.EphemeralIPv4Network') + @mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery') + def test_obtain_centos_lease_parses_static_routes(self, m_maybe, m_ipv4): + """ + EphemeralDHPCv4 parses rfc3442 routes for EphemeralIPv4Network + for Centos Lease format + """ + lease = [ + {'interface': 'wlp3s0', 'fixed-address': '192.168.2.74', + 'subnet-mask': '255.255.255.0', 'routers': '192.168.2.1', + 'classless-static-routes': '0 130.56.240.1', + 'renew': '4 2017/07/27 18:02:30', + 'expire': '5 2017/07/28 07:08:15'}] + m_maybe.return_value = lease + eph = net.dhcp.EphemeralDHCPv4() + eph.obtain_lease() + expected_kwargs = { + 'interface': 'wlp3s0', + 'ip': '192.168.2.74', + 'prefix_or_mask': '255.255.255.0', + 'broadcast': '192.168.2.255', + 'static_routes': [('0.0.0.0/0', '130.56.240.1')], + 'router': '192.168.2.1'} + m_ipv4.assert_called_with(**expected_kwargs) + class TestDHCPParseStaticRoutes(CiTestCase): @@ -181,6 +232,20 @@ class TestDHCPParseStaticRoutes(CiTestCase): logs = self.logs.getvalue() self.assertIn(rfc3442, logs.splitlines()[0]) + def test_redhat_format(self): + redhat_format = "24.191.168.128 192.168.128.1,0 192.168.128.1" + self.assertEqual(sorted([ + ("191.168.128.0/24", "192.168.128.1"), + ("0.0.0.0/0", "192.168.128.1") + ]), sorted(parse_static_routes(redhat_format))) + + def test_redhat_format_with_a_space_too_much_after_comma(self): + redhat_format = "24.191.168.128 192.168.128.1, 0 192.168.128.1" + self.assertEqual(sorted([ + ("191.168.128.0/24", "192.168.128.1"), + ("0.0.0.0/0", "192.168.128.1") + ]), sorted(parse_static_routes(redhat_format))) + class TestDHCPDiscoveryClean(CiTestCase): with_logs = True -- cgit v1.2.3