US2853 fix: switch clique creation

- bug fix: switch-switch_pnic link was created only for uplink
- clique finder: handle switch_pnic-switch_pnic links: search both
  directions

Change-Id: I7b8b4c790212c6f527112cce6d52e40fb615ec52
Signed-off-by: yayogev <yaronyogev@gmail.com>

diff --git a/app/discover/clique_finder.py b/app/discover/clique_finder.py
index 4c4e636..2d845b7 100644 (file)
--- a/app/discover/clique_finder.py
+++ b/app/discover/clique_finder.py
@@ -14,6 +14,9 @@ from utils.inventory_mgr import InventoryMgr
 
 
 class CliqueFinder(Fetcher):
+
+    link_type_reversed = {}
+
     def __init__(self):
         super().__init__()
         self.inv = InventoryMgr()
@@ -42,7 +45,8 @@ class CliqueFinder(Fetcher):
 
     def get_clique_types(self):
         if not self.clique_types_by_type:
-            clique_types = self.clique_types.find({"environment": self.get_env()})
+            clique_types = self.clique_types \
+                .find({"environment": self.get_env()})
             default_clique_types = \
                 self.clique_types.find({'environment': 'ANY'})
             for clique_type in clique_types:
@@ -58,8 +62,9 @@ class CliqueFinder(Fetcher):
             return self.clique_types_by_type
 
     def find_cliques_for_type(self, clique_type):
-        type = clique_type["focal_point_type"]
-        constraint = self.clique_constraints.find_one({"focal_point_type": type})
+        focal_point_type = clique_type["focal_point_type"]
+        constraint = self.clique_constraints \
+            .find_one({"focal_point_type": focal_point_type})
         constraints = [] if not constraint else constraint["constraints"]
         object_type = clique_type["focal_point_type"]
         objects_for_focal_point_type = self.inventory.find({
@@ -71,19 +76,21 @@ class CliqueFinder(Fetcher):
 
     def rebuild_clique(self, clique):
         focal_point_db_id = clique['focal_point']
-        constraint = self.clique_constraints.find_one({"focal_point_type": type})
+        o = self.inventory.find_one({'_id': focal_point_db_id})
+        constraint = self.clique_constraints \
+            .find_one({"focal_point_type": o['type']})
         constraints = [] if not constraint else constraint["constraints"]
         clique_types = self.get_clique_types()
-        o = self.inventory.find_one({'_id': focal_point_db_id})
         clique_type = clique_types[o['type']]
-        new_clique = self.construct_clique_for_focal_point(o, clique_type, constraints)
+        new_clique = self.construct_clique_for_focal_point(o, clique_type,
+                                                           constraints)
         if not new_clique:
             self.cliques.delete({'_id': clique['_id']})
 
     def construct_clique_for_focal_point(self, o, clique_type, constraints):
         # keep a hash of nodes in clique that were visited for each type
         # start from the focal point
-        nodes_of_type = {o["type"]: {str(o["_id"]): 1}}
+        nodes_of_type = {o["type"]: {str(o["_id"])}}
         clique = {
             "environment": self.env,
             "focal_point": o["_id"],
@@ -96,49 +103,7 @@ class CliqueFinder(Fetcher):
             val = o[c] if c in o else None
             clique["constraints"][c] = val
         for link_type in clique_type["link_types"]:
-            # check if it's backwards
-            link_type_parts = link_type.split('-')
-            link_type_parts.reverse()
-            link_type_reversed = '-'.join(link_type_parts)
-            self_linked = link_type == link_type_reversed
-            if self_linked:
-                reversed = False
-            else:
-                matches = self.links.find_one({
-                    "environment": self.env,
-                    "link_type": link_type_reversed
-                })
-                reversed = True if matches else False
-            if reversed:
-                link_type = link_type_reversed
-            from_type = link_type[:link_type.index("-")]
-            to_type = link_type[link_type.index("-") + 1:]
-            side_to_match = 'target' if reversed else 'source'
-            other_side = 'target' if not reversed else 'source'
-            match_type = to_type if reversed else from_type
-            if match_type not in nodes_of_type.keys():
-                continue
-            other_side_type = to_type if not reversed else from_type
-            nodes_to_add = {}
-            for match_point in nodes_of_type[match_type].keys():
-                matches = self.links.find({
-                    "environment": self.env,
-                    "link_type": link_type,
-                    side_to_match: ObjectId(match_point)
-                })
-                for link in matches:
-                    id = link["_id"]
-                    if id in clique["links"]:
-                        continue
-                    if not self.check_constraints(clique, link):
-                        continue
-                    clique["links"].append(id)
-                    clique["links_detailed"].append(link)
-                    other_side_point = str(link[other_side])
-                    nodes_to_add[other_side_point] = 1
-            if other_side_type not in nodes_of_type:
-                nodes_of_type[other_side_type] = {}
-            nodes_of_type[other_side_type].update(nodes_to_add)
+            self.check_link_type(clique, link_type, nodes_of_type)
 
         # after adding the links to the clique, create/update the clique
         if not clique["links"]:
@@ -162,7 +127,8 @@ class CliqueFinder(Fetcher):
             upsert=True)
         return clique_document
 
-    def check_constraints(self, clique, link):
+    @staticmethod
+    def check_constraints(clique, link):
         if "attributes" not in link:
             return True
         attributes = link["attributes"]
@@ -178,3 +144,82 @@ class CliqueFinder(Fetcher):
             elif link_val != constraints[c]:
                 return False
         return True
+
+    @staticmethod
+    def get_link_type_reversed(link_type: str) -> str:
+        if not CliqueFinder.link_type_reversed.get(link_type):
+            link_type_parts = link_type.split('-')
+            link_type_parts.reverse()
+            CliqueFinder.link_type_reversed[link_type] = \
+                '-'.join(link_type_parts)
+        return CliqueFinder.link_type_reversed.get(link_type)
+
+    def check_link_type(self, clique, link_type, nodes_of_type):
+        # check if it's backwards
+        link_type_reversed = self.get_link_type_reversed(link_type)
+        # handle case of links like T<-->T
+        self_linked = link_type == link_type_reversed
+        use_reversed = False
+        if not self_linked:
+            matches = self.links.find_one({
+                "environment": self.env,
+                "link_type": link_type_reversed
+            })
+            use_reversed = True if matches else False
+        if self_linked or not use_reversed:
+            self.check_link_type_forward(clique, link_type, nodes_of_type)
+        if self_linked or use_reversed:
+            self.check_link_type_back(clique, link_type, nodes_of_type)
+
+    def check_link_type_for_direction(self, clique, link_type, nodes_of_type,
+                                      is_reversed=False):
+        if is_reversed:
+            link_type = self.get_link_type_reversed(link_type)
+        from_type = link_type[:link_type.index("-")]
+        to_type = link_type[link_type.index("-") + 1:]
+        side_to_match = 'target' if is_reversed else 'source'
+        other_side = 'target' if not is_reversed else 'source'
+        match_type = to_type if is_reversed else from_type
+        if match_type not in nodes_of_type.keys():
+            return
+        other_side_type = to_type if not is_reversed else from_type
+        nodes_to_add = set()
+        for match_point in nodes_of_type[match_type]:
+            matches = self.find_matches_for_point(match_point,
+                                                  clique,
+                                                  link_type,
+                                                  side_to_match,
+                                                  other_side)
+            nodes_to_add = nodes_to_add | matches
+        if other_side_type not in nodes_of_type:
+            nodes_of_type[other_side_type] = set()
+        nodes_of_type[other_side_type] = \
+            nodes_of_type[other_side_type] | nodes_to_add
+
+    def find_matches_for_point(self, match_point, clique, link_type,
+                               side_to_match, other_side) -> set:
+        nodes_to_add = set()
+        matches = self.links.find({
+            "environment": self.env,
+            "link_type": link_type,
+            side_to_match: ObjectId(match_point)
+        })
+        for link in matches:
+            link_id = link["_id"]
+            if link_id in clique["links"]:
+                continue
+            if not self.check_constraints(clique, link):
+                continue
+            clique["links"].append(link_id)
+            clique["links_detailed"].append(link)
+            other_side_point = str(link[other_side])
+            nodes_to_add.add(other_side_point)
+        return nodes_to_add
+
+    def check_link_type_forward(self, clique, link_type, nodes_of_type):
+        self.check_link_type_for_direction(clique, link_type, nodes_of_type,
+                                           is_reversed=False)
+
+    def check_link_type_back(self, clique, link_type, nodes_of_type):
+        self.check_link_type_for_direction(clique, link_type, nodes_of_type,
+                                           is_reversed=True)

diff --git a/app/discover/find_links_for_pnics.py b/app/discover/find_links_for_pnics.py
index 372ff39..3f3eb29 100644 (file)
--- a/app/discover/find_links_for_pnics.py
+++ b/app/discover/find_links_for_pnics.py
@@ -33,11 +33,11 @@ class FindLinksForPnics(FindLinks):
         pnics = self.inv.find_items({
             "environment": self.get_env(),
             "type": "switch_pnic",
-            "role": "uplink"
         })
         for pnic in pnics:
-            self.add_switch_pnic_to_switch_pnic_link(pnic)
             self.add_switch_to_pnic_link(pnic)
+            if "role" == "uplink":
+                self.add_switch_pnic_to_switch_pnic_link(pnic)
 
     def add_pnic_network_links(self, pnic):
         host = pnic["host"]