Merge lp:~lamont/maas/bug-1513485 into lp:~maas-committers/maas/trunk

[LaMont Jones]

Fix handling of multiple StaticIPAddress rows with empty IP addresses. In some circumstances, (possibly migration from 1.8, or other unknown reasons) there can be multiple StaticIPAddress entries for a subnet with no IP address. Previously, this caused the code to crash on a get_or_create() call.

Migration means that sometimes, there are multiple empty StaticIPAddress entries for a subnet.

[Mike Pontillo (mpontillo)]

I'm going to approve this since it's a pretty simple change, but please see address my comments below before landing this.

review: Approve

=== modified file 'src/maasserver/models/staticipaddress.py'
--- src/maasserver/models/staticipaddress.py	2015-11-02 15:48:12 +0000
+++ src/maasserver/models/staticipaddress.py	2015-11-12 13:57:06 +0000
@@ -470,10 +470,19 @@
                     ipaddress.interface_set.exclude(mac_address__in=mac_list))
                 if len(other_interfaces) > 0:
                     # Get or create an empty DISCOVERED IP address for these
-                    # other interfaces, linked to the old subnet.
-                    empty_ip, _ = StaticIPAddress.objects.get_or_create(
-                        alloc_type=IPADDRESS_TYPE.DISCOVERED, ip=None,
-                        subnet=ipaddress.subnet)
+                    # other interfaces, linked to the old subnet.  if we have
+                    # migrated from 1.8 to 1.9, it's possible to have more
+                    # than one empty_ip for the subnet.  Use the first one if
+                    # it is there, if it isn't, then let get_or_create make
+                    # one for us.
+                    empty_ip = StaticIPAddress.objects.filter(
+                        ip=None,
+                        alloc_type=IPADDRESS_TYPE.DISCOVERED,
+                        subnet=ipaddress.subnet).first()
+                    if empty_ip is None:
+                        empty_ip, _ = StaticIPAddress.objects.get_or_create(
+                            alloc_type=IPADDRESS_TYPE.DISCOVERED, ip=None,
+                            subnet=ipaddress.subnet)
                     for other_interface in other_interfaces:
                         other_interface.ip_addresses.remove(ipaddress)
                         other_interface.ip_addresses.add(empty_ip)

=== modified file 'src/maasserver/models/tests/test_staticipaddress.py'
--- src/maasserver/models/tests/test_staticipaddress.py	2015-11-02 15:57:19 +0000
+++ src/maasserver/models/tests/test_staticipaddress.py	2015-11-12 13:57:06 +0000
@@ -395,6 +395,35 @@
         self.assertIsNone(extra_ip.ip)
         self.assertEquals(subnet, lease_ip.subnet)
 
+    def test_update_leases_handles_multiple_empty_ips(self):
+        cidr = unicode(factory.make_ipv4_network().cidr)
+        node = factory.make_Node_with_Interface_on_Subnet(cidr=cidr)
+        boot_interface = node.get_boot_interface()
+        ip_address = boot_interface.ip_addresses.first()
+        subnet = ip_address.subnet
+        ngi = subnet.nodegroupinterface_set.first()
+        # Create a pre-1.9 condition in the StaticIPAddress table.
+        factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.DISCOVERED, ip=None, subnet=subnet)
+        discovered_ip = unicode(IPAddress(ngi.get_dynamic_ip_range()[0]))
+        macs = [factory.make_mac_address() for i in range(2)]
+        StaticIPAddress.objects.update_leases(
+            node.nodegroup, [(discovered_ip, macs[0])])
+        # Now move to the new MAC, and ensure that the table is correctly
+        # updated, even when multiple empty IP addresses are in the table.
+        # (See also bug #1513485).
+        StaticIPAddress.objects.update_leases(
+            node.nodegroup, [(discovered_ip, macs[1])])
+        new_ips = StaticIPAddress.objects.filter(ip=discovered_ip)
+        self.assertEqual(1, len(new_ips))
+        self.assertEquals(
+            IPADDRESS_TYPE.DISCOVERED, new_ips[0].alloc_type)
+        self.assertEquals(
+            INTERFACE_TYPE.UNKNOWN, new_ips[0].interface_set.first().type)
+        empty_ips = StaticIPAddress.objects.filter(
+            ip=None, alloc_type=IPADDRESS_TYPE.DISCOVERED, subnet=subnet)
+        self.assertEqual(2, len(empty_ips))
+
     def test_update_leases_only_keeps_one_DISCOVERED_address(self):
         vlan = VLAN.objects.get_default_vlan()
         node = factory.make_Node_with_Interface_on_Subnet(vlan=vlan)