Merge ~blake-rouse/maas:fix-1710308-2.2 into maas:2.2

Proposed by Blake Rouse on 2017-08-22
Status: Merged
Approved by: Blake Rouse on 2017-08-22
Approved revision: f50e1f2bb58798d93f7596fcffade6163bb23d4b
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~blake-rouse/maas:fix-1710308-2.2
Merge into: maas:2.2
Diff against target: 332 lines (+196/-5)
4 files modified
src/maasserver/dns/config.py (+6/-0)
src/maasserver/dns/tests/test_config.py (+19/-0)
src/maasserver/region_controller.py (+46/-3)
src/maasserver/tests/test_region_controller.py (+125/-2)
Reviewer Review Type Date Requested Status
Blake Rouse Approve on 2017-08-22
Review via email: mp+329366@code.launchpad.net

Commit message

Backport eebad83a13bf1d1ec99d2bba4fdcd831cbede3f1 from master.

Fixes LP: #1710308, #1710278 - After BIND reload verify that the serial for all authoritative domains the new value before continuing.

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

Self-approving backport.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maasserver/dns/config.py b/src/maasserver/dns/config.py
2index 855cc47..e27773a 100644
3--- a/src/maasserver/dns/config.py
4+++ b/src/maasserver/dns/config.py
5@@ -88,6 +88,12 @@ def dns_update_all_zones(reload_retry=False):
6 else:
7 bind_reload()
8
9+ # Return the current serial and list of domain names.
10+ return serial, [
11+ domain.name
12+ for domain in domains
13+ ]
14+
15
16 def get_upstream_dns():
17 """Return the IP addresses of configured upstream DNS servers.
18diff --git a/src/maasserver/dns/tests/test_config.py b/src/maasserver/dns/tests/test_config.py
19index 029e874..c163420 100644
20--- a/src/maasserver/dns/tests/test_config.py
21+++ b/src/maasserver/dns/tests/test_config.py
22@@ -53,6 +53,7 @@ from testtools.matchers import (
23 Contains,
24 Equals,
25 FileContains,
26+ MatchesSetwise,
27 MatchesStructure,
28 )
29
30@@ -302,6 +303,24 @@ class TestDNSConfigModifications(TestDNSServer):
31 port=self.bind.config.port, commands=[ns_record, '+short'])
32 self.assertEqual(ip, ip_of_ns_record)
33
34+ def test_dns_update_all_zones_returns_serial_and_domains(self):
35+ self.patch(settings, 'DNS_CONNECT', True)
36+ domain = factory.make_Domain()
37+ # These domains should not show up. Just to test we create them.
38+ for _ in range(3):
39+ factory.make_Domain(authoritative=False)
40+ node, static = self.create_node_with_static_ip(domain=domain)
41+ fake_serial = random.randint(1, 1000)
42+ self.patch(
43+ dns_config_module,
44+ "current_zone_serial").return_value = fake_serial
45+ serial, domains = dns_update_all_zones()
46+ self.assertThat(serial, Equals(fake_serial))
47+ self.assertThat(domains, MatchesSetwise(*[
48+ Equals(domain.name)
49+ for domain in Domain.objects.filter(authoritative=True)
50+ ]))
51+
52
53 class TestDNSDynamicIPAddresses(TestDNSServer):
54 """Allocated nodes with IP addresses in the dynamic range get a DNS
55diff --git a/src/maasserver/region_controller.py b/src/maasserver/region_controller.py
56index f3a51f3..0f9e181 100644
57--- a/src/maasserver/region_controller.py
58+++ b/src/maasserver/region_controller.py
59@@ -31,16 +31,25 @@ from provisioningserver.logger import LegacyLogger
60 from provisioningserver.utils.twisted import (
61 asynchronous,
62 FOREVER,
63+ pause,
64 )
65 from twisted.application.service import Service
66 from twisted.internet import reactor
67-from twisted.internet.defer import DeferredList
68+from twisted.internet.defer import (
69+ DeferredList,
70+ inlineCallbacks,
71+)
72 from twisted.internet.task import LoopingCall
73+from twisted.names.client import Resolver
74
75
76 log = LegacyLogger()
77
78
79+class DNSReloadError(Exception):
80+ """Error raised when the bind never fully reloads the zone."""
81+
82+
83 class RegionControllerService(Service):
84 """
85 A service that controllers external services that are in MAAS's control on
86@@ -64,6 +73,9 @@ class RegionControllerService(Service):
87 self.needsDNSUpdate = False
88 self.needsProxyUpdate = False
89 self.postgresListener = postgresListener
90+ self.dnsResolver = Resolver(
91+ resolv=None, servers=[('127.0.0.1', 53)],
92+ timeout=(1,), reactor=clock)
93
94 @asynchronous(timeout=FOREVER)
95 def startService(self):
96@@ -108,9 +120,12 @@ class RegionControllerService(Service):
97 if self.needsDNSUpdate:
98 self.needsDNSUpdate = False
99 d = deferToDatabase(transactional(dns_update_all_zones))
100+ d.addCallback(self._check_serial)
101 d.addCallback(
102- lambda _: log.msg(
103- "Successfully configured DNS."))
104+ lambda domains: log.msg(
105+ "Successfully configured DNS "
106+ "authoritative zones: %s." % (
107+ ', '.join(domains))))
108 d.addErrback(
109 log.err,
110 "Failed configuring DNS.")
111@@ -131,3 +146,31 @@ class RegionControllerService(Service):
112 self.processingDefer = None
113 else:
114 return DeferredList(defers)
115+
116+ @inlineCallbacks
117+ def _check_serial(self, result):
118+ """Check that the serial of the domain is updated."""
119+ serial, domain_names = result
120+ not_matching_domains = set(domain_names)
121+ loop = 0
122+ while len(not_matching_domains) > 0 and loop != 30:
123+ for domain in list(not_matching_domains):
124+ try:
125+ answers, _, _ = yield self.dnsResolver.lookupAuthority(
126+ domain)
127+ except (ValueError, TimeoutError):
128+ answers = []
129+ if len(answers) > 0:
130+ if int(answers[0].payload.serial) == int(serial):
131+ not_matching_domains.remove(domain)
132+ loop += 1
133+ yield pause(2)
134+ # 30 retries with 2 second pauses (aka. 60 seconds) has passed and
135+ # there still is a domain that has the wrong serial. For now just
136+ # raise the error, in the future we should take action and force
137+ # restart bind.
138+ if len(not_matching_domains) > 0:
139+ raise DNSReloadError(
140+ "Failed to reload DNS; serial mismatch "
141+ "on domains %s" % ', '.join(not_matching_domains))
142+ return domain_names
143diff --git a/src/maasserver/tests/test_region_controller.py b/src/maasserver/tests/test_region_controller.py
144index 9dbe4da..a4e83d2 100644
145--- a/src/maasserver/tests/test_region_controller.py
146+++ b/src/maasserver/tests/test_region_controller.py
147@@ -5,6 +5,7 @@
148
149 __all__ = []
150
151+import random
152 from unittest.mock import (
153 ANY,
154 call,
155@@ -14,7 +15,10 @@ from unittest.mock import (
156
157 from crochet import wait_for
158 from maasserver import region_controller
159-from maasserver.region_controller import RegionControllerService
160+from maasserver.region_controller import (
161+ DNSReloadError,
162+ RegionControllerService,
163+)
164 from maasserver.testing.factory import factory
165 from maasserver.testing.testcase import MAASServerTestCase
166 from maastesting.matchers import (
167@@ -22,6 +26,7 @@ from maastesting.matchers import (
168 MockCallsMatch,
169 MockNotCalled,
170 )
171+from testtools import ExpectedException
172 from testtools.matchers import MatchesStructure
173 from twisted.internet import reactor
174 from twisted.internet.defer import (
175@@ -29,6 +34,12 @@ from twisted.internet.defer import (
176 inlineCallbacks,
177 succeed,
178 )
179+from twisted.names.dns import (
180+ A,
181+ Record_SOA,
182+ RRHeader,
183+ SOA,
184+)
185
186
187 wait_for_reactor = wait_for(30) # 30 seconds.
188@@ -166,16 +177,28 @@ class TestRegionControllerService(MAASServerTestCase):
189 def test_process_updates_zones(self):
190 service = RegionControllerService(sentinel.listener)
191 service.needsDNSUpdate = True
192+ dns_result = (
193+ random.randint(1, 1000), [
194+ factory.make_name('domain')
195+ for _ in range(3)
196+ ])
197 mock_dns_update_all_zones = self.patch(
198 region_controller, "dns_update_all_zones")
199+ mock_dns_update_all_zones.return_value = dns_result
200+ mock_check_serial = self.patch(service, "_check_serial")
201+ mock_check_serial.return_value = succeed(dns_result[1])
202 mock_msg = self.patch(
203 region_controller.log, "msg")
204 service.startProcessing()
205 yield service.processingDefer
206 self.assertThat(mock_dns_update_all_zones, MockCalledOnceWith())
207+ self.assertThat(mock_check_serial, MockCalledOnceWith(dns_result))
208 self.assertThat(
209 mock_msg,
210- MockCalledOnceWith("Successfully configured DNS."))
211+ MockCalledOnceWith(
212+ "Successfully configured DNS "
213+ "authoritative zones: %s." % (
214+ ', '.join(dns_result[1]))))
215
216 @wait_for_reactor
217 @inlineCallbacks
218@@ -236,8 +259,16 @@ class TestRegionControllerService(MAASServerTestCase):
219 service = RegionControllerService(sentinel.listener)
220 service.needsDNSUpdate = True
221 service.needsProxyUpdate = True
222+ dns_result = (
223+ random.randint(1, 1000), [
224+ factory.make_name('domain')
225+ for _ in range(3)
226+ ])
227 mock_dns_update_all_zones = self.patch(
228 region_controller, "dns_update_all_zones")
229+ mock_dns_update_all_zones.return_value = dns_result
230+ mock_check_serial = self.patch(service, "_check_serial")
231+ mock_check_serial.return_value = succeed(dns_result[1])
232 mock_proxy_update_config = self.patch(
233 region_controller, "proxy_update_config")
234 mock_proxy_update_config.return_value = succeed(None)
235@@ -245,5 +276,97 @@ class TestRegionControllerService(MAASServerTestCase):
236 yield service.processingDefer
237 self.assertThat(
238 mock_dns_update_all_zones, MockCalledOnceWith())
239+ self.assertThat(mock_check_serial, MockCalledOnceWith(dns_result))
240 self.assertThat(
241 mock_proxy_update_config, MockCalledOnceWith(reload_proxy=True))
242+
243+ def make_soa_result(self, serial):
244+ return RRHeader(
245+ type=SOA, cls=A, ttl=30, payload=Record_SOA(serial=serial))
246+
247+ @wait_for_reactor
248+ def test__check_serial_doesnt_raise_error_on_successful_serial_match(self):
249+ service = RegionControllerService(sentinel.listener)
250+ result_serial = random.randint(1, 1000)
251+ formatted_serial = '{0:10d}'.format(result_serial)
252+ dns_names = [
253+ factory.make_name('domain')
254+ for _ in range(3)
255+ ]
256+ # Mock pause so test runs faster.
257+ self.patch(region_controller, "pause").return_value = succeed(None)
258+ mock_lookup = self.patch(service.dnsResolver, "lookupAuthority")
259+ mock_lookup.side_effect = [
260+ # First pass no results.
261+ succeed(([], [], [])),
262+ succeed(([], [], [])),
263+ succeed(([], [], [])),
264+ # First domain valid result.
265+ succeed(([self.make_soa_result(result_serial)], [], [])),
266+ succeed(([], [], [])),
267+ succeed(([], [], [])),
268+ # Second domain wrong serial.
269+ succeed(([self.make_soa_result(result_serial - 1)], [], [])),
270+ succeed(([], [], [])),
271+ # Third domain correct serial.
272+ succeed(([], [], [])),
273+ succeed(([self.make_soa_result(result_serial)], [], [])),
274+ # Second domain correct serial.
275+ succeed(([self.make_soa_result(result_serial)], [], [])),
276+ ]
277+ # Error should not be raised.
278+ return service._check_serial((formatted_serial, dns_names))
279+
280+ @wait_for_reactor
281+ @inlineCallbacks
282+ def test__check_serial_raise_error_after_30_tries(self):
283+ service = RegionControllerService(sentinel.listener)
284+ result_serial = random.randint(1, 1000)
285+ formatted_serial = '{0:10d}'.format(result_serial)
286+ dns_names = [
287+ factory.make_name('domain')
288+ for _ in range(3)
289+ ]
290+ # Mock pause so test runs faster.
291+ self.patch(region_controller, "pause").return_value = succeed(None)
292+ mock_lookup = self.patch(service.dnsResolver, "lookupAuthority")
293+ mock_lookup.side_effect = lambda *args: succeed(([], [], []))
294+ # Error should not be raised.
295+ with ExpectedException(DNSReloadError):
296+ yield service._check_serial((formatted_serial, dns_names))
297+
298+ @wait_for_reactor
299+ @inlineCallbacks
300+ def test__check_serial_handles_ValueError(self):
301+ service = RegionControllerService(sentinel.listener)
302+ result_serial = random.randint(1, 1000)
303+ formatted_serial = '{0:10d}'.format(result_serial)
304+ dns_names = [
305+ factory.make_name('domain')
306+ for _ in range(3)
307+ ]
308+ # Mock pause so test runs faster.
309+ self.patch(region_controller, "pause").return_value = succeed(None)
310+ mock_lookup = self.patch(service.dnsResolver, "lookupAuthority")
311+ mock_lookup.side_effect = ValueError()
312+ # Error should not be raised.
313+ with ExpectedException(DNSReloadError):
314+ yield service._check_serial((formatted_serial, dns_names))
315+
316+ @wait_for_reactor
317+ @inlineCallbacks
318+ def test__check_serial_handles_TimeoutError(self):
319+ service = RegionControllerService(sentinel.listener)
320+ result_serial = random.randint(1, 1000)
321+ formatted_serial = '{0:10d}'.format(result_serial)
322+ dns_names = [
323+ factory.make_name('domain')
324+ for _ in range(3)
325+ ]
326+ # Mock pause so test runs faster.
327+ self.patch(region_controller, "pause").return_value = succeed(None)
328+ mock_lookup = self.patch(service.dnsResolver, "lookupAuthority")
329+ mock_lookup.side_effect = TimeoutError()
330+ # Error should not be raised.
331+ with ExpectedException(DNSReloadError):
332+ yield service._check_serial((formatted_serial, dns_names))

Subscribers

People subscribed via source and target branches

to all changes: