Merge lp:~mpontillo/maas/unmanaged-subnets into lp:~maas-committers/maas/trunk

[Mike Pontillo]

WIP

[MAAS Lander (maas-lander)]

Transitioned to Git.

lp:maas has now moved from Bzr to Git.
Please propose your branches with Launchpad using Git.

git clone https://git.launchpad.net/maas

Unmerged revisions

5573. By Mike Pontillo on 2016-11-29

Unmanaged subnets WIP.

=== modified file 'src/maasserver/migrations/builtin/maasserver/0027_replace_static_range_with_admin_reserved_ranges.py'
--- src/maasserver/migrations/builtin/maasserver/0027_replace_static_range_with_admin_reserved_ranges.py	2016-05-11 19:01:48 +0000
+++ src/maasserver/migrations/builtin/maasserver/0027_replace_static_range_with_admin_reserved_ranges.py	2016-11-29 18:04:20 +0000
@@ -37,7 +37,7 @@
         IPRange, subnet, ranges, created_time, range_description):
     unreserved_range_set = MAASIPSet(ranges)
     unreserved_ranges = unreserved_range_set.get_unused_ranges(
-        subnet.cidr, comment="reserved")
+        subnet.cidr, purpose="reserved")
     for iprange in unreserved_ranges:
         start_ip = str(IPAddress(iprange.first))
         end_ip = str(IPAddress(iprange.last))

=== added file 'src/maasserver/migrations/builtin/maasserver/0094_add_unmanaged_subnets.py'
--- src/maasserver/migrations/builtin/maasserver/0094_add_unmanaged_subnets.py	1970-01-01 00:00:00 +0000
+++ src/maasserver/migrations/builtin/maasserver/0094_add_unmanaged_subnets.py	2016-11-29 18:04:20 +0000
@@ -0,0 +1,19 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('maasserver', '0093_add_rdns_model'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='subnet',
+            name='managed',
+            field=models.BooleanField(default=True),
+        ),
+    ]

=== modified file 'src/maasserver/models/iprange.py'
--- src/maasserver/models/iprange.py	2016-10-19 19:20:24 +0000
+++ src/maasserver/models/iprange.py	2016-11-29 18:04:20 +0000
@@ -171,8 +171,9 @@
     def netaddr_iprange(self):
         return netaddr.IPRange(self.start_ip, self.end_ip)
 
-    def get_MAASIPRange(self):
-        purpose = self.type
+    def get_MAASIPRange(self, purpose=None):
+        if purpose is None:
+            purpose = self.type
         # Using '-' instead of '_' is just for consistency.
         # APIs in previous MAAS releases used '-' in range types.
         purpose = purpose.replace('_', '-')

=== modified file 'src/maasserver/models/subnet.py'
--- src/maasserver/models/subnet.py	2016-10-18 16:48:13 +0000
+++ src/maasserver/models/subnet.py	2016-11-29 18:04:20 +0000
@@ -65,6 +65,7 @@
     MaybeIPAddress,
     parse_integer,
 )
+from provisioningserver.utils.network import IPRANGE_TYPE as MAASIPRANGE_TYPE
 
 
 maaslog = get_maas_logger("subnet")
@@ -386,6 +387,9 @@
     active_discovery = BooleanField(
         editable=True, blank=False, null=False, default=False)
 
+    managed = BooleanField(
+        editable=True, blank=False, null=False, default=True)
+
     @property
     def label(self):
         """Returns a human-friendly label for this subnet."""
@@ -457,8 +461,8 @@
 
     def _get_ranges_for_allocated_ips(
             self, ipnetwork: IPNetwork, ignore_discovered_ips: bool) -> set:
-        """Returns a set of MAASIPRange objects created from the set of allocated
-        StaticIPAddress objects.
+        """Returns a set of MAASIPRange objects created from the set of
+        allocated StaticIPAddress objects.
         """
         # Note, the original implementation used .exclude() to filter,
         # but we'll filter at runtime so that prefetch_related in the
@@ -472,10 +476,49 @@
                     ranges.add(make_iprange(ip, purpose="assigned-ip"))
         return ranges
 
+    def _add_subnet_metadata(self, ranges, exclude_addresses,
+                             ignore_discovered_ips):
+        """Adds subnet metadata to the specified range set.
+
+        For IP addresses within the subnet which are either a default gateway
+        or are a gateway specified by a static route, adds the 'gateway-ip'
+        purpose to the set of ranges.
+
+        For IP addresses on the subnet which are also designated DNS servers
+        on the subnet, adds IP addresses with the `dns-server` purpose
+        to the set of ranges.
+
+        For IP addresses specified in the `exclude_addresses` parameter,
+        adds IP addresses with the `excluded` purpose to the set of ranges.
+        """
+        ipnetwork = self.get_ipnetwork()
+        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")}
+        if self.dns_servers is not None:
+            ranges |= set(
+                make_iprange(server, purpose="dns-server")
+                for server in self.dns_servers
+                if server in ipnetwork
+            )
+        for static_route in StaticRoute.objects.filter(source=self):
+            ranges |= {
+                make_iprange(
+                    static_route.gateway_ip, purpose="gateway-ip")}
+        ranges |= self._get_ranges_for_allocated_ips(
+            ipnetwork, ignore_discovered_ips)
+        ranges |= set(
+            make_iprange(address, purpose="excluded")
+            for address in exclude_addresses
+            if address in ipnetwork
+        )
+        return ranges
+
     def get_ipranges_in_use(
             self, exclude_addresses: IPAddressExcludeList=None,
             ranges_only: bool=False,
-            ignore_discovered_ips: bool=False) -> MAASIPSet:
+            ignore_discovered_ips: bool=False,
+            for_reserved_allocation: bool=False) -> MAASIPSet:
         """Returns a `MAASIPSet` of `MAASIPRange` objects which are currently
         in use on this `Subnet`.
 
@@ -483,6 +526,8 @@
         :param ignore_discovered_ips: DISCOVERED addresses are not "in use".
         :param ranges_only: if True, filters out gateway IPs, static routes,
             DNS servers, and `exclude_addresses`.
+        :param for_reserved_allocation: if True, filters out reserved IP
+            ranges, so that users can use them for allocation.
         """
         if exclude_addresses is None:
             exclude_addresses = []
@@ -499,9 +544,9 @@
             # *outside* both ranges, so that they won't conflict with addresses
             # reserved from this scheme in the future.
             first = str(IPAddress(network.first))
-            first_plus_one = str(IPAddress(network.first + 1))
-            second = str(IPAddress(network.first + 0xFFFFFFFF))
             if network.prefixlen == 64:
+                first_plus_one = str(IPAddress(network.first + 1))
+                second = str(IPAddress(network.first + 0xFFFFFFFF))
                 ranges |= {make_iprange(
                     first_plus_one, second, purpose="reserved")}
             # Reserve the subnet router anycast address, except for /127 and
@@ -509,29 +554,11 @@
             if network.prefixlen < 127:
                 ranges |= {make_iprange(
                     first, first, purpose="rfc-4291-2.6.1")}
-        ipnetwork = self.get_ipnetwork()
         if not ranges_only:
-            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")}
-            if self.dns_servers is not None:
-                ranges |= set(
-                    make_iprange(server, purpose="dns-server")
-                    for server in self.dns_servers
-                    if server in ipnetwork
-                )
-            for static_route in StaticRoute.objects.filter(source=self):
-                ranges |= {
-                    make_iprange(
-                        static_route.gateway_ip, purpose="gateway-ip")}
-            ranges |= self._get_ranges_for_allocated_ips(
-                ipnetwork, ignore_discovered_ips)
-            ranges |= set(
-                make_iprange(address, purpose="excluded")
-                for address in exclude_addresses
-                if address in network
-            )
-        ranges |= self.get_reserved_maasipset()
+            ranges |= self._add_subnet_metadata(
+                ranges, exclude_addresses, ignore_discovered_ips)
+        if not for_reserved_allocation:
+            ranges |= self.get_reserved_maasipset()
         ranges |= self.get_dynamic_maasipset()
         return MAASIPSet(ranges)
 
@@ -554,12 +581,24 @@
         """
         if exclude_addresses is None:
             exclude_addresses = []
+        for_reserved_allocation = self.managed is False and not ranges_only
         ranges = self.get_ipranges_in_use(
             exclude_addresses=exclude_addresses,
             ranges_only=ranges_only,
-            ignore_discovered_ips=ignore_discovered_ips)
+            ignore_discovered_ips=ignore_discovered_ips,
+            for_reserved_allocation=for_reserved_allocation
+        )
+        if for_reserved_allocation:
+            # For unmanaged networks, we must reserve everything NOT in a
+            # reserved range with the 'unmanaged' type.
+            reserved = self.get_reserved_maasipset()
+            unmanaged = reserved.get_unused_ranges(
+                self.get_ipnetwork(), purpose=MAASIPRANGE_TYPE.UNMANAGED)
+            ranges |= unmanaged
         if with_neighbours:
             ranges |= self.get_maasipset_for_neighbours()
+        from pprint import pprint
+        pprint(ranges)
         unused = ranges.get_unused_ranges(self.get_ipnetwork())
         return unused
 
@@ -574,7 +613,7 @@
         # IP addresses should already be covered by get_ipranges_in_use().
         neighbours = Discovery.objects.filter(subnet=self).by_unknown_ip()
         neighbour_set = {
-            make_iprange(neighbour.ip, purpose="neighbour")
+            make_iprange(neighbour.ip, purpose=MAASIPRANGE_TYPE.NEIGHBOUR)
             for neighbour in neighbours
         }
         return MAASIPSet(neighbour_set)
@@ -717,9 +756,9 @@
                     "%s is within the dynamic range from %s to %s" % (
                         ip, IPAddress(iprange.first), IPAddress(iprange.last)))
 
-    def get_reserved_maasipset(self):
+    def get_reserved_maasipset(self, purpose=None):
         reserved_ranges = MAASIPSet(
-            iprange.get_MAASIPRange()
+            iprange.get_MAASIPRange(purpose=purpose)
             for iprange in self.get_reserved_ranges()
         )
         return reserved_ranges

=== modified file 'src/maasserver/models/tests/test_subnet.py'
--- src/maasserver/models/tests/test_subnet.py	2016-11-09 08:14:00 +0000
+++ src/maasserver/models/tests/test_subnet.py	2016-11-29 18:04:20 +0000
@@ -2,6 +2,7 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for the Subnet model."""
+from maasserver.utils.orm import reload_object
 
 __all__ = []
 
@@ -898,6 +899,11 @@
 
 class TestSubnetGetNextIPForAllocation(MAASServerTestCase):
 
+    scenarios = (
+        ("managed", {'managed': True}),
+        # ("unmanaged", {'managed': False}),
+    )
+
     def setUp(self):
         register_view("maasserver_discovery")
         return super().setUp()
@@ -906,7 +912,7 @@
         # Note: 10.0.0.0/30 --> 10.0.0.1 and 10.0.0.0.2 are usable.
         subnet = factory.make_Subnet(
             cidr="10.0.0.0/30", gateway_ip="10.0.0.1",
-            dns_servers=["10.0.0.2"])
+            dns_servers=["10.0.0.2"], managed=self.managed)
         with ExpectedException(
                 StaticIPAddressExhaustion,
                 "No more IPs available in subnet: 10.0.0.0/30."):
@@ -915,35 +921,65 @@
     def test__allocates_next_free_address(self):
         # Note: 10.0.0.0/30 --> 10.0.0.1 and 10.0.0.0.2 are usable.
         subnet = factory.make_Subnet(
-            cidr="10.0.0.0/30", gateway_ip=None, dns_servers=None)
+            cidr="10.0.0.0/30", gateway_ip=None, dns_servers=None,
+            managed=self.managed)
+        if not self.managed:
+            factory.make_IPRange(
+                subnet, start_ip='10.0.0.1', end_ip='10.0.0.2',
+                type=IPRANGE_TYPE.RESERVED)
+            subnet = reload_object(subnet)
         ip = subnet.get_next_ip_for_allocation()
         self.assertThat(ip, Equals("10.0.0.1"))
 
     def test__avoids_gateway_ip(self):
         # Note: 10.0.0.0/30 --> 10.0.0.1 and 10.0.0.0.2 are usable.
         subnet = factory.make_Subnet(
-            cidr="10.0.0.0/30", gateway_ip="10.0.0.1", dns_servers=None)
+            cidr="10.0.0.0/30", gateway_ip="10.0.0.1", dns_servers=None,
+            managed=self.managed)
+        if not self.managed:
+            factory.make_IPRange(
+                subnet, start_ip='10.0.0.1', end_ip='10.0.0.2',
+                type=IPRANGE_TYPE.RESERVED)
+            subnet = reload_object(subnet)
         ip = subnet.get_next_ip_for_allocation()
         self.assertThat(ip, Equals("10.0.0.2"))
 
     def test__avoids_excluded_addresses(self):
         # Note: 10.0.0.0/30 --> 10.0.0.1 and 10.0.0.0.2 are usable.
         subnet = factory.make_Subnet(
-            cidr="10.0.0.0/30", gateway_ip=None, dns_servers=None)
+            cidr="10.0.0.0/30", gateway_ip=None, dns_servers=None,
+            managed=self.managed)
+        if not self.managed:
+            factory.make_IPRange(
+                subnet, start_ip='10.0.0.1', end_ip='10.0.0.2',
+                type=IPRANGE_TYPE.RESERVED)
+            subnet = reload_object(subnet)
         ip = subnet.get_next_ip_for_allocation(exclude_addresses=["10.0.0.1"])
         self.assertThat(ip, Equals("10.0.0.2"))
 
     def test__avoids_dns_servers(self):
         # Note: 10.0.0.0/30 --> 10.0.0.1 and 10.0.0.0.2 are usable.
         subnet = factory.make_Subnet(
-            cidr="10.0.0.0/30", gateway_ip=None, dns_servers=["10.0.0.1"])
+            cidr="10.0.0.0/30", gateway_ip=None, dns_servers=["10.0.0.1"],
+            managed=self.managed)
+        if not self.managed:
+            factory.make_IPRange(
+                subnet, start_ip='10.0.0.1', end_ip='10.0.0.2',
+                type=IPRANGE_TYPE.RESERVED)
+            subnet = reload_object(subnet)
         ip = subnet.get_next_ip_for_allocation()
         self.assertThat(ip, Equals("10.0.0.2"))
 
     def test__avoids_observed_neighbours(self):
         # Note: 10.0.0.0/30 --> 10.0.0.1 and 10.0.0.0.2 are usable.
         subnet = factory.make_Subnet(
-            cidr="10.0.0.0/30", gateway_ip=None, dns_servers=None)
+            cidr="10.0.0.0/30", gateway_ip=None, dns_servers=None,
+            managed=self.managed)
+        if not self.managed:
+            factory.make_IPRange(
+                subnet, start_ip='10.0.0.1', end_ip='10.0.0.2',
+                type=IPRANGE_TYPE.RESERVED)
+            subnet = reload_object(subnet)
         rackif = factory.make_Interface(vlan=subnet.vlan)
         factory.make_Discovery(ip="10.0.0.1", interface=rackif)
         ip = subnet.get_next_ip_for_allocation()
@@ -952,7 +988,13 @@
     def test__logs_if_suggests_previously_observed_neighbour(self):
         # Note: 10.0.0.0/30 --> 10.0.0.1 and 10.0.0.0.2 are usable.
         subnet = factory.make_Subnet(
-            cidr="10.0.0.0/30", gateway_ip=None, dns_servers=None)
+            cidr="10.0.0.0/30", gateway_ip=None, dns_servers=None,
+            managed=self.managed)
+        if not self.managed:
+            factory.make_IPRange(
+                subnet, start_ip='10.0.0.1', end_ip='10.0.0.2',
+                type=IPRANGE_TYPE.RESERVED)
+            subnet = reload_object(subnet)
         rackif = factory.make_Interface(vlan=subnet.vlan)
         now = datetime.now()
         yesterday = now - timedelta(days=1)
@@ -970,7 +1012,13 @@
     def test__uses_smallest_free_range_when_not_considering_neighbours(self):
         # Note: 10.0.0.0/29 --> 10.0.0.1 through 10.0.0.0.6 are usable.
         subnet = factory.make_Subnet(
-            cidr="10.0.0.0/29", gateway_ip=None, dns_servers=None)
+            cidr="10.0.0.0/29", gateway_ip=None, dns_servers=None,
+            managed=self.managed)
+        if not self.managed:
+            factory.make_IPRange(
+                subnet, start_ip='10.0.0.1', end_ip='10.0.0.6',
+                type=IPRANGE_TYPE.RESERVED)
+            subnet = reload_object(subnet)
         # With .4 in use, the free ranges are {1, 2, 3}, {5, 6}. So MAAS should
         # select 10.0.0.5, since that is the first address in the smallest
         # available range.

=== modified file 'src/maasserver/testing/factory.py'
--- src/maasserver/testing/factory.py	2016-11-11 17:21:57 +0000
+++ src/maasserver/testing/factory.py	2016-11-29 18:04:20 +0000
@@ -848,7 +848,8 @@
     def make_Subnet(self, name=None, vlan=None, space=None, cidr=None,
                     gateway_ip=RANDOM, dns_servers=None, host_bits=None,
                     fabric=None, vid=None, dhcp_on=False, version=None,
-                    rdns_mode=RDNS_MODE.DEFAULT, allow_proxy=True):
+                    rdns_mode=RDNS_MODE.DEFAULT, allow_proxy=True,
+                    managed=True):
         if name is None:
             name = factory.make_name('name')
         if vlan is None:
@@ -869,7 +870,7 @@
         subnet = Subnet(
             name=name, vlan=vlan, cidr=cidr, gateway_ip=gateway_ip,
             space=space, dns_servers=dns_servers, rdns_mode=rdns_mode,
-            allow_proxy=allow_proxy)
+            allow_proxy=allow_proxy, managed=managed)
         subnet.save()
         return subnet
 

=== modified file 'src/provisioningserver/utils/network.py'
--- src/provisioningserver/utils/network.py	2016-11-03 15:39:17 +0000
+++ src/provisioningserver/utils/network.py	2016-11-29 18:04:20 +0000
@@ -35,6 +35,7 @@
 import struct
 from typing import (
     Iterable,
+    Tuple,
     List,
     Optional,
     TypeVar,
@@ -100,6 +101,8 @@
     GATEWAY_IP = 'gateway-ip'
     DYNAMIC = 'dynamic'
     PROPOSED_DYNAMIC = 'proposed-dynamic'
+    UNMANAGED = 'unmanaged'
+    NEIGHBOUR = 'neighbour'
 
 
 class MAASIPRange(IPRange):
@@ -152,31 +155,173 @@
         return json
 
 
-def _combine_overlapping_maasipranges(
+def get_iprange_intersection(a: MAASIPRange, b: MAASIPRange):
+    """Given two overlapping MAASIPRange objects, return the best possible
+    combination of the two without any information loss.
+    """
+    # This ensures r0's start index is less than or equal to r1's start index.
+    r0, r1 = sorted([a, b])
+    if r0.first == r1.first and r0.last == r1.last:
+        # Easy case: exact match.
+        #   +------------+
+        #   | r0         |
+        #   +------------+
+        #   +------------+
+        #   | r1         |
+        # + +------------+
+        # ================
+        #   +------------+
+        #   | x          |
+        #   +------------+
+        # purpose(x) = r0 | r1
+        return (
+            MAASIPRange(r0.first, r0.last, purpose=r0.purpose | r1. purpose),
+        )
+    elif r0.first == r1.first and r0.last > r1.last:
+        # Note: if the start index is equal, larger ranges always sort before
+        # smaller ranges.
+        #   +------------+
+        #   | r0         |
+        #   +------------+
+        #   +--------+
+        #   | r1     |
+        # + +--------+
+        # ================
+        #   +---+--------+
+        #   | x      | y |
+        #   +---+--------+
+        # purpose(x) = r0 | r1
+        # purpose(y) = r0
+        return (
+            MAASIPRange(r0.first, r1.last, purpose=r0.purpose | r1.purpose),
+            MAASIPRange(r1.last + 1, r0.last, purpose=r0.purpose),
+        )
+    elif r0.first < r1.first and r0.last == r1.last:
+        #   +------------+
+        #   | r0         |
+        #   +------------+
+        #       +--------+
+        #       | r1     |
+        # +     +--------+
+        # ================
+        #   +---+--------+
+        #   | x | y      |
+        #   +---+--------+
+        # purpose(x) = r0
+        # purpose(y) = r0 | r1
+        return (
+            MAASIPRange(r0.first, r1.first - 1, purpose=r0.purpose),
+            MAASIPRange(r1.first, r0.last, purpose=r0.purpose | r1.purpose),
+        )
+    elif r0.first < r1.first and r0.last < r1.last:
+        #   +--------+
+        #   | r0     |
+        #   +--------+
+        #       +--------+
+        #       | r1     |
+        # +     +--------+
+        # ================
+        #   +---+----+---+
+        #   | x | y  | z |
+        #   +---+----+---+
+        # purpose(x) = r0
+        # purpose(y) = r0 | r1
+        # purpose(z) = r1
+        return (
+            MAASIPRange(r0.first, r1.first - 1, purpose=r0.purpose),
+            MAASIPRange(
+                r1.first, r0.last, purpose=r0.purpose | r1.purpose),
+            MAASIPRange(r0.last + 1, r1.last, purpose=r1.purpose),
+        )
+    elif r0.first < r1.first and r0.last > r1.last:
+        #   +------------+
+        #   | r0         |
+        #   +------------+
+        #       +----+
+        #       | r1 |
+        # +     +----+
+        # ================
+        #   +---+----+---+
+        #   | x | y  | z |
+        #   +---+----+---+
+        # x = r0
+        # y = r0 | r1
+        # z = r0
+        return (
+            MAASIPRange(r0.first, r1.first - 1, purpose=r0.purpose),
+            MAASIPRange(
+                r1.first, r1.last, purpose=r0.purpose | r1.purpose),
+            MAASIPRange(r1.last + 1, r0.last, purpose=r0.purpose),
+        )
+    else:
+        raise ValueError(
+            "Unable to combine IP ranges: (%s, %s); do they actually overlap?"
+            % (a, b))
+
+
+def _split_overlapping_ipranges_for_mismatched_purposes(
         ranges: Iterable[MAASIPRange]) -> List[MAASIPRange]:
     """Returns the specified ranges after combining any overlapping ranges.
 
     Given a sorted list of `MAASIPRange` objects, returns a new (sorted)
     list where any adjacent overlapping ranges have been combined into a single
     range.
+
+    Must be run after combining overlapping IP ranges (if purpose matches).
     """
     new_ranges = []
     previous_min = None
     previous_max = None
+    previous_purpose = None
     for item in ranges:
         if previous_min is not None and previous_max is not None:
             # Check for an overlapping range.
             min_overlaps = previous_min <= item.first <= previous_max
             max_overlaps = previous_min <= item.last <= previous_max
-            if min_overlaps or max_overlaps:
+            if previous_purpose != item.purpose and (
+                    min_overlaps or max_overlaps):
+                # Replace the previous range with one or more ranges.
                 previous = new_ranges.pop()
+                ranges = get_iprange_intersection(previous, item)
+                new_ranges.extend(ranges)
+                item = new_ranges[-1]
+            else:
+                new_ranges.append(item)
+        previous_min = item.first
+        previous_max = item.last
+        previous_purpose = item.purpose
+    return new_ranges
+
+
+def _combine_adjacent_maasipranges(
+        ranges: Iterable[MAASIPRange]) -> List[MAASIPRange]:
+    """Returns the specified ranges after combining any overlapping ranges.
+
+    Given a sorted list of `MAASIPRange` objects, returns a new (sorted)
+    list where any adjacent overlapping ranges have been combined into a single
+    range.
+    """
+    new_ranges = []
+    previous_min = None
+    previous_max = None
+    previous_purpose = None
+    for item in ranges:
+        if previous_min is not None and previous_max is not None:
+            # Check for an overlapping range.
+            min_overlaps = previous_min <= item.first <= previous_max
+            max_overlaps = previous_min <= item.last <= previous_max
+            if item.purpose == previous_purpose and (
+                    min_overlaps or max_overlaps):
+                # Replace the previous range with a new, combined range.
+                new_ranges.pop()
                 item = make_iprange(
                     min(item.first, previous_min),
                     max(item.last, previous_max),
-                    previous.purpose | item.purpose)
+                    item.purpose)
+        new_ranges.append(item)
         previous_min = item.first
         previous_max = item.last
-        new_ranges.append(item)
+        previous_purpose = item.purpose
     return new_ranges
 
 
@@ -398,8 +543,11 @@
         (3) Combining adjacent ranges with an identical purpose.
         """
         self.ranges = _normalize_ipranges(self.ranges)
-        self.ranges = _combine_overlapping_maasipranges(self.ranges)
         self.ranges = _coalesce_adjacent_purposes(self.ranges)
+        self.ranges = _split_overlapping_ipranges_for_mismatched_purposes(
+            self.ranges)
+        self.ranges = _combine_adjacent_maasipranges(
+            self.ranges)
 
     def __ior__(self, other):
         """Return self |= other."""
@@ -521,7 +669,7 @@
 
     def get_unused_ranges(
             self, outer_range: OuterRange,
-            comment=IPRANGE_TYPE.UNUSED) -> 'MAASIPSet':
+            purpose=IPRANGE_TYPE.UNUSED) -> 'MAASIPSet':
         """Calculates and returns a list of unused IP ranges, based on
         the supplied range of desired addresses.
 
@@ -557,7 +705,7 @@
             # range.
             if candidate_end - candidate_start >= 0:
                 unused_ranges.append(
-                    make_iprange(candidate_start, candidate_end, comment))
+                    make_iprange(candidate_start, candidate_end, purpose))
             candidate_start = used_range.last + 1
         # Skip the broadcast address, if this is an IPv4 network
         if type(outer_range) == IPNetwork:
@@ -572,7 +720,7 @@
         # of the range we're checking against.
         if candidate_end - candidate_start >= 0:
             unused_ranges.append(
-                make_iprange(candidate_start, candidate_end, comment))
+                make_iprange(candidate_start, candidate_end, purpose))
         return MAASIPSet(unused_ranges)
 
     def get_full_range(self, outer_range):