← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~jtv/maas/parse-leases into lp:maas

 

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/parse-leases into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/parse-leases/+merge/113967

As discussed in today's standup call: this lets us figure out all the current DHCP leases in a node group.  It's not as complete as I'd like it to be, in terms of syntax it supports, but the easiest way to implement it seemed to be a 2008 example by Paul McGuire: http://www.koders.com/python/fidB7EA557CD9669A42C83C931FDB895BE2451DFA36.aspx

It's based on the pyparsing module, which we already have as a dependency through python-celery.  That means we shouldn't need it in HACKING.txt, although of course we do now need it as a direct dependency.

This parser differs from the example I used in several ways:
 * It has unit tests.
 * Regexes were more familiar than pyparsing, so I used those where it was easier.
 * They also allowed me to parse all of a statements arguments as one (which was more convenient here).
 * Most of the statements are irrelevant to our purposes, so I just skip them.
 * Where possible I used Python's standard library, e.g. for date/time parsing.
 * Technically the leases file's keywords are case-insensitive.
 * Comments are now stripped out; the example parsed them as statements.
 * You'll also find the structure of the grammar a bit different, for convenience.

The parser was originally going to live in the leases.py file, but with all the globals etc. it ended up more at home in its own module.

We'll have to do some practical testing to see that it works on real ISC-generated leases files.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/parse-leases/+merge/113967
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/parse-leases into lp:maas.
=== modified file 'src/provisioningserver/dhcp/leases.py'
--- src/provisioningserver/dhcp/leases.py	2012-07-06 05:39:28 +0000
+++ src/provisioningserver/dhcp/leases.py	2012-07-09 12:48:20 +0000
@@ -35,6 +35,7 @@
 from os import stat
 
 from celeryconfig import DHCP_LEASES_FILE
+from provisioningserver.dhcp.leases_parser import parse_leases
 
 # Modification time on last-processed leases file.
 recorded_leases_time = None
@@ -48,21 +49,23 @@
     return stat(DHCP_LEASES_FILE).st_mtime
 
 
-def parse_leases():
+def parse_leases_file():
     """Parse the DHCP leases file.
 
     :return: A tuple: (timestamp, leases).  The `timestamp` is the last
         modification time of the leases file, and `leases` is a dict
         mapping leased IP addresses to their associated MAC addresses.
     """
-    # TODO: Implement leases-file parser here.
+    with open(DHCP_LEASES_FILE) as leases_file:
+        contents = leases_file.read().decode('utf-8')
+    return get_leases_timestamp(), parse_leases(contents)
 
 
 def check_lease_changes():
     """Has the DHCP leases file changed in any significant way?"""
     if get_leases_timestamp() == recorded_leases_time:
         return None
-    timestamp, leases = parse_leases()
+    timestamp, leases = parse_leases_file()
     if leases == recorded_leases:
         return None
     else:
@@ -96,7 +99,7 @@
     server restarts, or zone-file update commands getting lost on their
     way to the DNS server.
     """
-    timestamp, leases = parse_leases()
+    timestamp, leases = parse_leases_file()
     record_lease_state(timestamp, leases)
     send_leases(leases)
 

=== added file 'src/provisioningserver/dhcp/leases_parser.py'
--- src/provisioningserver/dhcp/leases_parser.py	1970-01-01 00:00:00 +0000
+++ src/provisioningserver/dhcp/leases_parser.py	2012-07-09 12:48:20 +0000
@@ -0,0 +1,102 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Parser for ISC dhcpd leases file.
+
+The parser is very minimal.  All we really care about is which IP
+addresses are currently associated with which respective MAC addresses.
+The parser works out no other information than that, and does not
+pretend to parse the full format of the leases file faithfully.
+"""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = [
+    'parse_leases',
+    ]
+
+from datetime import datetime
+
+from pyparsing import (
+    CaselessKeyword,
+    Dict,
+    Group,
+    oneOf,
+    QuotedString,
+    Regex,
+    restOfLine,
+    Suppress,
+    ZeroOrMore,
+    )
+
+
+ip = Regex("[0-9]{1,3}(\.[0-9]{1,3}){3}")
+mac = Regex("[0-9a-fA-F]{2}(:[0-9a-fA-F]{2}){5}")
+hardware_type = Regex('[A-Za-z0-9_-]+')
+args = Regex('[^"{;]+') | QuotedString('"')
+expiry = Regex('[0-9]\s+[0-9/-]+\s+[0-9:]+') | 'never'
+
+hardware = CaselessKeyword("hardware") + hardware_type("type") + mac("mac")
+ends = CaselessKeyword("ends") + expiry("expiry")
+other_statement = (
+    oneOf(
+        ['starts', 'tstp', 'tsfp', 'uid', 'binding'], caseless=True) +
+    args
+    )
+
+lease_statement = (hardware | ends | other_statement) + Suppress(';')
+lease_parser = (
+    CaselessKeyword("lease") + ip("ip") +
+    Suppress('{') +
+    Dict(ZeroOrMore(Group(lease_statement))) +
+    Suppress('}')
+    )
+lease_parser.ignore('#' + restOfLine)
+
+
+def get_expiry_date(lease):
+    """Get the expiry date for a lease, if any.
+
+    :param lease: A lease as returned by the parser.
+    :return: A UTC-based timestamp representing the lease's moment of
+        expiry, or None if the lease has no expiry date.
+    """
+    ends = getattr(lease, 'ends', None)
+    if ends is None or len(ends) == 0 or ends.lower() == 'never':
+        return None
+    else:
+        return datetime.strptime(ends, '%w %Y/%m/%d %H:%M:%S')
+
+
+def has_expired(lease, now):
+    """Has `lease` expired?
+
+    :param lease: A lease as returned by the parser.
+    :param now: The current UTC-based timestamp to check expiry against.
+    :return: Whether the lease has expired.
+    """
+    expiry_date = get_expiry_date(lease)
+    if expiry_date is None:
+        return False
+    else:
+        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.
+    """
+    now = datetime.utcnow()
+    leases = lease_parser.searchString(leases_contents)
+    return {
+        lease.ip: lease.hardware.mac
+        for lease in leases
+            if not has_expired(lease, now)}

=== modified file 'src/provisioningserver/dhcp/tests/test_leases.py'
--- src/provisioningserver/dhcp/tests/test_leases.py	2012-07-06 05:26:28 +0000
+++ src/provisioningserver/dhcp/tests/test_leases.py	2012-07-09 12:48:20 +0000
@@ -16,6 +16,7 @@
     datetime,
     timedelta,
     )
+from textwrap import dedent
 
 from maastesting.factory import factory
 from maastesting.fakemethod import FakeMethod
@@ -27,6 +28,7 @@
 from provisioningserver.dhcp import leases as leases_module
 from provisioningserver.dhcp.leases import (
     check_lease_changes,
+    parse_leases_file,
     record_lease_state,
     update_leases,
     upload_leases,
@@ -48,8 +50,16 @@
         """Create a leases dict with one, arbitrary lease in it."""
         return {factory.getRandomIPAddress(): factory.getRandomMACAddress()}
 
+    def redirect_parser(self, path):
+        """Make the leases parser read from a file at `path`."""
+        self.patch(leases_module, 'DHCP_LEASES_FILE', path)
+
     def fake_leases_file(self, leases=None, age=None):
-        """Create a fake leases file.
+        """Fake the presence of a leases file.
+
+        This does not go through the leases parser.  It patches out the
+        leases parser with a fake that returns the lease data you pass in
+        here.
 
         :param leases: Dict of leases (mapping IP addresses to MACs).
         :param age: Number of seconds since last modification to leases file.
@@ -62,10 +72,22 @@
         if age is not None:
             age_file(leases_file, age)
         timestamp = get_write_time(leases_file)
-        self.patch(leases_module, 'DHCP_LEASES_FILE', leases_file)
-        # TODO: We don't have a lease-file parser yet.  For now, just
-        # fake up a "parser" that returns the given data.
-        self.patch(leases_module, 'parse_leases', lambda: (timestamp, leases))
+        self.redirect_parser(leases_file)
+        self.patch(
+            leases_module, 'parse_leases_file', lambda: (timestamp, leases))
+        return leases_file
+
+    def write_leases_file(self, contents):
+        """Create a leases file, and cause the parser to read from it.
+
+        This patches out the leases parser to read from the new file.
+
+        :param contents: Text contents for the leases file.
+        :return: Path of temporary leases file.
+        """
+        leases_file = self.make_file(
+            contents=dedent(contents).encode('utf-8'))
+        self.redirect_parser(leases_file)
         return leases_file
 
     def test_check_lease_changes_returns_tuple_if_no_state_cached(self):
@@ -90,7 +112,7 @@
     def test_check_lease_changes_does_not_parse_unchanged_leases_file(self):
         parser = FakeMethod()
         leases_file = self.fake_leases_file()
-        self.patch(leases_module, 'parse_leases', parser)
+        self.patch(leases_module, 'parse_leases_file', parser)
         record_lease_state(get_write_time(leases_file), {})
         update_leases()
         self.assertSequenceEqual([], parser.calls)
@@ -186,3 +208,22 @@
         except StopExecuting:
             pass
         self.assertIsNone(check_lease_changes())
+
+    def test_parse_leases_file_parses_leases(self):
+        params = {
+            'ip': factory.getRandomIPAddress(),
+            'mac': factory.getRandomMACAddress(),
+        }
+        leases_file = self.write_leases_file("""\
+            lease %(ip)s {
+                starts 5 2010/01/01 00:00:01;
+                ends never;
+                tstp 6 2010/01/02 05:00:00;
+                tsfp 6 2010/01/02 05:00:00;
+                binding state free;
+                hardware ethernet %(mac)s;
+            }
+            """ % params)
+        self.assertEqual(
+            (get_write_time(leases_file), {params['ip']: params['mac']}),
+            parse_leases_file())

=== added file 'src/provisioningserver/dhcp/tests/test_leases_parser.py'
--- src/provisioningserver/dhcp/tests/test_leases_parser.py	1970-01-01 00:00:00 +0000
+++ src/provisioningserver/dhcp/tests/test_leases_parser.py	2012-07-09 12:48:20 +0000
@@ -0,0 +1,170 @@
+# Copyright 2012 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""..."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+__metaclass__ = type
+__all__ = []
+
+from collections import namedtuple
+from datetime import datetime
+from textwrap import dedent
+
+from maastesting.factory import factory
+from maastesting.testcase import TestCase
+from provisioningserver.dhcp.leases_parser import (
+    get_expiry_date,
+    has_expired,
+    parse_leases,
+    )
+
+
+class TestLeasesParser(TestCase):
+
+    def fake_parsed_lease(self, ip=None, mac=None, ends=None):
+        """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)
+
+    def test_get_expiry_date_parses_expiry_date(self):
+        lease = self.fake_parsed_lease(ends='0 2011/01/02 03:04:05')
+        self.assertEqual(
+            datetime(
+                year=2011, month=01, day=02,
+                hour=03, minute=04, second=05),
+            get_expiry_date(lease))
+
+    def test_get_expiry_date_returns_None_for_never(self):
+        self.assertIsNone(
+            get_expiry_date(self.fake_parsed_lease(ends='never')))
+
+    def test_get_expiry_date_returns_None_if_no_expiry_given(self):
+        self.assertIsNone(get_expiry_date(self.fake_parsed_lease(ends=None)))
+
+    def test_has_expired_returns_False_for_eternal_lease(self):
+        now = datetime.utcnow()
+        self.assertFalse(has_expired(self.fake_parsed_lease(ends=None), now))
+
+    def test_has_expired_returns_False_for_future_expiry_date(self):
+        now = datetime.utcnow()
+        later = '1 2035/12/31 23:59:59'
+        self.assertFalse(has_expired(self.fake_parsed_lease(ends=later), now))
+
+    def test_has_expired_returns_True_for_past_expiry_date(self):
+        now = datetime.utcnow()
+        earlier = '1 2001/01/01 00:00:00'
+        self.assertTrue(
+            has_expired(self.fake_parsed_lease(ends=earlier), now))
+
+    def test_parse_leases_copes_with_empty_file(self):
+        self.assertEqual({}, parse_leases(""))
+
+    def test_parse_leases_parses_lease(self):
+        params = {
+            'ip': factory.getRandomIPAddress(),
+            'mac': factory.getRandomMACAddress(),
+        }
+        leases = parse_leases(dedent("""\
+            lease %(ip)s {
+                starts 5 2010/01/01 00:00:01;
+                ends never;
+                tstp 6 2010/01/02 05:00:00;
+                tsfp 6 2010/01/02 05:00:00;
+                binding state free;
+                hardware ethernet %(mac)s;
+            }
+            """ % params))
+        self.assertEqual({params['ip']: params['mac']}, leases)
+
+    def test_parse_leases_ignores_incomplete_lease_at_end(self):
+        params = {
+            'ip': factory.getRandomIPAddress(),
+            'mac': factory.getRandomMACAddress(),
+            'incomplete_ip': factory.getRandomIPAddress(),
+        }
+        leases = parse_leases(dedent("""\
+            lease %(ip)s {
+                hardware ethernet %(mac)s;
+            }
+            lease %(incomplete_ip)s {
+                starts 5 2010/01/01 00:00:05;
+            """ % params))
+        self.assertEqual({params['ip']: params['mac']}, leases)
+
+    def test_parse_leases_ignores_comments(self):
+        params = {
+            'ip': factory.getRandomIPAddress(),
+            'mac': factory.getRandomMACAddress(),
+        }
+        leases = parse_leases(dedent("""\
+            # Top comment (ignored).
+            lease %(ip)s { # End-of-line comment (ignored).
+                # Comment in lease block (ignored).
+                hardware ethernet %(mac)s;  # EOL comment in lease (ignored).
+            } # Comment right after closing brace (ignored).
+            # End comment (ignored).
+            """ % params))
+        self.assertEqual({params['ip']: params['mac']}, leases)
+
+    def test_parse_leases_ignores_expired_leases(self):
+        params = {
+            'ip': factory.getRandomIPAddress(),
+            'mac': factory.getRandomMACAddress(),
+        }
+        leases = parse_leases(dedent("""\
+            lease %(ip)s {
+                hardware ethernet %(mac)s;
+                ends 1 2001/01/01 00:00:00;
+            }
+            """ % params))
+        self.assertEqual({}, leases)
+
+    def test_parse_leases_treats_never_as_eternity(self):
+        params = {
+            'ip': factory.getRandomIPAddress(),
+            'mac': factory.getRandomMACAddress(),
+        }
+        leases = parse_leases(dedent("""\
+            lease %(ip)s {
+                hardware ethernet %(mac)s;
+                ends never;
+            }
+            """ % params))
+        self.assertEqual({params['ip']: params['mac']}, leases)
+
+    def test_parse_leases_treats_missing_end_date_as_eternity(self):
+        params = {
+            'ip': factory.getRandomIPAddress(),
+            'mac': factory.getRandomMACAddress(),
+        }
+        leases = parse_leases(dedent("""\
+            lease %(ip)s {
+                hardware ethernet %(mac)s;
+            }
+            """ % params))
+        self.assertEqual({params['ip']: params['mac']}, leases)
+
+    def test_parse_leases_takes_latest_lease_for_address(self):
+        params = {
+            'ip': factory.getRandomIPAddress(),
+            'old_owner': factory.getRandomMACAddress(),
+            'new_owner': factory.getRandomMACAddress(),
+        }
+        leases = parse_leases(dedent("""\
+            lease %(ip)s {
+                hardware ethernet %(old_owner)s;
+            }
+            lease %(ip)s {
+                hardware ethernet %(new_owner)s;
+            }
+            """ % params))
+        self.assertEqual({params['ip']: params['new_owner']}, leases)