Merge lp:~gmb/maas/backport-to-1.7-bug-1382108 into lp:maas/1.7

Proposed by Graham Binns on 2014-10-22
Status: Merged
Approved by: Graham Binns on 2014-10-22
Approved revision: 3271
Merged at revision: 3274
Proposed branch: lp:~gmb/maas/backport-to-1.7-bug-1382108
Merge into: lp:maas/1.7
Diff against target: 107 lines (+75/-1)
2 files modified
src/maasserver/models/node.py (+12/-1)
src/maasserver/models/tests/test_node.py (+63/-0)
To merge this branch: bzr merge lp:~gmb/maas/backport-to-1.7-bug-1382108
Reviewer Review Type Date Requested Status
Julian Edwards (community) 2014-10-22 Approve on 2014-10-22
Review via email: mp+239299@code.launchpad.net

Commit message

Backport the fix for bug 1382108 to 1.7:

Don't write DHCP host maps when starting a node if its PXE MAC is not on a managed cluster interface, they are not needed. This also removes a related crash (see bug 1382108).

To post a comment you must log in.
Christian Reis (kiko) wrote :

Ah, this looks much better. If you can get someone who knows their stuff to review the actual code changes (I don't know the internal API well enough) this is clear to land in 1.7. Thanks!

Julian Edwards (julian-edwards) wrote :

Looks almost like my trunk change, therefore it must be fine :)

review: Approve
Julian Edwards (julian-edwards) wrote :

Thanks for backporting it.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/models/node.py'
2--- src/maasserver/models/node.py 2014-10-15 20:00:42 +0000
3+++ src/maasserver/models/node.py 2014-10-22 20:24:21 +0000
4@@ -414,7 +414,10 @@
5 for node in nodes:
6 if node.status == NODE_STATUS.ALLOCATED:
7 claims = node.claim_static_ip_addresses()
8- static_mappings[node.nodegroup].update(claims)
9+ # If the PXE mac is on a managed interface then we can ask
10+ # the cluster to generate the DHCP host map(s).
11+ if node.is_pxe_mac_on_managed_interface():
12+ static_mappings[node.nodegroup].update(claims)
13 node.start_deployment()
14
15 # XXX 2014-06-17 bigjools bug=1330765
16@@ -1550,3 +1553,11 @@
17 return self.pxe_mac
18
19 return self.macaddress_set.first()
20+
21+ def is_pxe_mac_on_managed_interface(self):
22+ pxe_mac = self.get_pxe_mac()
23+ if pxe_mac is not None:
24+ cluster_interface = pxe_mac.cluster_interface
25+ if cluster_interface is not None:
26+ return cluster_interface.is_managed
27+ return False
28
29=== modified file 'src/maasserver/models/tests/test_node.py'
30--- src/maasserver/models/tests/test_node.py 2014-10-15 22:29:41 +0000
31+++ src/maasserver/models/tests/test_node.py 2014-10-22 20:24:21 +0000
32@@ -1834,6 +1834,28 @@
33 self.assertEqual(node.macaddress_set.first(), node.get_pxe_mac())
34
35
36+class TestNode_pxe_mac_on_managed_interface(MAASServerTestCase):
37+
38+ def test__returns_true_if_managed(self):
39+ node = factory.make_node_with_mac_attached_to_nodegroupinterface()
40+ self.assertTrue(node.is_pxe_mac_on_managed_interface())
41+
42+ def test__returns_false_if_no_pxe_mac(self):
43+ node = factory.make_Node()
44+ self.assertFalse(node.is_pxe_mac_on_managed_interface())
45+
46+ def test__returns_false_if_no_attached_cluster_interface(self):
47+ node = factory.make_Node()
48+ node.pxe_mac = factory.make_MACAddress(node=node)
49+ node.save()
50+ self.assertFalse(node.is_pxe_mac_on_managed_interface())
51+
52+ def test__returns_false_if_cluster_interface_unmanaged(self):
53+ node = factory.make_node_with_mac_attached_to_nodegroupinterface(
54+ management=NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED)
55+ self.assertFalse(node.is_pxe_mac_on_managed_interface())
56+
57+
58 class NodeRoutersTest(MAASServerTestCase):
59
60 def test_routers_stores_mac_address(self):
61@@ -2375,6 +2397,47 @@
62 # demonstrates this behaviour.
63 self.assertItemsEqual([node1], nodes_started)
64
65+ def test__does_not_generate_host_maps_if_not_on_managed_interface(self):
66+ cluster = factory.make_NodeGroup()
67+ managed_interface = factory.make_NodeGroupInterface(
68+ nodegroup=cluster,
69+ management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS)
70+ unmanaged_interface = factory.make_NodeGroupInterface(
71+ nodegroup=cluster,
72+ management=NODEGROUPINTERFACE_MANAGEMENT.DEFAULT)
73+ user = factory.make_User()
74+ [node1, node2] = self.make_acquired_nodes_with_macs(
75+ user, nodegroup=cluster, count=2)
76+ # Give the node a PXE MAC address on the cluster's interface.
77+ node1_mac = node1.get_pxe_mac()
78+ node1_mac.cluster_interface = managed_interface
79+ node1_mac.save()
80+ node2_mac = node1.get_pxe_mac()
81+ node2_mac.cluster_interface = unmanaged_interface
82+ node2_mac.save()
83+
84+ node1_ip = factory.make_ipv4_address()
85+ claim_static_ip_addresses = self.patch(
86+ node_module.Node, 'claim_static_ip_addresses')
87+ claim_static_ip_addresses.side_effect = [
88+ [(node1_ip, node1_mac.mac_address)],
89+ [(factory.make_ipv4_address(), node2_mac.mac_address)],
90+ ]
91+
92+ update_host_maps = self.patch(node_module, "update_host_maps")
93+ Node.objects.start_nodes([node1.system_id, node2.system_id], user)
94+ self.expectThat(update_host_maps, MockCalledOnceWith(ANY))
95+
96+ observed_static_mappings = update_host_maps.call_args[0][0]
97+
98+ [observed_cluster] = observed_static_mappings.keys()
99+ self.expectThat(observed_cluster.uuid, Equals(cluster.uuid))
100+
101+ observed_claims = observed_static_mappings.values()
102+ self.expectThat(
103+ observed_claims,
104+ Equals([{node1_ip: node1_mac.mac_address}]))
105+
106
107 class NodeManagerTest_StopNodes(MAASServerTestCase):
108

Subscribers

People subscribed via source and target branches

to all changes: