Filtering BGP VPN list with resource associations

Code Issues Proposed changes

For db based drivers, use the Neutron db query hooks mechanism to apply
resource association filters on query results. That improvement permits to
remove two mandatory methods of the driver interface
'find_bgpvpns_for_network' and 'find_bgpvpns_for_router' which can be
replaced by the 'get_bgpvpns' method with appropriated filters.
For non db based drivers, improves utils method to filter resources with
a list of values instead of one value only. In that case, the resource
attribute value must contains filtered values at least (like it's done
for db based drivers).
Change-Id: Icbbb7ba704fbb3c025aabbddbb5121a2ecd852cc
Closes-Bug: #1659895
Implements blueprint api-find-bgpvpn-for-net

This commit is contained in:

Édouard Thuleau

2017年01月27日 18:09:11 +01:00

parent fbebea5ac9

commit 3fd8f29493

11 changed files with 252 additions and 74 deletions

Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL

Download Patch File Download Diff File Expand all files Collapse all files

36
networking_bgpvpn/neutron/db/bgpvpn_db.py

View File

@@ -132,6 +132,26 @@ class BGPVPNPluginDb(common_db_mixin.CommonDbMixin):

}

return self._fields(res, fields)

def _list_bgpvpns_result_filter_hook(self, query, filters):

values = filters and filters.get('networks', [])

if values:

query = query.join(BGPVPNNetAssociation)

query = query.filter(BGPVPNNetAssociation.network_id.in_(values))

values = filters and filters.get('routers', [])

if values:

query = query.join(BGPVPNRouterAssociation)

query = query.filter(BGPVPNRouterAssociation.router_id.in_(values))

return query

common_db_mixin.CommonDbMixin.register_model_query_hook(

BGPVPN,

"bgpvpn_filter_by_resource_association",

None,

'_list_bgpvpns_result_filter_hook')

def create_bgpvpn(self, context, bgpvpn):

rt = utils.rtrd_list2str(bgpvpn['route_targets'])

i_rt = utils.rtrd_list2str(bgpvpn['import_targets'])

@@ -194,15 +214,6 @@ class BGPVPNPluginDb(common_db_mixin.CommonDbMixin):

context.session.delete(bgpvpn_db)

return bgpvpn

def find_bgpvpns_for_network(self, context, network_id, bgpvpn_type=None):

# Note : we could use added backref in the network table

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'],

'tenant_id': net_assoc_db['tenant_id'],

@@ -259,13 +270,6 @@ class BGPVPNPluginDb(common_db_mixin.CommonDbMixin):

context.session.delete(net_assoc_db)

return net_assoc

def find_bgpvpns_for_router(self, context, router_id):

query = (context.session.query(BGPVPN).

join(BGPVPNRouterAssociation).

filter(BGPVPNRouterAssociation.router_id == router_id))

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'],

'tenant_id': router_assoc_db['tenant_id'],

11
networking_bgpvpn/neutron/services/common/utils.py

View File

@@ -41,10 +41,13 @@ def filter_resource(resource, filters=None):

filters = {}

for key, value in filters.items():

if key in resource.keys():

if isinstance(value, list):

if resource[key] not in value:

return False

elif resource[key] != value:

if not isinstance(value, list):

value = [value]

if isinstance(resource[key], list):

resource_value = resource[key]

else:

resource_value = [resource[key]]

if not set(value).issubset(set(resource_value)):

return False

return True

17
networking_bgpvpn/neutron/services/plugin.py

View File

@@ -70,12 +70,21 @@ class BGPVPNPlugin(bgpvpn.BGPVPNPluginBase):

network_id = kwargs.get('network_id')

router_id = kwargs.get('router_id')

try:

routers_bgpvpns = self.driver.find_bgpvpns_for_router(context,

router_id)

routers_bgpvpns = self.driver.get_bgpvpns(

context,

filters={

'routers': [router_id],

},

)

except bgpvpn.BGPVPNRouterAssociationNotSupported:

return

nets_bgpvpns = self.driver.find_bgpvpns_for_network(

context, network_id, bgpvpn_type=constants.BGPVPN_L3)

nets_bgpvpns = self.driver.get_bgpvpns(

context,

filters={

'networks': [network_id],

'type': [constants.BGPVPN_L3],

},

)

if routers_bgpvpns and nets_bgpvpns:

msg = _('It is not allowed to add an interface to a router if '

10
networking_bgpvpn/neutron/services/service_drivers/bagpipe/bagpipe.py

View File

@@ -271,9 +271,13 @@ class BaGPipeBGPVPNDriver(driver_api.BGPVPNDriver):

def _bgpvpns_for_network(self, context, network_id):

return (

self.bgpvpn_db.find_bgpvpns_for_network(context, network_id)

or self.retrieve_bgpvpns_of_router_assocs_by_network(context,

network_id)

self.bgpvpn_db.get_bgpvpns(

context,

filters={

'networks': [network_id],

},

) or self.retrieve_bgpvpns_of_router_assocs_by_network(context,

network_id)

)

def _retrieve_bgpvpn_network_info_for_port(self, context, port):

15
networking_bgpvpn/neutron/services/service_drivers/driver_api.py

View File

@@ -70,10 +70,6 @@ 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

@@ -90,10 +86,6 @@ 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):

@@ -158,10 +150,6 @@ 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,

@@ -184,9 +172,6 @@ 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

10
networking_bgpvpn/neutron/services/service_drivers/opencontrail/opencontrail.py

View File

@@ -406,12 +406,6 @@ 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)

@@ -427,7 +421,3 @@ 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)

8
networking_bgpvpn/neutron/services/service_drivers/opendaylight/odl.py

View File

@@ -74,8 +74,12 @@ class OpenDaylightBgpvpnDriver(driver_api.BGPVPNDriver):

self.client.sendjson('put', url, {BGPVPNS[:-1]: bgpvpn})

def create_net_assoc_precommit(self, context, net_assoc):

bgpvpns = self.bgpvpn_db.find_bgpvpns_for_network(

context, net_assoc['network_id'])

bgpvpns = self.bgpvpn_db.get_bgpvpns(

context,

filters={

'networks': [net_assoc['network_id']],

},

)

if len(bgpvpns) > 1:

raise bgpvpn_ext.BGPVPNNetworkAssocExistsAnotherBgpvpn(

driver=OPENDAYLIGHT_BGPVPN_DRIVER_NAME,

154
networking_bgpvpn/tests/unit/db/test_db.py

View File

@@ -23,6 +23,10 @@ from networking_bgpvpn.neutron.services.common import constants

from networking_bgpvpn.tests.unit.services import test_plugin

def _id_list(list):

return [bgpvpn['id'] for bgpvpn in list]

class BgpvpnDBTestCase(test_plugin.BgpvpnTestCaseMixin):

def setUp(self):

@@ -113,10 +117,12 @@ class BgpvpnDBTestCase(test_plugin.BgpvpnTestCaseMixin):

self.assertEqual([], bgpvpn2['route_distinguishers'])

# find bgpvpn by network_id

bgpvpn3 = self.plugin_db.find_bgpvpns_for_network(

bgpvpn3 = self.plugin_db.get_bgpvpns(

self.ctx,

net['network']['id']

)

filters={

'networks': [net['network']['id']],

},

)

self.assertEqual(1, len(bgpvpn3))

self.assertEqual(bgpvpn2['id'], bgpvpn3[0]['id'])

@@ -201,7 +207,7 @@ class BgpvpnDBTestCase(test_plugin.BgpvpnTestCaseMixin):

id, {'tenant_id': self._tenant_id,

'network_id': net_id})

def test_db_find_bgpvpn_for_net(self):

def test_db_find_bgpvpn_for_associated_network(self):

with self.network() as net, \

self.bgpvpn(type=constants.BGPVPN_L2) as bgpvpn_l2, \

self.bgpvpn() as bgpvpn_l3, \

@@ -211,25 +217,36 @@ class BgpvpnDBTestCase(test_plugin.BgpvpnTestCaseMixin):

net['network']['id']):

net_id = net['network']['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.plugin_db.get_bgpvpns(

self.ctx,

filters={'networks': [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.get_bgpvpns(

self.ctx,

filters={

'networks': [net_id],

'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.plugin_db.get_bgpvpns(

self.ctx,

filters={

'networks': [net_id],

'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])

@@ -253,14 +270,16 @@ class BgpvpnDBTestCase(test_plugin.BgpvpnTestCaseMixin):

bgpvpn = self.plugin_db.get_bgpvpn(self.ctx, id)

self.assertEqual([], bgpvpn['routers'])

def test_db_find_bgpvpn_for_router(self):

def test_db_find_bgpvpn_for_associated_router(self):

with self.router(tenant_id=self._tenant_id) as router:

router_id = router['router']['id']

with self.bgpvpn() as bgpvpn:

id = bgpvpn['bgpvpn']['id']

with self.assoc_router(id, router_id=router_id):

bgpvpn_list = self.plugin_db.\

find_bgpvpns_for_router(self.ctx, router_id)

bgpvpn_list = self.plugin_db.get_bgpvpns(

self.ctx,

filters={'routers': [router_id]},

)

self.assertEqual(id, bgpvpn_list[0]['id'])

def test_db_delete_router(self):

@@ -272,3 +291,98 @@ class BgpvpnDBTestCase(test_plugin.BgpvpnTestCaseMixin):

do_disassociate=False)

bgpvpn_db = self.plugin_db.get_bgpvpn(self.ctx, id)

self.assertEqual([], bgpvpn_db['routers'])

def test_db_list_bgpvpn_filtering_associated_resources(self):

with self.network() as network1, \

self.network() as network2, \

self.router(tenant_id=self._tenant_id) as router1, \

self.router(tenant_id=self._tenant_id) as router2, \

self.bgpvpn() as bgpvpn1, \

self.bgpvpn() as bgpvpn2, \

self.bgpvpn() as bgpvpn3, \

self.assoc_net(bgpvpn1['bgpvpn']['id'],

network1['network']['id']), \

self.assoc_router(bgpvpn3['bgpvpn']['id'],

router1['router']['id']), \

self.assoc_net(bgpvpn2['bgpvpn']['id'],

network2['network']['id']), \

self.assoc_router(bgpvpn2['bgpvpn']['id'],

router2['router']['id']):

network1_id = network1['network']['id']

network2_id = network2['network']['id']

router1_id = router1['router']['id']

router2_id = router2['router']['id']

bgpvpn_id_list = _id_list(

self.plugin_db.get_bgpvpns(

self.ctx,

filters={

'networks': [network1_id],

},

)

self.assertIn(bgpvpn1['bgpvpn']['id'], bgpvpn_id_list)

self.assertNotIn(bgpvpn2['bgpvpn']['id'], bgpvpn_id_list)

self.assertNotIn(bgpvpn3['bgpvpn']['id'], bgpvpn_id_list)

bgpvpn_id_list = _id_list(

self.plugin_db.get_bgpvpns(

self.ctx,

filters={

'networks': [network1_id, network2_id],

},

)

self.assertIn(bgpvpn1['bgpvpn']['id'], bgpvpn_id_list)

self.assertIn(bgpvpn2['bgpvpn']['id'], bgpvpn_id_list)

self.assertNotIn(bgpvpn3['bgpvpn']['id'], bgpvpn_id_list)

bgpvpn_id_list = _id_list(

self.plugin_db.get_bgpvpns(

self.ctx,

filters={

'routers': [router1_id],

},

)

self.assertNotIn(bgpvpn1['bgpvpn']['id'], bgpvpn_id_list)

self.assertNotIn(bgpvpn2['bgpvpn']['id'], bgpvpn_id_list)

self.assertIn(bgpvpn3['bgpvpn']['id'], bgpvpn_id_list)

bgpvpn_id_list = _id_list(

self.plugin_db.get_bgpvpns(

self.ctx,

filters={

'routers': [router1_id, router2_id],

},

)

self.assertNotIn(bgpvpn1['bgpvpn']['id'], bgpvpn_id_list)

self.assertIn(bgpvpn2['bgpvpn']['id'], bgpvpn_id_list)

self.assertIn(bgpvpn3['bgpvpn']['id'], bgpvpn_id_list)

bgpvpn_id_list = _id_list(

self.plugin_db.get_bgpvpns(

self.ctx,

filters={

'networks': [network1_id],

'routers': [router1_id],

},

)

self.assertNotIn(bgpvpn1['bgpvpn']['id'], bgpvpn_id_list)

self.assertNotIn(bgpvpn2['bgpvpn']['id'], bgpvpn_id_list)

self.assertNotIn(bgpvpn3['bgpvpn']['id'], bgpvpn_id_list)

bgpvpn_id_list = _id_list(

self.plugin_db.get_bgpvpns(

self.ctx,

filters={

'networks': [network2_id],

'routers': [router2_id],

},

)

self.assertNotIn(bgpvpn1['bgpvpn']['id'], bgpvpn_id_list)

self.assertIn(bgpvpn2['bgpvpn']['id'], bgpvpn_id_list)

self.assertNotIn(bgpvpn3['bgpvpn']['id'], bgpvpn_id_list)

0
networking_bgpvpn/tests/unit/services/common/init.py Normal file

View File

61
networking_bgpvpn/tests/unit/services/common/test_utils.py Normal file

View File

@@ -0,0 +1,61 @@

#

# Licensed under the Apache License, Version 2.0 (the "License"); you may

# not use this file except in compliance with the License. You may obtain

# a copy of the License at

#

# http://www.apache.org/licenses/LICENSE-2.0

#

# Unless required by applicable law or agreed to in writing, software

# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT

# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the

# License for the specific language governing permissions and limitations

# under the License.

from neutron.tests import base

from networking_bgpvpn.neutron.services.common.utils import filter_resource

class TestFilterResource(base.BaseTestCase):

_fake_resource_string = {

'fake_attribute': 'fake_value1',

}

_fake_resource_list = {

'fake_attribute': ['fake_value1', 'fake_value2', 'fake_value3'],

}

def test_filter_resource_succeeds_with_one_value(self):

filters = {

'fake_attribute': 'fake_value1',

}

self.assertTrue(filter_resource(self._fake_resource_string, filters))

self.assertTrue(filter_resource(self._fake_resource_list, filters))

def test_filter_resource_fails_with_one_value(self):

filters = {

'fake_attribute': 'wrong_fake_value1',

}

self.assertFalse(filter_resource(self._fake_resource_string, filters))

self.assertFalse(filter_resource(self._fake_resource_list, filters))

def test_filter_resource_succeeds_with_list_of_values(self):

filters = {

'fake_attribute': ['fake_value1'],

}

self.assertTrue(filter_resource(self._fake_resource_string, filters))

filters = {

'fake_attribute': ['fake_value1', 'fake_value2'],

}

self.assertTrue(filter_resource(self._fake_resource_list, filters))

def test_filter_resource_fails_with_list_of_values(self):

filters = {

'fake_attribute': ['wrong_fake_value1'],

}

self.assertFalse(filter_resource(self._fake_resource_string, filters))

filters = {

'fake_attribute': ['wrong_fake_value1', 'fake_value2'],

}

self.assertFalse(filter_resource(self._fake_resource_list, filters))

4
releasenotes/notes/filtering-on-resource-association-2acdbc5b59d1a40a.yaml Normal file

View File

@@ -0,0 +1,4 @@

---

features:

- The API now supports filtering BGPVPN resources based

onthe networks or routers they are associated with.