Merge lp:~blake-rouse/maas/fix-1519527-1.9 into lp:maas/1.9

[Blake Rouse]

Prevent claim_static_ips from allocated more than one IP address per discovered subnet.

[Mike Pontillo (mpontillo)]

Nice find. Looks good!

review: Approve

=== modified file 'src/maasserver/models/interface.py'
--- src/maasserver/models/interface.py	2015-12-03 18:59:49 +0000
+++ src/maasserver/models/interface.py	2015-12-04 16:56:51 +0000
@@ -1317,6 +1317,11 @@
                 if ngi is not None and ngi.subnet is not None:
                     discovered_subnets.append(ngi.subnet)
 
+        # This must be a set because it is highly possible that the parent
+        # has multiple subnets on the same interface or same subnet on multiple
+        # interfaces. We only want to allocate one ip address per subnet.
+        discovered_subnets = set(discovered_subnets)
+
         if len(discovered_subnets) == 0:
             node = self.node
             if parent is not None:

=== modified file 'src/maasserver/models/tests/test_interface.py'
--- src/maasserver/models/tests/test_interface.py	2015-12-03 19:51:52 +0000
+++ src/maasserver/models/tests/test_interface.py	2015-12-04 16:56:51 +0000
@@ -2575,6 +2575,42 @@
                 call(INTERFACE_LINK_TYPE.STATIC, subnet_v4),
                 call(INTERFACE_LINK_TYPE.STATIC, subnet_v6)))
 
+    def test__without_address_calls_link_subnet_once_per_subnet(self):
+        nodegroup = factory.make_NodeGroup(status=NODEGROUP_STATUS.ENABLED)
+        interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
+        network_v4 = factory.make_ipv4_network()
+        subnet_v4 = factory.make_Subnet(cidr=unicode(network_v4.cidr))
+        factory.make_NodeGroupInterface(
+            nodegroup, management=NODEGROUPINTERFACE_MANAGEMENT.DHCP,
+            subnet=subnet_v4)
+        factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.DISCOVERED, ip="",
+            subnet=subnet_v4, interface=interface)
+        # Make it have the same subnet twice.
+        factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.DISCOVERED, ip="",
+            subnet=subnet_v4, interface=interface)
+        network_v6 = factory.make_ipv6_network()
+        subnet_v6 = factory.make_Subnet(cidr=unicode(network_v6.cidr))
+        factory.make_NodeGroupInterface(
+            nodegroup, management=NODEGROUPINTERFACE_MANAGEMENT.DHCP,
+            subnet=subnet_v6)
+        factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.DISCOVERED, ip="",
+            subnet=subnet_v6, interface=interface)
+        # Make it have the same subnet twice.
+        factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.DISCOVERED, ip="",
+            subnet=subnet_v6, interface=interface)
+
+        mock_link_subnet = self.patch_autospec(interface, "link_subnet")
+        interface.claim_static_ips()
+        self.assertThat(
+            mock_link_subnet,
+            MockCallsMatch(
+                call(INTERFACE_LINK_TYPE.STATIC, subnet_v4),
+                call(INTERFACE_LINK_TYPE.STATIC, subnet_v6)))
+
     def test__without_address_does_nothing_if_none_managed(self):
         interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
         network_v4 = factory.make_ipv4_network()
@@ -2659,6 +2695,49 @@
                 call(INTERFACE_LINK_TYPE.STATIC, subnet_v4),
                 call(INTERFACE_LINK_TYPE.STATIC, subnet_v6)))
 
+    def test__device_no_address_calls_link_subnet_once_per_subnet(self):
+        nodegroup = factory.make_NodeGroup(status=NODEGROUP_STATUS.ENABLED)
+        parent = factory.make_Node()
+        parent_nic0 = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, node=parent)
+        parent_nic1 = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, node=parent)
+        network_v4 = factory.make_ipv4_network()
+        subnet_v4 = factory.make_Subnet(cidr=unicode(network_v4.cidr))
+        factory.make_NodeGroupInterface(
+            nodegroup, management=NODEGROUPINTERFACE_MANAGEMENT.DHCP,
+            subnet=subnet_v4)
+        factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.DISCOVERED, ip="",
+            subnet=subnet_v4, interface=parent_nic0)
+        # Make second interface on the parent have the same subnet.
+        factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.DISCOVERED, ip="",
+            subnet=subnet_v4, interface=parent_nic1)
+        network_v6 = factory.make_ipv6_network()
+        subnet_v6 = factory.make_Subnet(cidr=unicode(network_v6.cidr))
+        factory.make_NodeGroupInterface(
+            nodegroup, management=NODEGROUPINTERFACE_MANAGEMENT.DHCP,
+            subnet=subnet_v6)
+        factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.DISCOVERED, ip="",
+            subnet=subnet_v6, interface=parent_nic0)
+        # Make second interface on the parent have the same subnet.
+        factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.DISCOVERED, ip="",
+            subnet=subnet_v6, interface=parent_nic1)
+        device = factory.make_Device(parent=parent)
+        device_interface = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, node=device)
+
+        mock_link_subnet = self.patch_autospec(device_interface, "link_subnet")
+        device_interface.claim_static_ips()
+        self.assertThat(
+            mock_link_subnet,
+            MockCallsMatch(
+                call(INTERFACE_LINK_TYPE.STATIC, subnet_v4),
+                call(INTERFACE_LINK_TYPE.STATIC, subnet_v6)))
+
     def test__device_with_address_calls_link_subnet_with_ip_address(self):
         parent = factory.make_Node()
         interface = factory.make_Interface(