Skip to content

Commit

Permalink
Fix snmp agent Initialize config DB multiple times issue (#245)
Browse files Browse the repository at this point in the history
<!--
Please make sure you've read and understood our contributing guidelines;
https://github.com/Azure/SONiC/blob/gh-pages/CONTRIBUTING.md

** Make sure all your commits include a signature generated with `git commit -s` **

If this is a bug fix, make sure your description includes "fixes #xxxx", or
"closes #xxxx"

Please provide the following information:
-->

**- What I did**
    Fix following code issue:
        1. When initialize SonicDBConfig on multi ASIC device, not check if the global config already initialized issue.
        2. Initialize SonicDBConfig multiple times with dupe code.

**- How I did it**
    Code change to check isGlobalInit before load global config.
    Move dupe code to Namespace.init_sonic_db_config() and initialize config only once.
    Add new mock method for SonicDBConfig.isGlobalInit

**- How to verify it**
    Pass all UT and E2E test.

**- Description for the changelog**
<!--
Write a short (one line) summary that describes the changes in this
pull request for inclusion in the changelog:
-->
  • Loading branch information
liuh-80 authored Mar 18, 2022
1 parent 6bd51c4 commit 2654f4a
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 13 deletions.
4 changes: 3 additions & 1 deletion src/sonic_ax_impl/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import sys

import ax_interface
from sonic_ax_impl.mibs import ieee802_1ab
from sonic_ax_impl.mibs import ieee802_1ab, Namespace
from . import logger
from .mibs.ietf import rfc1213, rfc2737, rfc2863, rfc3433, rfc4292, rfc4363
from .mibs.vendor import dell, cisco
Expand Down Expand Up @@ -58,6 +58,8 @@ def main(update_frequency=None):
global event_loop

try:
Namespace.init_sonic_db_config()

# initialize handler and set update frequency (or use the default)
agent = ax_interface.Agent(SonicMIB, update_frequency or DEFAULT_UPDATE_FREQUENCY, event_loop)

Expand Down
37 changes: 25 additions & 12 deletions src/sonic_ax_impl/mibs/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,16 +218,11 @@ def init_db():
Connects to DB
:return: db_conn
"""
if not SonicDBConfig.isInit():
if multi_asic.is_multi_asic():
# Load the global config file database_global.json once.
SonicDBConfig.load_sonic_global_db_config()
else:
SonicDBConfig.load_sonic_db_config()
Namespace.init_sonic_db_config()

# 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)

return db_conn

def init_mgmt_interface_tables(db_conn):
Expand Down Expand Up @@ -536,14 +531,32 @@ def get_oidvalue(self, oid):
return self.oid_map[oid]

class Namespace:

"""
Sonic database initialized flag.
"""
db_config_loaded = False

@staticmethod
def init_sonic_db_config():
"""
Initialize SonicDBConfig
"""
if Namespace.db_config_loaded:
return

if multi_asic.is_multi_asic():
# Load the global config file database_global.json once.
SonicDBConfig.load_sonic_global_db_config()
else:
SonicDBConfig.load_sonic_db_config()

Namespace.db_config_loaded = True

@staticmethod
def init_namespace_dbs():
db_conn = []
if not SonicDBConfig.isInit():
if multi_asic.is_multi_asic():
SonicDBConfig.load_sonic_global_db_config()
else:
SonicDBConfig.load_sonic_db_config()
Namespace.init_sonic_db_config()
host_namespace_idx = 0
for idx, namespace in enumerate(SonicDBConfig.get_ns_list()):
if namespace == multi_asic.DEFAULT_NAMESPACE:
Expand Down
4 changes: 4 additions & 0 deletions tests/mock_tables/dbconnector.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ def clean_up_config():
SonicDBConfig._sonic_db_global_config_init = False
SonicDBConfig._sonic_db_config_init = False

def mock_SonicDBConfig_isGlobalInit():
return SonicDBConfig._sonic_db_global_config_init


# TODO Convert this to fixture as all Test classes require it.
def load_namespace_config():
Expand Down Expand Up @@ -140,6 +143,7 @@ def keys(self, pattern='*'):
SonicV2Connector.connect = connect_SonicV2Connector
swsscommon.SonicV2Connector = SonicV2Connector
swsscommon.SonicDBConfig = SonicDBConfig
swsscommon.SonicDBConfig.isGlobalInit = mock_SonicDBConfig_isGlobalInit

# pytest case collecting will import some module before monkey patch, so reload
from importlib import reload
Expand Down

0 comments on commit 2654f4a

Please sign in to comment.