openstack/neutron
Code Issues Proposed changes

Relax subnet pool network affinity constraints

Browse Source 
This change relaxes the constraint that all subnets of the same
address family on a network must be allocated from the same subnet
pool. It allows subnets that share the same address scope to
co-exist on a network. If there is no address scope involved the
subnet pool / network affinity constraints continue to  enforced
as done previously.

Change-Id: I33bd17c723b3e8d409415bda008440f8ed9cfa68
Closes: 1830240
Implements: subnets-different-pools-same-net
This commit is contained in:
Ryan Tidwell 2019-06-26 00:29:00 -05:00
parent 1421c63f4b
commit 32182010c2
No known key found for this signature in database
GPG Key ID: A1C63854C1CDF372
7 changed files with 325 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
neutron/db/db_base_plugin_v2.py
View File

@ -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,

54
neutron/db/ipam_backend_mixin.py
View File

@ -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,
new_subnetpool_id, ip_version):
def _validate_network_subnetpools(self, network, subnet_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
new_subnetpool_id != subnet.subnetpool_id):
raise exc.NetworkSubnetPoolAffinityError()
if ipv6_pd_subnetpool:
# Check the prefix delegation case. Since there is no
# 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', [])

18
neutron/objects/address_scope.py
View File

@ -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

5
neutron/tests/unit/db/test_db_base_plugin_v2.py
View File

@ -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):

29
neutron/tests/unit/db/test_ipam_backend_mixin.py
View File

@ -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):

173
neutron/tests/unit/extensions/test_address_scope.py
View File

@ -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)

12
releasenotes/notes/relax-subnetpool-network-affinity-837c1fc28f835de5.yaml Normal file
View File

@ -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>`_.