From: Moshe Levi Date: Wed, 22 Apr 2015 11:17:28 +0000 (+0300) Subject: Add client id option support to dhcp agent X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=d4a39439727055fed2cc0661f1ba02c73fd523dc;p=openstack-build%2Fneutron-build.git Add client id option support to dhcp agent According to the dnsmasq man client id option should be written to dhcp-hostsfile and not to the dhcp-optsfile. Also this patch update the dhcp_release command to take into account the client id when releasing old leases. Closes-Bug: #1447105 Change-Id: I6f11b12040ad4e00ae871be45edda3b52b4ee0da --- diff --git a/neutron/agent/linux/dhcp.py b/neutron/agent/linux/dhcp.py index b9af4e337..d4fbcb1d9 100644 --- a/neutron/agent/linux/dhcp.py +++ b/neutron/agent/linux/dhcp.py @@ -33,6 +33,7 @@ from neutron.common import constants from neutron.common import exceptions from neutron.common import ipv6_utils from neutron.common import utils as commonutils +from neutron.extensions import extra_dhcp_opt as edo_ext from neutron.i18n import _LE, _LI, _LW from neutron.openstack.common import uuidutils @@ -281,6 +282,8 @@ class Dnsmasq(DhcpLocalProcess): _TAG_PREFIX = 'tag%d' + _ID = 'id:' + @classmethod def check_version(cls): pass @@ -399,9 +402,11 @@ class Dnsmasq(DhcpLocalProcess): service_name=DNSMASQ_SERVICE_NAME, monitored_process=pm) - def _release_lease(self, mac_address, ip): + def _release_lease(self, mac_address, ip, client_id): """Release a DHCP lease.""" cmd = ['dhcp_release', self.interface_name, ip, mac_address] + if client_id: + cmd.append(client_id) ip_wrapper = ip_lib.IPWrapper(namespace=self.network.namespace) ip_wrapper.netns.execute(cmd, run_as_root=True) @@ -461,6 +466,9 @@ class Dnsmasq(DhcpLocalProcess): fqdn = '%s.%s' % (fqdn, self.conf.dhcp_domain) yield (port, alloc, hostname, fqdn) + def _get_port_extra_dhcp_opts(self, port): + return getattr(port, edo_ext.EXTRADHCPOPTS, False) + def _output_hosts_file(self): """Writes a dnsmasq compatible dhcp hosts file. @@ -487,7 +495,7 @@ class Dnsmasq(DhcpLocalProcess): # avoid potential performance drop when lots of hosts are dumped for (port, alloc, hostname, name) in self._iter_hosts(): if not alloc: - if getattr(port, 'extra_dhcp_opts', False): + if self._get_port_extra_dhcp_opts(port): buf.write('%s,%s%s\n' % (port.mac_address, 'set:', port.id)) continue @@ -503,10 +511,20 @@ class Dnsmasq(DhcpLocalProcess): if netaddr.valid_ipv6(ip_address): ip_address = '[%s]' % ip_address - if getattr(port, 'extra_dhcp_opts', False): - buf.write('%s,%s,%s,%s%s\n' % - (port.mac_address, name, ip_address, - 'set:', port.id)) + if self._get_port_extra_dhcp_opts(port): + client_id = self._get_client_id(port) + if client_id and len(port.extra_dhcp_opts) > 1: + buf.write('%s,%s%s,%s,%s,%s%s\n' % + (port.mac_address, self._ID, client_id, name, + ip_address, 'set:', port.id)) + elif client_id and len(port.extra_dhcp_opts) == 1: + buf.write('%s,%s%s,%s,%s\n' % + (port.mac_address, self._ID, client_id, name, + ip_address)) + else: + buf.write('%s,%s,%s,%s%s\n' % + (port.mac_address, name, ip_address, + 'set:', port.id)) else: buf.write('%s,%s,%s\n' % (port.mac_address, name, ip_address)) @@ -516,13 +534,28 @@ class Dnsmasq(DhcpLocalProcess): buf.getvalue()) return filename + def _get_client_id(self, port): + if self._get_port_extra_dhcp_opts(port): + for opt in port.extra_dhcp_opts: + if opt.opt_name == edo_ext.CLIENT_ID: + return opt.opt_value + def _read_hosts_file_leases(self, filename): leases = set() - if os.path.exists(filename): + try: with open(filename) as f: for l in f.readlines(): host = l.strip().split(',') - leases.add((host[2].strip('[]'), host[0])) + mac = host[0] + client_id = None + if host[1].startswith(self._ID): + ip = host[3].strip('[]') + client_id = host[1][len(self._ID):] + else: + ip = host[2].strip('[]') + leases.add((ip, mac, client_id)) + except (OSError, IOError): + LOG.debug('Error while reading hosts file %s', filename) return leases def _release_unused_leases(self): @@ -531,11 +564,12 @@ class Dnsmasq(DhcpLocalProcess): new_leases = set() for port in self.network.ports: + client_id = self._get_client_id(port) for alloc in port.fixed_ips: - new_leases.add((alloc.ip_address, port.mac_address)) + new_leases.add((alloc.ip_address, port.mac_address, client_id)) - for ip, mac in old_leases - new_leases: - self._release_lease(mac, ip) + for ip, mac, client_id in old_leases - new_leases: + self._release_lease(mac, ip, client_id) def _output_addn_hosts_file(self): """Writes a dnsmasq compatible additional hosts file. @@ -645,11 +679,13 @@ class Dnsmasq(DhcpLocalProcess): options = [] dhcp_ips = collections.defaultdict(list) for port in self.network.ports: - if getattr(port, 'extra_dhcp_opts', False): + if self._get_port_extra_dhcp_opts(port): port_ip_versions = set( [netaddr.IPAddress(ip.ip_address).version for ip in port.fixed_ips]) for opt in port.extra_dhcp_opts: + if opt.opt_name == edo_ext.CLIENT_ID: + continue opt_ip_version = opt.ip_version if opt_ip_version in port_ip_versions: options.append( diff --git a/neutron/extensions/extra_dhcp_opt.py b/neutron/extensions/extra_dhcp_opt.py index 6bc8d6c3a..0de062c7c 100644 --- a/neutron/extensions/extra_dhcp_opt.py +++ b/neutron/extensions/extra_dhcp_opt.py @@ -46,6 +46,8 @@ EXTRADHCPOPTS = 'extra_dhcp_opts' DHCP_OPT_NAME_MAX_LEN = 64 DHCP_OPT_VALUE_MAX_LEN = 255 +CLIENT_ID = "client-id" + EXTENDED_ATTRIBUTES_2_0 = { 'ports': { EXTRADHCPOPTS: diff --git a/neutron/tests/unit/agent/linux/test_dhcp.py b/neutron/tests/unit/agent/linux/test_dhcp.py index b9f349942..fae303ab0 100644 --- a/neutron/tests/unit/agent/linux/test_dhcp.py +++ b/neutron/tests/unit/agent/linux/test_dhcp.py @@ -27,6 +27,7 @@ from neutron.agent.linux import external_process from neutron.agent.linux import utils from neutron.common import config as base_config from neutron.common import constants +from neutron.extensions import extra_dhcp_opt as edo_ext from neutron.tests import base LOG = logging.getLogger(__name__) @@ -100,6 +101,38 @@ class FakePort4(object): self.extra_dhcp_opts = [] +class FakePort5(object): + id = 'eeeeeeee-eeee-eeee-eeee-eeeeeeeeeee' + admin_state_up = True + device_owner = 'foo5' + fixed_ips = [FakeIPAllocation('192.168.0.5', + 'dddddddd-dddd-dddd-dddd-dddddddddddd')] + mac_address = '00:00:0f:aa:bb:55' + + def __init__(self): + self.extra_dhcp_opts = [ + DhcpOpt(opt_name=edo_ext.CLIENT_ID, + opt_value='test5')] + + +class FakePort6(object): + id = 'ccccccccc-cccc-cccc-cccc-ccccccccc' + admin_state_up = True + device_owner = 'foo6' + fixed_ips = [FakeIPAllocation('192.168.0.6', + 'dddddddd-dddd-dddd-dddd-dddddddddddd')] + mac_address = '00:00:0f:aa:bb:66' + + def __init__(self): + self.extra_dhcp_opts = [ + DhcpOpt(opt_name=edo_ext.CLIENT_ID, + opt_value='test6', + ip_version=4), + DhcpOpt(opt_name='dns-server', + opt_value='123.123.123.45', + ip_version=4)] + + class FakeV6Port(object): id = 'hhhhhhhh-hhhh-hhhh-hhhh-hhhhhhhhhhhh' admin_state_up = True @@ -366,6 +399,13 @@ class FakeV4Network(object): namespace = 'qdhcp-ns' +class FakeV4NetworkClientId(object): + id = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' + subnets = [FakeV4Subnet()] + ports = [FakePort1(), FakePort5(), FakePort6()] + namespace = 'qdhcp-ns' + + class FakeV6Network(object): id = 'bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb' subnets = [FakeV6Subnet()] @@ -1304,7 +1344,7 @@ class TestDnsmasq(TestBase): ip2 = '192.168.1.3' mac2 = '00:00:80:cc:bb:aa' - old_leases = set([(ip1, mac1), (ip2, mac2)]) + old_leases = set([(ip1, mac1, None), (ip2, mac2, None)]) dnsmasq._read_hosts_file_leases = mock.Mock(return_value=old_leases) dnsmasq._output_hosts_file = mock.Mock() dnsmasq._release_lease = mock.Mock() @@ -1312,10 +1352,33 @@ class TestDnsmasq(TestBase): dnsmasq._release_unused_leases() - dnsmasq._release_lease.assert_has_calls([mock.call(mac1, ip1), - mock.call(mac2, ip2)], + dnsmasq._release_lease.assert_has_calls([mock.call(mac1, ip1, None), + mock.call(mac2, ip2, None)], any_order=True) + def test_release_unused_leases_with_client_id(self): + dnsmasq = self._get_dnsmasq(FakeDualNetwork()) + + ip1 = '192.168.1.2' + mac1 = '00:00:80:aa:bb:cc' + client_id1 = 'client1' + ip2 = '192.168.1.3' + mac2 = '00:00:80:cc:bb:aa' + client_id2 = 'client2' + + old_leases = set([(ip1, mac1, client_id1), (ip2, mac2, client_id2)]) + 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, client_id1), + mock.call(mac2, ip2, client_id2)], + any_order=True) + def test_release_unused_leases_one_lease(self): dnsmasq = self._get_dnsmasq(FakeDualNetwork()) @@ -1324,7 +1387,7 @@ class TestDnsmasq(TestBase): ip2 = '192.168.0.3' mac2 = '00:00:80:cc:bb:aa' - old_leases = set([(ip1, mac1), (ip2, mac2)]) + old_leases = set([(ip1, mac1, None), (ip2, mac2, None)]) dnsmasq._read_hosts_file_leases = mock.Mock(return_value=old_leases) dnsmasq._output_hosts_file = mock.Mock() dnsmasq._release_lease = mock.Mock() @@ -1332,27 +1395,63 @@ class TestDnsmasq(TestBase): dnsmasq._release_unused_leases() - dnsmasq._release_lease.assert_has_calls([mock.call(mac2, ip2)], - any_order=True) + dnsmasq._release_lease.assert_called_once_with( + mac2, ip2, None) + + def test_release_unused_leases_one_lease_with_client_id(self): + dnsmasq = self._get_dnsmasq(FakeDualNetwork()) + + ip1 = '192.168.0.2' + mac1 = '00:00:80:aa:bb:cc' + client_id1 = 'client1' + ip2 = '192.168.0.5' + mac2 = '00:00:0f:aa:bb:55' + client_id2 = 'test5' + + old_leases = set([(ip1, mac1, client_id1), (ip2, mac2, client_id2)]) + 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 = [FakePort5()] + + dnsmasq._release_unused_leases() + + dnsmasq._release_lease.assert_called_once_with( + mac1, ip1, client_id1) 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", - "00:00:80:aa:bb:cc,inst-name,[fdca:3ba5:a17a::1]"] - mock_open.return_value.readlines.return_value = lines - - dnsmasq = self._get_dnsmasq(FakeDualNetwork()) - leases = dnsmasq._read_hosts_file_leases(filename) - - self.assertEqual(set([("192.168.0.1", "00:00:80:aa:bb:cc"), - ("fdca:3ba5:a17a::1", "00:00:80:aa:bb:cc")]), - leases) - mock_exists.assert_called_once_with(filename) + 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", + "00:00:80:aa:bb:cc,inst-name,[fdca:3ba5:a17a::1]"] + mock_open.return_value.readlines.return_value = lines + + dnsmasq = self._get_dnsmasq(FakeDualNetwork()) + leases = dnsmasq._read_hosts_file_leases(filename) + + self.assertEqual(set([("192.168.0.1", "00:00:80:aa:bb:cc", None), + ("fdca:3ba5:a17a::1", "00:00:80:aa:bb:cc", + None)]), leases) + mock_open.assert_called_once_with(filename) + + def test_read_hosts_file_leases_with_client_id(self): + filename = '/path/to/file' + 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,id:client1,inst-name,192.168.0.1", + "00:00:80:aa:bb:cc,id:client2,inst-name," + "[fdca:3ba5:a17a::1]"] + mock_open.return_value.readlines.return_value = lines + + dnsmasq = self._get_dnsmasq(FakeDualNetwork()) + leases = dnsmasq._read_hosts_file_leases(filename) + + self.assertEqual(set([("192.168.0.1", "00:00:80:aa:bb:cc", 'client1'), + ("fdca:3ba5:a17a::1", "00:00:80:aa:bb:cc", + 'client2')]), leases) mock_open.assert_called_once_with(filename) def test_make_subnet_interface_ip_map(self): @@ -1426,6 +1525,22 @@ class TestDnsmasq(TestBase): self.safe.assert_has_calls([mock.call(exp_host_name, exp_host_data)]) + def test_only_populates_dhcp_client_id(self): + exp_host_name = '/dhcp/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa/host' + exp_host_data = ('00:00:80:aa:bb:cc,host-192-168-0-2.openstacklocal,' + '192.168.0.2\n' + '00:00:0f:aa:bb:55,id:test5,' + 'host-192-168-0-5.openstacklocal,' + '192.168.0.5\n' + '00:00:0f:aa:bb:66,id:test6,' + 'host-192-168-0-6.openstacklocal,192.168.0.6,' + 'set:ccccccccc-cccc-cccc-cccc-ccccccccc\n').lstrip() + + dm = self._get_dnsmasq(FakeV4NetworkClientId) + dm._output_hosts_file() + self.safe.assert_has_calls([mock.call(exp_host_name, + exp_host_data)]) + def test_only_populates_dhcp_enabled_subnet_on_a_network(self): exp_host_name = '/dhcp/cccccccc-cccc-cccc-cccc-cccccccccccc/host' exp_host_data = ('00:00:80:aa:bb:cc,host-192-168-0-2.openstacklocal,'