Merge lp:~trapnine/maas/dynamic_range_overlaps_discovered_ip_1580772 into lp:~maas-committers/maas/trunk

[Jeffrey C Jones]

Dynamic ranges can overlap DISCOVERED IPs.

Dynamic ranges can overlap DISCOVERED IPs.

[Mike Pontillo (mpontillo)]

Looks good. Thanks for the fixes.

review: Approve

=== modified file 'src/maasserver/models/subnet.py'
--- src/maasserver/models/subnet.py	2016-06-02 18:16:20 +0000
+++ src/maasserver/models/subnet.py	2016-06-09 23:39:12 +0000
@@ -27,6 +27,7 @@
 from django.db.models.query import QuerySet
 from maasserver import DefaultMeta
 from maasserver.enum import (
+    IPADDRESS_TYPE,
     IPRANGE_TYPE,
     RDNS_MODE,
     RDNS_MODE_CHOICES,
@@ -401,20 +402,21 @@
                 "IP range. (Delete the dynamic range or disable DHCP first.)")
         super().delete(*args, **kwargs)
 
-    def get_staticipaddresses_in_use(self):
-        """Returns a list of `netaddr.IPAddress` objects to represent each
-        IP address in use in this `Subnet`."""
-        # We could exclude DISCOVERED addresses here, but that wouldn't be
-        # genuine. (we'd be allowing something we have observed as an in-use
-        # address to potentially be claimed for something else, which could
-        # be a conflict.)
+    def _get_ranges_for_allocated_ips(self, ipnetwork):
+        """Returns a set of IPrange objects created from the set of allocated
+        StaticIPAddress objects.."""
+        # We exclude DISCOVERED addresses here because they aren't considered
+        # allocated ranges, and new ranges can overlap them - lp:1580772).
         # Note, the original implementation used .exclude() to filter,
         # but we'll filter at runtime so that prefetch_related in the
         # websocket works properly.
-        return set(
-            IPAddress(ip.ip)
-            for ip in self.staticipaddress_set.all()
-            if ip.ip)
+        ranges = set()
+        for sip in self.staticipaddress_set.all():
+            if sip.ip and sip.alloc_type != IPADDRESS_TYPE.DISCOVERED:
+                ip = IPAddress(sip.ip)
+                if ip in ipnetwork:
+                    ranges.add(make_iprange(ip, purpose="assigned-ip"))
+        return ranges
 
     def get_ipranges_in_use(self, exclude_addresses=[], ranges_only=False):
         """Returns a `MAASIPSet` of `MAASIPRange` objects which are currently
@@ -447,7 +449,6 @@
                     first, first, purpose="rfc-4291-2.6.1")}
         ipnetwork = self.get_ipnetwork()
         if not ranges_only:
-            assigned_ip_addresses = self.get_staticipaddresses_in_use()
             if (self.gateway_ip is not None and self.gateway_ip != '' and
                     self.gateway_ip in ipnetwork):
                 ranges |= {make_iprange(self.gateway_ip, purpose="gateway-ip")}
@@ -457,11 +458,7 @@
                     for server in self.dns_servers
                     if server in ipnetwork
                 )
-            ranges |= set(
-                make_iprange(ip, purpose="assigned-ip")
-                for ip in assigned_ip_addresses
-                if ip in ipnetwork
-            )
+            ranges |= self._get_ranges_for_allocated_ips(ipnetwork)
             ranges |= set(
                 make_iprange(address, purpose="excluded")
                 for address in exclude_addresses

=== modified file 'src/maasserver/models/tests/test_iprange.py'
--- src/maasserver/models/tests/test_iprange.py	2016-05-12 19:07:37 +0000
+++ src/maasserver/models/tests/test_iprange.py	2016-06-09 23:39:12 +0000
@@ -5,6 +5,8 @@
 
 __all__ = []
 
+import random
+
 from django.core.exceptions import ValidationError
 from maasserver.enum import (
     IPADDRESS_TYPE,
@@ -432,23 +434,15 @@
         with ExpectedException(ValidationError, self.reserved_no_fit):
             iprange.save()
 
-    def test__dynamic_range_cannot_overlap_static_ip(self):
+    def test__reserved_range_can_overlap_most_ip_types(self):
         subnet = make_plain_subnet()
         factory.make_StaticIPAddress(
-            alloc_type=IPADDRESS_TYPE.USER_RESERVED, subnet=subnet)
-        iprange = IPRange(
             subnet=subnet,
-            type=IPRANGE_TYPE.DYNAMIC,
-            start_ip="192.168.0.2",
-            end_ip="192.168.0.254",
-        )
-        with ExpectedException(ValidationError, self.dynamic_overlaps):
-            iprange.save()
-
-    def test__reserved_range_can_overlap_static_ip(self):
-        subnet = make_plain_subnet()
-        factory.make_StaticIPAddress(
-            alloc_type=IPADDRESS_TYPE.USER_RESERVED, subnet=subnet)
+            alloc_type=random.choice((
+                IPADDRESS_TYPE.AUTO,
+                IPADDRESS_TYPE.STICKY,
+                IPADDRESS_TYPE.USER_RESERVED,
+                IPADDRESS_TYPE.DISCOVERED)))
         iprange = IPRange(
             subnet=subnet,
             type=IPRANGE_TYPE.RESERVED,
@@ -457,6 +451,53 @@
         )
         iprange.save()
 
+    def test__dynamic_range_cannot_overlap_most_ip_types(self):
+        subnet = make_plain_subnet()
+        factory.make_StaticIPAddress(
+            subnet=subnet,
+            alloc_type=random.choice((
+                IPADDRESS_TYPE.AUTO,
+                IPADDRESS_TYPE.STICKY,
+                IPADDRESS_TYPE.USER_RESERVED)))
+        iprange = IPRange(
+            subnet=subnet,
+            type=IPRANGE_TYPE.DYNAMIC,
+            start_ip="192.168.0.2",
+            end_ip="192.168.0.254",
+        )
+        with ExpectedException(ValidationError, self.dynamic_overlaps):
+            iprange.save()
+
+    # Regression for lp:1580772.
+    def test__dynamic_range_can_overlap_discovered_ip(self):
+        subnet = make_plain_subnet()
+        factory.make_StaticIPAddress(
+            ip="192.168.0.3",
+            alloc_type=IPADDRESS_TYPE.DISCOVERED,
+            subnet=subnet)
+        iprange = IPRange(
+            subnet=subnet,
+            type=IPRANGE_TYPE.DYNAMIC,
+            start_ip="192.168.0.2",
+            end_ip="192.168.0.254",
+        )
+        iprange.save()
+
+    # Regression for lp:1580772.
+    def test__dynamic_range_can_match_discovered_ip(self):
+        subnet = make_plain_subnet()
+        factory.make_StaticIPAddress(
+            ip="192.168.0.3",
+            alloc_type=IPADDRESS_TYPE.DISCOVERED,
+            subnet=subnet)
+        iprange = IPRange(
+            subnet=subnet,
+            type=IPRANGE_TYPE.DYNAMIC,
+            start_ip="192.168.0.3",
+            end_ip="192.168.0.3",
+        )
+        iprange.save()
+
     def test__dynamic_range_cannot_overlap_dns_servers(self):
         subnet = factory.make_Subnet(
             cidr='192.168.0.0/24',