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

[Graham Binns]

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).

[Christian Reis (kiko)]

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)]

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

review: Approve

[Julian Edwards (julian-edwards)]

Thanks for backporting it.

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2014-10-15 20:00:42 +0000
+++ src/maasserver/models/node.py	2014-10-22 20:24:21 +0000
@@ -414,7 +414,10 @@
         for node in nodes:
             if node.status == NODE_STATUS.ALLOCATED:
                 claims = node.claim_static_ip_addresses()
-                static_mappings[node.nodegroup].update(claims)
+                # If the PXE mac is on a managed interface then we can ask
+                # the cluster to generate the DHCP host map(s).
+                if node.is_pxe_mac_on_managed_interface():
+                    static_mappings[node.nodegroup].update(claims)
                 node.start_deployment()
 
         # XXX 2014-06-17 bigjools bug=1330765
@@ -1550,3 +1553,11 @@
             return self.pxe_mac
 
         return self.macaddress_set.first()
+
+    def is_pxe_mac_on_managed_interface(self):
+        pxe_mac = self.get_pxe_mac()
+        if pxe_mac is not None:
+            cluster_interface = pxe_mac.cluster_interface
+            if cluster_interface is not None:
+                return cluster_interface.is_managed
+        return False

=== modified file 'src/maasserver/models/tests/test_node.py'
--- src/maasserver/models/tests/test_node.py	2014-10-15 22:29:41 +0000
+++ src/maasserver/models/tests/test_node.py	2014-10-22 20:24:21 +0000
@@ -1834,6 +1834,28 @@
         self.assertEqual(node.macaddress_set.first(), node.get_pxe_mac())
 
 
+class TestNode_pxe_mac_on_managed_interface(MAASServerTestCase):
+
+    def test__returns_true_if_managed(self):
+        node = factory.make_node_with_mac_attached_to_nodegroupinterface()
+        self.assertTrue(node.is_pxe_mac_on_managed_interface())
+
+    def test__returns_false_if_no_pxe_mac(self):
+        node = factory.make_Node()
+        self.assertFalse(node.is_pxe_mac_on_managed_interface())
+
+    def test__returns_false_if_no_attached_cluster_interface(self):
+        node = factory.make_Node()
+        node.pxe_mac = factory.make_MACAddress(node=node)
+        node.save()
+        self.assertFalse(node.is_pxe_mac_on_managed_interface())
+
+    def test__returns_false_if_cluster_interface_unmanaged(self):
+        node = factory.make_node_with_mac_attached_to_nodegroupinterface(
+            management=NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED)
+        self.assertFalse(node.is_pxe_mac_on_managed_interface())
+
+
 class NodeRoutersTest(MAASServerTestCase):
 
     def test_routers_stores_mac_address(self):
@@ -2375,6 +2397,47 @@
         # demonstrates this behaviour.
         self.assertItemsEqual([node1], nodes_started)
 
+    def test__does_not_generate_host_maps_if_not_on_managed_interface(self):
+        cluster = factory.make_NodeGroup()
+        managed_interface = factory.make_NodeGroupInterface(
+            nodegroup=cluster,
+            management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS)
+        unmanaged_interface = factory.make_NodeGroupInterface(
+            nodegroup=cluster,
+            management=NODEGROUPINTERFACE_MANAGEMENT.DEFAULT)
+        user = factory.make_User()
+        [node1, node2] = self.make_acquired_nodes_with_macs(
+            user, nodegroup=cluster, count=2)
+        # Give the node a PXE MAC address on the cluster's interface.
+        node1_mac = node1.get_pxe_mac()
+        node1_mac.cluster_interface = managed_interface
+        node1_mac.save()
+        node2_mac = node1.get_pxe_mac()
+        node2_mac.cluster_interface = unmanaged_interface
+        node2_mac.save()
+
+        node1_ip = factory.make_ipv4_address()
+        claim_static_ip_addresses = self.patch(
+            node_module.Node, 'claim_static_ip_addresses')
+        claim_static_ip_addresses.side_effect = [
+            [(node1_ip, node1_mac.mac_address)],
+            [(factory.make_ipv4_address(), node2_mac.mac_address)],
+            ]
+
+        update_host_maps = self.patch(node_module, "update_host_maps")
+        Node.objects.start_nodes([node1.system_id, node2.system_id], user)
+        self.expectThat(update_host_maps, MockCalledOnceWith(ANY))
+
+        observed_static_mappings = update_host_maps.call_args[0][0]
+
+        [observed_cluster] = observed_static_mappings.keys()
+        self.expectThat(observed_cluster.uuid, Equals(cluster.uuid))
+
+        observed_claims = observed_static_mappings.values()
+        self.expectThat(
+            observed_claims,
+            Equals([{node1_ip: node1_mac.mac_address}]))
+
 
 class NodeManagerTest_StopNodes(MAASServerTestCase):