From e0748a58efe6fdb2931047dffa267fa4e2d54615 Mon Sep 17 00:00:00 2001 From: Gaudenz Steinlin Date: Mon, 16 Nov 2020 17:41:18 +0100 Subject: [PATCH] Copy existing IPv6 leases to generated lease file Because the DHCP agent does not know the IAID (identity association identifier) of assigned IPv6 addresses it's not possible to generate the lease file including IPv6 leases. Because of this IPv6 addresses are excluded when generating the lease file in case of DHCP agent restarts. This causes DHCPv6 clients to fail to RENEW their lease and to go through a full address discovery cycle with possible short connectivity disruption. This commit copies the existing IPv6 leaes from an already existing lease file if present. While this does not allow for DHCP agent failover, this is still better than just skipping the IPv6 addresses. A lease file without the IPv6 addresses is still generated if an agent is migrated to a different host. This commit complements the fix implemented in Ib1b2f284ab81f1c4af7b08b5257b45a3f6e79c3e which just skips the IPv6 leases as otherwise the lease file would be invalid and all leases would be lost. It does not change the behavior for still valid IPv4 leases. With this issue fixed an additional fix is required to not loose DHCPv6 leases when the agent restarts dnsmasq. Currently the DHCP agent regenerates all configuration files on restart. This means that DHCPv6 leases are lost as they can't be regenerated. This changes the agent to only delete the config files if the agent's ports are also removed. Closes-Bug: #1722126 Related-Change: Ib1b2f284ab81f1c4af7b08b5257b45a3f6e79c3e Change-Id: I40761b30563749251b9d74731bbe7a80a124da89 (cherry picked from commit 6bc1c00d66ced82dd142739790222e8408684696) --- neutron/agent/linux/dhcp.py | 49 +++++++++++++--- neutron/tests/unit/agent/linux/test_dhcp.py | 64 +++++++++++++++++++++ 2 files changed, 105 insertions(+), 8 deletions(-) diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index 8c18eb32c3b..96aab3b5b54 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -309,7 +309,7 @@ class DhcpLocalProcess(DhcpBase): common_utils.wait_until_true(lambda: not self.active) if not retain_port: self._destroy_namespace_and_port() - self._remove_config_files() + self._remove_config_files() def _destroy_namespace_and_port(self): try: @@ -798,17 +798,50 @@ class Dnsmasq(DhcpLocalProcess): dhcpv4_enabled_subnet_ids = [ s.id for s in self._get_all_subnets(self.network) if s.enable_dhcp and s.ip_version == constants.IP_VERSION_4] + dhcpv6_enabled_subnet_ids = [ + s.id for s in self._get_all_subnets(self.network) + if s.enable_dhcp and s.ip_version == constants.IP_VERSION_6] + + existing_ipv6_leases = {} + if os.path.isfile(filename): + # The IPv6 leases can't be generated as their IAID is unknown. To + # not loose active leases, read the existing leases and add them to + # the generated file. + LOG.debug('Reading IPv6 leases from existing lease file.') + with open(filename) as leasefile: + for line in leasefile: + if line.startswith('duid '): + # Keep the DUID + buf.write(line) + continue + try: + ts, mac, ip, host, iaid = line.split(' ') + except ValueError: + # not the correct format for a lease, skip this line + continue + + if netaddr.valid_ipv6(ip): + existing_ipv6_leases[netaddr.IPAddress(ip)] = line + for host_tuple in self._iter_hosts(): port, alloc, hostname, name, no_dhcp, no_opts, tag = host_tuple - # don't write ip address which belongs to a dhcp disabled subnet - # or an IPv6 subnet. - if no_dhcp or alloc.subnet_id not in dhcpv4_enabled_subnet_ids: + + if no_dhcp: continue - # all that matters is the mac address and IP. the hostname and - # client ID will be overwritten on the next renewal. - buf.write('%s %s %s * *\n' % - (timestamp, port.mac_address, alloc.ip_address)) + if alloc.subnet_id in dhcpv4_enabled_subnet_ids: + # all that matters is the mac address and IP. the hostname and + # client ID will be overwritten on the next renewal. + buf.write('%s %s %s * *\n' % + (timestamp, port.mac_address, alloc.ip_address)) + elif (alloc.subnet_id in dhcpv6_enabled_subnet_ids and + netaddr.IPAddress(alloc.ip_address) in existing_ipv6_leases): + # Keep the existing IPv6 lease if the port still exists and is + # still configured for DHCPv6 + buf.write( + existing_ipv6_leases[netaddr.IPAddress(alloc.ip_address)] + ) + contents = buf.getvalue() file_utils.replace_file(filename, contents) LOG.debug('Done building initial lease file %s with contents:\n%s', diff --git a/neutron/tests/unit/agent/linux/test_dhcp.py b/neutron/tests/unit/agent/linux/test_dhcp.py index 27e71abb71b..fb9d9a27237 100644 --- a/neutron/tests/unit/agent/linux/test_dhcp.py +++ b/neutron/tests/unit/agent/linux/test_dhcp.py @@ -1227,6 +1227,7 @@ class TestDhcpLocalProcess(TestBase): mocks['interface_name'].__get__ = mock.Mock(return_value='tap0') lp = LocalChild(self.conf, network) lp.disable(retain_port=True) + self.rmtree.assert_not_called() self._assert_disabled(lp) def test_disable(self): @@ -1238,6 +1239,7 @@ class TestDhcpLocalProcess(TestBase): with mock.patch('neutron.agent.linux.ip_lib.' 'delete_network_namespace') as delete_ns: lp.disable() + self.rmtree.assert_called_once() self._assert_disabled(lp) @@ -1560,6 +1562,68 @@ class TestDnsmasq(TestBase): timestamp = 0 self._test_output_init_lease_file(timestamp) + @mock.patch('time.time') + @mock.patch('os.path.isfile', return_value=True) + def test_output_init_lease_file_existing(self, isfile, tmock): + + duid = 'duid 00:01:00:01:27:da:58:97:fa:16:3e:6c:ad:c1' + ipv4_leases = ( + '1623162161 00:00:80:aa:bb:cc 192.168.0.2 host-192-168-0-2 *\n' + '1623147425 00:00:0f:aa:bb:cc 192.168.0.3 host-192-168-0-3 ' + 'ff:b5:5e:67:ff:00:02:00:00:ab:11:43:e5:86:52:f3:d7:2c:97\n' + '1623138717 00:00:0f:rr:rr:rr 192.168.0.1 host-192-168-0-1 ' + 'ff:b5:5e:67:ff:00:02:00:00:ab:11:f6:f2:aa:cb:94:c1:b4:86' + ) + ipv6_lease_v6_port = ( + '1623083263 755752236 fdca:3ba5:a17a:4ba3::2 ' + 'host-fdca-3ba5-a17a-4ba3--2 ' + '00:01:00:01:28:50:e8:31:5a:42:2d:0b:dd:2c' + ) + additional_ipv6_leases = ( + '1623143299 3042863103 2001:db8::45 host-2001-db8--45 ' + '00:02:00:00:ab:11:fa:c9:0e:0f:3d:90:73:f0\n' + '1623134168 3042863103 2001:db8::12 host-2001-db8--12 ' + '00:02:00:00:ab:11:f6:f2:aa:cb:94:c1:b4:86' + ) + existing_leases = '\n'.join((ipv4_leases, duid, ipv6_lease_v6_port, + additional_ipv6_leases)) + + # lease duration should be added to current time + timestamp = 1000000 + 500 + # The expected lease file contains: + # * The DHCPv6 servers DUID + # * A lease for all IPv4 addresses + # * A lease for the IPv6 addresses present in the existing lease file + # (IPv6 of FakeV6Port) + # * No lease for the IPv6 addresses NOT present in the existing lease + # file (IPv6 of FakeDualPort) + # * No lease for the IPv6 addresses present in the existing lease file + # which are no longer assigned to any port + expected = ( + '%s\n' + '%s 00:00:80:aa:bb:cc 192.168.0.2 * *\n' + '%s\n' + '%s 00:00:0f:aa:bb:cc 192.168.0.3 * *\n' + '%s 00:00:0f:rr:rr:rr 192.168.0.1 * *\n' + ) % (duid, timestamp, ipv6_lease_v6_port, timestamp, timestamp) + + self.conf.set_override('dhcp_lease_duration', 500) + tmock.return_value = 1000000 + + with mock.patch.object(dhcp.Dnsmasq, 'get_conf_file_name') as conf_fn: + conf_fn.return_value = '/foo/leases' + dm = self._get_dnsmasq(FakeDualNetwork()) + + # Patch __iter__ into mock for Python < 3.8 compatibility + open_mock = mock.mock_open(read_data=existing_leases) + open_mock.return_value.__iter__ = lambda s: iter(s.readline, '') + + with mock.patch('builtins.open', open_mock): + dm._output_init_lease_file() + + # Assert the lease file contains the existing ipv6_leases + self.safe.assert_called_once_with('/foo/leases', expected) + def _test_output_opts_file(self, expected, network, ipm_retval=None): with mock.patch.object(dhcp.Dnsmasq, 'get_conf_file_name') as conf_fn: conf_fn.return_value = '/foo/opts'