AIM Policy Driver - Part 6 - Add Router Interfaces

This patch adds interface for the PTG subnets to the default Router.
Corresponding UTs are added which check that the correct port is created for
the router interface.

A bug due to which the apic_aim_l3 was not getting set for the GBP aim_mapping
tests is also being fixed.

Change-Id: Ia8e96da15b2571491412c649af5a99261ceb8a84

gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_mapping.py

@@ -281,8 +281,13 @@ class AIMMappingDriver(nrd.CommonNeutronBase):
             for subnet_id in subnet_ids:
                 if not context._plugin._get_ptgs_for_subnet(
                     context._plugin_context, subnet_id):
-                    # TODO(Sumit): pass router_id of default router
+                    l2p_id = context.current['l2_policy_id']
+                    router_id = None
+                    if l2p_id:
+                        l3p = self._get_l3p_for_l2policy(context, l2p_id)
+                        router_id = l3p['routers'][0]
                     self._cleanup_subnet(plugin_context, subnet_id,
+                                         router_id=router_id,
                                          clean_session=False)
 
         if ptg_db['l2_policy_id']:
@@ -770,7 +775,13 @@ class AIMMappingDriver(nrd.CommonNeutronBase):
             context.add_subnets(subs - set(context.current['subnets']))
             for subnet in added:
                 self._sync_ptg_subnets(context, l2p)
-            # TODO(Sumit): This subnet needs to added to the default router
+                l3p = self._get_l3p_for_l2policy(context, l2p_id)
+                # TODO(Sumit): Consider uplinking to multiple routers
+                # which is done to provide multiple external gateways
+                # in different Neutron external networks.
+                router_id = l3p['routers'][0]
+                self._add_router_interface_for_subnet(context, router_id,
+                                                      subnet['id'])
 
     def _create_implicit_contracts_and_configure_default_epg(
         self, context, l2p, epg_dn):

gbpservice/neutron/services/grouppolicy/drivers/resource_mapping.py

@@ -523,7 +523,8 @@ class ImplicitResourceOperations(local_api.LocalAPI):
         if router_id:
             try:
                 self._remove_router_interface(plugin_context, router_id,
-                                              interface_info)
+                                              interface_info,
+                                              clean_session=clean_session)
             except ext_l3.RouterInterfaceNotFoundForSubnet:
                 LOG.debug("Ignoring RouterInterfaceNotFoundForSubnet cleaning "
                           "up subnet: %s", subnet_id)
@@ -627,6 +628,27 @@ class ImplicitResourceOperations(local_api.LocalAPI):
             self._delete_router(plugin_context, router_id,
                                 clean_session=clean_session)
 
+    def _plug_router_to_subnet(self, plugin_context, subnet_id, router_id):
+        interface_info = {'subnet_id': subnet_id}
+        if router_id:
+            try:
+                self._add_router_interface(plugin_context, router_id,
+                                           interface_info)
+            except n_exc.BadRequest as e:
+                LOG.exception(_LE("Adding subnet to router failed, exception:"
+                                  "%s"), e)
+                raise exc.GroupPolicyInternalError()
+
+    def _add_router_interface_for_subnet(self, context, router_id, subnet_id):
+        self._plug_router_to_subnet(
+            context._plugin_context, subnet_id, router_id)
+
+    def _get_l3p_for_l2policy(self, context, l2p_id):
+        l2p = context._plugin.get_l2_policy(context._plugin_context, l2p_id)
+        l3p_id = l2p['l3_policy_id']
+        l3p = context._plugin.get_l3_policy(context._plugin_context, l3p_id)
+        return l3p
+
 
 class ResourceMappingDriver(api.PolicyDriver, ImplicitResourceOperations,
                             nsp_manager.NetworkServicePolicyMappingMixin,
@@ -1825,12 +1847,6 @@ class ResourceMappingDriver(api.PolicyDriver, ImplicitResourceOperations,
     def update_network_service_policy_precommit(self, context):
         self._validate_nsp_parameters(context)
 
-    def _get_l3p_for_l2policy(self, context, l2p_id):
-        l2p = context._plugin.get_l2_policy(context._plugin_context, l2p_id)
-        l3p_id = l2p['l3_policy_id']
-        l3p = context._plugin.get_l3_policy(context._plugin_context, l3p_id)
-        return l3p
-
     def _plug_router_to_external_segment(self, context, es_dict):
         es_list = context._plugin.get_external_segments(
             context._plugin_context, filters={'id': es_dict.keys()})
@@ -1895,16 +1911,6 @@ class ResourceMappingDriver(api.PolicyDriver, ImplicitResourceOperations,
         self._mark_subnet_owned(context._plugin_context.session, subnet['id'])
         return subnet
 
-    def _plug_router_to_subnet(self, plugin_context, subnet_id, router_id):
-        interface_info = {'subnet_id': subnet_id}
-        if router_id:
-            try:
-                self._add_router_interface(plugin_context, router_id,
-                                           interface_info)
-            except n_exc.BadRequest:
-                LOG.exception(_LE("Adding subnet to router failed"))
-                raise exc.GroupPolicyInternalError()
-
     def _stitch_ptg_to_l3p(self, context, ptg, l3p, subnet_ids):
         if l3p['routers']:
             router_id = l3p['routers'][0]

gbpservice/neutron/tests/unit/db/grouppolicy/test_group_policy_mapping_db.py

@@ -42,14 +42,15 @@ SC_PLUGIN_KLASS = (
 class GroupPolicyMappingDbTestCase(tgpdb.GroupPolicyDbTestCase,
                                    test_l3.L3NatTestCaseMixin):
 
-    def setUp(self, core_plugin=None, gp_plugin=None, service_plugins=None,
+    def setUp(self, core_plugin=None, l3_plugin=None, gp_plugin=None,
-              sc_plugin=None):
+              service_plugins=None, sc_plugin=None):
         if not gp_plugin:
             gp_plugin = DB_GP_PLUGIN_KLASS
         if not service_plugins:
             service_plugins = {
-                'l3_plugin_name': "router", 'gp_plugin_name': gp_plugin,
+                'gp_plugin_name': gp_plugin,
                 'servicechain_plugin': sc_plugin or SC_PLUGIN_KLASS}
+        service_plugins['l3_plugin_name'] = l3_plugin or "router"
         super(GroupPolicyMappingDbTestCase, self).setUp(
             core_plugin=core_plugin, gp_plugin=gp_plugin,
             service_plugins=service_plugins

gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_mapping_driver.py

@@ -60,8 +60,10 @@ class AIMBaseTestCase(test_nr_base.CommonNeutronBaseTestCase,
     _extension_path = None
 
     def setUp(self, policy_drivers=None, core_plugin=None, ml2_options=None,
-              sc_plugin=None, **kwargs):
+              l3_plugin=None, sc_plugin=None, **kwargs):
         core_plugin = core_plugin or ML2PLUS_PLUGIN
+        if not l3_plugin:
+            l3_plugin = "apic_aim_l3"
         # The dummy driver configured here is meant to be the second driver
         # invoked and helps in rollback testing. We mock the dummy driver
         # methods to raise an exception and validate that DB operations
@@ -74,7 +76,8 @@ class AIMBaseTestCase(test_nr_base.CommonNeutronBaseTestCase,
                                    'tenant_network_types': ['opflex']}
         super(AIMBaseTestCase, self).setUp(
             policy_drivers=policy_drivers, core_plugin=core_plugin,
-            ml2_options=ml2_opts, sc_plugin=sc_plugin)
+            ml2_options=ml2_opts, l3_plugin=l3_plugin,
+            sc_plugin=sc_plugin)
         config.cfg.CONF.set_override('network_vlan_ranges',
                                      ['physnet1:1000:1099'],
                                      group='ml2_type_vlan')
@@ -556,6 +559,23 @@ class TestPolicyTargetGroup(AIMBaseTestCase):
                                implicit_contract_name],
                               aim_epg.consumed_contract_names)
 
+    def _validate_router_interface_created(self):
+        # check port is created on default router
+        ports = self._plugin.get_ports(self._context)
+        self.assertEqual(1, len(ports))
+        router_port = ports[0]
+        self.assertEqual('network:router_interface',
+                         router_port['device_owner'])
+        routers = self._l3_plugin.get_routers(self._context)
+        self.assertEqual(1, len(routers))
+        self.assertEqual(routers[0]['id'],
+                         router_port['device_id'])
+        subnets = self._plugin.get_subnets(self._context)
+        self.assertEqual(1, len(subnets))
+        self.assertEqual(1, len(router_port['fixed_ips']))
+        self.assertEqual(subnets[0]['id'],
+                         router_port['fixed_ips'][0]['subnet_id'])
+
     def test_policy_target_group_lifecycle_implicit_l2p(self):
         prs_lists = self._get_provided_consumed_prs_lists()
         ptg = self.create_policy_target_group(
@@ -577,6 +597,9 @@ class TestPolicyTargetGroup(AIMBaseTestCase):
         self.assertIsNotNone(subnet['id'])
         self.assertEqual(l3p['subnetpools_v4'][0],
                          subnet['subnetpool_id'])
+
+        self._validate_router_interface_created()
+
         ptg_name = ptg['name']
         aim_epg_name = str(self.name_mapper.policy_target_group(
             self._neutron_context.session, ptg_id, ptg_name))
@@ -625,6 +648,8 @@ class TestPolicyTargetGroup(AIMBaseTestCase):
         req = self.new_show_request('subnets', ptg['subnets'][0], fmt=self.fmt)
         res = req.get_response(self.api)
         self.assertEqual(webob.exc.HTTPNotFound.code, res.status_int)
+        # check router ports are deleted too
+        self.assertEqual([], self._plugin.get_ports(self._context))
         # Implicitly created L2P should be deleted
         self.show_l2_policy(ptg['l2_policy_id'], expected_res_status=404)
 
@@ -646,6 +671,9 @@ class TestPolicyTargetGroup(AIMBaseTestCase):
         req = self.new_show_request('subnets', ptg['subnets'][0], fmt=self.fmt)
         res = self.deserialize(self.fmt, req.get_response(self.api))
         self.assertIsNotNone(res['subnet']['id'])
+
+        self._validate_router_interface_created()
+
         ptg_name = ptg['name']
         aim_epg_name = str(self.name_mapper.policy_target_group(
             self._neutron_context.session, ptg_id, ptg_name))
@@ -700,6 +728,7 @@ class TestPolicyTargetGroup(AIMBaseTestCase):
         req = self.new_show_request('subnets', ptg['subnets'][0], fmt=self.fmt)
         res = self.deserialize(self.fmt, req.get_response(self.api))
         self.assertIsNotNone(res['subnet']['id'])
+        self._validate_router_interface_created()
 
 
 # TODO(Sumit): Add tests here which tests different scenarios for subnet
@@ -720,6 +749,7 @@ class TestPolicyTargetGroupRollback(AIMBaseTestCase):
         self.dummy.create_policy_target_group_precommit = mock.Mock(
             side_effect=Exception)
         self.create_policy_target_group(name="ptg1", expected_res_status=500)
+        self.assertEqual([], self._plugin.get_ports(self._context))
         self.assertEqual([], self._plugin.get_subnets(self._context))
         self.assertEqual([], self._plugin.get_networks(self._context))
         self.assertEqual([], self._gbp_plugin.get_policy_target_groups(
@@ -800,12 +830,14 @@ class TestPolicyTargetRollback(AIMBaseTestCase):
             side_effect=Exception)
         ptg_id = self.create_policy_target_group(
             name="ptg1")['policy_target_group']['id']
+        ports = self._plugin.get_ports(self._context)
         self.create_policy_target(name="pt1",
                                   policy_target_group_id=ptg_id,
                                   expected_res_status=500)
         self.assertEqual([],
                          self._gbp_plugin.get_policy_targets(self._context))
-        self.assertEqual([], self._plugin.get_ports(self._context))
+        new_ports = self._plugin.get_ports(self._context)
+        self.assertItemsEqual(ports, new_ports)
         # restore mock
         self.dummy.create_policy_target_precommit = orig_func
 

gbpservice/neutron/tests/unit/services/grouppolicy/test_extension_driver_api.py

@@ -33,7 +33,7 @@ class ExtensionDriverTestBase(test_plugin.GroupPolicyPluginTestCase):
     _extension_path = os.path.dirname(os.path.abspath(test_ext.__file__))
 
     def setUp(self, policy_drivers=None, core_plugin=None,
-              ml2_options=None, sc_plugin=None):
+              l3_plugin=None, ml2_options=None, sc_plugin=None):
         config.cfg.CONF.set_override('extension_drivers',
                                      self._extension_drivers,
                                      group='group_policy')
@@ -41,8 +41,8 @@ class ExtensionDriverTestBase(test_plugin.GroupPolicyPluginTestCase):
             config.cfg.CONF.set_override(
                 'api_extensions_path', self._extension_path)
         super(ExtensionDriverTestBase, self).setUp(
-            core_plugin=core_plugin, ml2_options=ml2_options,
+            core_plugin=core_plugin, l3_plugin=l3_plugin,
-            sc_plugin=sc_plugin)
+            ml2_options=ml2_options, sc_plugin=sc_plugin)
 
 
 class ExtensionDriverTestCase(ExtensionDriverTestBase):

gbpservice/neutron/tests/unit/services/grouppolicy/test_grouppolicy_plugin.py

@@ -59,8 +59,8 @@ def get_status_for_test(self, context):
 
 class GroupPolicyPluginTestBase(tgpmdb.GroupPolicyMappingDbTestCase):
 
-    def setUp(self, core_plugin=None, gp_plugin=None, ml2_options=None,
+    def setUp(self, core_plugin=None, l3_plugin=None, gp_plugin=None,
-              sc_plugin=None):
+              ml2_options=None, sc_plugin=None):
         if not gp_plugin:
             gp_plugin = GP_PLUGIN_KLASS
         ml2_opts = ml2_options or {'mechanism_drivers': ['openvswitch']}
@@ -68,6 +68,7 @@ class GroupPolicyPluginTestBase(tgpmdb.GroupPolicyMappingDbTestCase):
             cfg.CONF.set_override(opt, val, 'ml2')
         core_plugin = core_plugin or test_plugin.PLUGIN_NAME
         super(GroupPolicyPluginTestBase, self).setUp(core_plugin=core_plugin,
+                                                     l3_plugin=l3_plugin,
                                                      gp_plugin=gp_plugin,
                                                      sc_plugin=sc_plugin)
 

gbpservice/neutron/tests/unit/services/grouppolicy/test_neutron_resources_driver.py

@@ -34,8 +34,8 @@ CORE_PLUGIN = ('gbpservice.neutron.tests.unit.services.grouppolicy.'
 class CommonNeutronBaseTestCase(test_plugin.GroupPolicyPluginTestBase):
 
     def setUp(self, policy_drivers=None,
-              core_plugin=n_test_plugin.PLUGIN_NAME, ml2_options=None,
+              core_plugin=n_test_plugin.PLUGIN_NAME, l3_plugin=None,
-              sc_plugin=None):
+              ml2_options=None, sc_plugin=None):
         policy_drivers = policy_drivers or ['neutron_resources']
         config.cfg.CONF.set_override('policy_drivers',
                                      policy_drivers,
@@ -44,6 +44,7 @@ class CommonNeutronBaseTestCase(test_plugin.GroupPolicyPluginTestBase):
                                      ['dummy'], group='servicechain')
         config.cfg.CONF.set_override('allow_overlapping_ips', True)
         super(CommonNeutronBaseTestCase, self).setUp(core_plugin=core_plugin,
+                                                     l3_plugin=l3_plugin,
                                                      ml2_options=ml2_options,
                                                      sc_plugin=sc_plugin)
         engine = db_api.get_engine()