launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #13311
[Merge] lp:~jtv/maas/bug-1064227 into lp:maas
Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/bug-1064227 into lp:maas.
Commit message:
Change the DHCP leases parser: an IP/MAC mapping remains current as long as there is an unexpired lease entry for it, or a host declaration without a subsequent rubout declaration.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1064227 in MAAS: "Two nodes with IP address yet only one CNAME"
https://bugs.launchpad.net/maas/+bug/1064227
For more details, see:
https://code.launchpad.net/~jtv/maas/bug-1064227/+merge/129115
This reflects an important change in our understanding of how leases and host declarations interact in the DHCP leases file. We do not see lease renewals for clients that we have host maps for, and it appears that the expired leases for those clients are not removed from the hosts file on cleanup.
In this branch I change the parser to recognize host maps as well as leases. Either is accepted as establishing an IP/MAC mapping.
Host maps have a different "expiry" mechanism: whereas a lease may have an expiry date that passes, a host map is retired by appending a new host map for the same IP address that contains the "deleted" statement.
Jeroen
--
https://code.launchpad.net/~jtv/maas/bug-1064227/+merge/129115
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/bug-1064227 into lp:maas.
=== modified file 'src/provisioningserver/dhcp/leases_parser.py'
--- src/provisioningserver/dhcp/leases_parser.py 2012-10-05 06:16:48 +0000
+++ src/provisioningserver/dhcp/leases_parser.py 2012-10-11 08:27:42 +0000
@@ -47,24 +47,50 @@
Suppress('=') +
QuotedString('"'))
+# For our purposes, leases and host declarations are similar enough that
+# we can parse them as the same construct with different names.
+lease_or_host = oneOf(['lease', 'host'], caseless=True)
+
hardware = CaselessKeyword("hardware") + hardware_type("type") + mac("mac")
ends = CaselessKeyword("ends") + expiry("expiry")
-other_statement = (
- oneOf(
- ['starts', 'tstp', 'atsfp', 'tsfp', 'cltt', 'uid', 'binding', 'next',
- 'client-hostname', 'abandoned', 'option', 'ddns-text',
- 'ddns-fwd-name', 'ddns-client-fqdn', 'ddns-rev-name',
- 'vendor-class-identifier', 'bootp', 'reserved', 'rewind'],
- caseless=True) + args
- )
-lone_statement = (
- oneOf(['abandoned', 'bootp', 'reserved'], caseless=True))
+
+lone_statement_names = [
+ 'abandoned',
+ 'bootp',
+ 'deleted',
+ 'dynamic',
+ 'reserved',
+ ]
+lone_statement = oneOf(lone_statement_names, caseless=True)
+
+other_statement_names = [
+ 'atsfp',
+ 'binding',
+ 'bootp',
+ 'client-hostname',
+ 'cltt',
+ 'ddns-client-fqdn',
+ 'ddns-fwd-name',
+ 'ddns-rev-name',
+ 'ddns-text',
+ 'fixed-address',
+ 'next',
+ 'option',
+ 'reserved',
+ 'rewind',
+ 'starts',
+ 'tstp',
+ 'tsfp',
+ 'uid',
+ 'vendor-class-identifier',
+ ]
+other_statement = oneOf(other_statement_names, caseless=True) + args
lease_statement = (
hardware | ends | set_statement | lone_statement | other_statement
) + Suppress(';')
lease_parser = (
- CaselessKeyword("lease") + ip("ip") +
+ lease_or_host("lease_or_host") + ip("ip") +
Suppress('{') +
Dict(ZeroOrMore(Group(lease_statement))) +
Suppress('}')
@@ -72,6 +98,19 @@
lease_parser.ignore('#' + restOfLine)
+def is_lease(entry):
+ """Is `entry` a lease declaration?"""
+ entry_type = entry.lease_or_host.lower()
+ assert entry_type in {'host', 'lease'}, (
+ "Unknown entry type (not a host or lease): %s" % entry_type)
+ return entry_type == 'lease'
+
+
+def is_host(entry):
+ """Is `entry` a host declaration?"""
+ return not is_lease(entry)
+
+
def get_expiry_date(lease):
"""Get the expiry date for a lease, if any.
@@ -79,6 +118,7 @@
:return: A UTC-based timestamp representing the lease's moment of
expiry, or None if the lease has no expiry date.
"""
+ assert is_lease(lease)
ends = getattr(lease, 'ends', None)
if ends is None or len(ends) == 0 or ends.lower() == 'never':
return None
@@ -93,6 +133,7 @@
:param now: The current UTC-based timestamp to check expiry against.
:return: Whether the lease has expired.
"""
+ assert is_lease(lease)
expiry_date = get_expiry_date(lease)
if expiry_date is None:
return False
@@ -100,19 +141,65 @@
return expiry_date < now
-def parse_leases(leases_contents):
- """Parse contents of a leases file.
-
- :param leases_contents: Contents (as unicode) of the leases file.
- :return: A dict mapping each currently leased IP address to the MAC
- address that it is associated with.
- """
+def gather_leases(hosts_and_leases):
+ """Find current leases among `hosts_and_leases`."""
now = datetime.utcnow()
- leases = lease_parser.searchString(leases_contents)
# If multiple leases for the same address are valid at the same
# time, for whatever reason, the dict will contain the one that was
# last appended to the leases file.
return {
lease.ip: lease.hardware.mac
- for lease in leases
+ for lease in filter(is_lease, hosts_and_leases)
if not has_expired(lease, now)}
+
+
+def get_host_mac(host):
+ """Get the MAC address from a host declaration. A rubout has none."""
+ assert is_host(host)
+ deleted_statement = getattr(host, 'deleted', None)
+ if deleted_statement not in (None, '', b''):
+ return None
+ hardware = getattr(host, 'hardware', None)
+ if hardware in (None, '', b''):
+ return None
+ else:
+ return hardware.mac
+
+
+def gather_hosts(hosts_and_leases):
+ """Find current host declarations among `hosts_and_leases`."""
+ # Get MAC address mappings for host entries. A newer entry
+ # overwrites an older one for the same IP address. A rubout entry
+ # will have no MAC address.
+ host_maps = {
+ host.ip: get_host_mac(host)
+ for host in filter(is_host, hosts_and_leases)}
+ # Now filter out mappings where the last entry was a rubout.
+ return {
+ ip: mac
+ for ip, mac in host_maps.items()
+ if mac is not None}
+
+
+def combine_entries(entries):
+ """Combine the hosts and leases declarations in a parsed leases file.
+
+ :param entries: Parsed host/leases entries from a leases file.
+ :return: A dict mapping leased IP addresses to the respective MAC
+ addresses that currently own them (regardless of whether they
+ were found in a lease or in a host declaration).
+ """
+ leases = gather_leases(entries)
+ leases.update(gather_hosts(entries))
+ return leases
+
+
+def parse_leases(leases_contents):
+ """Parse contents of a leases file.
+
+ :param leases_contents: Contents (as unicode) of the leases file.
+ :return: A dict mapping each currently leased IP address to the MAC
+ address that it is associated with.
+ """
+ entries = lease_parser.searchString(leases_contents)
+ return combine_entries(entries)
=== modified file 'src/provisioningserver/dhcp/tests/test_leases_parser.py'
--- src/provisioningserver/dhcp/tests/test_leases_parser.py 2012-10-05 06:16:24 +0000
+++ src/provisioningserver/dhcp/tests/test_leases_parser.py 2012-10-11 08:27:42 +0000
@@ -1,7 +1,7 @@
# Copyright 2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
-"""..."""
+"""Tests for the DHCP leases parser."""
from __future__ import (
absolute_import,
@@ -19,6 +19,9 @@
from maastesting.factory import factory
from maastesting.testcase import TestCase
from provisioningserver.dhcp.leases_parser import (
+ combine_entries,
+ gather_hosts,
+ gather_leases,
get_expiry_date,
has_expired,
parse_leases,
@@ -27,13 +30,28 @@
class TestLeasesParser(TestCase):
- def fake_parsed_lease(self, ip=None, mac=None, ends=None):
+ def fake_parsed_lease(self, ip=None, mac=None, ends=None,
+ entry_type='lease'):
"""Fake a lease as produced by the parser."""
if ip is None:
ip = factory.getRandomIPAddress()
if mac is None:
mac = factory.getRandomMACAddress()
- return namedtuple('lease', ['ip', 'mac', 'ends'])(ip, mac, ends)
+ Hardware = namedtuple('Hardware', ['mac'])
+ Lease = namedtuple(
+ 'Lease', ['lease_or_host', 'ip', 'hardware', 'ends'])
+ return Lease(entry_type, ip, Hardware(mac), ends)
+
+ def fake_parsed_host(self, ip=None, mac=None):
+ """Fake a host declaration as produced by the parser."""
+ return self.fake_parsed_lease(ip=ip, mac=mac, entry_type='host')
+
+ def fake_parsed_rubout(self, ip=None):
+ """Fake a "rubout" host declaration."""
+ if ip is None:
+ ip = factory.getRandomIPAddress()
+ Rubout = namedtuple('Rubout', ['lease_or_host', 'ip'])
+ return Rubout('host', ip)
def test_get_expiry_date_parses_expiry_date(self):
lease = self.fake_parsed_lease(ends='0 2011/01/02 03:04:05')
@@ -65,6 +83,67 @@
self.assertTrue(
has_expired(self.fake_parsed_lease(ends=earlier), now))
+ def test_gather_leases_finds_current_leases(self):
+ lease = self.fake_parsed_lease()
+ self.assertEqual(
+ {lease.ip: lease.hardware.mac},
+ gather_leases([lease]))
+
+ def test_gather_leases_ignores_expired_leases(self):
+ earlier = '1 2001/01/01 00:00:00'
+ lease = self.fake_parsed_lease(ends=earlier)
+ self.assertEqual({}, gather_leases([lease]))
+
+ def test_gather_leases_combines_expired_and_current_leases(self):
+ earlier = '1 2001/01/01 00:00:00'
+ ip = factory.getRandomIPAddress()
+ old_owner = factory.getRandomMACAddress()
+ new_owner = factory.getRandomMACAddress()
+ leases = [
+ self.fake_parsed_lease(ip=ip, mac=old_owner, ends=earlier),
+ self.fake_parsed_lease(ip=ip, mac=new_owner),
+ ]
+ self.assertEqual({ip: new_owner}, gather_leases(leases))
+
+ def test_gather_leases_ignores_ordering(self):
+ earlier = '1 2001/01/01 00:00:00'
+ ip = factory.getRandomIPAddress()
+ old_owner = factory.getRandomMACAddress()
+ new_owner = factory.getRandomMACAddress()
+ leases = [
+ self.fake_parsed_lease(ip=ip, mac=new_owner),
+ self.fake_parsed_lease(ip=ip, mac=old_owner, ends=earlier),
+ ]
+ self.assertEqual({ip: new_owner}, gather_leases(leases))
+
+ def test_gather_leases_ignores_host_declarations(self):
+ self.assertEqual({}, gather_leases([self.fake_parsed_host()]))
+
+ def test_gather_hosts_finds_hosts(self):
+ host = self.fake_parsed_host()
+ self.assertEqual({host.ip: host.hardware.mac}, gather_hosts([host]))
+
+ def test_gather_hosts_ignores_unaccompanied_rubouts(self):
+ self.assertEqual({}, gather_hosts([self.fake_parsed_rubout()]))
+
+ def test_gather_hosts_ignores_rubbed_out_entries(self):
+ ip = factory.getRandomIPAddress()
+ hosts = [
+ self.fake_parsed_host(ip=ip),
+ self.fake_parsed_rubout(ip=ip),
+ ]
+ self.assertEqual({}, gather_hosts(hosts))
+
+ def test_gather_hosts_follows_reassigned_host(self):
+ ip = factory.getRandomIPAddress()
+ new_owner = factory.getRandomMACAddress()
+ hosts = [
+ self.fake_parsed_host(ip=ip),
+ self.fake_parsed_rubout(ip=ip),
+ self.fake_parsed_host(ip=ip, mac=new_owner),
+ ]
+ self.assertEqual({ip: new_owner}, gather_hosts(hosts))
+
def test_parse_leases_copes_with_empty_file(self):
self.assertEqual({}, parse_leases(""))
@@ -102,6 +181,28 @@
""" % params))
self.assertEqual({params['ip']: params['mac']}, leases)
+ def test_parse_leases_parses_host(self):
+ params = {
+ 'ip': factory.getRandomIPAddress(),
+ 'mac': factory.getRandomMACAddress(),
+ }
+ leases = parse_leases(dedent("""\
+ host %(ip)s {
+ dynamic;
+ hardware ethernet %(mac)s;
+ fixed-address %(ip)s;
+ }
+ """ % params))
+ self.assertEqual({params['ip']: params['mac']}, leases)
+
+ def test_parse_leases_parses_host_rubout(self):
+ leases = parse_leases(dedent("""\
+ host %s {
+ deleted;
+ }
+ """ % factory.getRandomIPAddress()))
+ self.assertEqual({}, leases)
+
def test_parse_leases_ignores_incomplete_lease_at_end(self):
params = {
'ip': factory.getRandomIPAddress(),
@@ -185,3 +286,88 @@
}
""" % params))
self.assertEqual({params['ip']: params['new_owner']}, leases)
+
+ def test_host_declaration_is_like_an_unexpired_lease(self):
+ params = {
+ 'ip': factory.getRandomIPAddress(),
+ 'mac': factory.getRandomMACAddress(),
+ }
+ leases = parse_leases(dedent("""\
+ host %(ip)s {
+ hardware ethernet %(mac)s;
+ fixed-address %(ip)s;
+ }
+ """ % params))
+ self.assertEqual({params['ip']: params['mac']}, leases)
+
+ def test_combine_entries_accepts_host_followed_by_expired_lease(self):
+ ip = factory.getRandomIPAddress()
+ mac = factory.getRandomMACAddress()
+ earlier = '1 2001/01/01 00:00:00'
+ entries = [
+ self.fake_parsed_host(ip=ip, mac=mac),
+ self.fake_parsed_lease(ip=ip, ends=earlier),
+ ]
+ self.assertEqual({ip: mac}, combine_entries(entries))
+
+ def test_combine_entries_accepts_expired_lease_followed_by_host(self):
+ ip = factory.getRandomIPAddress()
+ mac = factory.getRandomMACAddress()
+ earlier = '1 2001/01/01 00:00:00'
+ entries = [
+ self.fake_parsed_lease(ip=ip, ends=earlier),
+ self.fake_parsed_host(ip=ip, mac=mac),
+ ]
+ self.assertEqual({ip: mac}, combine_entries(entries))
+
+ def test_combine_entries_accepts_rubout_followed_by_current_lease(self):
+ ip = factory.getRandomIPAddress()
+ mac = factory.getRandomMACAddress()
+ entries = [
+ self.fake_parsed_host(ip=ip),
+ self.fake_parsed_rubout(ip=ip),
+ self.fake_parsed_lease(ip=ip, mac=mac),
+ ]
+ self.assertEqual({ip: mac}, combine_entries(entries))
+
+ def test_combine_entries_ignores_rubout_followed_by_expired_lease(self):
+ ip = factory.getRandomIPAddress()
+ mac = factory.getRandomMACAddress()
+ earlier = '1 2001/01/01 00:00:00'
+ entries = [
+ self.fake_parsed_host(ip=ip),
+ self.fake_parsed_rubout(ip=ip),
+ self.fake_parsed_lease(ip=ip, mac=mac, ends=earlier),
+ ]
+ self.assertEqual({}, combine_entries(entries))
+
+ def test_combine_entries_ignores_expired_lease_followed_by_rubout(self):
+ ip = factory.getRandomIPAddress()
+ mac = factory.getRandomMACAddress()
+ earlier = '1 2001/01/01 00:00:00'
+ entries = [
+ self.fake_parsed_host(ip=ip),
+ self.fake_parsed_lease(ip=ip, mac=mac, ends=earlier),
+ self.fake_parsed_rubout(ip=ip),
+ ]
+ self.assertEqual({}, combine_entries(entries))
+
+ def test_combine_entries_accepts_valid_lease_followed_by_rubout(self):
+ ip = factory.getRandomIPAddress()
+ mac = factory.getRandomMACAddress()
+ entries = [
+ self.fake_parsed_host(ip=ip),
+ self.fake_parsed_lease(ip=ip, mac=mac),
+ self.fake_parsed_rubout(ip=ip),
+ ]
+ self.assertEqual({ip: mac}, combine_entries(entries))
+
+ def test_combine_entries_accepts_reassigned_host(self):
+ ip = factory.getRandomIPAddress()
+ mac = factory.getRandomMACAddress()
+ entries = [
+ self.fake_parsed_host(ip=ip),
+ self.fake_parsed_rubout(ip=ip),
+ self.fake_parsed_host(ip=ip, mac=mac),
+ ]
+ self.assertEqual({ip: mac}, combine_entries(entries))
