Merge lp:~mpontillo/maas/fix-discovered-ip-uniqueness--bug-1686736 into lp:~maas-committers/maas/trunk

[Mike Pontillo]

Relax unique constraint on StaticIPAddress model to allow DISCOVERED IP addresses to coexist with other types.

 * Switches from unique=True on the 'ip' field to Django's unique_together.
 * Adds a partial index to allow only DISCOVERED IP addresses to coexist with other allocation types (all the other types' addresses still must be unique among themselves).

This was also tested manually as follows:

    http://paste.ubuntu.com/24467822/

[Blake Rouse (blake-rouse)]

Looks good.

review: Approve

=== added file 'src/maasserver/migrations/builtin/maasserver/0121_relax_staticipaddress_unique_constraint.py'
--- src/maasserver/migrations/builtin/maasserver/0121_relax_staticipaddress_unique_constraint.py	1970-01-01 00:00:00 +0000
+++ src/maasserver/migrations/builtin/maasserver/0121_relax_staticipaddress_unique_constraint.py	2017-04-27 18:18:29 +0000
@@ -0,0 +1,36 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import (
+    migrations,
+    models,
+)
+import maasserver.fields
+
+# IPs must be unique across the entire table, except DISCOVERED addresses
+# (which are type 6). The unique_together ensures that IP addresses are also
+# unique amongst their own alloc_type. This index strengthens that.
+staticipaddress_unique_index_create = (
+    "CREATE UNIQUE INDEX maasserver_staticipaddress__discovered_unique"
+    "    ON maasserver_staticipaddress (ip)"
+    "    WHERE alloc_type != 6"
+)
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('maasserver', '0120_bootsourcecache_extra'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='staticipaddress',
+            name='ip',
+            field=maasserver.fields.MAASIPAddressField(editable=False, verbose_name='IP', blank=True, null=True, default=None),
+        ),
+        migrations.AlterUniqueTogether(
+            name='staticipaddress',
+            unique_together=set([('alloc_type', 'ip')]),
+        ),
+        migrations.RunSQL(staticipaddress_unique_index_create),
+    ]

=== modified file 'src/maasserver/models/staticipaddress.py'
--- src/maasserver/models/staticipaddress.py	2017-04-06 18:19:08 +0000
+++ src/maasserver/models/staticipaddress.py	2017-04-27 18:18:29 +0000
@@ -668,12 +668,13 @@
     class Meta(DefaultMeta):
         verbose_name = "Static IP Address"
         verbose_name_plural = "Static IP Addresses"
+        unique_together = ('alloc_type', 'ip')
 
     # IP can be none when a DHCP lease has expired: in this case the entry
     # in the StaticIPAddress only materializes the connection between an
     # interface and a subnet.
     ip = MAASIPAddressField(
-        unique=True, null=True, editable=False, blank=True,
+        unique=False, null=True, editable=False, blank=True,
         default=None, verbose_name='IP')
 
     alloc_type = IntegerField(

=== modified file 'src/maasserver/models/tests/test_staticipaddress.py'
--- src/maasserver/models/tests/test_staticipaddress.py	2017-04-06 16:38:44 +0000
+++ src/maasserver/models/tests/test_staticipaddress.py	2017-04-27 18:18:29 +0000
@@ -1358,3 +1358,42 @@
         self.assertThat(
             ip.alloc_type_name,
             Equals(""))
+
+
+class TestUniqueConstraints(MAASServerTestCase):
+
+    def test__rejects_duplicate_address_of_same_type(self):
+        subnet = factory.make_Subnet(cidr='10.0.0.0/8')
+        factory.make_StaticIPAddress(
+            ip='10.0.0.1', subnet=subnet,
+            alloc_type=IPADDRESS_TYPE.USER_RESERVED)
+        with ExpectedException(IntegrityError):
+            factory.make_StaticIPAddress(
+                ip='10.0.0.1', alloc_type=IPADDRESS_TYPE.USER_RESERVED)
+
+    def test__rejects_duplicate_address_for_two_different_static_types(self):
+        subnet = factory.make_Subnet(cidr='10.0.0.0/8')
+        factory.make_StaticIPAddress(
+            ip='10.0.0.1', subnet=subnet, alloc_type=IPADDRESS_TYPE.STICKY)
+        with ExpectedException(IntegrityError):
+            factory.make_StaticIPAddress(
+                ip='10.0.0.1', subnet=subnet,
+                alloc_type=IPADDRESS_TYPE.USER_RESERVED)
+
+    def test__rejects_duplicate_discovered(self):
+        subnet = factory.make_Subnet(cidr='10.0.0.0/8')
+        factory.make_StaticIPAddress(
+            ip='10.0.0.1', subnet=subnet, alloc_type=IPADDRESS_TYPE.DISCOVERED)
+        with ExpectedException(IntegrityError):
+            factory.make_StaticIPAddress(
+                ip='10.0.0.1', subnet=subnet,
+                alloc_type=IPADDRESS_TYPE.DISCOVERED)
+
+    def test__allows_discovered_to_coexist_with_static(self):
+        subnet = factory.make_Subnet(cidr='10.0.0.0/8')
+        factory.make_StaticIPAddress(
+            ip='10.0.0.1', subnet=subnet, alloc_type=IPADDRESS_TYPE.DISCOVERED)
+        factory.make_StaticIPAddress(
+            ip='10.0.0.1', subnet=subnet, alloc_type=IPADDRESS_TYPE.STICKY)
+        self.assertThat(
+            StaticIPAddress.objects.filter(ip='10.0.0.1').count(), Equals(2))