From b32426c05e16a0d4aab3dbe5d2d7287dfec13ce1 Mon Sep 17 00:00:00 2001 From: Carl Baldwin Date: Tue, 10 May 2016 22:21:22 +0000 Subject: [PATCH] Make IPAM segment aware on port create With this change, Neutron will filter subnets based on binding information, or lack thereof, which is provided with the port create call. Partially-Implements: blueprint routed-networks Change-Id: I39e1644ea6831c3efa082e4952ac1365e2f95e0d --- neutron/db/ipam_backend_mixin.py | 61 +++++- neutron/db/ipam_non_pluggable_backend.py | 8 +- neutron/db/ipam_pluggable_backend.py | 8 +- neutron/services/segments/exceptions.py | 12 ++ .../unit/db/test_ipam_pluggable_backend.py | 2 +- neutron/tests/unit/extensions/test_segment.py | 189 ++++++++++++++++-- 6 files changed, 249 insertions(+), 31 deletions(-) diff --git a/neutron/db/ipam_backend_mixin.py b/neutron/db/ipam_backend_mixin.py index d8473185181..00254e8fdb7 100644 --- a/neutron/db/ipam_backend_mixin.py +++ b/neutron/db/ipam_backend_mixin.py @@ -23,6 +23,7 @@ from neutron_lib import exceptions as exc from oslo_config import cfg from oslo_db import exception as db_exc from oslo_log import log as logging +from sqlalchemy import and_, or_ from sqlalchemy.orm import exc as orm_exc from neutron._i18n import _, _LI @@ -35,6 +36,7 @@ from neutron.db import models_v2 from neutron.db import segments_db from neutron.extensions import segment from neutron.ipam import utils as ipam_utils +from neutron.services.segments import db as segment_svc_db from neutron.services.segments import exceptions as segment_exc LOG = logging.getLogger(__name__) @@ -532,12 +534,59 @@ class IpamBackendMixin(db_base_plugin_common.DbBasePluginCommon): fixed_ip_list.append({'subnet_id': subnet['id']}) return fixed_ip_list - def _ipam_get_subnets(self, context, network_id, segment_id): - query = self._get_collection_query(context, models_v2.Subnet) - query = query.filter(models_v2.Subnet.network_id == network_id) - query = query.filter(models_v2.Subnet.segment_id == segment_id) - subnets = [self._make_subnet_dict(c, context=context) for c in query] - return subnets + def _ipam_get_subnets(self, context, network_id, host): + Subnet = models_v2.Subnet + SegmentHostMapping = segment_svc_db.SegmentHostMapping + + query = self._get_collection_query(context, Subnet) + query = query.filter(Subnet.network_id == network_id) + if not validators.is_attr_set(host): + query = query.filter(Subnet.segment_id.is_(None)) + return [self._make_subnet_dict(c, context=context) for c in query] + + # A host has been provided. Consider these two scenarios + # 1. Not a routed network: subnets are not on segments + # 2. Is a routed network: only subnets on segments mapped to host + # The following join query returns results for either. The two are + # guaranteed to be mutually exclusive when subnets are created. + query = query.add_entity(SegmentHostMapping) + query = query.outerjoin( + SegmentHostMapping, + and_(Subnet.segment_id == SegmentHostMapping.segment_id, + SegmentHostMapping.host == host)) + # Essentially "segment_id IS NULL XNOR host IS NULL" + query = query.filter(or_(and_(Subnet.segment_id.isnot(None), + SegmentHostMapping.host.isnot(None)), + and_(Subnet.segment_id.is_(None), + SegmentHostMapping.host.is_(None)))) + + results = query.all() + + # See if results are empty because the host isn't mapped to a segment + if not results: + # Check if it's a routed network (i.e subnets on segments) + query = self._get_collection_query(context, Subnet) + query = query.filter(Subnet.network_id == network_id) + query = query.filter(Subnet.segment_id.isnot(None)) + if query.count() == 0: + return [] + # It is a routed network but no subnets found for host + raise segment_exc.HostNotConnectedToAnySegment( + host=host, network_id=network_id) + + # For now, we're using a simplifying assumption that a host will only + # touch one segment in a given routed network. Raise exception + # otherwise. This restriction may be relaxed as use cases for multiple + # mappings are understood. + segment_ids = {subnet.segment_id + for subnet, mapping in results + if mapping} + if 1 < len(segment_ids): + raise segment_exc.HostConnectedToMultipleSegments( + host=host, network_id=network_id) + + return [self._make_subnet_dict(subnet, context=context) + for subnet, _mapping in results] def _make_subnet_args(self, detail, subnet, subnetpool_id): args = super(IpamBackendMixin, self)._make_subnet_args( diff --git a/neutron/db/ipam_non_pluggable_backend.py b/neutron/db/ipam_non_pluggable_backend.py index abc0098c972..3d8fbaac472 100644 --- a/neutron/db/ipam_non_pluggable_backend.py +++ b/neutron/db/ipam_non_pluggable_backend.py @@ -27,6 +27,7 @@ from neutron.common import constants as n_const from neutron.common import ipv6_utils from neutron.db import ipam_backend_mixin from neutron.db import models_v2 +from neutron.extensions import portbindings from neutron.ipam import requests as ipam_req from neutron.ipam import subnet_alloc @@ -319,7 +320,7 @@ class IpamNonPluggableBackend(ipam_backend_mixin.IpamBackendMixin): changes = self._get_changed_ips_for_port(context, original_ips, new_ips, device_owner) subnets = self._ipam_get_subnets( - context, network_id=network_id, segment_id=None) + context, network_id=network_id, host=None) # Check if the IP's to add are OK to_add = self._test_fixed_ips_for_port(context, network_id, changes.add, device_owner, @@ -351,8 +352,9 @@ class IpamNonPluggableBackend(ipam_backend_mixin.IpamBackendMixin): a subnet_id then allocate an IP address accordingly. """ p = port['port'] - subnets = self._ipam_get_subnets( - context, network_id=p['network_id'], segment_id=None) + subnets = self._ipam_get_subnets(context, + network_id=p['network_id'], + host=p.get(portbindings.HOST_ID)) v4, v6_stateful, v6_stateless = self._classify_subnets( context, subnets) diff --git a/neutron/db/ipam_pluggable_backend.py b/neutron/db/ipam_pluggable_backend.py index 7be6620d8ae..8b9581b0560 100644 --- a/neutron/db/ipam_pluggable_backend.py +++ b/neutron/db/ipam_pluggable_backend.py @@ -27,6 +27,7 @@ from neutron._i18n import _, _LE, _LW from neutron.common import ipv6_utils from neutron.db import ipam_backend_mixin from neutron.db import models_v2 +from neutron.extensions import portbindings from neutron.ipam import driver from neutron.ipam import exceptions as ipam_exc from neutron.ipam import requests as ipam_req @@ -188,8 +189,9 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin): a subnet_id then allocate an IP address accordingly. """ p = port['port'] - subnets = self._ipam_get_subnets( - context, network_id=p['network_id'], segment_id=None) + subnets = self._ipam_get_subnets(context, + network_id=p['network_id'], + host=p.get(portbindings.HOST_ID)) v4, v6_stateful, v6_stateless = self._classify_subnets( context, subnets) @@ -269,7 +271,7 @@ class IpamPluggableBackend(ipam_backend_mixin.IpamBackendMixin): changes = self._get_changed_ips_for_port( context, original_ips, new_ips, port['device_owner']) subnets = self._ipam_get_subnets( - context, network_id=port['network_id'], segment_id=None) + context, network_id=port['network_id'], host=None) # Check if the IP's to add are OK to_add = self._test_fixed_ips_for_port( context, port['network_id'], changes.add, diff --git a/neutron/services/segments/exceptions.py b/neutron/services/segments/exceptions.py index 77bcb1a589f..1197a893b21 100644 --- a/neutron/services/segments/exceptions.py +++ b/neutron/services/segments/exceptions.py @@ -36,3 +36,15 @@ class SubnetCantAssociateToDynamicSegment(exceptions.BadRequest): class NetworkIdsDontMatch(exceptions.BadRequest): message = _("The subnet's network id, '%(subnet_network)s', doesn't match " "the network_id of segment '%(segment_id)s'") + + +class HostConnectedToMultipleSegments(exceptions.Conflict): + message = _("Host %(host)s is connected to multiple segments on routed " + "provider network '%(network_id)s'. It should be connected " + "to one.") + + +class HostNotConnectedToAnySegment(exceptions.Conflict): + message = _("Host %(host)s is not connected to any segments on routed " + "provider network '%(network_id)s'. It should be connected " + "to one.") diff --git a/neutron/tests/unit/db/test_ipam_pluggable_backend.py b/neutron/tests/unit/db/test_ipam_pluggable_backend.py index 08e2bb83810..04c942b8457 100644 --- a/neutron/tests/unit/db/test_ipam_pluggable_backend.py +++ b/neutron/tests/unit/db/test_ipam_pluggable_backend.py @@ -621,7 +621,7 @@ class TestDbBasePluginIpam(test_db_base.NeutronDbPluginV2TestCase): original_ips, new_ips, mac) mocks['driver'].get_address_request_factory.assert_called_once_with() mocks['ipam']._ipam_get_subnets.assert_called_once_with( - context, network_id=port_dict['network_id'], segment_id=None) + context, network_id=port_dict['network_id'], host=None) # Validate port_dict is passed into address_factory address_factory.get_request.assert_called_once_with(context, port_dict, diff --git a/neutron/tests/unit/extensions/test_segment.py b/neutron/tests/unit/extensions/test_segment.py index f5b29824f61..8dec1594fa6 100644 --- a/neutron/tests/unit/extensions/test_segment.py +++ b/neutron/tests/unit/extensions/test_segment.py @@ -13,6 +13,7 @@ # under the License. import mock +import netaddr from neutron_lib import constants from oslo_utils import uuidutils import webob.exc @@ -22,10 +23,12 @@ from neutron import context from neutron.db import agents_db from neutron.db import db_base_plugin_v2 from neutron.db import segments_db +from neutron.extensions import portbindings from neutron.extensions import segment as ext_segment from neutron.plugins.common import constants as p_constants from neutron.plugins.ml2 import config from neutron.services.segments import db +from neutron.services.segments import exceptions as segment_exc from neutron.tests.common import helpers from neutron.tests.unit.db import test_db_base_plugin_v2 @@ -99,7 +102,7 @@ class SegmentTestPlugin(db_base_plugin_v2.NeutronDbPluginV2, __native_pagination_support = True __native_sorting_support = True - supported_extension_aliases = ["segment"] + supported_extension_aliases = ["segment", "binding"] def get_plugin_description(self): return "Network Segments" @@ -258,23 +261,6 @@ class TestSegmentSubnetAssociation(SegmentTestCase): segment_id=segment['id']) self.assertEqual(webob.exc.HTTPBadRequest.code, res.status_int) - def test_port_create_with_segment_subnets(self): - with self.network() as network: - net = network['network'] - - segment = self._test_create_segment(network_id=net['id']) - segment_id = segment['segment']['id'] - - with self.subnet(network=network, segment_id=segment_id) as subnet: - subnet = subnet['subnet'] - - response = self._create_port(self.fmt, - net_id=net['id'], - tenant_id=net['tenant_id']) - res = self.deserialize(self.fmt, response) - # Don't allocate IPs in this case because it doesn't have binding info - self.assertEqual(0, len(res['port']['fixed_ips'])) - class HostSegmentMappingTestCase(SegmentTestCase): _mechanism_drivers = ['logger'] @@ -495,3 +481,170 @@ class TestMl2HostSegmentMappingAgentServerSynch(HostSegmentMappingTestCase): plugin=self.plugin, start_flag=False) self.assertTrue(host in db.reported_hosts) mock_function.assert_not_called() + + +class TestSegmentAwareIpam(SegmentTestCase): + def _setup_host_mappings(self, mappings=()): + ctx = context.get_admin_context() + with ctx.session.begin(subtransactions=True): + for segment_id, host in mappings: + record = db.SegmentHostMapping( + segment_id=segment_id, + host=host) + ctx.session.add(record) + + def _create_test_segment_with_subnet(self, + network=None, + cidr='2001:db8:0:0::/64', + physnet='physnet'): + """Creates one network with one segment and one subnet""" + if not network: + with self.network() as network: + pass + + segment = self._test_create_segment( + network_id=network['network']['id'], + physical_network=physnet) + + ip_version = netaddr.IPNetwork(cidr).version if cidr else None + with self.subnet(network=network, + segment_id=segment['segment']['id'], + ip_version=ip_version, + cidr=cidr) as subnet: + return network, segment, subnet + + def _create_test_segments_with_subnets(self, num): + """Creates one network with num segments and num subnets""" + with self.network() as network: + segments, subnets = [], [] + for i in range(num): + cidr = '2001:db8:0:%s::/64' % i + physnet = 'physnet%s' % i + _net, segment, subnet = self._create_test_segment_with_subnet( + network=network, cidr=cidr, physnet=physnet) + segments.append(segment) + subnets.append(subnet) + return network, segments, subnets + + def test_port_create_with_segment_subnets(self): + """No binding information is provided, defer IP allocation""" + network, segment, subnet = self._create_test_segment_with_subnet() + response = self._create_port(self.fmt, + net_id=network['network']['id'], + tenant_id=network['network']['tenant_id']) + res = self.deserialize(self.fmt, response) + # Don't allocate IPs in this case because we didn't give binding info + self.assertEqual(0, len(res['port']['fixed_ips'])) + + def test_port_create_with_binding_information(self): + """Binding information is provided, subnets are on segments""" + network, segments, subnets = self._create_test_segments_with_subnets(3) + + # Map the host to the middle segment (by mocking host/segment mapping) + self._setup_host_mappings([ + (segments[1]['segment']['id'], 'fakehost'), + (segments[1]['segment']['id'], 'otherhost'), + (segments[0]['segment']['id'], 'thirdhost')]) + + response = self._create_port(self.fmt, + net_id=network['network']['id'], + tenant_id=network['network']['tenant_id'], + arg_list=(portbindings.HOST_ID,), + **{portbindings.HOST_ID: 'fakehost'}) + res = self.deserialize(self.fmt, response) + + # Since host mapped to middle segment, IP must come from middle subnet + ip = res['port']['fixed_ips'][0]['ip_address'] + ip_net = netaddr.IPNetwork(subnets[1]['subnet']['cidr']) + self.assertIn(ip, ip_net) + + def test_port_create_with_binding_and_no_subnets(self): + """Binding information is provided, no subnets.""" + with self.network() as network: + segment = self._test_create_segment( + network_id=network['network']['id'], + physical_network='physnet') + + # Map the host to the segment + self._setup_host_mappings([(segment['segment']['id'], 'fakehost')]) + + response = self._create_port(self.fmt, + net_id=network['network']['id'], + tenant_id=network['network']['tenant_id'], + arg_list=(portbindings.HOST_ID,), + **{portbindings.HOST_ID: 'fakehost'}) + res = self.deserialize(self.fmt, response) + + # No subnets, so no allocation. But, it shouldn't be an error. + self.assertEqual(0, len(res['port']['fixed_ips'])) + + def test_port_create_with_binding_information_fallback(self): + """Binding information is provided, subnets not on segments""" + with self.network() as network: + with self.subnet(network=network, + ip_version=6, + cidr='2001:db8:0:0::/64') as subnet: + segment = self._test_create_segment( + network_id=network['network']['id'], + physical_network='physnet') + + # Map the host to the segment + self._setup_host_mappings([(segment['segment']['id'], 'fakehost')]) + + response = self._create_port(self.fmt, + net_id=network['network']['id'], + tenant_id=network['network']['tenant_id'], + arg_list=(portbindings.HOST_ID,), + **{portbindings.HOST_ID: 'fakehost'}) + res = self.deserialize(self.fmt, response) + + # Since the subnet is not on a segment, fall back to it + ip = res['port']['fixed_ips'][0]['ip_address'] + ip_net = netaddr.IPNetwork(subnet['subnet']['cidr']) + self.assertIn(ip, ip_net) + + def test_port_create_on_unconnected_host(self): + """Binding information provided, host not connected to any segment""" + network, segment, _subnet = self._create_test_segment_with_subnet() + response = self._create_port(self.fmt, + net_id=network['network']['id'], + tenant_id=network['network']['tenant_id'], + arg_list=(portbindings.HOST_ID,), + **{portbindings.HOST_ID: 'fakehost'}) + res = self.deserialize(self.fmt, response) + + self.assertEqual(webob.exc.HTTPConflict.code, response.status_int) + self.assertEqual(segment_exc.HostNotConnectedToAnySegment.__name__, + res['NeutronError']['type']) + + # Ensure that mapping the segment to other hosts doesn't trip it up + self._setup_host_mappings([(segment['segment']['id'], 'otherhost')]) + response = self._create_port(self.fmt, + net_id=network['network']['id'], + tenant_id=network['network']['tenant_id'], + arg_list=(portbindings.HOST_ID,), + **{portbindings.HOST_ID: 'fakehost'}) + res = self.deserialize(self.fmt, response) + + self.assertEqual(webob.exc.HTTPConflict.code, response.status_int) + self.assertEqual(segment_exc.HostNotConnectedToAnySegment.__name__, + res['NeutronError']['type']) + + def test_port_create_on_multiconnected_host(self): + """Binding information provided, host connected to multiple segments""" + network, segments, subnets = self._create_test_segments_with_subnets(2) + + # This host is bound to multiple hosts + self._setup_host_mappings([(segments[0]['segment']['id'], 'fakehost'), + (segments[1]['segment']['id'], 'fakehost')]) + + response = self._create_port(self.fmt, + net_id=network['network']['id'], + tenant_id=network['network']['tenant_id'], + arg_list=(portbindings.HOST_ID,), + **{portbindings.HOST_ID: 'fakehost'}) + res = self.deserialize(self.fmt, response) + + self.assertEqual(webob.exc.HTTPConflict.code, response.status_int) + self.assertEqual(segment_exc.HostConnectedToMultipleSegments.__name__, + res['NeutronError']['type'])