Merge lp:~mpontillo/maas/subnet-dhcp-dns-settings--bug-1600720--2.0 into lp:maas/2.0

Proposed by Mike Pontillo on 2016-07-15
Status: Merged
Approved by: Mike Pontillo on 2016-07-15
Approved revision: 5164
Merged at revision: 5163
Proposed branch: lp:~mpontillo/maas/subnet-dhcp-dns-settings--bug-1600720--2.0
Merge into: lp:maas/2.0
Diff against target: 347 lines (+173/-63)
3 files modified
docs/changelog.rst (+3/-1)
src/maasserver/dhcp.py (+59/-32)
src/maasserver/tests/test_dhcp.py (+111/-30)
To merge this branch: bzr merge lp:~mpontillo/maas/subnet-dhcp-dns-settings--bug-1600720--2.0
Reviewer Review Type Date Requested Status
Mike Pontillo (community) Approve on 2016-07-15
Review via email: mp+300211@code.launchpad.net

Commit message

Merge revision 5169 from trunk.

Fix DHCP configuration to properly override subnet DNS with user-configured DNS. (This makes MAAS DHCP consistent with what curtin does.)

Drive-by fix to log an error when DHCP is enabled for a particular address family, but no subnet can be found. (Previous behavior was to silently fail to write the configuration; now if IPv6 is misconfigured, it will not prevent the IPv4 configuration from being written, and vice versa.)

To post a comment you must log in.
5164. By Mike Pontillo on 2016-07-15

Update CHANGELOG.

Mike Pontillo (mpontillo) wrote :

Self-reviewed backport.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'docs/changelog.rst'
2--- docs/changelog.rst 2016-07-15 02:22:48 +0000
3+++ docs/changelog.rst 2016-07-15 15:53:19 +0000
4@@ -15,7 +15,9 @@
5
6 LP: #1603147 Commissioning dropdown is grey and checkmarks are missing.
7
8-LP: #1576116 MAAS adds regions controller as DNS resolver
9+LP: #1576116 [2.0rc1] MAAS does not respect default subnet's DNS server when choosing default DNS
10+
11+LP: #1600720 [2.0rc1] MAAS doesn't honor DNS settings for a subnet for DHCP
12
13
14 2.0.0 (rc2)
15
16=== modified file 'src/maasserver/dhcp.py'
17--- src/maasserver/dhcp.py 2016-04-13 13:18:40 +0000
18+++ src/maasserver/dhcp.py 2016-07-15 15:53:19 +0000
19@@ -316,10 +316,17 @@
20
21
22 def make_subnet_config(
23- rack_controller, subnet, dns_servers, ntp_server, default_domain,
24+ rack_controller, subnet, maas_dns_server, ntp_server, default_domain,
25 failover_peer=None, subnets_dhcp_snippets=[]):
26 """Return DHCP subnet configuration dict for a rack interface."""
27 ip_network = subnet.get_ipnetwork()
28+ if subnet.dns_servers is not None and len(subnet.dns_servers) > 0:
29+ # Replace MAAS DNS with the servers defined on the subnet.
30+ dns_servers = ", ".join(subnet.dns_servers)
31+ elif maas_dns_server is not None and len(maas_dns_server) > 0:
32+ dns_servers = maas_dns_server
33+ else:
34+ dns_servers = ""
35 return {
36 'subnet': str(ip_network.network),
37 'subnet_mask': str(ip_network.netmask),
38@@ -368,12 +375,10 @@
39 from maasserver.dns.zonegenerator import get_dns_server_address
40
41 try:
42- dns_servers = get_dns_server_address(
43+ maas_dns_server = get_dns_server_address(
44 rack_controller, ipv4=(ip_version == 4), ipv6=(ip_version == 6))
45 except UnresolvableHost:
46- # No IPv6 DNS server addresses found. As a space-separated string,
47- # that becomes the empty string.
48- dns_servers = ''
49+ maas_dns_server = None
50
51 # Select the best interface for this VLAN. This is an interface that
52 # at least has an IP address.
53@@ -382,9 +387,10 @@
54 interface = get_best_interface(interfaces)
55 if interface is None:
56 raise DHCPConfigurationError(
57- "No interface on rack controller '%s' has an IP address on any "
58- "subnet on VLAN '%s.%d'." % (
59- rack_controller.hostname, vlan.fabric.name, vlan.vid))
60+ "No IPv%d interface on rack controller '%s' has an IP address on "
61+ "any subnet on VLAN '%s.%d'." % (
62+ ip_version, rack_controller.hostname, vlan.fabric.name,
63+ vlan.vid))
64
65 # Generate the failover peer for this VLAN.
66 if vlan.secondary_rack_id is not None:
67@@ -406,7 +412,7 @@
68 for subnet in subnets:
69 subnet_configs.append(
70 make_subnet_config(
71- rack_controller, subnet, dns_servers, ntp_server,
72+ rack_controller, subnet, maas_dns_server, ntp_server,
73 domain, peer_name, subnets_dhcp_snippets))
74
75 # Generate the hosts for all subnets.
76@@ -470,31 +476,52 @@
77 for vlan, (subnets_v4, subnets_v6) in vlan_subnets.items():
78 # IPv4
79 if len(subnets_v4) > 0:
80- failover_peer, subnets, hosts, interface = get_dhcp_configure_for(
81- 4, rack_controller, vlan, subnets_v4,
82- ntp_server, default_domain, dhcp_snippets)
83- if failover_peer is not None:
84- failover_peers_v4.append(failover_peer)
85- shared_networks_v4.append({
86- "name": "vlan-%d" % vlan.id,
87- "subnets": subnets,
88- })
89- hosts_v4.extend(hosts)
90- interfaces_v4.add(interface)
91-
92+ try:
93+ config = get_dhcp_configure_for(
94+ 4, rack_controller, vlan, subnets_v4, ntp_server,
95+ default_domain, dhcp_snippets)
96+ except DHCPConfigurationError as e:
97+ # XXX bug #1602412: this silently breaks DHCPv4, but we cannot
98+ # allow it to crash here since DHCPv6 might be able to run.
99+ # This error may be irrelevant if there is an IPv4 network in
100+ # the MAAS model which is not configured on the rack, and the
101+ # user only wants to serve DHCPv6. But it is still something
102+ # worth noting, so log it and continue.
103+ log.err(e)
104+ else:
105+ failover_peer, subnets, hosts, interface = config
106+ if failover_peer is not None:
107+ failover_peers_v4.append(failover_peer)
108+ shared_networks_v4.append({
109+ "name": "vlan-%d" % vlan.id,
110+ "subnets": subnets,
111+ })
112+ hosts_v4.extend(hosts)
113+ interfaces_v4.add(interface)
114 # IPv6
115 if len(subnets_v6) > 0:
116- failover_peer, subnets, hosts, interface = get_dhcp_configure_for(
117- 6, rack_controller, vlan, subnets_v6,
118- ntp_server, default_domain, dhcp_snippets)
119- if failover_peer is not None:
120- failover_peers_v6.append(failover_peer)
121- shared_networks_v6.append({
122- "name": "vlan-%d" % vlan.id,
123- "subnets": subnets,
124- })
125- hosts_v6.extend(hosts)
126- interfaces_v6.add(interface)
127+ try:
128+ config = get_dhcp_configure_for(
129+ 6, rack_controller, vlan, subnets_v6,
130+ ntp_server, default_domain, dhcp_snippets)
131+ except DHCPConfigurationError as e:
132+ # XXX bug #1602412: this silently breaks DHCPv6, but we cannot
133+ # allow it to crash here since DHCPv4 might be able to run.
134+ # This error may be irrelevant if there is an IPv6 network in
135+ # the MAAS model which is not configured on the rack, and the
136+ # user only wants to serve DHCPv4. But it is still something
137+ # worth noting, so log it and continue.
138+ log.err(e)
139+ else:
140+ failover_peer, subnets, hosts, interface = config
141+ if failover_peer is not None:
142+ failover_peers_v6.append(failover_peer)
143+ shared_networks_v6.append({
144+ "name": "vlan-%d" % vlan.id,
145+ "subnets": subnets,
146+ })
147+ hosts_v6.extend(hosts)
148+ interfaces_v6.add(interface)
149 return (
150 get_omapi_key(),
151 failover_peers_v4, shared_networks_v4, hosts_v4, interfaces_v4,
152
153=== modified file 'src/maasserver/tests/test_dhcp.py'
154--- src/maasserver/tests/test_dhcp.py 2016-05-12 19:07:37 +0000
155+++ src/maasserver/tests/test_dhcp.py 2016-07-15 15:53:19 +0000
156@@ -45,6 +45,7 @@
157 from maastesting.twisted import (
158 always_fail_with,
159 always_succeed_with,
160+ TwistedLoggerFixture,
161 )
162 from netaddr import (
163 IPAddress,
164@@ -627,23 +628,78 @@
165 'dhcp_snippets',
166 ]))
167
168- def test__sets_dns_and_ntp_from_arguments(self):
169- rack_controller = factory.make_RackController(interface=False)
170- vlan = factory.make_VLAN()
171- subnet = factory.make_Subnet(vlan=vlan)
172- factory.make_Interface(
173- INTERFACE_TYPE.PHYSICAL, vlan=vlan, node=rack_controller)
174- dns = '%s %s' % (
175- factory.make_ipv4_address(),
176- factory.make_ipv6_address(),
177- )
178- ntp = factory.make_name('ntp')
179- default_domain = Domain.objects.get_default_domain()
180- config = dhcp.make_subnet_config(
181- rack_controller, subnet, dns, ntp, default_domain)
182- self.expectThat(config['dns_servers'], Equals(dns))
183+ def test__sets_ipv4_dns_from_arguments(self):
184+ rack_controller = factory.make_RackController(interface=False)
185+ vlan = factory.make_VLAN()
186+ subnet = factory.make_Subnet(vlan=vlan, dns_servers=[], version=4)
187+ factory.make_Interface(
188+ INTERFACE_TYPE.PHYSICAL, vlan=vlan, node=rack_controller)
189+ maas_dns = factory.make_ipv4_address()
190+ ntp = factory.make_name('ntp')
191+ default_domain = Domain.objects.get_default_domain()
192+ config = dhcp.make_subnet_config(
193+ rack_controller, subnet, maas_dns, ntp, default_domain)
194+ self.expectThat(config['dns_servers'], Equals(maas_dns))
195+
196+ def test__sets_ipv6_dns_from_arguments(self):
197+ rack_controller = factory.make_RackController(interface=False)
198+ vlan = factory.make_VLAN()
199+ subnet = factory.make_Subnet(vlan=vlan, dns_servers=[], version=6)
200+ factory.make_Interface(
201+ INTERFACE_TYPE.PHYSICAL, vlan=vlan, node=rack_controller)
202+ maas_dns = factory.make_ipv6_address()
203+ ntp = factory.make_name('ntp')
204+ default_domain = Domain.objects.get_default_domain()
205+ config = dhcp.make_subnet_config(
206+ rack_controller, subnet, maas_dns, ntp, default_domain)
207+ self.expectThat(config['dns_servers'], Equals(maas_dns))
208+
209+ def test__sets_ntp_from_arguments(self):
210+ rack_controller = factory.make_RackController(interface=False)
211+ vlan = factory.make_VLAN()
212+ subnet = factory.make_Subnet(vlan=vlan, dns_servers=[])
213+ factory.make_Interface(
214+ INTERFACE_TYPE.PHYSICAL, vlan=vlan, node=rack_controller)
215+ ntp = factory.make_name('ntp')
216+ default_domain = Domain.objects.get_default_domain()
217+ config = dhcp.make_subnet_config(
218+ rack_controller, subnet, "", ntp, default_domain)
219 self.expectThat(config['ntp_server'], Equals(ntp))
220
221+ def test__overrides_ipv4_dns_from_subnet(self):
222+ rack_controller = factory.make_RackController(interface=False)
223+ vlan = factory.make_VLAN()
224+ subnet = factory.make_Subnet(vlan=vlan, version=4)
225+ maas_dns = factory.make_ipv4_address()
226+ subnet_dns_servers = ["8.8.8.8", "8.8.4.4"]
227+ subnet.dns_servers = subnet_dns_servers
228+ subnet.save()
229+ factory.make_Interface(
230+ INTERFACE_TYPE.PHYSICAL, vlan=vlan, node=rack_controller)
231+ ntp = factory.make_name('ntp')
232+ default_domain = Domain.objects.get_default_domain()
233+ config = dhcp.make_subnet_config(
234+ rack_controller, subnet, maas_dns, ntp, default_domain)
235+ self.expectThat(
236+ config['dns_servers'], Equals(", ".join(subnet_dns_servers)))
237+
238+ def test__overrides_ipv6_dns_from_subnet(self):
239+ rack_controller = factory.make_RackController(interface=False)
240+ vlan = factory.make_VLAN()
241+ subnet = factory.make_Subnet(vlan=vlan, version=6)
242+ maas_dns = factory.make_ipv6_address()
243+ subnet_dns_servers = ["2001:db8::1", "2001:db8::2"]
244+ subnet.dns_servers = subnet_dns_servers
245+ subnet.save()
246+ factory.make_Interface(
247+ INTERFACE_TYPE.PHYSICAL, vlan=vlan, node=rack_controller)
248+ ntp = factory.make_name('ntp')
249+ default_domain = Domain.objects.get_default_domain()
250+ config = dhcp.make_subnet_config(
251+ rack_controller, subnet, maas_dns, ntp, default_domain)
252+ self.expectThat(
253+ config['dns_servers'], Equals(", ".join(subnet_dns_servers)))
254+
255 def test__sets_domain_name_from_passed_domain(self):
256 rack_controller = factory.make_RackController(interface=False)
257 vlan = factory.make_VLAN()
258@@ -1121,7 +1177,8 @@
259 ha_vlan = factory.make_VLAN(
260 dhcp_on=True, primary_rack=primary_rack,
261 secondary_rack=secondary_rack)
262- ha_subnet = factory.make_ipv4_Subnet_with_IPRanges(vlan=ha_vlan)
263+ ha_subnet = factory.make_ipv4_Subnet_with_IPRanges(
264+ vlan=ha_vlan, dns_servers=['127.0.0.1'])
265 ha_network = ha_subnet.get_ipnetwork()
266 ha_dhcp_snippets = [
267 factory.make_DHCPSnippet(subnet=ha_subnet, enabled=True)
268@@ -1136,7 +1193,8 @@
269 secondary_ip = factory.make_StaticIPAddress(
270 alloc_type=IPADDRESS_TYPE.AUTO, subnet=ha_subnet,
271 interface=secondary_interface)
272- other_subnet = factory.make_ipv4_Subnet_with_IPRanges(vlan=ha_vlan)
273+ other_subnet = factory.make_ipv4_Subnet_with_IPRanges(
274+ vlan=ha_vlan, dns_servers=['127.0.0.1'])
275 other_network = other_subnet.get_ipnetwork()
276 other_dhcp_snippets = [
277 factory.make_DHCPSnippet(subnet=other_subnet, enabled=True)
278@@ -1358,7 +1416,8 @@
279 return cluster, cluster.ConfigureDHCPv4, cluster.ConfigureDHCPv6
280
281 @transactional
282- def create_rack_controller(self, dhcp_on=True):
283+ def create_rack_controller(
284+ self, dhcp_on=True, missing_ipv4=False, missing_ipv6=False):
285 """Create a `rack_controller` in a state that will call both
286 `ConfigureDHCPv4` and `ConfigureDHCPv6` with data."""
287 primary_rack = factory.make_RackController(interface=False)
288@@ -1382,18 +1441,20 @@
289 subnet_v6, "fd38:c341:27da:c831:0:1::",
290 "fd38:c341:27da:c831:0:1:ffff:0")
291
292- factory.make_StaticIPAddress(
293- alloc_type=IPADDRESS_TYPE.AUTO, subnet=subnet_v4,
294- interface=primary_interface)
295- factory.make_StaticIPAddress(
296- alloc_type=IPADDRESS_TYPE.AUTO, subnet=subnet_v4,
297- interface=secondary_interface)
298- factory.make_StaticIPAddress(
299- alloc_type=IPADDRESS_TYPE.AUTO, subnet=subnet_v6,
300- interface=primary_interface)
301- factory.make_StaticIPAddress(
302- alloc_type=IPADDRESS_TYPE.AUTO, subnet=subnet_v6,
303- interface=secondary_interface)
304+ if not missing_ipv4:
305+ factory.make_StaticIPAddress(
306+ alloc_type=IPADDRESS_TYPE.AUTO, subnet=subnet_v4,
307+ interface=primary_interface)
308+ factory.make_StaticIPAddress(
309+ alloc_type=IPADDRESS_TYPE.AUTO, subnet=subnet_v4,
310+ interface=secondary_interface)
311+ if not missing_ipv6:
312+ factory.make_StaticIPAddress(
313+ alloc_type=IPADDRESS_TYPE.AUTO, subnet=subnet_v6,
314+ interface=primary_interface)
315+ factory.make_StaticIPAddress(
316+ alloc_type=IPADDRESS_TYPE.AUTO, subnet=subnet_v6,
317+ interface=secondary_interface)
318
319 for _ in range(3):
320 factory.make_DHCPSnippet(subnet=subnet_v4, enabled=True)
321@@ -1448,6 +1509,26 @@
322
323 @wait_for_reactor
324 @inlineCallbacks
325+ def test__logs_DHCPConfigurationError_ipv4(self):
326+ self.patch(dhcp.settings, "DHCP_CONNECT", True)
327+ with TwistedLoggerFixture() as logger:
328+ yield deferToDatabase(
329+ self.create_rack_controller, missing_ipv4=True)
330+ self.assertDocTestMatches(
331+ "...No IPv4 interface...", logger.output)
332+
333+ @wait_for_reactor
334+ @inlineCallbacks
335+ def test__logs_DHCPConfigurationError_ipv6(self):
336+ self.patch(dhcp.settings, "DHCP_CONNECT", True)
337+ with TwistedLoggerFixture() as logger:
338+ yield deferToDatabase(
339+ self.create_rack_controller, missing_ipv6=True)
340+ self.assertDocTestMatches(
341+ "...No IPv6 interface...", logger.output)
342+
343+ @wait_for_reactor
344+ @inlineCallbacks
345 def test__doesnt_call_configure_for_both_ipv4_and_ipv6(self):
346 rack_controller, args = yield deferToDatabase(
347 self.create_rack_controller)

Subscribers

People subscribed via source and target branches

to all changes: