Fix undefined variable and warning message (#134)

* Fix errors introduced.
Add namespace test case for fdb related MIB.
* Make sure that db connections are done in MIB class init
functions. Multiple connect is causing an increase in run
time causing agentx connection to close.
* Minor fix
* Add mock asic_db.
* Remove trailing whitespace.
* Revert "Make sure that db connections are done in MIB class init"
* Fix to make sure pubsub channel in LLDPRemManAddrUpdater is
not created everytime in update_data but is created only if
the channel was not created previously.
* Fix as per review comments.

src/sonic_ax_impl/mibs/__init__.py

@@ -150,7 +150,7 @@ def init_db():
     """
     # SyncD database connector. THIS MUST BE INITIALIZED ON A PER-THREAD BASIS.
     # Redis PubSub objects (such as those within swsssdk) are NOT thread-safe.
-    db_conn = SonicV2Connector(**redis_kwargs) 
+    db_conn = SonicV2Connector(**redis_kwargs)
 
     return db_conn
 
@@ -389,6 +389,13 @@ def get_transceiver_sensor_sub_id(ifindex, sensor):
     transceiver_oid, = get_transceiver_sub_id(ifindex)
     return (transceiver_oid + SENSOR_PART_ID_MAP[sensor], )
 
+def get_redis_pubsub(db_conn, db_name, pattern):
+    redis_client = db_conn.get_redis_client(db_name)
+    db = db_conn.get_dbid(db_name)
+    pubsub = redis_client.pubsub()
+    pubsub.psubscribe("__keyspace@{}__:{}".format(db, pattern))
+    return pubsub
+
 class RedisOidTreeUpdater(MIBUpdater):
     def __init__(self, prefix_str):
         super().__init__()
@@ -419,7 +426,7 @@ def update_data(self):
         self.oid_list = []
         self.oid_map = {}
 
-        keys = Namespace.dbs_keys(self.db_conn, SNMP_OVERLAY_DB, self.prefix_str + '*') 
+        keys = Namespace.dbs_keys(self.db_conn, SNMP_OVERLAY_DB, self.prefix_str + '*')
         # TODO: fix db_conn.keys to return empty list instead of None if there is no match
         if keys is None:
             keys = []
@@ -428,7 +435,7 @@ def update_data(self):
             key = key.decode()
             oid = oid2tuple(key, dot_prefix=False)
             self.oid_list.append(oid)
-            value = Namespace.dbs_get_all(self.db_conn, SNMP_OVERLAY_DB, key) 
+            value = Namespace.dbs_get_all(self.db_conn, SNMP_OVERLAY_DB, key)
             if value[b'type'] in [b'COUNTER_32', b'COUNTER_64']:
                 self.oid_map[oid] = int(value[b'data'])
             else:
@@ -587,6 +594,6 @@ def dbs_get_bridge_port_map(dbs, db_name):
     def dbs_get_vlan_id_from_bvid(dbs, bvid):
         for db_conn in Namespace.get_non_host_dbs(dbs):
             db_conn.connect('ASIC_DB')
-            vlan_obj = db.keys('ASIC_DB', "ASIC_STATE:SAI_OBJECT_TYPE_VLAN:" + bvid)
+            vlan_obj = db_conn.keys('ASIC_DB', "ASIC_STATE:SAI_OBJECT_TYPE_VLAN:" + bvid)
             if vlan_obj is not None:
                 return port_util.get_vlan_id_from_bvid(db_conn, bvid)

src/sonic_ax_impl/mibs/ieee802_1ab.py

@@ -71,7 +71,6 @@ class ManAddrConst:
     """
     man_addr_oid = (1, 3, 6, 1, 2, 1, 2, 2, 1, 1)
 
-
 def poll_lldp_entry_updates(pubsub):
     ret = None, None, None
     msg = pubsub.get_message()
@@ -157,7 +156,7 @@ def __init__(self):
         # cache of port data
         # { if_name -> { 'key': 'value' } }
         self.loc_port_data = {}
-        self.pubsub = [None] * len(self.db_conn) 
+        self.pubsub = [None] * len(self.db_conn)
 
     def reinit_data(self):
         """
@@ -222,16 +221,10 @@ def get_next(self, sub_id):
             return None
         return self.if_range[right]
 
-    def _update_per_namespace_data(self, db_conn, pubsub):
+    def _update_per_namespace_data(self, pubsub):
         """
         Listen to updates in APP DB, update local cache
         """
-        if not pubsub:
-            redis_client = db_conn.get_redis_client(db_conn.APPL_DB)
-            db = db_conn.get_dbid(db_conn.APPL_DB)
-            pubsub = redis_client.pubsub()
-            pubsub.psubscribe("__keyspace@{}__:{}".format(db, mibs.lldp_entry_table(b'*')))
-
         while True:
             data, interface, if_id = poll_lldp_entry_updates(pubsub)
 
@@ -243,7 +236,10 @@ def _update_per_namespace_data(self, db_conn, pubsub):
 
     def update_data(self):
         for i in range(len(self.db_conn)):
-            self._update_per_namespace_data(self.db_conn[i], self.pubsub[i])
+            if not self.pubsub[i]:
+                pattern = mibs.lldp_entry_table(b'*')
+                self.pubsub[i] = mibs.get_redis_pubsub(self.db_conn[i], self.db_conn[i].APPL_DB, pattern)
+            self._update_per_namespace_data(self.pubsub[i])
 
     def local_port_num(self, sub_id):
         if len(sub_id) == 0:
@@ -307,7 +303,7 @@ def reinit_data(self):
         self.mgmt_ip_str = None
 
         # establish connection to application database.
-        self.db_conn.connect(mibs.APPL_DB) 
+        self.db_conn.connect(mibs.APPL_DB)
         mgmt_ip_bytes = self.db_conn.get(mibs.APPL_DB, mibs.LOC_CHASSIS_TABLE, b'lldp_loc_man_addr')
 
         if not mgmt_ip_bytes:
@@ -488,7 +484,7 @@ class LLDPRemManAddrUpdater(MIBUpdater):
     def __init__(self):
         super().__init__()
 
-        self.db_conn = Namespace.init_namespace_dbs() 
+        self.db_conn = Namespace.init_namespace_dbs()
         # establish connection to application database.
         Namespace.connect_all_dbs(self.db_conn, mibs.APPL_DB)
         self.if_range = []
@@ -536,16 +532,10 @@ def update_rem_if_mgmt(self, if_oid, if_name):
             return
         self.if_range.sort()
 
-    def _update_per_namespace_data(self, db_conn, pubsub):
+    def _update_per_namespace_data(self, pubsub):
         """
         Listen to updates in APP DB, update local cache
         """
-        if not pubsub:
-            redis_client = db_conn.get_redis_client(db_conn.APPL_DB)
-            db = db_conn.get_dbid(db_conn.APPL_DB)
-            pubsub = redis_client.pubsub()
-            pubsub.psubscribe("__keyspace@{}__:{}".format(db, mibs.lldp_entry_table(b'*')))
-
         while True:
             data, interface, if_index = poll_lldp_entry_updates(pubsub)
 
@@ -561,7 +551,10 @@ def _update_per_namespace_data(self, db_conn, pubsub):
 
     def update_data(self):
         for i in range(len(self.db_conn)):
-             self._update_per_namespace_data(self.db_conn[i], self.pubsub[i])
+            if not self.pubsub[i]:
+                pattern = mibs.lldp_entry_table(b'*')
+                self.pubsub[i] = mibs.get_redis_pubsub(self.db_conn[i], self.db_conn[i].APPL_DB, pattern)
+            self._update_per_namespace_data(self.pubsub[i])
 
 
     def reinit_data(self):

src/sonic_ax_impl/mibs/ietf/rfc2737.py

@@ -181,7 +181,7 @@ def reinit_data(self):
             interface = transceiver_entry.split(mibs.TABLE_NAME_SEPARATOR_VBAR)[-1]
             self._update_transceiver_cache(interface)
 
-    def _update_per_namespace_data(self, statedb, pubsub):
+    def _update_per_namespace_data(self, pubsub):
         """
         Update cache.
         Here we listen to changes in STATE_DB TRANSCEIVER_INFO table
@@ -190,12 +190,6 @@ def _update_per_namespace_data(self, statedb, pubsub):
 
         # This code is not executed in unit test, since mockredis
         # does not support pubsub
-        if not pubsub:
-            redis_client = statedb.get_redis_client(statedb.STATE_DB)
-            db = statedb.get_dbid(statedb.STATE_DB)
-            pubsub = redis_client.pubsub()
-            pubsub.psubscribe("__keyspace@{}__:{}".format(db, self.TRANSCEIVER_KEY_PATTERN))
-
         while True:
             msg = pubsub.get_message()
 
@@ -233,8 +227,13 @@ def _update_per_namespace_data(self, statedb, pubsub):
                         self.physical_entities.remove(sub_id)
 
     def update_data(self):
+        # This code is not executed in unit test, since mockredis
+        # does not support pubsub
         for i in range(len(self.statedb)):
-             self._update_per_namespace_data(self.statedb[i], self.pubsub[i])
+            if not self.pubsub[i]:
+                pattern = self.TRANSCEIVER_KEY_PATTERN
+                self.pubsub[i] = mibs.get_redis_pubsub(self.statedb[i], self.statedb[i].STATE_DB, pattern)
+            self._update_per_namespace_data(self.pubsub[i])
 
     def _update_transceiver_cache(self, interface):
         """

src/sonic_ax_impl/mibs/ietf/rfc3433.py

@@ -250,7 +250,7 @@ def __init__(self):
 
         super().__init__()
 
-        self.statedb = Namespace.init_namespace_dbs() 
+        self.statedb = Namespace.init_namespace_dbs()
         Namespace.connect_all_dbs(self.statedb, mibs.STATE_DB)
 
         # list of available sub OIDs
@@ -378,7 +378,7 @@ def get_ent_physical_sensor_precision(self, sub_id):
         """
         Get sensor precision value based on sub OID
         :param sub_id: sub ID of the sensor
-        :return: sensor precision in range (-8, 9) 
+        :return: sensor precision in range (-8, 9)
                  or None if sub_id not in the cache.
         """
 

src/sonic_ax_impl/mibs/vendor/cisco/ciscoPfcExtMIB.py

@@ -191,7 +191,7 @@ def requests_per_priority(self, sub_id):
         """
         :param sub_id: The 0-based sub-identifier query.
         :return: the counter for the respective sub_id/table.
-        """        
+        """
         port_oid = ''
         queue_index = ''
         try:

tests/mock_tables/asic0/asic_db.json

@@ -1,2 +1,27 @@
 {
+  "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY:{\"bridge_id\":\"oid:0x0\",\"bridge_type\":\"SAI_FDB_ENTRY_BRIDGE_TYPE_1Q\",\"mac\":\"7C:FE:90:80:9F:04\",\"switch_id\":\"oid:0x21000000000000\",\"vlan\":\"1000\"}": {
+    "SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID": "oid:0x3a000000000608",
+    "SAI_FDB_ENTRY_ATTR_TYPE": "SAI_FDB_ENTRY_TYPE_DYNAMIC"
+  },
+  "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY:{\"bridge_id\":\"oid:0x0\",\"bridge_type\":\"SAI_FDB_ENTRY_BRIDGE_TYPE_1Q\",\"mac\":\"7C:FE:90:80:9F:06\",\"switch_id\":\"oid:0x21000000000000\",\"vlan\":\"1000\"}": {
+    "SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID": "oid:0x3a000000000616",
+    "SAI_FDB_ENTRY_ATTR_TYPE": "SAI_FDB_ENTRY_TYPE_DYNAMIC"
+  },
+  "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY:{\"bvid\":\"oid:0x26000000000016\",\"mac\":\"7C:FE:90:80:9F:08\",\"switch_id\":\"oid:0x21000000000000\"}": {
+    "SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID": "oid:0x3a000000000620",
+    "SAI_FDB_ENTRY_ATTR_TYPE": "SAI_FDB_ENTRY_TYPE_DYNAMIC"
+  },
+  "ASIC_STATE:SAI_OBJECT_TYPE_VLAN:oid:0x26000000000016": {
+    "SAI_VLAN_ATTR_VLAN_ID": "1000"
+  },
+  "ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT:oid:0x3a000000000608": {
+    "SAI_BRIDGE_PORT_ATTR_TYPE": "SAI_BRIDGE_PORT_TYPE_PORT",
+    "SAI_BRIDGE_PORT_ATTR_PORT_ID": "oid:0x1000000000003",
+    "SAI_BRIDGE_PORT_ATTR_ADMIN_STATE": "true"
+  },
+  "ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT:oid:0x3a000000000616": {
+    "SAI_BRIDGE_PORT_ATTR_TYPE": "SAI_BRIDGE_PORT_TYPE_PORT",
+    "SAI_BRIDGE_PORT_ATTR_PORT_ID": "oid:0x1000000000005",
+    "SAI_BRIDGE_PORT_ATTR_ADMIN_STATE": "true"
+  }
 }

tests/mock_tables/asic1/asic_db.json

@@ -1,2 +1,11 @@
 {
+  "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY:{\"bridge_id\":\"oid:0x0\",\"bridge_type\":\"SAI_FDB_ENTRY_BRIDGE_TYPE_1Q\",\"mac\":\"7C:FE:90:80:9F:10\",\"switch_id\":\"oid:0x21000000000000\",\"vlan\":\"1000\"}": {
+    "SAI_FDB_ENTRY_ATTR_BRIDGE_PORT_ID": "oid:0x3a0000000006208",
+    "SAI_FDB_ENTRY_ATTR_TYPE": "SAI_FDB_ENTRY_TYPE_DYNAMIC"
+  },
+  "ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT:oid:0x3a000000000620": {
+    "SAI_BRIDGE_PORT_ATTR_TYPE": "SAI_BRIDGE_PORT_TYPE_PORT",
+    "SAI_BRIDGE_PORT_ATTR_PORT_ID": "oid:0x1000000000010",
+    "SAI_BRIDGE_PORT_ATTR_ADMIN_STATE": "true"
+  }
 }

tests/mock_tables/global_db/asic_db.json

@@ -0,0 +1,2 @@
+{
+}