Merge "Relax subnet pool network affinity constraints"

neutron/db/db_base_plugin_v2.py

@@ -32,6 +32,7 @@ from neutron_lib.db import model_query
 from neutron_lib.db import resource_extend
 from neutron_lib.db import utils as ndb_utils
 from neutron_lib import exceptions as exc
+from neutron_lib.exceptions import address_scope as addr_scope_exc
 from neutron_lib.exceptions import l3 as l3_exc
 from neutron_lib.plugins import constants as plugin_constants
 from neutron_lib.plugins import directory
@@ -1131,6 +1132,9 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon,
                 subnetpool_id=subnetpool_id, address_scope_id=address_scope_id,
                 ip_version=as_ip_version)
 
+        self._check_subnetpool_address_scope_network_affinity(
+            context, subnetpool_id, ip_version)
+
         subnetpools = subnetpool_obj.SubnetPool.get_objects(
             context, address_scope_id=address_scope_id)
 
@@ -1142,6 +1146,44 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon,
             if sp_set.intersection(new_set):
                 raise exc.AddressScopePrefixConflict()
 
+    def _check_subnetpool_address_scope_network_affinity(self, context,
+                                                         subnetpool_id,
+                                                         ip_version):
+        """Check whether updating a subnet pool's address scope is allowed.
+
+        - Identify the subnets that would be re-scoped
+        - Identify the networks that would be affected by re-scoping
+        - Find all subnets associated with the affected networks
+        - Perform set difference (all - to_be_rescoped)
+        - If the set difference yields non-zero result size, re-scoping the
+        subnet pool will leave subnets in different address scopes and result
+        in address scope / network affinity violations so raise an exception to
+        block the operation.
+        """
+
+        # TODO(tidwellr) potentially lots of subnets here, optimize this code
+        subnets_to_rescope = self._get_subnets_by_subnetpool(context,
+                                                             subnetpool_id)
+        rescoped_subnet_ids = set()
+        affected_source_network_ids = set()
+        for subnet in subnets_to_rescope:
+            rescoped_subnet_ids.add(subnet.id)
+            affected_source_network_ids.add(subnet.network_id)
+
+        all_network_subnets = subnet_obj.Subnet.get_objects(
+            context,
+            network_id=affected_source_network_ids,
+            ip_version=ip_version)
+        all_affected_subnet_ids = set(
+            [subnet.id for subnet in all_network_subnets])
+
+        # Use set difference to identify the subnets that would be
+        # violating address scope affinity constraints if the subnet
+        # pool's address scope was changed.
+        violations = all_affected_subnet_ids.difference(rescoped_subnet_ids)
+        if violations:
+            raise addr_scope_exc.NetworkAddressScopeAffinityError()
+
     def _check_subnetpool_update_allowed(self, context, subnetpool_id,
                                          address_scope_id):
         """Check if the subnetpool can be updated or not.
@@ -1178,7 +1220,7 @@ class NeutronDbPluginV2(db_base_plugin_common.DbBasePluginCommon,
             self._check_default_subnetpool_exists(context,
                                                   sp_reader.ip_version)
         self._validate_address_scope_id(context, sp_reader.address_scope_id,
-                                        id, sp_reader.prefixes,
+                                        sp_reader.id, sp_reader.prefixes,
                                         sp_reader.ip_version)
         pool_args = {'project_id': sp['tenant_id'],
                      'id': sp_reader.id,

neutron/db/ipam_backend_mixin.py

@@ -25,6 +25,7 @@ from neutron_lib import constants as const
 from neutron_lib.db import api as db_api
 from neutron_lib.db import utils as db_utils
 from neutron_lib import exceptions as exc
+from neutron_lib.exceptions import address_scope as addr_scope_exc
 from neutron_lib.utils import net as net_utils
 from oslo_config import cfg
 from oslo_log import log as logging
@@ -37,6 +38,7 @@ from neutron.db import models_v2
 from neutron.extensions import segment
 from neutron.ipam import exceptions as ipam_exceptions
 from neutron.ipam import utils as ipam_utils
+from neutron.objects import address_scope as addr_scope_obj
 from neutron.objects import network as network_obj
 from neutron.objects import subnet as subnet_obj
 from neutron.services.segments import exceptions as segment_exc
@@ -252,15 +254,43 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon):
                           'cidr': subnet.cidr})
                 raise exc.InvalidInput(error_message=err_msg)
 
-    def _validate_network_subnetpools(self, network,
+    def _validate_network_subnetpools(self, network, subnet_ip_version,
-                                      new_subnetpool_id, ip_version):
+                                      new_subnetpool, network_scope):
         """Validate all subnets on the given network have been allocated from
-           the same subnet pool as new_subnetpool_id
+           the same subnet pool as new_subnetpool if no address scope is
+           used. If address scopes are used, validate that all subnets on the
+           given network participate in the same address scope.
         """
+        # 'new_subnetpool' might just be the Prefix Delegation ID
+        ipv6_pd_subnetpool = new_subnetpool == const.IPV6_PD_POOL_ID
+
+        # Check address scope affinities
+        if network_scope:
+            if (ipv6_pd_subnetpool or
+                    new_subnetpool and
+                    new_subnetpool.address_scope_id != network_scope.id):
+                raise addr_scope_exc.NetworkAddressScopeAffinityError()
+
+        # Checks for situations where address scopes aren't involved
         for subnet in network.subnets:
-            if (subnet.ip_version == ip_version and
+            if ipv6_pd_subnetpool:
-                    new_subnetpool_id != subnet.subnetpool_id):
+                # Check the prefix delegation case.  Since there is no
-                raise exc.NetworkSubnetPoolAffinityError()
+                # subnetpool object, we just check against the PD ID.
+                if (subnet.ip_version == const.IP_VERSION_6 and
+                        subnet.subnetpool_id != const.IPV6_PD_POOL_ID):
+                    raise exc.NetworkSubnetPoolAffinityError()
+            else:
+                if new_subnetpool:
+                    # In this case we have the new subnetpool object, so
+                    # we can check the ID and IP version.
+                    if (subnet.subnetpool_id != new_subnetpool.id and
+                            subnet.ip_version == new_subnetpool.ip_version and
+                            not network_scope):
+                        raise exc.NetworkSubnetPoolAffinityError()
+                else:
+                    if (subnet.subnetpool_id and
+                            subnet.ip_version == subnet_ip_version):
+                        raise exc.NetworkSubnetPoolAffinityError()
 
     def validate_allocation_pools(self, ip_pools, subnet_cidr):
         """Validate IP allocation pools.
@@ -517,10 +547,18 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon):
                      dns_nameservers,
                      host_routes,
                      subnet_request):
+        network_scope = addr_scope_obj.AddressScope.get_network_address_scope(
+            context, network.id, subnet_args['ip_version'])
+        # 'subnetpool' is not necessarily an object
+        subnetpool = subnet_args.get('subnetpool_id')
+        if subnetpool and subnetpool != const.IPV6_PD_POOL_ID:
+            subnetpool = self._get_subnetpool(context, subnetpool)
+
         self._validate_subnet_cidr(context, network, subnet_args['cidr'])
         self._validate_network_subnetpools(network,
-                                           subnet_args['subnetpool_id'],
+                                           subnet_args['ip_version'],
-                                           subnet_args['ip_version'])
+                                           subnetpool,
+                                           network_scope)
 
         service_types = subnet_args.pop('service_types', [])
 

neutron/objects/address_scope.py

@@ -15,6 +15,7 @@
 from oslo_versionedobjects import fields as obj_fields
 
 from neutron.db.models import address_scope as models
+from neutron.db import models_v2
 from neutron.objects import base
 from neutron.objects import common_types
 
@@ -33,3 +34,20 @@ class AddressScope(base.NeutronDbObject):
         'shared': obj_fields.BooleanField(),
         'ip_version': common_types.IPVersionEnumField(),
     }
+
+    @classmethod
+    def get_network_address_scope(cls, context, network_id, ip_version):
+        query = context.session.query(cls.db_model)
+        query = query.join(
+            models_v2.SubnetPool,
+            models_v2.SubnetPool.address_scope_id == cls.db_model.id)
+        query = query.filter(
+            cls.db_model.ip_version == ip_version,
+            models_v2.Subnet.subnetpool_id == models_v2.SubnetPool.id,
+            models_v2.Subnet.network_id == network_id)
+        scope_model_obj = query.one_or_none()
+
+        if scope_model_obj:
+            return cls._load_object(context, scope_model_obj)
+
+        return None

neutron/tests/unit/db/test_db_base_plugin_v2.py

@@ -6868,7 +6868,10 @@ class NeutronDbPluginV2AsMixinTestCase(NeutronDbPluginV2TestCase,
         new_subnetpool_id = None
         self.assertRaises(lib_exc.NetworkSubnetPoolAffinityError,
                           self.plugin.ipam._validate_network_subnetpools,
-                          network, new_subnetpool_id, 4)
+                          network,
+                          constants.IP_VERSION_4,
+                          new_subnetpool_id,
+                          None)
 
 
 class TestNetworks(testlib_api.SqlTestCase):

neutron/tests/unit/db/test_ipam_backend_mixin.py

@@ -17,6 +17,8 @@ import mock
 import netaddr
 from neutron_lib.api.definitions import portbindings
 from neutron_lib import constants
+from neutron_lib import exceptions as exc
+from neutron_lib.exceptions import address_scope as addr_scope_exc
 from oslo_utils import uuidutils
 import webob.exc
 
@@ -299,6 +301,33 @@ class TestIpamBackendMixin(base.BaseTestCase):
         self.assertFalse(result)
         self.assertTrue(self.mixin._get_subnet_object.called)
 
+    def test__validate_network_subnetpools_mismatch_address_scopes(self):
+        address_scope_id = "dummy-scope"
+        subnetpool = mock.MagicMock()
+        address_scope = mock.MagicMock()
+        subnetpool.address_scope.return_value = address_scope_id
+        address_scope.id.return_value = address_scope_id
+        self.assertRaises(addr_scope_exc.NetworkAddressScopeAffinityError,
+                          self.mixin._validate_network_subnetpools,
+                          mock.MagicMock(),
+                          constants.IP_VERSION_4,
+                          subnetpool,
+                          address_scope)
+
+    def test__validate_network_subnetpools_subnetpool_mismatch(self):
+        subnet = mock.MagicMock(ip_version=constants.IP_VERSION_4)
+        subnet.subnetpool_id = 'fake-subnetpool'
+        network = mock.MagicMock(subnets=[subnet])
+        subnetpool = mock.MagicMock(id=uuidutils.generate_uuid())
+        subnetpool.ip_version = constants.IP_VERSION_4
+
+        self.assertRaises(exc.NetworkSubnetPoolAffinityError,
+                          self.mixin._validate_network_subnetpools,
+                          network,
+                          constants.IP_VERSION_4,
+                          subnetpool,
+                          None)
+
 
 class TestPlugin(db_base_plugin_v2.NeutronDbPluginV2,
                  portbindings_db.PortBindingMixin):

neutron/tests/unit/extensions/test_address_scope.py

@@ -22,6 +22,7 @@ from neutron_lib.callbacks import registry
 from neutron_lib.callbacks import resources
 from neutron_lib import constants
 from neutron_lib import context
+from neutron_lib.plugins import directory
 import webob.exc
 
 from neutron.db import address_scope_db
@@ -516,3 +517,175 @@ class TestSubnetPoolsWithAddressScopes(AddressScopeTestCase):
                 res = req.get_response(api)
                 self.assertEqual(webob.exc.HTTPBadRequest.code,
                                  res.status_int)
+
+    def test_create_two_subnets_different_subnetpools_same_network(self):
+        with self.address_scope(constants.IP_VERSION_4,
+                                name='foo-address-scope') as addr_scope:
+            addr_scope = addr_scope['address_scope']
+            with self.subnetpool(
+                        ['10.10.0.0/16'],
+                        name='subnetpool_a',
+                        tenant_id=addr_scope['tenant_id'],
+                        default_prefixlen=24,
+                        address_scope_id=addr_scope['id']) as subnetpool_a,\
+                self.subnetpool(
+                         ['10.20.0.0/16'],
+                         name='subnetpool_b',
+                         tenant_id=addr_scope['tenant_id'],
+                         default_prefixlen=24,
+                         address_scope_id=addr_scope['id']) as subnetpool_b:
+                subnetpool_a = subnetpool_a['subnetpool']
+                subnetpool_b = subnetpool_b['subnetpool']
+
+                with self.network(
+                        tenant_id=addr_scope['tenant_id']) as network:
+                    subnet_a = self._make_subnet(
+                        self.fmt,
+                        network,
+                        constants.ATTR_NOT_SPECIFIED,
+                        None,
+                        subnetpool_id=subnetpool_a['id'],
+                        ip_version=constants.IP_VERSION_4,
+                        tenant_id=addr_scope['tenant_id'])
+                    subnet_b = self._make_subnet(
+                        self.fmt,
+                        network,
+                        constants.ATTR_NOT_SPECIFIED,
+                        None,
+                        subnetpool_id=subnetpool_b['id'],
+                        ip_version=constants.IP_VERSION_4,
+                        tenant_id=addr_scope['tenant_id'])
+
+                    # Look up subnet counts and perform assertions
+                    ctx = context.Context('', addr_scope['tenant_id'])
+                    pl = directory.get_plugin()
+                    total_count = pl.get_subnets_count(
+                        ctx,
+                        filters={'network_id':
+                                 [network['network']['id']]})
+                    subnets_pool_a_count = pl.get_subnets_count(
+                        ctx,
+                        filters={'id': [subnet_a['subnet']['id']],
+                                 'subnetpool_id': [subnetpool_a['id']],
+                                 'network_id': [network['network']['id']]})
+                    subnets_pool_b_count = pl.get_subnets_count(
+                        ctx,
+                        filters={'id': [subnet_b['subnet']['id']],
+                                 'subnetpool_id': [subnetpool_b['id']],
+                                 'network_id': [network['network']['id']]})
+                    self.assertEqual(2, total_count)
+                    self.assertEqual(1, subnets_pool_a_count)
+                    self.assertEqual(1, subnets_pool_b_count)
+
+    def test_block_update_subnetpool_network_affinity(self):
+        with self.address_scope(constants.IP_VERSION_4,
+                                name='scope-a') as scope_a,\
+            self.address_scope(constants.IP_VERSION_4,
+                               name='scope-b') as scope_b:
+            scope_a = scope_a['address_scope']
+            scope_b = scope_b['address_scope']
+
+            with self.subnetpool(
+                        ['10.10.0.0/16'],
+                        name='subnetpool_a',
+                        tenant_id=scope_a['tenant_id'],
+                        default_prefixlen=24,
+                        address_scope_id=scope_a['id']) as subnetpool_a,\
+                self.subnetpool(
+                         ['10.20.0.0/16'],
+                         name='subnetpool_b',
+                         tenant_id=scope_a['tenant_id'],
+                         default_prefixlen=24,
+                         address_scope_id=scope_a['id']) as subnetpool_b:
+                subnetpool_a = subnetpool_a['subnetpool']
+                subnetpool_b = subnetpool_b['subnetpool']
+
+                with self.network(
+                        tenant_id=scope_a['tenant_id']) as network:
+                    self._make_subnet(
+                        self.fmt,
+                        network,
+                        constants.ATTR_NOT_SPECIFIED,
+                        None,
+                        subnetpool_id=subnetpool_a['id'],
+                        ip_version=constants.IP_VERSION_4,
+                        tenant_id=scope_a['tenant_id'])
+                    self._make_subnet(
+                        self.fmt,
+                        network,
+                        constants.ATTR_NOT_SPECIFIED,
+                        None,
+                        subnetpool_id=subnetpool_b['id'],
+                        ip_version=constants.IP_VERSION_4,
+                        tenant_id=scope_a['tenant_id'])
+
+                    # Attempt to update subnetpool_b's address scope and
+                    # assert failure.
+                    data = {'subnetpool': {'address_scope_id':
+                                           scope_b['id']}}
+                    req = self.new_update_request('subnetpools', data,
+                                                  subnetpool_b['id'])
+                    api = self._api_for_resource('subnetpools')
+                    res = req.get_response(api)
+                    self.assertEqual(webob.exc.HTTPBadRequest.code,
+                                     res.status_int)
+
+    def test_ipv6_pd_add_non_pd_subnet_to_same_network(self):
+        with self.address_scope(constants.IP_VERSION_6,
+                                name='foo-address-scope') as addr_scope:
+            addr_scope = addr_scope['address_scope']
+            with self.subnetpool(
+                        ['2001:db8:1234::/48'],
+                        name='non_pd_pool',
+                        tenant_id=addr_scope['tenant_id'],
+                        default_prefixlen=64,
+                        address_scope_id=addr_scope['id']) as non_pd_pool:
+                non_pd_pool = non_pd_pool['subnetpool']
+
+                with self.network(
+                        tenant_id=addr_scope['tenant_id']) as network:
+                    with self.subnet(cidr=None,
+                                     network=network,
+                                     ip_version=constants.IP_VERSION_6,
+                                     subnetpool_id=constants.IPV6_PD_POOL_ID,
+                                     ipv6_ra_mode=constants.IPV6_SLAAC,
+                                     ipv6_address_mode=constants.IPV6_SLAAC):
+                        res = self._create_subnet(
+                            self.fmt,
+                            cidr=None,
+                            net_id=network['network']['id'],
+                            subnetpool_id=non_pd_pool['id'],
+                            tenant_id=addr_scope['tenant_id'],
+                            ip_version=constants.IP_VERSION_6)
+                        self.assertEqual(webob.exc.HTTPBadRequest.code,
+                                         res.status_int)
+
+    def test_ipv6_non_pd_add_pd_subnet_to_same_network(self):
+        with self.address_scope(constants.IP_VERSION_6,
+                                name='foo-address-scope') as addr_scope:
+            addr_scope = addr_scope['address_scope']
+            with self.subnetpool(
+                        ['2001:db8:1234::/48'],
+                        name='non_pd_pool',
+                        tenant_id=addr_scope['tenant_id'],
+                        default_prefixlen=64,
+                        address_scope_id=addr_scope['id']) as non_pd_pool:
+                non_pd_pool = non_pd_pool['subnetpool']
+
+                with self.network(
+                        tenant_id=addr_scope['tenant_id']) as network:
+                    with self.subnet(cidr=None,
+                                     network=network,
+                                     ip_version=constants.IP_VERSION_6,
+                                     subnetpool_id=non_pd_pool['id']):
+                        res = self._create_subnet(
+                            self.fmt,
+                            cidr=None,
+                            net_id=network['network']['id'],
+                            tenant_id=addr_scope['tenant_id'],
+                            subnetpool_id=constants.IPV6_PD_POOL_ID,
+                            ip_version=constants.IP_VERSION_6,
+                            ipv6_ra_mode=constants.IPV6_SLAAC,
+                            ipv6_address_mode=constants.IPV6_SLAAC)
+                        self.assertEqual(webob.exc.HTTPBadRequest.code,
+                                         res.status_int)

releasenotes/notes/relax-subnetpool-network-affinity-837c1fc28f835de5.yaml

@@ -0,0 +1,12 @@
+---
+features:
+  - |
+    When different subnet pools participate in the same address scope, the
+    constraints disallowing subnets to be allocated from different pools on
+    the same network have been relaxed. As long as subnet pools participate
+    in the same address scope, subnets can now be created from different
+    subnet pools when multiple subnets are created on a network. When
+    address scopes are not used, subnets with the same ``ip_version`` on the
+    same network must still be allocated from the same subnet pool.
+    For more information, see bug
+    `1830240 <https://bugs.launchpad.net/neutron/+bug/1830240>`_.