NSXv BGP: Add more unittests

Also adding a check that only external network could be associtated with
a BGP speaker

Change-Id: I4f19f305c37b98f9dad8a79294e1ec198e8da22a

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 "

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 "
+            raise nsx_exc.NsxBgpGatewayNetworkHasNoSubnets(net_id=network_id)
-                      "associating it with BGP speaker.")
-            return False
 
         # REVISIT(roeyc): Currently not allowing more than one bgp speaker per
         # gateway network.

vmware_nsx/tests/unit/nsx_v/vshield/fake_vcns.py

@@ -1483,9 +1483,12 @@ 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 = {
+            'featureType': '',
+            'ospf': {},
+            'routingGlobalConfig': {
                 'routerId': '172.24.4.12',
                 'ipPrefixes': {
                     'ipPrefixes': [
@@ -1499,8 +1502,13 @@ class FakeVcns(object):
                 },
                 '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 = {

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,9 +115,6 @@ 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'])
 
@@ -95,8 +123,26 @@ class TestNSXvBgpPlugin(test_plugin.NsxVPluginV2TestCase,
             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.
+        bgp_peer = {'bgp_peer':
-        pass
+                    {'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