Merge lp:~mpontillo/maas/ntp-issues--bug-1695083 into lp:maas/trunk

Proposed by Mike Pontillo on 2017-06-03
Status: Merged
Approved by: Mike Pontillo on 2017-06-05
Approved revision: 6074
Merged at revision: 6073
Proposed branch: lp:~mpontillo/maas/ntp-issues--bug-1695083
Merge into: lp:maas/trunk
Diff against target: 85 lines (+51/-7)
2 files modified
src/maasserver/dhcp.py (+24/-7)
src/maasserver/tests/test_dhcp.py (+27/-0)
To merge this branch: bzr merge lp:~mpontillo/maas/ntp-issues--bug-1695083
Reviewer Review Type Date Requested Status
Blake Rouse 2017-06-03 Approve on 2017-06-05
Review via email: mp+325037@code.launchpad.net

Commit message

When selecting an NTP server address to provide via DHCP, prefer subnets on VLANs where DHCP is enabled.

To post a comment you must log in.
Blake Rouse (blake-rouse) wrote :

Looks good.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/dhcp.py'
2--- src/maasserver/dhcp.py 2017-05-22 13:39:28 +0000
3+++ src/maasserver/dhcp.py 2017-06-03 19:25:25 +0000
4@@ -256,19 +256,36 @@
5 IPADDRESS_TYPE.STICKY, IPADDRESS_TYPE.USER_RESERVED})
6 rack_addresses = rack_addresses.exclude(subnet__isnull=True)
7 rack_addresses = rack_addresses.order_by(
8- "subnet__vlan__space_id", "subnet__cidr", "ip")
9+ # Prefer subnets with DHCP enabled.
10+ "-subnet__vlan__dhcp_on",
11+ "subnet__vlan__space_id",
12+ "subnet__cidr",
13+ "ip"
14+ )
15 rack_addresses = rack_addresses.values_list(
16- "subnet__vlan__space_id", "subnet__cidr", "ip")
17+ "subnet__vlan__dhcp_on",
18+ "subnet__vlan__space_id",
19+ "subnet__cidr",
20+ "ip"
21+ )
22
23 def get_space_id_and_family(record):
24- space_id, cidr, ip = record
25+ dhcp_on, space_id, cidr, ip = record
26 return space_id, IPNetwork(cidr).version
27
28+ def sort_key__dhcp_on__ip(record):
29+ dhcp_on, space_id, cidr, ip = record
30+ return -int(dhcp_on), IPAddress(ip)
31+
32+ best_ntp_servers = {
33+ space_id_and_family:
34+ min(group, key=sort_key__dhcp_on__ip)
35+ for space_id_and_family, group in groupby(
36+ rack_addresses, get_space_id_and_family)
37+ }
38 return {
39- space_id_and_family: min(
40- (ip for _, _, ip in group), key=IPAddress)
41- for space_id_and_family, group in groupby(
42- rack_addresses, get_space_id_and_family)
43+ key: value[3]
44+ for key, value in best_ntp_servers.items()
45 }
46
47
48
49=== modified file 'src/maasserver/tests/test_dhcp.py'
50--- src/maasserver/tests/test_dhcp.py 2017-05-30 18:36:16 +0000
51+++ src/maasserver/tests/test_dhcp.py 2017-06-03 19:25:25 +0000
52@@ -748,6 +748,33 @@
53 (address.ip for address in addresses), key=IPAddress),
54 }))
55
56+ def test__returned_dict_prefers_vlans_with_dhcp_on(self):
57+ rack = factory.make_RackController()
58+ space = factory.make_Space()
59+ ip_version = random.choice([4, 6])
60+ cidr1 = factory.make_ip4_or_6_network(version=ip_version, host_bits=16)
61+ cidr2 = factory.make_ip4_or_6_network(version=ip_version, host_bits=16)
62+ subnet1 = factory.make_Subnet(space=space, cidr=cidr1)
63+ subnet2 = factory.make_Subnet(space=space, cidr=cidr2)
64+ # Expect subnet2 to be selected, since DHCP is enabled.
65+ subnet2.vlan.dhcp_on = True
66+ subnet2.vlan.save()
67+ interface = factory.make_Interface(node=rack)
68+ # Make some addresses that won't be selected since they're on the
69+ # incorrect VLAN (without DHCP enabled).
70+ for _ in range(3):
71+ factory.make_StaticIPAddress(
72+ interface=interface, subnet=subnet1,
73+ alloc_type=IPADDRESS_TYPE.STICKY)
74+ expected_address = factory.make_StaticIPAddress(
75+ interface=interface, subnet=subnet2,
76+ alloc_type=IPADDRESS_TYPE.STICKY)
77+ self.assertThat(
78+ dhcp.get_ntp_server_addresses_for_rack(rack), Equals({
79+ (space.id, subnet2.get_ipnetwork().version):
80+ expected_address.ip
81+ }))
82+
83 def test__constant_query_count(self):
84 rack = factory.make_RackController()
85 interface = factory.make_Interface(node=rack)