=== modified file 'src/maasserver/rpc/leases.py' --- src/maasserver/rpc/leases.py 2015-08-27 16:07:30 +0000 +++ src/maasserver/rpc/leases.py 2017-02-07 17:35:36 +0000 @@ -16,6 +16,7 @@ "update_leases", ] +from maasserver.dns.config import dns_update_all_zones from maasserver.models.nodegroup import NodeGroup from maasserver.models.staticipaddress import StaticIPAddress from maasserver.utils.orm import transactional @@ -49,4 +50,5 @@ else: leases = convert_mappings_to_leases(mappings) StaticIPAddress.objects.update_leases(nodegroup, leases) + dns_update_all_zones() return {} === modified file 'src/maasserver/rpc/tests/test_regionservice.py' --- src/maasserver/rpc/tests/test_regionservice.py 2015-11-06 16:27:35 +0000 +++ src/maasserver/rpc/tests/test_regionservice.py 2017-02-07 17:35:36 +0000 @@ -57,6 +57,7 @@ from maasserver.models.nodegroup import NodeGroup from maasserver.rpc import ( events as events_module, + leases as leases_module, regionservice, ) from maasserver.rpc.regionservice import ( @@ -84,6 +85,7 @@ from maasserver.utils.threads import deferToDatabase from maastesting.djangotestcase import DjangoTransactionTestCase from maastesting.matchers import ( + MockCalledOnce, MockCalledOnceWith, MockCallsMatch, MockNotCalled, @@ -424,9 +426,11 @@ @wait_for_reactor @inlineCallbacks - def test__stores_leases(self): + def test__stores_leases_and_updates_dns(self): + dns_update = self.patch(leases_module, 'dns_update_all_zones') interface, ngi, nodegroup = yield deferToDatabase( self.make_interface_on_managed_cluster_interface) + mapping = { "ip": ngi.ip_range_low, "mac": interface.mac_address.get_raw(), @@ -436,6 +440,7 @@ b"uuid": nodegroup.uuid, b"mappings": [mapping]}) self.assertThat(response, Equals({})) + self.assertThat(dns_update, MockCalledOnce()) [(ip, mac)] = yield deferToDatabase( self.get_leases_for, nodegroup=nodegroup) === modified file 'src/provisioningserver/dhcp/leases_parser.py' --- src/provisioningserver/dhcp/leases_parser.py 2016-05-11 18:50:52 +0000 +++ src/provisioningserver/dhcp/leases_parser.py 2017-02-07 17:35:36 +0000 @@ -38,7 +38,9 @@ ) -ip = Regex("[:0-9a-fA-F][:.0-9a-fA-F]{2,38}") +LEASE_TIME_FORMAT = '%w %Y/%m/%d %H:%M:%S' + +ip = Regex("[:0-9a-fA-F][-:.0-9a-fA-F]{2,38}") mac = Regex("[0-9a-fA-F]{2}(:[0-9a-fA-F]{2}){5}") hardware_type = Regex('[A-Za-z0-9_-]+') args = Regex('[^"{;]+') | QuotedString('"') @@ -125,11 +127,20 @@ 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': + end_time = getattr(lease, 'ends', '') + if end_time is None or len(end_time) == 0: + binding_state = getattr(lease, 'binding state', '') + binding_state = binding_state.lower() + if binding_state == 'free': + # For a 'free' lease, the release time is the 'starts' time. + start_time = getattr(lease, 'starts', '') + if start_time is None or len(start_time) == 0: + return None + return datetime.strptime(start_time, LEASE_TIME_FORMAT) + elif end_time.lower() == 'never': return None else: - return datetime.strptime(ends, '%w %Y/%m/%d %H:%M:%S') + return datetime.strptime(end_time, LEASE_TIME_FORMAT) def has_expired(lease, now): @@ -153,13 +164,17 @@ def gather_leases(hosts_and_leases): """Find current leases among `hosts_and_leases`.""" now = datetime.utcnow() - # If multiple leases for the same address are valid at the same - # time, for whatever reason, the list will contain all of them. - return [ - (lease.host, lease.hardware.mac) - for lease in filter(is_lease, hosts_and_leases) - if not has_expired(lease, now) - ] + # Ensure we have the most recent MAC leased to each IP address. + leases = OrderedDict() + for lease in filter(is_lease, hosts_and_leases): + lease_mac = get_lease_mac(lease) + lease_ip = lease.host + if not has_expired(lease, now) and lease_mac is not None: + leases[lease_ip] = lease_mac + else: + if lease_ip in leases: + del leases[lease_ip] + return [(ip, mac) for ip, mac in leases.items()] def get_host_mac(host): @@ -173,6 +188,12 @@ return None else: return host + # In this case, it's stored the same way a lease MAC is stored. + return get_lease_mac(host) + + +def get_lease_mac(host): + """Get the MAC address from a lease declaration.""" hardware = getattr(host, 'hardware', None) if hardware in (None, '', b''): return None === modified file 'src/provisioningserver/dhcp/leases_parser_fast.py' --- src/provisioningserver/dhcp/leases_parser_fast.py 2015-09-09 15:02:30 +0000 +++ src/provisioningserver/dhcp/leases_parser_fast.py 2017-02-07 17:35:36 +0000 @@ -23,7 +23,10 @@ 'parse_leases', ] -from collections import defaultdict +from collections import ( + defaultdict, + OrderedDict, +) from datetime import datetime from itertools import chain import re @@ -31,6 +34,7 @@ from provisioningserver.dhcp.leases_parser import ( get_host_ip, get_host_mac, + get_lease_mac, has_expired, is_host, is_lease, @@ -43,7 +47,7 @@ ^\s* # Ignore leading whitespace on each line. (host|lease) # Look only for host or lease stanzas. \s+ # Mandatory whitespace. - ([0-9a-fA-F.:]+) # Capture the IP/MAC address for this stanza. + ([0-9a-fA-F.:-]+) # Capture the IP/MAC address for this stanza. \s*{ # Optional whitespace then an opening brace. ''', re.MULTILINE | re.DOTALL | re.VERBOSE) @@ -71,15 +75,29 @@ def parse_leases(leases_contents): - results = [] + leases = OrderedDict() + hosts = [] now = datetime.utcnow() for entry in extract_leases(leases_contents): if is_lease(entry): - if not has_expired(entry, now): - results.append((entry.host, entry.hardware.mac)) + mac = get_lease_mac(entry) + if not has_expired(entry, now) and mac is not None: + leases[entry.host] = entry.hardware.mac + else: + # Expired or released lease. + if entry.host in leases: + del leases[entry.host] elif is_host(entry): mac = get_host_mac(entry) ip = get_host_ip(entry) if ip and mac: - results.append((ip, mac)) + # A host entry came later than a lease entry for the same IP + # address. Letting them both stay will confuse MAAS by allowing + # an ephemeral lease to exist at the same time as a static + # host map. + if ip in leases: + del leases[ip] + hosts.append((ip, mac)) + results = leases.items() + results.extend(hosts) return results === modified file 'src/provisioningserver/dhcp/tests/test_leases_parser.py' --- src/provisioningserver/dhcp/tests/test_leases_parser.py 2016-05-11 18:50:52 +0000 +++ src/provisioningserver/dhcp/tests/test_leases_parser.py 2017-02-07 17:35:36 +0000 @@ -41,26 +41,32 @@ class Lease(object): - def __init__(self, lease_or_host, host, fixed_address, hardware, ends): + def __init__( + self, lease_or_host, host, fixed_address, hardware, starts, ends, + binding_state=None): self.lease_or_host = lease_or_host self.host = host self.hardware = hardware + self.starts = starts setattr(self, 'fixed-address', fixed_address) self.ends = ends + if binding_state is not None: + setattr(self, 'binding state', binding_state) def __iter__(self): return iter(self.__dict__.keys()) -def fake_parsed_lease(ip=None, mac=None, ends=None, - entry_type='lease'): +def fake_parsed_lease(ip=None, mac=None, starts=None, ends=None, + entry_type='lease', state=None): """Fake a lease as produced by the parser.""" if ip is None: ip = factory.make_ipv4_address() if mac is None: mac = factory.make_mac_address() Hardware = namedtuple('Hardware', ['mac']) - lease = Lease(entry_type, ip, ip, Hardware(mac), ends) + lease = Lease( + entry_type, ip, ip, Hardware(mac), starts, ends, binding_state=state) return lease @@ -312,7 +318,13 @@ """ % params)) self.assertEqual([(params['ip'], params['mac'])], leases) - def test_parse_leases_takes_all_leases_for_address(self): + def test_parse_leases_takes_current_lease_for_address(self): + # Note: a previous version of this test case checked that two leases + # can exist at the same time for a single IP address. This has been + # removed in order to prevent stale entries from polluting MAAS's + # discoveries with old information. To support bonds, static host + # mappings will still allow more than one MAC address to be assigned to + # the same IP address. params = { 'ip': factory.make_ipv4_address(), 'old_owner': factory.make_mac_address(), @@ -321,15 +333,35 @@ leases = self.parse(dedent("""\ lease %(ip)s { hardware ethernet %(old_owner)s; + ends 0 1990/01/01 00:00:00; } lease %(ip)s { hardware ethernet %(new_owner)s; } """ % params)) self.assertEqual( - [(params['ip'], params['old_owner']), - (params['ip'], params['new_owner'])], - leases) + [(params['ip'], params['new_owner'])], leases) + + def test_multiple_host_declarations_are_reported(self): + params = { + 'ip': factory.make_ipv4_address(), + 'bondmac1': factory.make_mac_address(), + 'bondmac2': factory.make_mac_address(), + } + leases = self.parse(dedent("""\ + host %(bondmac1)s { + hardware ethernet %(bondmac1)s; + fixed-address %(ip)s; + } + host %(bondmac2)s { + hardware ethernet %(bondmac2)s; + fixed-address %(ip)s; + } + """ % params)) + self.assertEqual([ + (params['ip'], params['bondmac1']), + (params['ip'], params['bondmac2']) + ], leases) def test_parse_leases_recognizes_host_deleted_statement_as_rubout(self): params = { @@ -362,6 +394,39 @@ class TestLeasesParserFast(MAASTestCase): + def test_handles_dash_separator_for_host_mapping(self): + ip = factory.make_ipv4_address() + mac = factory.make_mac_address() + mac_dash = mac.replace(":", "-") + leases = leases_parser_fast.parse_leases(dedent("""\ + host %s { + hardware ethernet %s; + fixed-address %s; + } + """ % (mac_dash, mac, ip))) + self.assertEqual([(ip, mac)], leases) + + def test_multiple_host_declarations_are_reported(self): + params = { + 'ip': factory.make_ipv4_address(), + 'bondmac1': factory.make_mac_address(), + 'bondmac2': factory.make_mac_address(), + } + leases = leases_parser_fast.parse_leases(dedent("""\ + host %(bondmac1)s { + hardware ethernet %(bondmac1)s; + fixed-address %(ip)s; + } + host %(bondmac2)s { + hardware ethernet %(bondmac2)s; + fixed-address %(ip)s; + } + """ % params)) + self.assertEqual([ + (params['ip'], params['bondmac1']), + (params['ip'], params['bondmac2']) + ], leases) + def test_expired_lease_does_not_shadow_earlier_host_stanza(self): params = { 'ip': factory.make_ipv4_address(), @@ -402,33 +467,55 @@ } """ % params)) # The lease hasn't expired, so shadows the earlier host stanza. - self.assertEqual( - [(params["ip"], params["mac1"]), (params["ip"], params["mac2"])], - leases) - - def test_host_stanza_shadows_earlier_active_lease(self): - params = { - 'ip': factory.make_ipv4_address(), - 'mac1': factory.make_mac_address(), - 'mac2': factory.make_mac_address(), - } - leases = leases_parser_fast.parse_leases(dedent("""\ - lease %(ip)s { - starts 5 2010/01/01 00:00:01; - hardware ethernet %(mac2)s; - } - host %(ip)s { - dynamic; - hardware ethernet %(mac1)s; - fixed-address %(ip)s; - } - """ % params)) - # The lease hasn't expired, but the host entry is later, so it - # shadows the earlier lease stanza. + # (But since the host mapping has not been removed, it takes precedence + # by coming later in the list, since that should better describe the + # intent of the MAAS administrator.) self.assertEqual( [(params["ip"], params["mac2"]), (params["ip"], params["mac1"])], leases) + def test_host_stanza_replaces_earlier_active_lease(self): + params = { + 'ip': factory.make_ipv4_address(), + 'mac1': factory.make_mac_address(), + 'mac2': factory.make_mac_address(), + } + leases = leases_parser_fast.parse_leases(dedent("""\ + lease %(ip)s { + starts 5 2010/01/01 00:00:01; + hardware ethernet %(mac2)s; + } + host %(ip)s { + dynamic; + hardware ethernet %(mac1)s; + fixed-address %(ip)s; + } + """ % params)) + # The lease hasn't expired, but the host entry is later. So the host + # mapping takes precedence. + self.assertEqual( + [(params["ip"], params["mac1"])], leases) + + def test_released_lease_with_no_end_time_is_released(self): + params = { + 'ip': factory.make_ipv4_address(), + 'mac1': factory.make_mac_address(), + 'mac2': factory.make_mac_address(), + } + leases = leases_parser_fast.parse_leases(dedent("""\ + lease %(ip)s { + starts 5 2010/01/01 00:00:01; + hardware ethernet %(mac2)s; + } + lease %(ip)s { + dynamic; + binding state free; + starts 0 1990/01/01 00:00:00; + } + """ % params)) + # The lease was added and then removed, so we expect a no-op. + self.assertEqual([], leases) + class TestLeasesParserFunctions(MAASTestCase): @@ -440,6 +527,14 @@ hour=03, minute=04, second=05), get_expiry_date(lease)) + def test_get_expiry_date_uses_start_date_for_free_lease(self): + lease = fake_parsed_lease(starts='0 2011/01/02 03:04:05', state='free') + 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(fake_parsed_lease(ends='never'))) @@ -492,7 +587,7 @@ ] self.assertEqual([(ip, new_owner)], gather_leases(leases)) - def test_gather_leases_ignores_ordering(self): + def test_ordering_is_important_to_gather_leases(self): earlier = '1 2001/01/01 00:00:00' ip = factory.make_ipv4_address() old_owner = factory.make_mac_address() @@ -501,7 +596,7 @@ fake_parsed_lease(ip=ip, mac=new_owner), fake_parsed_lease(ip=ip, mac=old_owner, ends=earlier), ] - self.assertEqual([(ip, new_owner)], gather_leases(leases)) + self.assertEqual([], gather_leases(leases)) def test_gather_leases_ignores_host_declarations(self): self.assertEqual([], gather_leases([fake_parsed_host()]))