Merge "Make IPAM segment aware on port create"

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):
+    def _ipam_get_subnets(self, context, network_id, host):
-        query = self._get_collection_query(context, models_v2.Subnet)
+        Subnet = models_v2.Subnet
-        query = query.filter(models_v2.Subnet.network_id == network_id)
+        SegmentHostMapping = segment_svc_db.SegmentHostMapping
-        query = query.filter(models_v2.Subnet.segment_id == segment_id)
+
-        subnets = [self._make_subnet_dict(c, context=context) for c in query]
+        query = self._get_collection_query(context, Subnet)
-        return subnets
+        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(

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(
+        subnets = self._ipam_get_subnets(context,
-            context, network_id=p['network_id'], segment_id=None)
+                                         network_id=p['network_id'],
+                                         host=p.get(portbindings.HOST_ID))
 
         v4, v6_stateful, v6_stateless = self._classify_subnets(
             context, subnets)

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(
+        subnets = self._ipam_get_subnets(context,
-            context, network_id=p['network_id'], segment_id=None)
+                                         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,

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.")

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,

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
 
@@ -101,7 +104,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"
@@ -260,23 +263,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']
@@ -493,3 +479,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'])