From 587836da40e2b87de065361cc204a3469622b4cc Mon Sep 17 00:00:00 2001 From: Carl Baldwin Date: Tue, 12 Nov 2013 22:52:47 +0000 Subject: [PATCH] Use information from the dnsmasq hosts file to call dhcp_release Certain situations can cause the DHCP agent's local cache to get out of sync with the leases held internally by dnsmasq. This method of detecting when to call dhcp_release is idempotent and not dependent on the cache. It is more robust. Change-Id: I4eafd9cfb94a77a2f0229f89de5483dad23725cf Closes-Bug: #1250644 --- neutron/agent/linux/dhcp.py | 40 ++++++++++++---- neutron/tests/unit/test_linux_dhcp.py | 66 +++++++++++++++++++++++---- 2 files changed, 88 insertions(+), 18 deletions(-) diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index 1cf1a8e1e..fe31f58e1 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -381,15 +381,17 @@ class Dnsmasq(DhcpLocalProcess): utils.execute(cmd, self.root_helper) def release_lease(self, mac_address, removed_ips): + pass + + def _release_lease(self, mac_address, ip): """Release a DHCP lease.""" - for ip in removed_ips or []: - cmd = ['dhcp_release', self.interface_name, ip, mac_address] - if self.network.namespace: - ip_wrapper = ip_lib.IPWrapper(self.root_helper, - self.network.namespace) - ip_wrapper.netns.execute(cmd) - else: - utils.execute(cmd, self.root_helper) + cmd = ['dhcp_release', self.interface_name, ip, mac_address] + if self.network.namespace: + ip_wrapper = ip_lib.IPWrapper(self.root_helper, + self.network.namespace) + ip_wrapper.netns.execute(cmd) + else: + utils.execute(cmd, self.root_helper) def reload_allocations(self): """Rebuild the dnsmasq config and signal the dnsmasq to reload.""" @@ -401,6 +403,7 @@ class Dnsmasq(DhcpLocalProcess): 'turned off DHCP: %s'), self.network.id) return + self._release_unused_leases() self._output_hosts_file() self._output_opts_file() if self.active: @@ -436,6 +439,27 @@ class Dnsmasq(DhcpLocalProcess): utils.replace_file(name, buf.getvalue()) return name + def _read_hosts_file_leases(self, filename): + leases = set() + if os.path.exists(filename): + with open(filename) as f: + for l in f.readlines(): + host = l.strip().split(',') + leases.add((host[2], host[0])) + return leases + + def _release_unused_leases(self): + filename = self.get_conf_file_name('host') + old_leases = self._read_hosts_file_leases(filename) + + new_leases = set() + for port in self.network.ports: + for alloc in port.fixed_ips: + new_leases.add((alloc.ip_address, port.mac_address)) + + for ip, mac in old_leases - new_leases: + self._release_lease(mac, ip) + def _output_opts_file(self): """Write a dnsmasq compatible options file.""" diff --git a/neutron/tests/unit/test_linux_dhcp.py b/neutron/tests/unit/test_linux_dhcp.py index 9dec635de..6d0cccaa5 100644 --- a/neutron/tests/unit/test_linux_dhcp.py +++ b/neutron/tests/unit/test_linux_dhcp.py @@ -864,16 +864,6 @@ tag:tag0,option:router,192.168.1.1""".lstrip() self.safe.assert_called_once_with('/foo/opts', expected) - def test_release_lease(self): - dm = dhcp.Dnsmasq(self.conf, FakeDualNetwork(), version=float(2.59)) - dm.release_lease(mac_address=FakePort2.mac_address, - removed_ips=[FakePort2.fixed_ips[0].ip_address]) - exp_args = ['ip', 'netns', 'exec', 'qdhcp-ns', 'dhcp_release', - dm.interface_name, FakePort2.fixed_ips[0].ip_address, - FakePort2.mac_address] - self.execute.assert_called_once_with(exp_args, root_helper='sudo', - check_exit_code=True) - def test_output_opts_file_pxe_2port_1net(self): expected = """ tag:tag0,option:dns-server,8.8.8.8 @@ -1069,6 +1059,62 @@ tag:tag1,249,%s,%s""".lstrip() % (fake_v6, mock.call(exp_opt_name, exp_opt_data)]) mock_open.assert_called_once_with('/proc/5/cmdline', 'r') + def test_release_unused_leases(self): + dnsmasq = dhcp.Dnsmasq(self.conf, FakeDualNetwork()) + + ip1 = '192.168.1.2' + mac1 = '00:00:80:aa:bb:cc' + ip2 = '192.168.1.3' + mac2 = '00:00:80:cc:bb:aa' + + old_leases = set([(ip1, mac1), (ip2, mac2)]) + dnsmasq._read_hosts_file_leases = mock.Mock(return_value=old_leases) + dnsmasq._output_hosts_file = mock.Mock() + dnsmasq._release_lease = mock.Mock() + dnsmasq.network.ports = [] + + dnsmasq._release_unused_leases() + + dnsmasq._release_lease.assert_has_calls([mock.call(mac1, ip1), + mock.call(mac2, ip2)], + any_order=True) + + def test_release_unused_leases_one_lease(self): + dnsmasq = dhcp.Dnsmasq(self.conf, FakeDualNetwork()) + + ip1 = '192.168.0.2' + mac1 = '00:00:80:aa:bb:cc' + ip2 = '192.168.0.3' + mac2 = '00:00:80:cc:bb:aa' + + old_leases = set([(ip1, mac1), (ip2, mac2)]) + dnsmasq._read_hosts_file_leases = mock.Mock(return_value=old_leases) + dnsmasq._output_hosts_file = mock.Mock() + dnsmasq._release_lease = mock.Mock() + dnsmasq.network.ports = [FakePort1()] + + dnsmasq._release_unused_leases() + + dnsmasq._release_lease.assert_has_calls([mock.call(mac2, ip2)], + any_order=True) + + def test_read_hosts_file_leases(self): + filename = '/path/to/file' + with mock.patch('os.path.exists') as mock_exists: + mock_exists.return_value = True + with mock.patch('__builtin__.open') as mock_open: + mock_open.return_value.__enter__ = lambda s: s + mock_open.return_value.__exit__ = mock.Mock() + lines = ["00:00:80:aa:bb:cc,inst-name,192.168.0.1"] + mock_open.return_value.readlines.return_value = lines + + dnsmasq = dhcp.Dnsmasq(self.conf, FakeDualNetwork()) + leases = dnsmasq._read_hosts_file_leases(filename) + + self.assertEqual(set([("192.168.0.1", "00:00:80:aa:bb:cc")]), leases) + mock_exists.assert_called_once_with(filename) + mock_open.assert_called_once_with(filename) + def test_make_subnet_interface_ip_map(self): with mock.patch('neutron.agent.linux.ip_lib.IPDevice') as ip_dev: ip_dev.return_value.addr.list.return_value = [ -- 2.45.2