diff --git a/networking_bgpvpn/neutron/db/bgpvpn_db.py b/networking_bgpvpn/neutron/db/bgpvpn_db.py index d4450850..e0338f13 100644 --- a/networking_bgpvpn/neutron/db/bgpvpn_db.py +++ b/networking_bgpvpn/neutron/db/bgpvpn_db.py @@ -192,17 +192,14 @@ class BGPVPNPluginDb(common_db_mixin.CommonDbMixin): context.session.delete(bgpvpn_db) return bgpvpn - def find_bgpvpns_for_network(self, context, network_id): + def find_bgpvpns_for_network(self, context, network_id, bgpvpn_type=None): # Note : we could use added backref in the network table - try: - query = (context.session.query(BGPVPN). - join(BGPVPNNetAssociation). - filter(BGPVPNNetAssociation.network_id == network_id)) - except exc.NoResultFound: - return - - return [self._make_bgpvpn_dict(bgpvpn) - for bgpvpn in query.all()] + query = (context.session.query(BGPVPN). + join(BGPVPNNetAssociation). + filter(BGPVPNNetAssociation.network_id == network_id)) + if bgpvpn_type is not None: + query = query.filter(BGPVPN.type == bgpvpn_type) + return [self._make_bgpvpn_dict(bgpvpn) for bgpvpn in query.all()] def _make_net_assoc_dict(self, net_assoc_db, fields=None): res = {'id': net_assoc_db['id'], @@ -261,15 +258,11 @@ class BGPVPNPluginDb(common_db_mixin.CommonDbMixin): return net_assoc def find_bgpvpns_for_router(self, context, router_id): - try: - query = (context.session.query(BGPVPN). - join(BGPVPNRouterAssociation). - filter(BGPVPNRouterAssociation.router_id == router_id)) - except exc.NoResultFound: - return + query = (context.session.query(BGPVPN). + join(BGPVPNRouterAssociation). + filter(BGPVPNRouterAssociation.router_id == router_id)) - return [self._make_bgpvpn_dict(bgpvpn) - for bgpvpn in query.all()] + return [self._make_bgpvpn_dict(bgpvpn) for bgpvpn in query.all()] def _make_router_assoc_dict(self, router_assoc_db, fields=None): res = {'id': router_assoc_db['id'], diff --git a/networking_bgpvpn/neutron/extensions/bgpvpn.py b/networking_bgpvpn/neutron/extensions/bgpvpn.py index fbae48be..8cba38e1 100755 --- a/networking_bgpvpn/neutron/extensions/bgpvpn.py +++ b/networking_bgpvpn/neutron/extensions/bgpvpn.py @@ -65,6 +65,11 @@ class BGPVPNRDNotSupported(n_exc.BadRequest): "route distinguisher") +class BGPVPNFindFromNetNotSupported(n_exc.BadRequest): + message = _("BGPVPN %(driver)s driver does not support to fetch BGPVPNs " + "associated to network id %(net_id)") + + class BGPVPNNetAssocAlreadyExists(n_exc.BadRequest): message = _("network %(net_id)s is already associated to " "BGPVPN %(bgpvpn_id)s") diff --git a/networking_bgpvpn/neutron/services/plugin.py b/networking_bgpvpn/neutron/services/plugin.py index 01986e1a..c453fd2e 100644 --- a/networking_bgpvpn/neutron/services/plugin.py +++ b/networking_bgpvpn/neutron/services/plugin.py @@ -13,6 +13,9 @@ # License for the specific language governing permissions and limitations # under the License. +from neutron.callbacks import events +from neutron.callbacks import registry +from neutron.callbacks import resources from neutron.db import servicetype_db as st_db from neutron import manager from neutron.plugins.common import constants as plugin_constants @@ -26,13 +29,13 @@ from oslo_log import log from networking_bgpvpn._i18n import _LI -from networking_bgpvpn.neutron.extensions.bgpvpn import BGPVPNPluginBase +from networking_bgpvpn.neutron.extensions import bgpvpn from networking_bgpvpn.neutron.services.common import constants LOG = log.getLogger(__name__) -class BGPVPNPlugin(BGPVPNPluginBase): +class BGPVPNPlugin(bgpvpn.BGPVPNPluginBase): supported_extension_aliases = ["bgpvpn"] path_prefix = "/bgpvpn" @@ -57,6 +60,28 @@ class BGPVPNPlugin(BGPVPNPluginBase): LOG.warning(_LI("Multiple drivers configured for BGPVPN, although" "running multiple drivers in parallel is not yet" "supported")) + registry.subscribe(self._notify_adding_interface_to_router, + resources.ROUTER_INTERFACE, + events.BEFORE_CREATE) + + def _notify_adding_interface_to_router(self, resource, event, trigger, + **kwargs): + context = kwargs.get('context') + network_id = kwargs.get('network_id') + router_id = kwargs.get('router_id') + try: + routers_bgpvpns = self.driver.find_bgpvpns_for_router(context, + router_id) + except bgpvpn.BGPVPNRouterAssociationNotSupported: + return + nets_bgpvpns = self.driver.find_bgpvpns_for_network( + context, network_id, bgpvpn_type=constants.BGPVPN_L3) + + if routers_bgpvpns and nets_bgpvpns: + msg = _('It is not allowed to add an interface to a router if ' + 'both the router and the network are bound to an ' + 'L3 BGPVPN.') + raise n_exc.BadRequest(resource='bgpvpn', msg=msg) def _validate_network(self, context, net_id): plugin = manager.NeutronManager.get_plugin() diff --git a/networking_bgpvpn/neutron/services/service_drivers/driver_api.py b/networking_bgpvpn/neutron/services/service_drivers/driver_api.py index 82e06a8f..a76436c2 100644 --- a/networking_bgpvpn/neutron/services/service_drivers/driver_api.py +++ b/networking_bgpvpn/neutron/services/service_drivers/driver_api.py @@ -70,6 +70,10 @@ class BGPVPNDriverBase(object): def delete_net_assoc(self, context, assoc_id, bgpvpn_id): pass + @abc.abstractmethod + def find_bgpvpns_for_network(self, context, network_id, bgpvpn_type=None): + pass + @abc.abstractmethod def create_router_assoc(self, context, bgpvpn_id, router_association): pass @@ -86,6 +90,10 @@ class BGPVPNDriverBase(object): def delete_router_assoc(self, context, assoc_id, bgpvpn_id): pass + @abc.abstractmethod + def find_bgpvpns_for_router(self, context, router_id): + pass + @six.add_metaclass(abc.ABCMeta) class BGPVPNDriverDBMixin(BGPVPNDriverBase): @@ -150,6 +158,10 @@ class BGPVPNDriverDBMixin(BGPVPNDriverBase): bgpvpn_id) self.delete_net_assoc_postcommit(context, net_assoc) + def find_bgpvpns_for_network(self, context, network_id, bgpvpn_type=None): + return self.bgpvpn_db.find_bgpvpns_for_network( + context, network_id, bgpvpn_type) + def create_router_assoc(self, context, bgpvpn_id, router_association): with context.session.begin(subtransactions=True): assoc = self.bgpvpn_db.create_router_assoc(context, bgpvpn_id, @@ -172,6 +184,9 @@ class BGPVPNDriverDBMixin(BGPVPNDriverBase): bgpvpn_id) self.delete_router_assoc_postcommit(context, router_assoc) + def find_bgpvpns_for_router(self, context, router_id): + return self.bgpvpn_db.find_bgpvpns_for_router(context, router_id) + @abc.abstractmethod def create_bgpvpn_postcommit(self, context, bgpvpn): pass diff --git a/networking_bgpvpn/neutron/services/service_drivers/opencontrail/opencontrail.py b/networking_bgpvpn/neutron/services/service_drivers/opencontrail/opencontrail.py index 5065e907..2d69205c 100644 --- a/networking_bgpvpn/neutron/services/service_drivers/opencontrail/opencontrail.py +++ b/networking_bgpvpn/neutron/services/service_drivers/opencontrail/opencontrail.py @@ -411,6 +411,12 @@ class OpenContrailBGPVPNDriver(driver_api.BGPVPNDriverBase): return net_assoc + def find_bgpvpns_for_network(self, context, network_id, bgpvpn_type=None): + # TODO(matrohon) : get bgpvpns from the kv store + raise bgpvpn_ext.BGPVPNFindFromNetNotSupported( + driver=OPENCONTRAIL_BGPVPN_DRIVER_NAME, + net_id=network_id) + def create_router_assoc(self, context, bgpvpn_id, router_association): raise bgpvpn_ext.BGPVPNRouterAssociationNotSupported( driver=OPENCONTRAIL_BGPVPN_DRIVER_NAME) @@ -426,3 +432,7 @@ class OpenContrailBGPVPNDriver(driver_api.BGPVPNDriverBase): def delete_router_assoc(self, context, assoc_id, bgpvpn_id): raise bgpvpn_ext.BGPVPNRouterAssociationNotSupported( driver=OPENCONTRAIL_BGPVPN_DRIVER_NAME) + + def find_bgpvpns_for_router(self, context, router_id): + raise bgpvpn_ext.BGPVPNRouterAssociationNotSupported( + driver=OPENCONTRAIL_BGPVPN_DRIVER_NAME) diff --git a/networking_bgpvpn/tests/unit/db/test_db.py b/networking_bgpvpn/tests/unit/db/test_db.py index 4784f11c..04cf5c32 100644 --- a/networking_bgpvpn/tests/unit/db/test_db.py +++ b/networking_bgpvpn/tests/unit/db/test_db.py @@ -19,6 +19,7 @@ from networking_bgpvpn.neutron.extensions.bgpvpn \ import BGPVPNNetAssocAlreadyExists from networking_bgpvpn.neutron.extensions.bgpvpn import BGPVPNNetAssocNotFound from networking_bgpvpn.neutron.extensions.bgpvpn import BGPVPNNotFound +from networking_bgpvpn.neutron.services.common import constants from networking_bgpvpn.tests.unit.services import test_plugin @@ -201,14 +202,36 @@ class BgpvpnDBTestCase(test_plugin.BgpvpnTestCaseMixin): 'network_id': net_id}) def test_db_find_bgpvpn_for_net(self): - with self.network() as net: + with self.network() as net, \ + self.bgpvpn(type=constants.BGPVPN_L2) as bgpvpn_l2, \ + self.bgpvpn() as bgpvpn_l3, \ + self.assoc_net(bgpvpn_l2['bgpvpn']['id'], + net['network']['id']), \ + self.assoc_net(bgpvpn_l3['bgpvpn']['id'], + net['network']['id']): net_id = net['network']['id'] - with self.bgpvpn() as bgpvpn: - id = bgpvpn['bgpvpn']['id'] - with self.assoc_net(id, net_id=net_id): - bgpvpn_list = self.plugin_db.\ - find_bgpvpns_for_network(self.ctx, net_id) - self.assertEqual(id, bgpvpn_list[0]['id']) + + def _id_list(list): + return [bgpvpn['id'] for bgpvpn in list] + + bgpvpn_id_list = _id_list( + self.plugin_db.find_bgpvpns_for_network(self.ctx, net_id)) + self.assertIn(bgpvpn_l2['bgpvpn']['id'], + bgpvpn_id_list) + self.assertIn(bgpvpn_l3['bgpvpn']['id'], + bgpvpn_id_list) + bgpvpn_l2_id_list = _id_list( + self.plugin_db.find_bgpvpns_for_network( + self.ctx, net_id, bgpvpn_type=constants.BGPVPN_L2)) + + self.assertIn(bgpvpn_l2['bgpvpn']['id'], bgpvpn_l2_id_list) + self.assertNotIn(bgpvpn_l3['bgpvpn']['id'], bgpvpn_l2_id_list) + + bgpvpn_l3_id_list = _id_list( + self.plugin_db.find_bgpvpns_for_network( + self.ctx, net_id, bgpvpn_type=constants.BGPVPN_L3)) + self.assertNotIn(bgpvpn_l2['bgpvpn']['id'], bgpvpn_l3_id_list[0]) + self.assertIn(bgpvpn_l3['bgpvpn']['id'], bgpvpn_l3_id_list[0]) def test_db_delete_net(self): with self.bgpvpn() as bgpvpn: diff --git a/networking_bgpvpn/tests/unit/services/test_plugin.py b/networking_bgpvpn/tests/unit/services/test_plugin.py index c92df2e6..9ba1b059 100644 --- a/networking_bgpvpn/tests/unit/services/test_plugin.py +++ b/networking_bgpvpn/tests/unit/services/test_plugin.py @@ -102,14 +102,15 @@ class BgpvpnTestCaseMixin(test_db_base_plugin_v2.NeutronDbPluginV2TestCase, @contextlib.contextmanager def bgpvpn(self, do_delete=True, **kwargs): + req_data = copy.deepcopy(self.bgpvpn_data) + fmt = 'json' if kwargs.get('data'): - bgpvpn_data = kwargs.get('data') + req_data = kwargs.get('data') else: - bgpvpn_data = copy.copy(self.bgpvpn_data) - bgpvpn_data['bgpvpn'].update(kwargs) + req_data['bgpvpn'].update(kwargs) bgpvpn_req = self.new_create_request( - 'bgpvpn/bgpvpns', bgpvpn_data, fmt=fmt) + 'bgpvpn/bgpvpns', req_data, fmt=fmt) res = bgpvpn_req.get_response(self.ext_api) if res.status_int>= 400: raise webob.exc.HTTPClientError(code=res.status_int) @@ -436,6 +437,45 @@ class TestBGPVPNServicePlugin(BgpvpnTestCaseMixin): res = bgpvpn_router_req.get_response(self.ext_api) self.assertEqual(res.status_int, webob.exc.HTTPBadRequest.code) + def test_attach_subnet_to_router_both_attached_to_bgpvpn(self): + with self.network() as net,\ + self.bgpvpn() as bgpvpn,\ + self.router(tenant_id=self._tenant_id) as router,\ + self.subnet(network={'network': net['network']}) as subnet,\ + self.assoc_net(bgpvpn['bgpvpn']['id'], net['network']['id']),\ + self.assoc_router(bgpvpn['bgpvpn']['id'], + router['router']['id']): + # Attach subnet to router + data = {"subnet_id": subnet['subnet']['id']} + bgpvpn_rtr_intf_req = self.new_update_request( + 'routers', + data=data, + fmt=self.fmt, + id=router['router']['id'], + subresource='add_router_interface') + res = bgpvpn_rtr_intf_req.get_response(self.ext_api) + self.assertEqual(res.status_int, webob.exc.HTTPConflict.code) + + def test_attach_port_to_router_both_attached_to_bgpvpn(self): + with self.network() as net,\ + self.bgpvpn() as bgpvpn,\ + self.router(tenant_id=self._tenant_id) as router,\ + self.subnet(network={'network': net['network']}) as subnet,\ + self.port(subnet={'subnet': subnet['subnet']}) as port,\ + self.assoc_net(bgpvpn['bgpvpn']['id'], net['network']['id']),\ + self.assoc_router(bgpvpn['bgpvpn']['id'], + router['router']['id']): + # Attach subnet to router + data = {"port_id": port['port']['id']} + bgpvpn_rtr_intf_req = self.new_update_request( + 'routers', + data=data, + fmt=self.fmt, + id=router['router']['id'], + subresource='add_router_interface') + res = bgpvpn_rtr_intf_req.get_response(self.ext_api) + self.assertEqual(res.status_int, webob.exc.HTTPConflict.code) + class TestBGPVPNServiceDriverDB(BgpvpnTestCaseMixin): diff --git a/networking_bgpvpn_tempest/tests/api/test_bgpvpn.py b/networking_bgpvpn_tempest/tests/api/test_bgpvpn.py index 45e29af4..835cbd4c 100644 --- a/networking_bgpvpn_tempest/tests/api/test_bgpvpn.py +++ b/networking_bgpvpn_tempest/tests/api/test_bgpvpn.py @@ -14,6 +14,7 @@ # under the License. from networking_bgpvpn_tempest.tests.base import BaseBgpvpnTest as base +from tempest.common.utils import data_utils from tempest.lib import exceptions from tempest import test from testtools import ExpectedException @@ -275,3 +276,32 @@ class BgpvpnTest(base): self.assertEqual(updated_bgpvpn['bgpvpn']['routers'], []) self.routers_client.delete_router(router_id) + + @test.attr(type=['negative']) + def test_attach_associated_subnet_to_associated_router(self): + # Create a first bgpvpn and associate a network with a subnet to it + bgpvpn_net = self.create_bgpvpn( + self.bgpvpn_admin_client, + tenant_id=self.bgpvpn_client.tenant_id) + network = self.create_network() + subnet = self.create_subnet(network) + self.bgpvpn_client.create_network_association( + bgpvpn_net['id'], network['id']) + + # Create a second bgpvpn and associate a router to it + bgpvpn_router = self.create_bgpvpn( + self.bgpvpn_admin_client, + tenant_id=self.bgpvpn_client.tenant_id) + + router = self.create_router( + router_name=data_utils.rand_name('test-bgpvpn-')) + self.bgpvpn_client.create_router_association( + bgpvpn_router['id'], + router['id']) + + # Attach the subnet of the network to the router + subnet_data = {'subnet_id': subnet['id']} + self.assertRaises(exceptions.Conflict, + self.routers_client.add_router_interface, + router['id'], + **subnet_data)