Merge lp:~mpontillo/maas/lease-parser-ignore-free-leases--1.9 into lp:maas/1.9

Proposed by Mike Pontillo on 2017-02-03
Status: Merged
Approved by: Mike Pontillo on 2017-02-17
Approved revision: 4605
Merged at revision: 4597
Proposed branch: lp:~mpontillo/maas/lease-parser-ignore-free-leases--1.9
Merge into: lp:maas/1.9
Diff against target: 449 lines (+192/-51)
5 files modified
src/maasserver/rpc/leases.py (+2/-0)
src/maasserver/rpc/tests/test_regionservice.py (+6/-1)
src/provisioningserver/dhcp/leases_parser.py (+32/-11)
src/provisioningserver/dhcp/leases_parser_fast.py (+24/-6)
src/provisioningserver/dhcp/tests/test_leases_parser.py (+128/-33)
To merge this branch: bzr merge lp:~mpontillo/maas/lease-parser-ignore-free-leases--1.9
Reviewer Review Type Date Requested Status
Blake Rouse (community) 2017-02-03 Approve on 2017-02-07
Review via email: mp+316300@code.launchpad.net

Commit message

DHCP lease parser fixes.

 * Fix improper handling of DHCP leases with a 'free' binding state,
   and/or no 'ends' time.
 * Fix lease parser to properly parse static host mappings with a '-'
   in the mapping key (such as the entries that MAAS itself writes).
 * Fix DNS zones to be updated when lease parser finishes and updates
   the region.
 * Fix lease parser to properly remove released leases.
 * Fix traceback that could occur when the fast lease parser attempts
   interpret a released lease.

To post a comment you must log in.
Mike Pontillo (mpontillo) wrote :

The way this code works is subtle. There is a difference between a "free" DHCP lease (free because it has not been renewed) and a "released" DHCP lease. And that difference is that when you don't explicitly release the address, you see something like:

lease 172.16.99.101 {
  starts 5 2017/02/03 02:05:01;
  ends 5 2017/02/03 02:05:31;
  tstp 5 2017/02/03 02:05:31;
  cltt 5 2017/02/03 02:05:01;
  binding state free;
  hardware ethernet 52:54:00:3d:e7:29;
}

So the lease is still valid, in truth, in the above case. (because it has not been renewed.) An explicitly released address will look like this:

lease 172.16.99.102 {
  starts 5 2017/02/03 02:06:06;
  cltt 5 2017/02/03 02:06:06;
  binding state free;
  hardware ethernet 52:54:00:8f:10:eb;
}

Notice that there is no 'ends' time.

So the root cause of the bug is: a free lease without an 'ends' time was considered an 'infinite' lease. This meant MAAS confusion*, because if IP address X was leased to host 'foo' and then explicitly released (and therefore did not have an 'ends' time, because the lease was cut short), and later host 'bar' leases IP address X, it will appear that two hosts have leased the same IP address. Ouch,

*: Bad pun, sorry.

Mike Pontillo (mpontillo) wrote :

To prevent the possibility of stale leases remaining in the StaticIPAddress table as DISCOVERED, an additional fix was needed to filter stale leases (in case an IP address was assigned to another host).

One issue remains that I found in testing: after the lease parser runs, the zone file isn't udpated. (Rather, I saw it get updated ~3 seconds before the lease parser finished and uploaded the leases.) Therefore I had an out-of-date zone file even though the lease parser had already determined the correct (hostname, ip) mapping, and that mapping was shown in the UI. (Workaround: `service maas-regiond restart`.)

Blake Rouse (blake-rouse) wrote :

Looks good. As for the restart of maas-regiond. Did it never get updated? As without it updating the zone files issues will still exists resolving dynamic IP addresses.

review: Approve
Mike Pontillo (mpontillo) wrote :

Blake, thanks for taking a look. It turned out there was a separate issue with BIND that also prevented the DNS zones from being properly updated. LaMont has landed a fix (and has queued a corresponding SRU for BIND).

Blake Rouse (blake-rouse) wrote :

Awesome. Glad this is resolved. Looks good!

review: Approve
MAAS Lander (maas-lander) wrote :
Download full text (1.3 MiB)

The attempt to merge lp:~mpontillo/maas/lease-parser-ignore-free-leases--1.9 into lp:maas/1.9 failed. Below is the output from the failed tests.

Ign http://prodstack-zone-1.clouds.archive.ubuntu.com trusty InRelease
Get:1 http://security.ubuntu.com trusty-security InRelease [65.9 kB]
Get:2 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates InRelease [65.9 kB]
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports InRelease
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty Release.gpg
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty Release
Get:3 http://security.ubuntu.com trusty-security/main Sources [124 kB]
Get:4 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/main Sources [390 kB]
Get:5 http://security.ubuntu.com trusty-security/universe Sources [49.3 kB]
Get:6 http://security.ubuntu.com trusty-security/main amd64 Packages [578 kB]
Get:7 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/restricted Sources [5,911 B]
Get:8 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/universe Sources [173 kB]
Get:9 http://security.ubuntu.com trusty-security/universe amd64 Packages [151 kB]
Get:10 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/multiverse Sources [7,535 B]
Get:11 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [943 kB]
Get:12 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/restricted amd64 Packages [16.4 kB]
Get:13 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [397 kB]
Get:14 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/multiverse amd64 Packages [14.0 kB]
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/main Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/restricted Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/universe Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/multiverse Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/main amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/restricted amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/universe amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/multiverse amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/main Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/restricted Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/universe Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/multiverse Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/restricted amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/multiverse amd64 Packages
Fetched 2,982 kB in 1s (1,802 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive...

MAAS Lander (maas-lander) wrote :
Download full text (1.3 MiB)

The attempt to merge lp:~mpontillo/maas/lease-parser-ignore-free-leases--1.9 into lp:maas/1.9 failed. Below is the output from the failed tests.

Ign http://prodstack-zone-1.clouds.archive.ubuntu.com trusty InRelease
Get:1 http://security.ubuntu.com trusty-security InRelease [65.9 kB]
Get:2 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates InRelease [65.9 kB]
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports InRelease
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty Release.gpg
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty Release
Get:3 http://security.ubuntu.com trusty-security/main Sources [124 kB]
Get:4 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/main Sources [390 kB]
Get:5 http://security.ubuntu.com trusty-security/universe Sources [49.3 kB]
Get:6 http://security.ubuntu.com trusty-security/main amd64 Packages [578 kB]
Get:7 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/restricted Sources [5,911 B]
Get:8 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/universe Sources [173 kB]
Get:9 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/multiverse Sources [7,535 B]
Get:10 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [943 kB]
Get:11 http://security.ubuntu.com trusty-security/universe amd64 Packages [151 kB]
Get:12 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/restricted amd64 Packages [16.4 kB]
Get:13 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [397 kB]
Get:14 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/multiverse amd64 Packages [14.0 kB]
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/main Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/restricted Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/universe Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/multiverse Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/main amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/restricted amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/universe amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/multiverse amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/main Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/restricted Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/universe Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/multiverse Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/restricted amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/multiverse amd64 Packages
Fetched 2,982 kB in 1s (1,718 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive...

Mike Pontillo (mpontillo) wrote :

Hm. I don't understand why this keeps failing. I just SSH'd to the lander and tried the same test, and it passed.

MAAS Lander (maas-lander) wrote :
Download full text (1.4 MiB)

The attempt to merge lp:~mpontillo/maas/lease-parser-ignore-free-leases--1.9 into lp:maas/1.9 failed. Below is the output from the failed tests.

Get:1 http://security.ubuntu.com trusty-security InRelease [65.9 kB]
Get:2 http://security.ubuntu.com trusty-security/main Sources [124 kB]
Get:3 http://security.ubuntu.com trusty-security/universe Sources [49.3 kB]
Get:4 http://security.ubuntu.com trusty-security/main amd64 Packages [578 kB]
Get:5 http://security.ubuntu.com trusty-security/universe amd64 Packages [151 kB]
Ign http://prodstack-zone-1.clouds.archive.ubuntu.com trusty InRelease
Get:6 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates InRelease [65.9 kB]
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports InRelease
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty Release.gpg
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty Release
Get:7 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/main Sources [390 kB]
Get:8 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/restricted Sources [5,911 B]
Get:9 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/universe Sources [173 kB]
Get:10 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/multiverse Sources [7,535 B]
Get:11 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [943 kB]
Get:12 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/restricted amd64 Packages [16.4 kB]
Get:13 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [397 kB]
Get:14 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/multiverse amd64 Packages [14.0 kB]
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/main Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/restricted Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/universe Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/multiverse Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/main amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/restricted amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/universe amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/multiverse amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/main Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/restricted Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/universe Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/multiverse Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/restricted amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/multiverse amd64 Packages
Fetched 2,982 kB in 2s (1,104 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/rpc/leases.py'
2--- src/maasserver/rpc/leases.py 2015-08-27 16:07:30 +0000
3+++ src/maasserver/rpc/leases.py 2017-02-07 17:35:36 +0000
4@@ -16,6 +16,7 @@
5 "update_leases",
6 ]
7
8+from maasserver.dns.config import dns_update_all_zones
9 from maasserver.models.nodegroup import NodeGroup
10 from maasserver.models.staticipaddress import StaticIPAddress
11 from maasserver.utils.orm import transactional
12@@ -49,4 +50,5 @@
13 else:
14 leases = convert_mappings_to_leases(mappings)
15 StaticIPAddress.objects.update_leases(nodegroup, leases)
16+ dns_update_all_zones()
17 return {}
18
19=== modified file 'src/maasserver/rpc/tests/test_regionservice.py'
20--- src/maasserver/rpc/tests/test_regionservice.py 2015-11-06 16:27:35 +0000
21+++ src/maasserver/rpc/tests/test_regionservice.py 2017-02-07 17:35:36 +0000
22@@ -57,6 +57,7 @@
23 from maasserver.models.nodegroup import NodeGroup
24 from maasserver.rpc import (
25 events as events_module,
26+ leases as leases_module,
27 regionservice,
28 )
29 from maasserver.rpc.regionservice import (
30@@ -84,6 +85,7 @@
31 from maasserver.utils.threads import deferToDatabase
32 from maastesting.djangotestcase import DjangoTransactionTestCase
33 from maastesting.matchers import (
34+ MockCalledOnce,
35 MockCalledOnceWith,
36 MockCallsMatch,
37 MockNotCalled,
38@@ -424,9 +426,11 @@
39
40 @wait_for_reactor
41 @inlineCallbacks
42- def test__stores_leases(self):
43+ def test__stores_leases_and_updates_dns(self):
44+ dns_update = self.patch(leases_module, 'dns_update_all_zones')
45 interface, ngi, nodegroup = yield deferToDatabase(
46 self.make_interface_on_managed_cluster_interface)
47+
48 mapping = {
49 "ip": ngi.ip_range_low,
50 "mac": interface.mac_address.get_raw(),
51@@ -436,6 +440,7 @@
52 b"uuid": nodegroup.uuid, b"mappings": [mapping]})
53
54 self.assertThat(response, Equals({}))
55+ self.assertThat(dns_update, MockCalledOnce())
56
57 [(ip, mac)] = yield deferToDatabase(
58 self.get_leases_for, nodegroup=nodegroup)
59
60=== modified file 'src/provisioningserver/dhcp/leases_parser.py'
61--- src/provisioningserver/dhcp/leases_parser.py 2016-05-11 18:50:52 +0000
62+++ src/provisioningserver/dhcp/leases_parser.py 2017-02-07 17:35:36 +0000
63@@ -38,7 +38,9 @@
64 )
65
66
67-ip = Regex("[:0-9a-fA-F][:.0-9a-fA-F]{2,38}")
68+LEASE_TIME_FORMAT = '%w %Y/%m/%d %H:%M:%S'
69+
70+ip = Regex("[:0-9a-fA-F][-:.0-9a-fA-F]{2,38}")
71 mac = Regex("[0-9a-fA-F]{2}(:[0-9a-fA-F]{2}){5}")
72 hardware_type = Regex('[A-Za-z0-9_-]+')
73 args = Regex('[^"{;]+') | QuotedString('"')
74@@ -125,11 +127,20 @@
75 expiry, or None if the lease has no expiry date.
76 """
77 assert is_lease(lease)
78- ends = getattr(lease, 'ends', None)
79- if ends is None or len(ends) == 0 or ends.lower() == 'never':
80+ end_time = getattr(lease, 'ends', '')
81+ if end_time is None or len(end_time) == 0:
82+ binding_state = getattr(lease, 'binding state', '')
83+ binding_state = binding_state.lower()
84+ if binding_state == 'free':
85+ # For a 'free' lease, the release time is the 'starts' time.
86+ start_time = getattr(lease, 'starts', '')
87+ if start_time is None or len(start_time) == 0:
88+ return None
89+ return datetime.strptime(start_time, LEASE_TIME_FORMAT)
90+ elif end_time.lower() == 'never':
91 return None
92 else:
93- return datetime.strptime(ends, '%w %Y/%m/%d %H:%M:%S')
94+ return datetime.strptime(end_time, LEASE_TIME_FORMAT)
95
96
97 def has_expired(lease, now):
98@@ -153,13 +164,17 @@
99 def gather_leases(hosts_and_leases):
100 """Find current leases among `hosts_and_leases`."""
101 now = datetime.utcnow()
102- # If multiple leases for the same address are valid at the same
103- # time, for whatever reason, the list will contain all of them.
104- return [
105- (lease.host, lease.hardware.mac)
106- for lease in filter(is_lease, hosts_and_leases)
107- if not has_expired(lease, now)
108- ]
109+ # Ensure we have the most recent MAC leased to each IP address.
110+ leases = OrderedDict()
111+ for lease in filter(is_lease, hosts_and_leases):
112+ lease_mac = get_lease_mac(lease)
113+ lease_ip = lease.host
114+ if not has_expired(lease, now) and lease_mac is not None:
115+ leases[lease_ip] = lease_mac
116+ else:
117+ if lease_ip in leases:
118+ del leases[lease_ip]
119+ return [(ip, mac) for ip, mac in leases.items()]
120
121
122 def get_host_mac(host):
123@@ -173,6 +188,12 @@
124 return None
125 else:
126 return host
127+ # In this case, it's stored the same way a lease MAC is stored.
128+ return get_lease_mac(host)
129+
130+
131+def get_lease_mac(host):
132+ """Get the MAC address from a lease declaration."""
133 hardware = getattr(host, 'hardware', None)
134 if hardware in (None, '', b''):
135 return None
136
137=== modified file 'src/provisioningserver/dhcp/leases_parser_fast.py'
138--- src/provisioningserver/dhcp/leases_parser_fast.py 2015-09-09 15:02:30 +0000
139+++ src/provisioningserver/dhcp/leases_parser_fast.py 2017-02-07 17:35:36 +0000
140@@ -23,7 +23,10 @@
141 'parse_leases',
142 ]
143
144-from collections import defaultdict
145+from collections import (
146+ defaultdict,
147+ OrderedDict,
148+)
149 from datetime import datetime
150 from itertools import chain
151 import re
152@@ -31,6 +34,7 @@
153 from provisioningserver.dhcp.leases_parser import (
154 get_host_ip,
155 get_host_mac,
156+ get_lease_mac,
157 has_expired,
158 is_host,
159 is_lease,
160@@ -43,7 +47,7 @@
161 ^\s* # Ignore leading whitespace on each line.
162 (host|lease) # Look only for host or lease stanzas.
163 \s+ # Mandatory whitespace.
164- ([0-9a-fA-F.:]+) # Capture the IP/MAC address for this stanza.
165+ ([0-9a-fA-F.:-]+) # Capture the IP/MAC address for this stanza.
166 \s*{ # Optional whitespace then an opening brace.
167 ''',
168 re.MULTILINE | re.DOTALL | re.VERBOSE)
169@@ -71,15 +75,29 @@
170
171
172 def parse_leases(leases_contents):
173- results = []
174+ leases = OrderedDict()
175+ hosts = []
176 now = datetime.utcnow()
177 for entry in extract_leases(leases_contents):
178 if is_lease(entry):
179- if not has_expired(entry, now):
180- results.append((entry.host, entry.hardware.mac))
181+ mac = get_lease_mac(entry)
182+ if not has_expired(entry, now) and mac is not None:
183+ leases[entry.host] = entry.hardware.mac
184+ else:
185+ # Expired or released lease.
186+ if entry.host in leases:
187+ del leases[entry.host]
188 elif is_host(entry):
189 mac = get_host_mac(entry)
190 ip = get_host_ip(entry)
191 if ip and mac:
192- results.append((ip, mac))
193+ # A host entry came later than a lease entry for the same IP
194+ # address. Letting them both stay will confuse MAAS by allowing
195+ # an ephemeral lease to exist at the same time as a static
196+ # host map.
197+ if ip in leases:
198+ del leases[ip]
199+ hosts.append((ip, mac))
200+ results = leases.items()
201+ results.extend(hosts)
202 return results
203
204=== modified file 'src/provisioningserver/dhcp/tests/test_leases_parser.py'
205--- src/provisioningserver/dhcp/tests/test_leases_parser.py 2016-05-11 18:50:52 +0000
206+++ src/provisioningserver/dhcp/tests/test_leases_parser.py 2017-02-07 17:35:36 +0000
207@@ -41,26 +41,32 @@
208
209 class Lease(object):
210
211- def __init__(self, lease_or_host, host, fixed_address, hardware, ends):
212+ def __init__(
213+ self, lease_or_host, host, fixed_address, hardware, starts, ends,
214+ binding_state=None):
215 self.lease_or_host = lease_or_host
216 self.host = host
217 self.hardware = hardware
218+ self.starts = starts
219 setattr(self, 'fixed-address', fixed_address)
220 self.ends = ends
221+ if binding_state is not None:
222+ setattr(self, 'binding state', binding_state)
223
224 def __iter__(self):
225 return iter(self.__dict__.keys())
226
227
228-def fake_parsed_lease(ip=None, mac=None, ends=None,
229- entry_type='lease'):
230+def fake_parsed_lease(ip=None, mac=None, starts=None, ends=None,
231+ entry_type='lease', state=None):
232 """Fake a lease as produced by the parser."""
233 if ip is None:
234 ip = factory.make_ipv4_address()
235 if mac is None:
236 mac = factory.make_mac_address()
237 Hardware = namedtuple('Hardware', ['mac'])
238- lease = Lease(entry_type, ip, ip, Hardware(mac), ends)
239+ lease = Lease(
240+ entry_type, ip, ip, Hardware(mac), starts, ends, binding_state=state)
241 return lease
242
243
244@@ -312,7 +318,13 @@
245 """ % params))
246 self.assertEqual([(params['ip'], params['mac'])], leases)
247
248- def test_parse_leases_takes_all_leases_for_address(self):
249+ def test_parse_leases_takes_current_lease_for_address(self):
250+ # Note: a previous version of this test case checked that two leases
251+ # can exist at the same time for a single IP address. This has been
252+ # removed in order to prevent stale entries from polluting MAAS's
253+ # discoveries with old information. To support bonds, static host
254+ # mappings will still allow more than one MAC address to be assigned to
255+ # the same IP address.
256 params = {
257 'ip': factory.make_ipv4_address(),
258 'old_owner': factory.make_mac_address(),
259@@ -321,15 +333,35 @@
260 leases = self.parse(dedent("""\
261 lease %(ip)s {
262 hardware ethernet %(old_owner)s;
263+ ends 0 1990/01/01 00:00:00;
264 }
265 lease %(ip)s {
266 hardware ethernet %(new_owner)s;
267 }
268 """ % params))
269 self.assertEqual(
270- [(params['ip'], params['old_owner']),
271- (params['ip'], params['new_owner'])],
272- leases)
273+ [(params['ip'], params['new_owner'])], leases)
274+
275+ def test_multiple_host_declarations_are_reported(self):
276+ params = {
277+ 'ip': factory.make_ipv4_address(),
278+ 'bondmac1': factory.make_mac_address(),
279+ 'bondmac2': factory.make_mac_address(),
280+ }
281+ leases = self.parse(dedent("""\
282+ host %(bondmac1)s {
283+ hardware ethernet %(bondmac1)s;
284+ fixed-address %(ip)s;
285+ }
286+ host %(bondmac2)s {
287+ hardware ethernet %(bondmac2)s;
288+ fixed-address %(ip)s;
289+ }
290+ """ % params))
291+ self.assertEqual([
292+ (params['ip'], params['bondmac1']),
293+ (params['ip'], params['bondmac2'])
294+ ], leases)
295
296 def test_parse_leases_recognizes_host_deleted_statement_as_rubout(self):
297 params = {
298@@ -362,6 +394,39 @@
299
300 class TestLeasesParserFast(MAASTestCase):
301
302+ def test_handles_dash_separator_for_host_mapping(self):
303+ ip = factory.make_ipv4_address()
304+ mac = factory.make_mac_address()
305+ mac_dash = mac.replace(":", "-")
306+ leases = leases_parser_fast.parse_leases(dedent("""\
307+ host %s {
308+ hardware ethernet %s;
309+ fixed-address %s;
310+ }
311+ """ % (mac_dash, mac, ip)))
312+ self.assertEqual([(ip, mac)], leases)
313+
314+ def test_multiple_host_declarations_are_reported(self):
315+ params = {
316+ 'ip': factory.make_ipv4_address(),
317+ 'bondmac1': factory.make_mac_address(),
318+ 'bondmac2': factory.make_mac_address(),
319+ }
320+ leases = leases_parser_fast.parse_leases(dedent("""\
321+ host %(bondmac1)s {
322+ hardware ethernet %(bondmac1)s;
323+ fixed-address %(ip)s;
324+ }
325+ host %(bondmac2)s {
326+ hardware ethernet %(bondmac2)s;
327+ fixed-address %(ip)s;
328+ }
329+ """ % params))
330+ self.assertEqual([
331+ (params['ip'], params['bondmac1']),
332+ (params['ip'], params['bondmac2'])
333+ ], leases)
334+
335 def test_expired_lease_does_not_shadow_earlier_host_stanza(self):
336 params = {
337 'ip': factory.make_ipv4_address(),
338@@ -402,33 +467,55 @@
339 }
340 """ % params))
341 # The lease hasn't expired, so shadows the earlier host stanza.
342- self.assertEqual(
343- [(params["ip"], params["mac1"]), (params["ip"], params["mac2"])],
344- leases)
345-
346- def test_host_stanza_shadows_earlier_active_lease(self):
347- params = {
348- 'ip': factory.make_ipv4_address(),
349- 'mac1': factory.make_mac_address(),
350- 'mac2': factory.make_mac_address(),
351- }
352- leases = leases_parser_fast.parse_leases(dedent("""\
353- lease %(ip)s {
354- starts 5 2010/01/01 00:00:01;
355- hardware ethernet %(mac2)s;
356- }
357- host %(ip)s {
358- dynamic;
359- hardware ethernet %(mac1)s;
360- fixed-address %(ip)s;
361- }
362- """ % params))
363- # The lease hasn't expired, but the host entry is later, so it
364- # shadows the earlier lease stanza.
365+ # (But since the host mapping has not been removed, it takes precedence
366+ # by coming later in the list, since that should better describe the
367+ # intent of the MAAS administrator.)
368 self.assertEqual(
369 [(params["ip"], params["mac2"]), (params["ip"], params["mac1"])],
370 leases)
371
372+ def test_host_stanza_replaces_earlier_active_lease(self):
373+ params = {
374+ 'ip': factory.make_ipv4_address(),
375+ 'mac1': factory.make_mac_address(),
376+ 'mac2': factory.make_mac_address(),
377+ }
378+ leases = leases_parser_fast.parse_leases(dedent("""\
379+ lease %(ip)s {
380+ starts 5 2010/01/01 00:00:01;
381+ hardware ethernet %(mac2)s;
382+ }
383+ host %(ip)s {
384+ dynamic;
385+ hardware ethernet %(mac1)s;
386+ fixed-address %(ip)s;
387+ }
388+ """ % params))
389+ # The lease hasn't expired, but the host entry is later. So the host
390+ # mapping takes precedence.
391+ self.assertEqual(
392+ [(params["ip"], params["mac1"])], leases)
393+
394+ def test_released_lease_with_no_end_time_is_released(self):
395+ params = {
396+ 'ip': factory.make_ipv4_address(),
397+ 'mac1': factory.make_mac_address(),
398+ 'mac2': factory.make_mac_address(),
399+ }
400+ leases = leases_parser_fast.parse_leases(dedent("""\
401+ lease %(ip)s {
402+ starts 5 2010/01/01 00:00:01;
403+ hardware ethernet %(mac2)s;
404+ }
405+ lease %(ip)s {
406+ dynamic;
407+ binding state free;
408+ starts 0 1990/01/01 00:00:00;
409+ }
410+ """ % params))
411+ # The lease was added and then removed, so we expect a no-op.
412+ self.assertEqual([], leases)
413+
414
415 class TestLeasesParserFunctions(MAASTestCase):
416
417@@ -440,6 +527,14 @@
418 hour=03, minute=04, second=05),
419 get_expiry_date(lease))
420
421+ def test_get_expiry_date_uses_start_date_for_free_lease(self):
422+ lease = fake_parsed_lease(starts='0 2011/01/02 03:04:05', state='free')
423+ self.assertEqual(
424+ datetime(
425+ year=2011, month=01, day=02,
426+ hour=03, minute=04, second=05),
427+ get_expiry_date(lease))
428+
429 def test_get_expiry_date_returns_None_for_never(self):
430 self.assertIsNone(
431 get_expiry_date(fake_parsed_lease(ends='never')))
432@@ -492,7 +587,7 @@
433 ]
434 self.assertEqual([(ip, new_owner)], gather_leases(leases))
435
436- def test_gather_leases_ignores_ordering(self):
437+ def test_ordering_is_important_to_gather_leases(self):
438 earlier = '1 2001/01/01 00:00:00'
439 ip = factory.make_ipv4_address()
440 old_owner = factory.make_mac_address()
441@@ -501,7 +596,7 @@
442 fake_parsed_lease(ip=ip, mac=new_owner),
443 fake_parsed_lease(ip=ip, mac=old_owner, ends=earlier),
444 ]
445- self.assertEqual([(ip, new_owner)], gather_leases(leases))
446+ self.assertEqual([], gather_leases(leases))
447
448 def test_gather_leases_ignores_host_declarations(self):
449 self.assertEqual([], gather_leases([fake_parsed_host()]))

Subscribers

People subscribed via source and target branches