ST: Configure Service Instances routes in primary RI of left vn/SC

This enhancement will eliminate the case of duplicate routes getting populated in routers.
This fix will also remove any existing service ris  already created by the old software.

Change-Id: I86bd6de851e76c0605395c55c82b15de64108fc7
Closes-Bug: #1554175

src/config/common/tests/test_common.py

@@ -467,7 +467,7 @@ def create_network_policy(self, vn1, vn2, service_list=None, service_mode=None):
                     service_template_properties=st_prop)
                 self._vnc_lib.service_template_create(service_template)
                 scale_out = ServiceScaleOutType()
-                if service_mode == 'in-network':
+                if service_mode in ['in-network', 'in-network-nat']:
                     if_list = [ServiceInstanceInterfaceType(virtual_network=vn1.get_fq_name_str()),
                                ServiceInstanceInterfaceType(virtual_network=vn2.get_fq_name_str())]
                     si_props = ServiceInstanceType(

src/config/schema-transformer/test/test_service.py

@@ -446,7 +446,7 @@ def test_add_delete_route(self):
         rvn = self.create_virtual_network(rvn_name, "20.0.0.0/24")
 
         service_name = self.id() + 's1'
-        np = self.create_network_policy(lvn, rvn, [service_name], "in-network")
+        np = self.create_network_policy(lvn, rvn, [service_name], service_mode="in-network-nat")
 
         vn_name = self.id() + 'vn100'
         vn = self.create_virtual_network(vn_name, "1.0.0.0/24")
@@ -466,14 +466,8 @@ def test_add_delete_route(self):
 
         @retries(5, hook=retry_exc_handler)
         def _match_route_table(rtgt_list):
-            sc = [x for x in to_bgp.ServiceChain]
-            if len(sc) == 0:
-                raise Exception("sc has 0 len")
-
-            sc_ri_name = ('service-'+sc[0] +
-                          '-default-domain_default-project_' + service_name)
             lri = self._vnc_lib.routing_instance_read(
-                fq_name=self.get_ri_name(lvn, sc_ri_name))
+                fq_name=self.get_ri_name(lvn))
             sr = lri.get_static_route_entries()
             if sr is None:
                 raise Exception("sr is None")
@@ -484,13 +478,13 @@ def _match_route_table(rtgt_list):
                 self.assertIn(rtgt, route.route_target)
             ri100 = self._vnc_lib.routing_instance_read(
                 fq_name=self.get_ri_name(vn))
-            rt100 = ri100.get_route_target_refs()[0]['to']
-            for rt_ref in lri.get_route_target_refs() or []:
-                if rt100 == rt_ref['to']:
-                    return sc_ri_name, rt100
+            rt100 = set(ref['to'][0] for ref in ri100.get_route_target_refs())
+            lrt = set(ref['to'][0] for ref in lri.get_route_target_refs() or [])
+            if rt100 & lrt:
+                return (rt100 & lrt)
             raise Exception("rt100 route-target ref not found")
 
-        sc_ri_name, rt100 = _match_route_table(rtgt_list.get_route_target())
+        rt100 = _match_route_table(rtgt_list.get_route_target())
 
         rtgt_list.add_route_target('target:1:2')
         vn.set_route_target_list(rtgt_list)
@@ -507,9 +501,9 @@ def _match_route_table(rtgt_list):
         self._vnc_lib.route_table_update(rt)
 
         @retries(5, hook=retry_exc_handler)
-        def _match_route_table_cleanup(sc_ri_name, rt100):
+        def _match_route_table_cleanup(rt100):
             lri = self._vnc_lib.routing_instance_read(
-                fq_name=self.get_ri_name(lvn, sc_ri_name))
+                fq_name=self.get_ri_name(lvn))
             sr = lri.get_static_route_entries()
             if sr and sr.route:
                 raise Exception("sr has route")
@@ -520,16 +514,18 @@ def _match_route_table_cleanup(sc_ri_name, rt100):
                 if rt100 == rt_ref['to']:
                     raise Exception("rt100 route-target ref found")
 
-        _match_route_table_cleanup(sc_ri_name, rt100)
+        _match_route_table_cleanup(rt100)
 
         # add the route again, then delete the network without deleting the
         # link to route table
+        route = RouteType(prefix="0.0.0.0/0",
+                          next_hop="default-domain:default-project:"+service_name)
         routes.add_route(route)
         rt.set_routes(routes)
         self._vnc_lib.route_table_update(rt)
         _match_route_table(rtgt_list.get_route_target())
         self._vnc_lib.virtual_network_delete(fq_name=vn.get_fq_name())
-        _match_route_table_cleanup(sc_ri_name, rt100)
+        _match_route_table_cleanup(rt100)
 
         self._vnc_lib.route_table_delete(fq_name=rt.get_fq_name())
         self.delete_network_policy(np, auto_policy=True)

src/config/schema-transformer/to_bgp.py

@@ -262,7 +262,6 @@ def __init__(self, name, obj=None, acl_dict=None, ri_dict=None):
         self._route_target = 0
         self.route_table_refs = set()
         self.route_table = {}
-        self.service_chains = {}
         prop = self.obj.get_virtual_network_properties(
         ) or VirtualNetworkType()
         self.allow_transit = prop.allow_transit
@@ -299,6 +298,7 @@ def __init__(self, name, obj=None, acl_dict=None, ri_dict=None):
         for policy in NetworkPolicyST.values():
             if policy.internal and name in policy.network_back_ref:
                 self.add_policy(policy.name)
+        self.service_chains = ServiceChain.find_service_chains(self.name)
         self.uve_send()
     # end __init__
 
@@ -376,7 +376,7 @@ def update_autonomous_system(cls, new_asn):
             ri.obj.add_route_target(new_rtgt_obj.obj, inst_tgt_data)
             _vnc_lib.routing_instance_update(ri.obj)
             for (prefix, nexthop) in vn.route_table.items():
-                left_ri = vn._get_routing_instance_from_route(nexthop)
+                (left_ri, _) = self._get_routing_instance_from_route(nexthop)
                 if left_ri is None:
                     continue
                 left_ri.update_route_target_list(
@@ -749,7 +749,7 @@ def set_route_target_list(self, rt_list):
                 ri_obj.update_route_target_list(rt_add, rt_del,
                                                 import_export='export')
         for (prefix, nexthop) in self.route_table.items():
-            left_ri = self._get_routing_instance_from_route(nexthop)
+            (left_ri, _) = self._get_routing_instance_from_route(nexthop)
             if left_ri is None:
                 continue
             left_ri.update_route_target_list(rt_add, rt_del,
@@ -785,7 +785,10 @@ def set_route_target_list(self, rt_list):
     # next-hop in a route contains fq-name of a service instance, which must
     # be an auto policy instance. This function will get the left vn for that
     # service instance and get the primary and service routing instances
+
     def _get_routing_instance_from_route(self, next_hop):
+        si = None
+        si_props = None
         try:
             si = _vnc_lib.service_instance_read(fq_name_str=next_hop)
             si_props = si.get_service_instance_properties()
@@ -794,44 +797,32 @@ def _get_routing_instance_from_route(self, next_hop):
         except NoIdError:
             _sandesh._logger.error("Cannot read service instance %s", next_hop)
             return None
-        if not si_props.auto_policy:
-            _sandesh._logger.error("%s: route table next hop must be service "
-                                   "instance with auto policy", self.name)
-            return None
-        left_vn_str, right_vn_str = get_si_vns(si, si_props)
-        if (not left_vn_str or not right_vn_str):
+        left_vn_str, _ = get_si_vns(si, si_props)
+        if not left_vn_str:
             _sandesh._logger.error("%s: route table next hop service instance "
-                                   "must have left and right virtual networks",
-                                   self.name)
-            return None
+                               "must have left virtual network", self.name)
+            return (None, None)
         left_vn = VirtualNetworkST.get(left_vn_str)
         if left_vn is None:
             _sandesh._logger.error("Virtual network %s not present",
-                                   left_vn_str)
-            return None
-        sc = ServiceChain.find(left_vn_str, right_vn_str, '<>',
-                               [PortType(0, -1)], [PortType(0, -1)], 'any')
-        if sc is None:
-            _sandesh._logger.error("Service chain between %s and %s not "
-                                   "present", left_vn_str, right_vn_str)
-            return None
-        left_ri_name = left_vn.get_service_name(sc.name, next_hop)
-        return left_vn.rinst.get(left_ri_name)
+                               left_vn_str)
+            return (None, None)
+
+        sc = ServiceChain(None, None, None, None, None, None, None, [si.get_fq_name_str()])
+        ret = sc.check_create() or {}
+        try:
+            left_ip = ret[si.get_fq_name_str()]['vm_list'][0]['left']['address']
+        except (KeyError, IndexError):
+            left_ip = None
+        return (left_vn.get_primary_routing_instance(), left_ip)
     # end _get_routing_instance_from_route
 
     def add_route(self, prefix, next_hop):
-        self.route_table[prefix] = next_hop
-        left_ri = self._get_routing_instance_from_route(next_hop)
-        if left_ri is None:
+        (left_ri, sc_address) = self._get_routing_instance_from_route(next_hop)
+        if left_ri is None or sc_address is None:
             _sandesh._logger.error(
-                "left routing instance is none for %s", next_hop)
+                "left routing instance or sc_address is none for %s", next_hop)
             return
-        service_info = left_ri.obj.get_service_chain_information()
-        if service_info is None:
-            _sandesh._logger.error(
-                "Service chain info not found for %s", left_ri.name)
-            return
-        sc_address = service_info.get_service_chain_address()
         static_route_entries = left_ri.obj.get_static_route_entries(
         ) or StaticRouteEntriesType()
         update = False
@@ -854,14 +845,15 @@ def add_route(self, prefix, next_hop):
         left_ri.update_route_target_list(
             rt_add=self.rt_list | set([self.get_route_target()]),
             import_export="import")
+        self.route_table[prefix] = next_hop
     # end add_route
 
     def delete_route(self, prefix):
         if prefix not in self.route_table:
             return
         next_hop = self.route_table[prefix]
         del self.route_table[prefix]
-        left_ri = self._get_routing_instance_from_route(next_hop)
+        (left_ri, _) = self._get_routing_instance_from_route(next_hop)
         if left_ri is None:
             return
         left_ri.update_route_target_list(rt_add=set(),
@@ -1251,7 +1243,7 @@ def add_rules(self, entries):
 
 class RouteTableST(DictST):
     _dict = {}
-
+    _si_dict = {}
     def __init__(self, name):
         self.name = name
         self.obj = RouteTableType(name)
@@ -1264,6 +1256,17 @@ def delete(cls, name):
         if name in cls._dict:
             del cls._dict[name]
     # end delete
+
+    def set_routes(self, routes=[]):
+        rt_list = [route.next_hop for route in routes]
+        for route in self.routes or []:
+            if route.next_hop not in rt_list:
+                self._si_dict.discard(route.next_hop)
+        self.routes = routes
+        for route in self.routes or []:
+            self._si_dict[route.next_hop] = self
+    #end set_routes
+
 # end RouteTableST
 
 # a struct to store attributes related to Security Group needed by schema
@@ -1659,6 +1662,20 @@ def __eq__(self, other):
         return True
     # end __eq__
 
+    @classmethod
+    def find_service_chains(cls, vn):
+        sc_map = {}
+        for sc in ServiceChain.values():
+            remote_vn = None
+            if (vn == sc.left_vn):
+                remote_vn = sc.right_vn
+            if (vn == sc.right_vn):
+                remote_vn = sc.left_vn
+            if remote_vn and vn != remote_vn:
+                sc_map.setdefault(remote_vn, []).append(sc)
+        return sc_map
+    # end find_service_chains
+
     @classmethod
     def find(cls, left_vn, right_vn, direction, sp_list, dp_list, protocol):
         for sc in ServiceChain.values():
@@ -3083,9 +3100,17 @@ def add_instance_ip_virtual_machine_interface(self, idents, meta):
         vmi_name = idents['virtual-machine-interface']
         ip_name = idents['instance-ip']
         vmi = VirtualMachineInterfaceST.locate(vmi_name)
-        if vmi is not None:
-            vmi.add_instance_ip(ip_name)
-            self.current_network_set |= vmi.rebake()
+        if vmi is None:
+            return
+        vmi.add_instance_ip(ip_name)
+        self.current_network_set |= vmi.rebake()
+        vm_obj = VirtualMachineST.get(vmi.virtual_machine)
+        if not vm_obj or not vm_obj.service_instance:
+            return
+        si_name = vm_obj.service_instance
+        rt = RouteTableST._si_dict.get(si_name)
+        if rt:
+            self.current_network_set |= rt.network_back_refs
     # end add_instance_ip_virtual_machine_interface
 
     def delete_instance_ip_virtual_machine_interface(self, idents, meta):
@@ -3156,6 +3181,8 @@ def add_virtual_machine_interface_virtual_machine(self, idents, meta):
         vm = VirtualMachineST.get(vm_name)
         if vm is not None:
             vm.add_interface(vmi_name)
+        if vmi is not None:
+            self.current_network_set |= vmi.rebake()
     # end add_virtual_machine_interface_virtual_machine
 
     def add_virtual_machine_virtual_machine_interface(self, idents, meta):
@@ -3186,6 +3213,9 @@ def add_virtual_machine_service_instance(self, idents, meta):
                     self.current_network_set.add(sc.left_vn)
                 if VirtualNetworkST.get(sc.right_vn):
                     self.current_network_set.add(sc.right_vn)
+        rt = RouteTableST._si_dict.get(si_name)
+        if rt:
+            self.current_network_set |= rt.network_back_refs
     # end add_virtual_machine_service_instance(self, idents, meta):
 
     def delete_virtual_machine_service_instance(self, idents, meta):
@@ -3236,24 +3266,29 @@ def add_service_instance_properties(self, idents, meta):
         si_name = idents['service-instance']
         si_props = ServiceInstanceType()
         si_props.build(meta)
-        if not si_props.auto_policy:
-            self.delete_service_instance_properties(idents, meta)
-            return
         try:
             si = _vnc_lib.service_instance_read(fq_name_str=si_name)
+            siprops = si.get_service_instance_properties()
+            left_vn_str, right_vn_str = get_si_vns(si, siprops)
+            if (not left_vn_str or not right_vn_str):
+                _sandesh._logger.error(
+                   "%s: route table next hop service instance must "
+                     "have left network", si_name)
+                self.delete_service_instance_properties(idents, meta)
+                return
+            vn1 = VirtualNetworkST.get(left_vn_str)
+            if vn1:
+                self.current_network_set.add(left_vn_str)
+            vn2 = VirtualNetworkST.get(right_vn_str)
+            if vn2:
+                self.current_network_set.add(right_vn_str)
+            if not siprops.auto_policy:
+                self.delete_service_instance_properties(idents, meta)
+                return
         except NoIdError:
             _sandesh._logger.error("NoIdError while reading service "
                                    "instance %s", si_name)
             return
-        si_props = si.get_service_instance_properties()
-        left_vn_str, right_vn_str = get_si_vns(si, si_props)
-        if (not left_vn_str or not right_vn_str):
-            _sandesh._logger.error(
-                "%s: route table next hop service instance must "
-                "have left and right virtual networks", si_name)
-            self.delete_service_instance_properties(idents, meta)
-            return
-
         policy_name = "_internal_" + si_name
         policy = NetworkPolicyST.locate(policy_name)
         addr1 = AddressType(virtual_network=left_vn_str)
@@ -3270,14 +3305,10 @@ def add_service_instance_properties(self, idents, meta):
         policy.network_back_ref = set([left_vn_str, right_vn_str])
         policy.internal = True
         policy.add_rules(pentry)
-        vn1 = VirtualNetworkST.get(left_vn_str)
         if vn1:
             vn1.add_policy(policy_name)
-            self.current_network_set.add(left_vn_str)
-        vn2 = VirtualNetworkST.get(right_vn_str)
         if vn2:
             vn2.add_policy(policy_name)
-            self.current_network_set.add(right_vn_str)
     # end add_service_instance_properties
 
     def delete_service_instance_properties(self, idents, meta):
@@ -3326,15 +3357,15 @@ def add_routes(self, idents, meta):
         if route_table:
             routes = RouteTableType()
             routes.build(meta)
-            route_table.routes = routes.get_route() or []
+            route_table.set_routes(routes.get_route() or [])
             self.current_network_set |= route_table.network_back_refs
     # end add_routes
 
     def delete_routes(self, idents, meta):
         route_table_name = idents['route-table']
         route_table = RouteTableST.get(route_table_name)
         if route_table:
-            route_table.routes = []
+            route_table.set_routes([])
             self.current_network_set |= route_table.network_back_refs
     # end delete_routes
 

src/config/vnc_openstack/vnc_openstack/neutron_plugin_db.py

@@ -2985,7 +2985,7 @@ def _router_set_external_gateway(self, router_obj, ext_net_obj):
         si_prop_obj = ServiceInstanceType(
             scale_out=ServiceScaleOutType(max_instances=2,
                                           auto_scale=True),
-            auto_policy=True)
+            auto_policy=False)
 
         # set right interface in order of [right, left] to match template
         left_if = ServiceInstanceInterfaceType()