diff --git a/vmware_nsx/common/exceptions.py b/vmware_nsx/common/exceptions.py index 3a6cb4770f..1d34ded646 100644 --- a/vmware_nsx/common/exceptions.py +++ b/vmware_nsx/common/exceptions.py @@ -184,6 +184,16 @@ class NsxBgpSpeakerUnableToAddGatewayNetwork(n_exc.BadRequest): "most.") +class NsxBgpNetworkNotExternal(n_exc.BadRequest): + message = _("Network %(net_id)s is not external, only external network " + "can be associated with a BGP speaker.") + + +class NsxBgpGatewayNetworkHasNoSubnets(n_exc.BadRequest): + message = _("Can't associate external network %(net_id)s with BGP " + "speaker, network doesn't contain any subnets.") + + class NsxRouterInterfaceDoesNotMatchAddressScope(n_exc.BadRequest): message = _("Unable to update no-NAT router %(router_id)s, " "only subnets allocated from address-scope " diff --git a/vmware_nsx/services/dynamic_routing/nsx_v/driver.py b/vmware_nsx/services/dynamic_routing/nsx_v/driver.py index 8a2bebdf67..429eb04336 100644 --- a/vmware_nsx/services/dynamic_routing/nsx_v/driver.py +++ b/vmware_nsx/services/dynamic_routing/nsx_v/driver.py @@ -21,6 +21,7 @@ from oslo_log import log as logging from oslo_utils import excutils from neutron.extensions import address_scope +from neutron.extensions import external_net from neutron_lib import constants as n_const from neutron_lib import exceptions as n_exc from neutron_lib.plugins import directory @@ -376,10 +377,10 @@ class NSXvBgpDriver(object): def _validate_gateway_network(self, context, speaker_id, network_id): ext_net = self._core_plugin.get_network(context, network_id) + if not ext_net.get(external_net.EXTERNAL): + raise nsx_exc.NsxBgpNetworkNotExternal(net_id=network_id) if not ext_net['subnets']: - LOG.debug("External network should have a subnet before " - "associating it with BGP speaker.") - return False + raise nsx_exc.NsxBgpGatewayNetworkHasNoSubnets(net_id=network_id) # REVISIT(roeyc): Currently not allowing more than one bgp speaker per # gateway network. diff --git a/vmware_nsx/tests/unit/nsx_v/vshield/fake_vcns.py b/vmware_nsx/tests/unit/nsx_v/vshield/fake_vcns.py index 410cf2aeb0..a5a0686ffc 100644 --- a/vmware_nsx/tests/unit/nsx_v/vshield/fake_vcns.py +++ b/vmware_nsx/tests/unit/nsx_v/vshield/fake_vcns.py @@ -1483,24 +1483,32 @@ class FakeVcns(object): } return self.return_helper(header, response) - def get_dynamic_routing_service(self, edge_id): + def get_edge_routing_config(self, edge_id): header = {'status': 200} response = { - 'routerId': '172.24.4.12', - 'ipPrefixes': { - 'ipPrefixes': [ - {'ipAddress': '10.0.0.0/24', - 'name': 'prefix-name'} - ] - }, - 'logging': { - 'logLevel': 'info', - 'enable': False - }, - 'ecmp': False + 'featureType': '', + 'ospf': {}, + 'routingGlobalConfig': { + 'routerId': '172.24.4.12', + 'ipPrefixes': { + 'ipPrefixes': [ + {'ipAddress': '10.0.0.0/24', + 'name': 'prefix-name'} + ] + }, + 'logging': { + 'logLevel': 'info', + 'enable': False + }, + 'ecmp': False + } } return self.return_helper(header, response) + def update_edge_routing_config(self, edge_id, request): + header = {'status': 200} + return self.return_helper(header, {}) + def update_bgp_dynamic_routing(self, edge_id, bgp_request): header = {"status": 201} response = { diff --git a/vmware_nsx/tests/unit/services/dynamic_routing/test_nsxv_bgp_driver.py b/vmware_nsx/tests/unit/services/dynamic_routing/test_nsxv_bgp_driver.py index 8925dd57f1..4eed00a1fe 100644 --- a/vmware_nsx/tests/unit/services/dynamic_routing/test_nsxv_bgp_driver.py +++ b/vmware_nsx/tests/unit/services/dynamic_routing/test_nsxv_bgp_driver.py @@ -16,6 +16,7 @@ import contextlib import mock from neutron.api import extensions +from neutron.extensions import address_scope from neutron_dynamic_routing.db import bgp_db # noqa from neutron_dynamic_routing import extensions as dr_extensions from neutron_dynamic_routing.extensions import bgp as ext_bgp @@ -24,6 +25,7 @@ from neutron_lib import context from neutron_lib import exceptions as n_exc from neutron_lib.plugins import directory +from vmware_nsx.common import exceptions as exc from vmware_nsx.services.dynamic_routing import bgp_plugin from vmware_nsx.services.dynamic_routing.nsx_v import driver as bgp_driver from vmware_nsx.tests.unit.nsx_v import test_plugin @@ -38,11 +40,40 @@ class TestNSXvBgpPlugin(test_plugin.NsxVPluginV2TestCase, service_plugins = {ext_bgp.BGP_EXT_ALIAS: BGP_PLUGIN} super(TestNSXvBgpPlugin, self).setUp(service_plugins=service_plugins) self.bgp_plugin = bgp_plugin.NSXvBgpPlugin() + self.bgp_plugin.nsxv_driver._validate_gateway_network = mock.Mock() + self.bgp_plugin.nsxv_driver._validate_bgp_configuration_on_peer_esg = ( + mock.Mock()) self.plugin = directory.get_plugin() self.l3plugin = self.plugin self.plugin.init_is_complete = True self.context = context.get_admin_context() + @contextlib.contextmanager + def gw_network(self, external=True, **kwargs): + with super(TestNSXvBgpPlugin, self).gw_network(external=external, + **kwargs) as gw_net: + if external: + gw_net['network']['router:external'] = True + gw_net['network'][address_scope.IPV4_ADDRESS_SCOPE] = True + yield gw_net + + @contextlib.contextmanager + def subnet(self, network=None, **kwargs): + if network and network['network'].get('router:external'): + kwargs['gateway_ip'] = None + kwargs['enable_dhcp'] = False + + with super(TestNSXvBgpPlugin, self).subnet(network=network, + **kwargs) as sub: + yield sub + + @contextlib.contextmanager + def router(self, **kwargs): + if 'external_gateway_info' in kwargs: + kwargs['external_gateway_info']['enable_snat'] = False + with super(TestNSXvBgpPlugin, self).router(**kwargs) as r: + yield r + @contextlib.contextmanager def esg_bgp_peer(self, esg_id): data = {'name': '', @@ -84,19 +115,34 @@ class TestNSXvBgpPlugin(test_plugin.NsxVPluginV2TestCase, def test_bgp_peer_esg_id(self): edge_id = 'edge-123' - with mock.patch.object(bgp_driver.NSXvBgpDriver, - '_validate_bgp_configuration_on_peer_esg', - side_effect=None): - with self.esg_bgp_peer(esg_id='edge-123') as esg_peer: - self.assertEqual(edge_id, esg_peer['esg_id']) + with self.esg_bgp_peer(esg_id='edge-123') as esg_peer: + self.assertEqual(edge_id, esg_peer['esg_id']) - peer_id = esg_peer['id'] - bgp_peer = self.bgp_plugin.get_bgp_peer(self.context, peer_id) - self.assertEqual(edge_id, bgp_peer['esg_id']) + peer_id = esg_peer['id'] + bgp_peer = self.bgp_plugin.get_bgp_peer(self.context, peer_id) + self.assertEqual(edge_id, bgp_peer['esg_id']) def test_create_bgp_peer_md5_auth_no_password(self): - # TODO(roeyc): Test requires a minor fix in base class. - pass + bgp_peer = {'bgp_peer': + {'auth_type': 'md5', 'password': None, + 'peer_ip': '10.0.0.3'}} + self.assertRaises(ext_bgp.InvalidBgpPeerMd5Authentication, + self.bgp_plugin.create_bgp_peer, + self.context, bgp_peer) + + def test_add_non_external_gateway_network(self): + self.bgp_plugin.nsxv_driver._validate_gateway_network = ( + bgp_driver.NSXvBgpDriver( + self.bgp_plugin)._validate_gateway_network) + with self.gw_network(external=False) as net,\ + self.subnetpool_with_address_scope(4, + prefixes=['8.0.0.0/8']) as sp: + network_id = net['network']['id'] + with self.bgp_speaker(sp['ip_version'], 1234) as speaker: + self.assertRaises(exc.NsxBgpNetworkNotExternal, + self.bgp_plugin.add_gateway_network, + self.context, speaker['id'], + {'network_id': network_id}) def test__bgp_speakers_for_gateway_network_by_ip_version(self): # REVISIT(roeyc): Base class test use ipv6 which is not supported. @@ -115,33 +161,6 @@ class TestNSXvBgpPlugin(test_plugin.NsxVPluginV2TestCase, pass def test__get_address_scope_ids_for_bgp_speaker(self): - # REVISIT(roeyc): Base class creates subnets with gateway-ip on - # external network, NSXv plugin requires that gateway-ip is not - # specified for subnets in BGP backed networks. - pass - - def test_get_ipv4_tenant_subnet_routes_by_bgp_speaker_dvr_router(self): - # REVISIT(roeyc): Base class creates subnets with gateway-ip on - # external network, NSXv plugin requires that gateway-ip is not - # specified for subnets in BGP backed networks. - pass - - def test_get_ipv4_tenant_subnet_routes_by_bgp_speaker_ipv4(self): - # REVISIT(roeyc): Base class creates subnets with gateway-ip on - # external network, NSXv plugin requires that gateway-ip is not - # specified for subnets in BGP backed networks. - pass - - def test_get_routes_by_bgp_speaker_binding(self): - # REVISIT(roeyc): Base class creates subnets with gateway-ip on - # external network, NSXv plugin requires that gateway-ip is not - # specified for subnets in BGP backed networks. - pass - - def test_get_routes_by_binding_network(self): - # REVISIT(roeyc): Base class creates subnets with gateway-ip on - # external network, NSXv plugin requires that gateway-ip is not - # specified for subnets in BGP backed networks. pass def test__get_dvr_fip_host_routes_by_binding(self): @@ -156,15 +175,6 @@ class TestNSXvBgpPlugin(test_plugin.NsxVPluginV2TestCase, def test__get_fip_next_hop_legacy(self): pass - def test__get_routes_by_router_with_fip(self): - pass - - def test_get_routes_by_bgp_speaker_binding_with_fip(self): - pass - - def test_get_routes_by_bgp_speaker_id_with_fip(self): - pass - def test_get_routes_by_bgp_speaker_id_with_fip_dvr(self): pass