Merge lp:~allenap/maas/edit-disconnected-cluster--bug-1439476 into lp:~maas-committers/maas/trunk

Proposed by Gavin Panella
Status: Merged
Approved by: Gavin Panella
Approved revision: no longer in the source branch.
Merged at revision: 4479
Proposed branch: lp:~allenap/maas/edit-disconnected-cluster--bug-1439476
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 475 lines (+204/-39)
4 files modified
src/maasserver/dhcp.py (+31/-12)
src/maasserver/rpc/regionservice.py (+35/-3)
src/maasserver/rpc/tests/test_regionservice.py (+76/-0)
src/maasserver/tests/test_dhcp.py (+62/-24)
To merge this branch: bzr merge lp:~allenap/maas/edit-disconnected-cluster--bug-1439476
Reviewer Review Type Date Requested Status
Blake Rouse (community) Approve
Review via email: mp+276996@code.launchpad.net

Commit message

Ignore disconnected clusters when configuring DHCP.

However, configure DHCP when the cluster does reconnect to the region.

To post a comment you must log in.
Revision history for this message
Blake Rouse (blake-rouse) wrote :

Code looks good and the implementation is okay. It solves the bug for 1.9 but I think in 2.0 we will need to do this better as HA will change this logic, I believe.

review: Approve
Revision history for this message
Gavin Panella (allenap) wrote :

Thanks for the review!

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 2015-09-24 16:22:12 +0000
3+++ src/maasserver/dhcp.py 2015-11-09 12:39:07 +0000
4@@ -18,6 +18,7 @@
5 ]
6
7 from collections import defaultdict
8+import logging
9 import threading
10
11 from django.conf import settings
12@@ -35,12 +36,16 @@
13 ConfigureDHCPv4,
14 ConfigureDHCPv6,
15 )
16+from provisioningserver.rpc.exceptions import NoConnectionsAvailable
17 from provisioningserver.utils.twisted import (
18 callOut,
19 synchronous,
20 )
21
22
23+logger = logging.getLogger(__name__)
24+
25+
26 def split_ipv4_ipv6_interfaces(interfaces):
27 """Divide `interfaces` into IPv4 ones and IPv6 ones.
28
29@@ -80,13 +85,14 @@
30
31
32 @synchronous
33-def do_configure_dhcp(ip_version, nodegroup, interfaces, ntp_server):
34+def do_configure_dhcp(ip_version, nodegroup, interfaces, ntp_server, client):
35 """Write DHCP configuration and restart the DHCP server.
36
37 Delegates the work to the cluster controller, and waits for it
38 to complete.
39
40 :param ip_version: The IP version to configure for, either 4 or 6.
41+ :param client: An RPC client for the given cluster.
42
43 :raise NoConnectionsAvailable: if the region controller could not get
44 an RPC connection to the cluster controller.
45@@ -122,30 +128,33 @@
46 make_subnet_config(interface, dns_servers, ntp_server)
47 for interface in interfaces
48 ]
49- client = getClientFor(nodegroup.uuid)
50 # XXX jtv 2014-08-26 bug=1361673: If this fails remotely, the error
51 # needs to be reported gracefully to the caller.
52 client(command, omapi_key=omapi_key, subnet_configs=subnets).wait(60)
53
54
55-def configure_dhcpv4(nodegroup, interfaces, ntp_server):
56+def configure_dhcpv4(nodegroup, interfaces, ntp_server, client):
57 """Call `do_configure_dhcp` for IPv4.
58
59 This serves mainly as a convenience for testing.
60 """
61- return do_configure_dhcp(4, nodegroup, interfaces, ntp_server)
62-
63-
64-def configure_dhcpv6(nodegroup, interfaces, ntp_server):
65+ return do_configure_dhcp(4, nodegroup, interfaces, ntp_server, client)
66+
67+
68+def configure_dhcpv6(nodegroup, interfaces, ntp_server, client):
69 """Call `do_configure_dhcp` for IPv6.
70
71 This serves mainly as a convenience for testing.
72 """
73- return do_configure_dhcp(6, nodegroup, interfaces, ntp_server)
74+ return do_configure_dhcp(6, nodegroup, interfaces, ntp_server, client)
75
76
77 def configure_dhcp_now(nodegroup):
78- """Write the DHCP configuration files and restart the DHCP servers."""
79+ """Write the DHCP configuration files and restart the DHCP servers.
80+
81+ :raises: :py:class:`~.exceptions.NoConnectionsAvailable` when there
82+ are no open connections to the specified cluster controller.
83+ """
84 # Let's get this out of the way first up shall we?
85 if not settings.DHCP_CONNECT:
86 # For the uninitiated, DHCP_CONNECT is set, by default, to False
87@@ -153,6 +162,10 @@
88 # calls to async tasks.
89 return
90
91+ # Get the client early; it's a cheap operation that may raise an
92+ # exception, meaning we can avoid some work if it fails.
93+ client = getClientFor(nodegroup.uuid)
94+
95 if nodegroup.status == NODEGROUP_STATUS.ENABLED:
96 # Cluster is an accepted one. Control DHCP for its managed interfaces.
97 interfaces = nodegroup.get_managed_interfaces()
98@@ -168,8 +181,8 @@
99
100 ipv4_interfaces, ipv6_interfaces = split_ipv4_ipv6_interfaces(interfaces)
101
102- configure_dhcpv4(nodegroup, ipv4_interfaces, ntp_server)
103- configure_dhcpv6(nodegroup, ipv6_interfaces, ntp_server)
104+ configure_dhcpv4(nodegroup, ipv4_interfaces, ntp_server, client)
105+ configure_dhcpv6(nodegroup, ipv6_interfaces, ntp_server, client)
106
107
108 class Changes:
109@@ -206,7 +219,13 @@
110 """Apply all requested changes."""
111 clusters = {cluster.id: cluster for cluster in self.clusters}
112 for cluster in clusters.viewvalues():
113- configure_dhcp_now(cluster)
114+ try:
115+ configure_dhcp_now(cluster)
116+ except NoConnectionsAvailable:
117+ logger.info(
118+ "Cluster %s (%s) is not connected at present so cannot "
119+ "be configured; it will catch up when it next connects.",
120+ cluster.cluster_name, cluster.uuid)
121
122
123 class ChangeConsolidator(threading.local):
124
125=== modified file 'src/maasserver/rpc/regionservice.py'
126--- src/maasserver/rpc/regionservice.py 2015-10-20 21:25:57 +0000
127+++ src/maasserver/rpc/regionservice.py 2015-11-09 12:39:07 +0000
128@@ -34,6 +34,8 @@
129 locks,
130 )
131 from maasserver.bootresources import get_simplestream_endpoint
132+from maasserver.dhcp import configure_dhcp_now
133+from maasserver.models.nodegroup import NodeGroup
134 from maasserver.rpc import (
135 clusters,
136 configuration,
137@@ -383,6 +385,31 @@
138 return d
139
140
141+@transactional
142+def configureCluster(ident):
143+ """Configure the cluster.
144+
145+ Typically this is called once, as soon as a successful handshake is made
146+ between the region and cluster. After that configuration can be done at
147+ the moment the change is made. In essence, this is "catches-up" the
148+ cluster with the region's current idea of how it should be.
149+
150+ Don't do anything particularly energetic in here because it will be called
151+ once for each region<-->cluster connection, of which each cluster can have
152+ many.
153+
154+ :param ident: The cluster's UUID.
155+ """
156+ try:
157+ cluster = NodeGroup.objects.get(uuid=ident)
158+ except NodeGroup.DoesNotExist:
159+ log.msg("Cluster '%s' is not recognised; cannot configure." % (ident,))
160+ else:
161+ configure_dhcp_now(cluster)
162+ log.msg("Cluster %s (%s) has been configured." % (
163+ cluster.cluster_name, cluster.uuid))
164+
165+
166 @implementer(IConnection)
167 class RegionServer(Region):
168 """The RPC protocol supported by a region controller, server version.
169@@ -428,13 +455,18 @@
170 "Cluster '%s' FAILED authentication; "
171 "dropping connection." % self.ident)
172 yield self.transport.loseConnection()
173+ returnValue(authenticated)
174
175- def handshakeSucceeded(self, result):
176+ def handshakeSucceeded(self, authenticated):
177 """The handshake (identify and authenticate) succeeded.
178
179- This does *NOT* mean that the cluster was successfully authenticated,
180- merely that the process of authentication did not encounter an error.
181+ :param authenticated: True if the remote cluster has successfully
182+ authenticated.
183+ :type authenticated: bool
184 """
185+ if authenticated:
186+ return deferToDatabase(configureCluster, self.ident).addErrback(
187+ log.err, "Failed to configure DHCP on %s." % (self.ident,))
188
189 def handshakeFailed(self, failure):
190 """The handshake (identify and authenticate) failed."""
191
192=== modified file 'src/maasserver/rpc/tests/test_regionservice.py'
193--- src/maasserver/rpc/tests/test_regionservice.py 2015-10-21 00:21:05 +0000
194+++ src/maasserver/rpc/tests/test_regionservice.py 2015-11-09 12:39:07 +0000
195@@ -1104,6 +1104,36 @@
196 "'%s'.", name, event_description, mac_address))
197
198
199+class TestConfigureCluster(MAASServerTestCase):
200+ """Tests for the `configureCluster` function."""
201+
202+ def test__logs_when_cluster_is_not_known(self):
203+ ident = factory.make_UUID()
204+ with TwistedLoggerFixture() as logger:
205+ regionservice.configureCluster(ident)
206+ self.assertDocTestMatches(
207+ "Cluster '...' is not recognised; cannot configure.",
208+ logger.output)
209+
210+ def test__logs_when_cluster_has_been_configured(self):
211+ ident = factory.make_NodeGroup().uuid
212+ with TwistedLoggerFixture() as logger:
213+ regionservice.configureCluster(ident)
214+ self.assertDocTestMatches(
215+ "Cluster ... (...) has been configured.",
216+ logger.output)
217+
218+ def test__configures_dhcp(self):
219+ ident = factory.make_NodeGroup().uuid
220+ self.patch_autospec(regionservice, "configure_dhcp_now")
221+ regionservice.configureCluster(ident)
222+ self.assertThat(
223+ regionservice.configure_dhcp_now,
224+ MockCalledOnceWith(ANY))
225+ [cluster] = regionservice.configure_dhcp_now.call_args[0]
226+ self.assertThat(cluster.uuid, Equals(ident))
227+
228+
229 class TestRegionServer(MAASServerTestCase):
230
231 def test_interfaces(self):
232@@ -1246,6 +1276,52 @@
233 # Nothing was logged.
234 self.assertEqual("", logger.output)
235
236+ def make_handshaking_server(self):
237+ service = RegionService()
238+ service.running = True # Pretend it's running.
239+ service.factory.protocol = HandshakingRegionServer
240+ return service.factory.buildProtocol(addr=None) # addr is unused.
241+
242+ def test_connectionMade_configures_cluster_if_auth_succeeds(self):
243+ self.patch_autospec(regionservice, "configureCluster")
244+ protocol = self.make_handshaking_server()
245+
246+ connectionMade = wait_for_reactor(protocol.connectionMade)
247+ connectionMade()
248+
249+ self.assertThat(
250+ regionservice.configureCluster,
251+ MockCalledOnceWith(protocol.ident))
252+
253+ def test_connectionMade_does_not_configure_cluster_if_auth_fails(self):
254+ self.patch_autospec(regionservice, "configureCluster")
255+ protocol = self.make_handshaking_server()
256+ self.patch_authenticate_for_failure(protocol)
257+ self.patch(protocol, "transport")
258+
259+ connectionMade = wait_for_reactor(protocol.connectionMade)
260+ connectionMade()
261+
262+ self.assertThat(regionservice.configureCluster, MockNotCalled())
263+
264+ def test_connectionMade_logs_cluster_configuration_failure(self):
265+ self.patch_autospec(regionservice, "configureCluster")
266+ regionservice.configureCluster.side_effect = ZeroDivisionError
267+ protocol = self.make_handshaking_server()
268+
269+ with TwistedLoggerFixture() as logger:
270+ connectionMade = wait_for_reactor(protocol.connectionMade)
271+ connectionMade()
272+
273+ self.assertDocTestMatches(
274+ """\
275+ ...
276+ ---
277+ Failed to configure DHCP on ...
278+ Traceback (most recent call last): ...
279+ """,
280+ logger.output)
281+
282 def make_running_server(self):
283 service = RegionService()
284 service.running = True # Pretend it's running.
285
286=== modified file 'src/maasserver/tests/test_dhcp.py'
287--- src/maasserver/tests/test_dhcp.py 2015-09-24 16:22:12 +0000
288+++ src/maasserver/tests/test_dhcp.py 2015-11-09 12:39:07 +0000
289@@ -18,6 +18,7 @@
290
291 from django.conf import settings
292 from django.db import transaction
293+from fixtures import LoggerFixture
294 from maasserver import dhcp
295 from maasserver.dhcp import (
296 configure_dhcp,
297@@ -34,6 +35,7 @@
298 Config,
299 NodeGroup,
300 )
301+from maasserver.rpc import getClientFor
302 from maasserver.rpc.testing.fixtures import MockLiveRegionToClusterRPCFixture
303 from maasserver.testing.eventloop import (
304 RegionEventLoopFixture,
305@@ -270,7 +272,9 @@
306
307 # Although the above nodegroup has managed interfaces, we pass the
308 # empty list here; do_configure_dhcp() dutifully believes us.
309- do_configure_dhcp(self.ip_version, nodegroup, [], ntp_server)
310+ do_configure_dhcp(
311+ self.ip_version, nodegroup, [], ntp_server,
312+ getClientFor(nodegroup.uuid))
313
314 self.assertThat(
315 command_stub, MockCalledOnceWith(
316@@ -297,7 +301,9 @@
317 protocol, command_stub = self.prepare_rpc(nodegroup)
318 command_stub.side_effect = always_succeed_with({})
319
320- do_configure_dhcp(self.ip_version, nodegroup, interfaces, ntp_server)
321+ do_configure_dhcp(
322+ self.ip_version, nodegroup, interfaces, ntp_server,
323+ getClientFor(nodegroup.uuid))
324
325 expected_subnet_configs = [
326 make_subnet_config(interface, dns_server, ntp_server)
327@@ -317,16 +323,20 @@
328 def test_configure_dhcpv4_calls_do_configure_dhcp(self):
329 do_configure_dhcp = self.patch_autospec(dhcp, "do_configure_dhcp")
330 dhcp.configure_dhcpv4(
331- sentinel.nodegroup, sentinel.interfaces, sentinel.ntp_server)
332+ sentinel.nodegroup, sentinel.interfaces, sentinel.ntp_server,
333+ sentinel.client)
334 self.assertThat(do_configure_dhcp, MockCalledOnceWith(
335- 4, sentinel.nodegroup, sentinel.interfaces, sentinel.ntp_server))
336+ 4, sentinel.nodegroup, sentinel.interfaces, sentinel.ntp_server,
337+ sentinel.client))
338
339 def test_configure_dhcpv6_calls_do_configure_dhcp(self):
340 do_configure_dhcp = self.patch_autospec(dhcp, "do_configure_dhcp")
341 dhcp.configure_dhcpv6(
342- sentinel.nodegroup, sentinel.interfaces, sentinel.ntp_server)
343+ sentinel.nodegroup, sentinel.interfaces, sentinel.ntp_server,
344+ sentinel.client)
345 self.assertThat(do_configure_dhcp, MockCalledOnceWith(
346- 6, sentinel.nodegroup, sentinel.interfaces, sentinel.ntp_server))
347+ 6, sentinel.nodegroup, sentinel.interfaces, sentinel.ntp_server,
348+ sentinel.client))
349
350
351 def patch_configure_funcs(test):
352@@ -383,6 +393,11 @@
353 class TestConfigureDHCP(MAASServerTestCase):
354 """Tests for `configure_dhcp`."""
355
356+ def setUp(self):
357+ super(TestConfigureDHCP, self).setUp()
358+ # Suppress checks for cluster availability.
359+ self.patch_autospec(dhcp, "getClientFor")
360+
361 def test__obeys_DHCP_CONNECT(self):
362 configure_dhcpv4, configure_dhcpv6 = patch_configure_funcs(self)
363 cluster = make_cluster(self)
364@@ -406,11 +421,12 @@
365 with post_commit_hooks:
366 configure_dhcp(cluster)
367
368- self.expectThat(configure_dhcpv4, MockCalledOnceWith(cluster, [], ANY))
369- self.expectThat(configure_dhcpv6, MockCalledOnceWith(cluster, [], ANY))
370+ self.expectThat(configure_dhcpv4, MockCalledOnceWith(
371+ cluster, [], ANY, dhcp.getClientFor.return_value))
372+ self.expectThat(configure_dhcpv6, MockCalledOnceWith(
373+ cluster, [], ANY, dhcp.getClientFor.return_value))
374
375 def test__configures_dhcpv4(self):
376- getClientFor = self.patch_autospec(dhcp, "getClientFor")
377 ip = factory.make_ipv4_address()
378 cluster = make_cluster(self, maas_url='http://%s/' % ip)
379 make_ipv4_interface(self, cluster)
380@@ -419,9 +435,8 @@
381 with post_commit_hooks:
382 configure_dhcp(cluster)
383
384- self.assertThat(getClientFor, MockCallsMatch(
385- call(cluster.uuid), call(cluster.uuid)))
386- client = getClientFor.return_value
387+ self.assertThat(dhcp.getClientFor, MockCalledOnceWith(cluster.uuid))
388+ client = dhcp.getClientFor.return_value
389 self.assertThat(client, MockCallsMatch(
390 call(ANY, omapi_key=ANY, subnet_configs=ANY),
391 call(ANY, omapi_key=ANY, subnet_configs=ANY),
392@@ -447,11 +462,30 @@
393
394 ntp_server = Config.objects.get_config('ntp_server')
395 self.assertThat(
396- configure_dhcpv4,
397- MockCalledOnceWith(ANY, ANY, ntp_server))
398+ configure_dhcpv4, MockCalledOnceWith(
399+ ANY, ANY, ntp_server, dhcp.getClientFor.return_value))
400 self.assertThat(
401- configure_dhcpv6,
402- MockCalledOnceWith(ANY, ANY, ntp_server))
403+ configure_dhcpv6, MockCalledOnceWith(
404+ ANY, ANY, ntp_server, dhcp.getClientFor.return_value))
405+
406+
407+class TestConfigureDHCPWithDisconnectedCluster(MAASServerTestCase):
408+ """Behaviour when the target cluster is not connected."""
409+
410+ def test__logs_about_disconnected_cluster(self):
411+ cluster = make_cluster(self)
412+ self.patch(settings, "DHCP_CONNECT", True)
413+
414+ with LoggerFixture(dhcp.__name__) as logger:
415+ with post_commit_hooks:
416+ configure_dhcp(cluster)
417+
418+ self.assertDocTestMatches(
419+ """\
420+ Cluster ... (...) is not connected at present so cannot be
421+ configured; it will catch up when it next connects.
422+ """,
423+ logger.output)
424
425
426 class TestConfigureDHCPTransactional(MAASTransactionServerTestCase):
427@@ -461,6 +495,14 @@
428 must be committed to the database in order that they're visible elsewhere.
429 """
430
431+ def setUp(self):
432+ super(TestConfigureDHCPTransactional, self).setUp()
433+ # Suppress checks for cluster availability.
434+ getClientFor = self.patch_autospec(dhcp, "getClientFor")
435+ getClientFor.return_value = sentinel.client
436+ # Connect DHCP changes.
437+ self.patch(settings, "DHCP_CONNECT", True)
438+
439 def test__passes_only_IPv4_interfaces_to_DHCPv4(self):
440 configure_dhcpv4, _ = patch_configure_funcs(self)
441
442@@ -468,14 +510,12 @@
443 cluster = make_cluster(self)
444 ipv4_interface = make_ipv4_interface(self, cluster)
445 make_ipv6_interface(self, cluster)
446- self.patch(settings, "DHCP_CONNECT", True)
447
448 with post_commit_hooks:
449 configure_dhcp(cluster)
450
451- self.assertThat(
452- configure_dhcpv4,
453- MockCalledOnceWith(cluster, [ipv4_interface], ANY))
454+ self.assertThat(configure_dhcpv4, MockCalledOnceWith(
455+ cluster, [ipv4_interface], ANY, sentinel.client))
456
457 def test__passes_only_IPv6_interfaces_to_DHCPv6(self):
458 _, configure_dhcpv6 = patch_configure_funcs(self)
459@@ -484,14 +524,12 @@
460 cluster = make_cluster(self)
461 ipv6_interface = make_ipv6_interface(self, cluster)
462 make_ipv4_interface(self, cluster)
463- self.patch(settings, "DHCP_CONNECT", True)
464
465 with post_commit_hooks:
466 configure_dhcp(cluster)
467
468- self.assertThat(
469- configure_dhcpv6,
470- MockCalledOnceWith(cluster, [ipv6_interface], ANY))
471+ self.assertThat(configure_dhcpv6, MockCalledOnceWith(
472+ cluster, [ipv6_interface], ANY, sentinel.client))
473
474
475 class TestDHCPConnect(MAASServerTestCase):