Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
178 changes: 139 additions & 39 deletions src/swsssdk/dbconnector.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,67 @@
# FIXME: Convert to metaclasses when Py2 support is removed. Metaclasses have unique interfaces to Python2/Python3.

class SonicDBConfig(object):
SONIC_DB_GLOBAL_CONFIG_FILE = "/var/run/redis/sonic-db/database_global.json"
SONIC_DB_CONFIG_FILE = "/var/run/redis/sonic-db/database_config.json"
_sonic_db_config_dir = "/var/run/redis/sonic-db"
_sonic_db_global_config_init = False
_sonic_db_config_init = False
_sonic_db_config = {}

"""This is the database_global.json parse and load API. This file has the namespace name and
the corresponding database_config.json file. The global file is significant for the
applications running in the linux host namespace, like eg: config/show cli, snmp etc which
needs to connect to databases running in other namesacpes. If the "namespace" attribute is not
specified for an "include" attribute, it referes to the linux host namespace.
"""
@staticmethod
def load_sonic_global_db_config(global_db_file_path=SONIC_DB_GLOBAL_CONFIG_FILE):
"""
Parse and load the global database config json file
"""
if SonicDBConfig._sonic_db_global_config_init == True:
return

if os.path.isfile(global_db_file_path) == True:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

global_db_file_path [](start = 26, length = 19)

If global_db_file_path does not exist, why set _sonic_db_global_config_init to True?

Also can you return _sonic_db_global_config_init value to caller?

global_db_config_dir = os.path.dirname(global_db_file_path)
with open(global_db_file_path, "r") as read_file:
all_ns_dbs = json.load(read_file)
for entry in all_ns_dbs['INCLUDES']:
if 'namespace' not in entry.keys():
# If the user already invoked load_sonic_db_config() explicitly to load the
# database_config.json file for current namesapce, skip loading the file
# referenced here in the global config file.
if SonicDBConfig._sonic_db_config_init == True:
continue
ns = ''
else:
ns = entry['namespace']

# Check if _sonic_db_config already have this namespace present
if ns in SonicDBConfig._sonic_db_config:
msg = "The database_config for this namespace '{}' is already parsed. !!".format(ns)
logger.warning(msg)
continue

db_include_file = os.path.join(global_db_config_dir, entry['include'])

# Not finding the database_config.json file for the namespace
if os.path.isfile(db_include_file) == False:
msg = "'{}' file is not found !!".format(db_include_file)
logger.warning(msg)
continue

# As we load the database_config.json file for current namesapce,
# set the _sonic_db_config_init flag to True to prevent loading again
# by the API load_sonic_db_config()
if ns is '':
SonicDBConfig._sonic_db_config_init = True

with open(db_include_file, "r") as inc_file:
SonicDBConfig._sonic_db_config[ns] = json.load(inc_file)

SonicDBConfig._sonic_db_global_config_init = True

@staticmethod
def load_sonic_db_config(sonic_db_file_path=SONIC_DB_CONFIG_FILE):
"""
Expand All @@ -27,91 +84,134 @@ def load_sonic_db_config(sonic_db_file_path=SONIC_DB_CONFIG_FILE):
logger.warning(msg)
sonic_db_file_path = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'config', 'database_config.json')
with open(sonic_db_file_path, "r") as read_file:
SonicDBConfig._sonic_db_config = json.load(read_file)
# The database_config.json is loaded into the '' index, which referes to the local namespace.
SonicDBConfig._sonic_db_config[''] = json.load(read_file)
except (OSError, IOError):
msg = "Could not open sonic database config file '{}'".format(sonic_db_file_path)
logger.exception(msg)
raise RuntimeError(msg)
SonicDBConfig._sonic_db_config_init = True

@staticmethod
def db_name_validation(db_name):
def namespace_validation(namespace):
# Check the namespace is valid.
if namespace is None:
msg = "invalid namespace name given as input"
logger.warning(msg)
raise RuntimeError(msg)
# Load global config if the namespace is an external one.
if namespace != '' and SonicDBConfig._sonic_db_global_config_init == False:
SonicDBConfig.load_sonic_global_db_config()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one more thing: namespace_validation() is a library API, if user use namespace_validation() to check namaspace stuff before any load_sonic_db_config, it calls load_sonic_global_db_config() which actually load ns='' case, but didn't set _sonic_db_config_init = TURE, which will make following lines SonicDBConfig.load_sonic_db_config() still load ns='' case again, better to set _sonic_db_config_init = TURE in load_sonic_global_db_config() when ns is ' '.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great catch! In stead of make it smarter, how about making it more stupid and simple?

Since use are required to call load_sonic_global_db_config() at this use case, let's make this function throw runtime exception and educate user to call load_sonic_global_db_config first.

Also take Dong's suggestion to ensure sonic_db_config_init == True after calling load_sonic_global_db_config()


In reply to: 408568390 [](ancestors = 408568390)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not 100% sure of this use case. The idea I had to make these two independent the load_global_config and local_db_config + to keep the existing behaviour of local_db_config intact.

I can set this flag sonic_db_config_init == True in load_global_config for ' ' case, but then the user cannot later if needed override the config present in global_config.json for the current namespace, with a config which he wants later .. by calling load_sonic_db_config(sonic_db_file_path=my_config_file)

Copy link
Copy Markdown
Collaborator

@dzhangalibaba dzhangalibaba Apr 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should NOT be allowed to make user override existing config once it is initialized . It should only init once to keep consistence among all applications. Otherwise some application want this config , others want that config, that will be messed up.

Copy link
Copy Markdown
Contributor Author

@judyjoseph judyjoseph Apr 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated with this flag sonic_db_config_init = True in the function load_sonic_global_db_config().

I haven't removed the implicit call to load_sonic_global_db_config() here if namespace is != '', as other wise the applications ( including scripts which uses it like sonic-db-cli, sonic_cfggen . db_migrator etc ) will have to call it explicitly SonicDBConfig.load_sonic_global_db_config() ? Additional checks on whether to load the global config or NOT depending on whether the user want to access a namespace other than '' needs to be added in multiple places in scripts/application.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am taking Qi's suggestion to make the user explicitly call load_sonic_global_db_config().

In addition, I am optimizing the logic a bit by adding a new argument "namespace" to this API load_sonic_global_db_config(). So if the user inputs an argument "namespace" this API will load the json file for that namespace alone .

This will improve performance of utilitiles like sonic-cfggen/sonic-db-cli etc where we pass the namespace argument.

if SonicDBConfig._sonic_db_config_init == False:
SonicDBConfig.load_sonic_db_config()
if namespace not in SonicDBConfig._sonic_db_config:
msg = "{} is not a valid namespace name in configuration file".format(namespace)
logger.warning(msg)
raise RuntimeError(msg)

@staticmethod
def db_name_validation(db_name, namespace=''):
if SonicDBConfig._sonic_db_config_init == False:
SonicDBConfig.load_sonic_db_config()
if db_name not in SonicDBConfig._sonic_db_config["DATABASES"]:
SonicDBConfig.namespace_validation(namespace)
db=SonicDBConfig._sonic_db_config[namespace]["DATABASES"]
if db_name not in db:
msg = "{} is not a valid database name in configuration file".format(db_name)
logger.exception(msg)
logger.warning(msg)
raise RuntimeError(msg)

@staticmethod
def inst_name_validation(inst_name):
def inst_name_validation(inst_name, namespace=''):
if SonicDBConfig._sonic_db_config_init == False:
SonicDBConfig.load_sonic_db_config()
if inst_name not in SonicDBConfig._sonic_db_config["INSTANCES"]:
SonicDBConfig.namespace_validation(namespace)
instances = SonicDBConfig._sonic_db_config[namespace]["INSTANCES"]
if inst_name not in instances:
msg = "{} is not a valid instance name in configuration file".format(inst_name)
logger.exception(msg)
logger.warning(msg)
raise RuntimeError(msg)

@staticmethod
def get_dblist():
def get_dblist(namespace=''):
if SonicDBConfig._sonic_db_config_init == False:
SonicDBConfig.load_sonic_db_config()
SonicDBConfig.namespace_validation(namespace)
return SonicDBConfig._sonic_db_config[namespace]["DATABASES"].keys()

@staticmethod
def get_ns_list():
if SonicDBConfig._sonic_db_config_init == False:
SonicDBConfig.load_sonic_db_config()
return SonicDBConfig._sonic_db_config["DATABASES"].keys()
return SonicDBConfig._sonic_db_config.keys()

@staticmethod
def get_instance(db_name):
def get_instance(db_name, namespace=''):
if SonicDBConfig._sonic_db_config_init == False:
SonicDBConfig.load_sonic_db_config()
SonicDBConfig.db_name_validation(db_name)
inst_name = SonicDBConfig._sonic_db_config["DATABASES"][db_name]["instance"]
SonicDBConfig.inst_name_validation(inst_name)
return SonicDBConfig._sonic_db_config["INSTANCES"][inst_name]
SonicDBConfig.db_name_validation(db_name, namespace)
inst_name = SonicDBConfig._sonic_db_config[namespace]["DATABASES"][db_name]["instance"]
SonicDBConfig.inst_name_validation(inst_name, namespace)
return SonicDBConfig._sonic_db_config[namespace]["INSTANCES"][inst_name]

@staticmethod
def get_instancelist():
def get_instancelist(namespace=''):
if SonicDBConfig._sonic_db_config_init == False:
SonicDBConfig.load_sonic_db_config()
return SonicDBConfig._sonic_db_config["INSTANCES"]
SonicDBConfig.namespace_validation(namespace)
return SonicDBConfig._sonic_db_config[namespace]["INSTANCES"]

@staticmethod
def get_socket(db_name):
def get_socket(db_name, namespace=''):
if SonicDBConfig._sonic_db_config_init == False:
SonicDBConfig.load_sonic_db_config()
SonicDBConfig.db_name_validation(db_name)
return SonicDBConfig.get_instance(db_name)["unix_socket_path"]
return SonicDBConfig.get_instance(db_name, namespace)["unix_socket_path"]

@staticmethod
def get_hostname(db_name):
def get_hostname(db_name, namespace=''):
if SonicDBConfig._sonic_db_config_init == False:
SonicDBConfig.load_sonic_db_config()
SonicDBConfig.db_name_validation(db_name)
return SonicDBConfig.get_instance(db_name)["hostname"]
return SonicDBConfig.get_instance(db_name, namespace)["hostname"]

@staticmethod
def get_port(db_name):
def get_port(db_name, namespace=''):
if SonicDBConfig._sonic_db_config_init == False:
SonicDBConfig.load_sonic_db_config()
SonicDBConfig.db_name_validation(db_name)
return SonicDBConfig.get_instance(db_name)["port"]
return SonicDBConfig.get_instance(db_name, namespace)["port"]

@staticmethod
def get_dbid(db_name):
def get_dbid(db_name, namespace=''):
if SonicDBConfig._sonic_db_config_init == False:
SonicDBConfig.load_sonic_db_config()
SonicDBConfig.db_name_validation(db_name)
return SonicDBConfig._sonic_db_config["DATABASES"][db_name]["id"]
SonicDBConfig.db_name_validation(db_name, namespace)
return SonicDBConfig._sonic_db_config[namespace]["DATABASES"][db_name]["id"]

@staticmethod
def get_separator(db_name):
def get_separator(db_name, namespace=''):
if SonicDBConfig._sonic_db_config_init == False:
SonicDBConfig.load_sonic_db_config()
SonicDBConfig.db_name_validation(db_name)
return SonicDBConfig._sonic_db_config["DATABASES"][db_name]["separator"]
SonicDBConfig.db_name_validation(db_name, namespace)
return SonicDBConfig._sonic_db_config[namespace]["DATABASES"][db_name]["separator"]

class SonicV2Connector(DBInterface):
def __init__(self, use_unix_socket_path=False, **kwargs):
def __init__(self, use_unix_socket_path=False, namespace='', **kwargs):
super(SonicV2Connector, self).__init__(**kwargs)
self.use_unix_socket_path = use_unix_socket_path
self.namespace = namespace

"""If the user don't give the namespace as input, it takes the default value of ''
'' (empty string) referes to the local namespace where this class is used.
(It could be a network namespace or linux host namesapce)
"""
if not isinstance(namespace, str):
msg = "{} is not a valid namespace name".format(namespace)
logger.warning(msg)
raise RuntimeError(msg)

# The TCP connection to a DB in another namespace in not supported.
if namespace != '' and use_unix_socket_path == False:
message = "TCP connectivity to the DB instance in a different namespace is not implemented!"
raise NotImplementedError(message)

for db_name in self.get_db_list():
# set a database name as a constant value attribute.
setattr(self, db_name, db_name)
Expand All @@ -133,25 +233,25 @@ def close(self, db_name):
super(SonicV2Connector, self).close(db_id)

def get_db_list(self):
return SonicDBConfig.get_dblist()
return SonicDBConfig.get_dblist(self.namespace)

def get_db_instance(self, db_name):
return SonicDBConfig.get_instance(db_name)
return SonicDBConfig.get_instance(db_name, self.namespace)

def get_db_socket(self, db_name):
return SonicDBConfig.get_socket(db_name)
return SonicDBConfig.get_socket(db_name, self.namespace)

def get_db_hostname(self, db_name):
return SonicDBConfig.get_hostname(db_name)
return SonicDBConfig.get_hostname(db_name, self.namespace)

def get_db_port(self, db_name):
return SonicDBConfig.get_port(db_name)
return SonicDBConfig.get_port(db_name, self.namespace)

def get_dbid(self, db_name):
return SonicDBConfig.get_dbid(db_name)
return SonicDBConfig.get_dbid(db_name, self.namespace)

def get_db_separator(self, db_name):
return SonicDBConfig.get_separator(db_name)
return SonicDBConfig.get_separator(db_name, self.namespace)

def get_redis_client(self, db_name):
db_id = self.get_dbid(db_name)
Expand Down
63 changes: 47 additions & 16 deletions src/swsssdk/scripts/sonic-db-cli
Original file line number Diff line number Diff line change
@@ -1,11 +1,24 @@
#!/usr/bin/python
from __future__ import print_function
import sys
import swsssdk
import redis
import argparse
from multiprocessing import Pool

def ping_single_instance(inst_info):
def ping_unix_path_single_instance(inst_info):
inst_hostname = inst_info['hostname']
inst_unix_socket_path = inst_info['unix_socket_path']
r = redis.Redis(host=inst_hostname, unix_socket_path=inst_unix_socket_path)
rsp = False
msg = 'Could not connect to Redis at {}:{}: Connection refused'.format(inst_hostname, inst_unix_socket_path)
try:
rsp = r.ping()
except redis.exceptions.ConnectionError as e:
pass
return 'PONG' if rsp else msg

def ping_tcp_single_instance(inst_info):
Copy link
Copy Markdown
Contributor

@qiluo-msft qiluo-msft Apr 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping_tcp_single_instance [](start = 4, length = 24)

Most code duplicated in ping_tcp_single_instance and ping_unix_path_single_instance. Not a blocking issue, and wish you could improve them.

inst_hostname = inst_info['hostname']
inst_port = inst_info['port']
r = redis.Redis(host=inst_hostname, port=inst_port)
Expand All @@ -17,8 +30,15 @@ def ping_single_instance(inst_info):
pass
return 'PONG' if rsp else msg

def ping_all_instances():
db_insts = swsssdk.SonicDBConfig.get_instancelist()
def ping_all_instances(namespace, use_unix_socket=False):
ping_single_instance = ping_unix_path_single_instance
# Use the tcp connectivity if namespace is local and unixsocket cmd_option is present.
if namespace is None:
namespace = ''
if not use_unix_socket:
ping_single_instance = ping_tcp_single_instance

db_insts = swsssdk.SonicDBConfig.get_instancelist(namespace)
# ping all redis instances together
# TODO: if one of the ping failed, it could fail quickly and not necessary to wait all other pings
p = Pool(len(db_insts))
Expand All @@ -28,19 +48,25 @@ def ping_all_instances():
if rsp != 'PONG':
msg.append(rsp)
if msg:
print '\n'.join(msg)
print('\n'.join(msg))
sys.exit(1)
else:
print 'PONG'
print('PONG')
sys.exit(0)

def execute_cmd(dbname, cmd):
dbconn = swsssdk.SonicV2Connector(use_unix_socket_path=False)
def execute_cmd(dbname, cmd, namespace, use_unix_socket=False):
if namespace is None:
if use_unix_socket:
dbconn = swsssdk.SonicV2Connector(use_unix_socket_path=True)
else:
dbconn = swsssdk.SonicV2Connector(use_unix_socket_path=False)
else:
dbconn = swsssdk.SonicV2Connector(use_unix_socket_path=True, namespace=namespace)
try:
dbconn.connect(dbname)
except RuntimeError:
msg = "Invalid database name input : '{}'".format(dbname)
print >> sys.stderr, msg
print(msg, file=sys.stderr)
sys.exit(1)
else:
client = dbconn.get_redis_client(dbname)
Expand All @@ -51,34 +77,39 @@ def execute_cmd(dbname, cmd):
with these changes, it is enough for us to mimic redis-cli in SONiC so far since no application uses tty mode redis-cli output
"""
if resp is None:
print ""
print()
elif isinstance(resp, list):
print "\n".join(resp)
print("\n".join(resp))
else:
print resp
print(resp)
sys.exit(0)

def main():
parser = argparse.ArgumentParser(description='SONiC DB CLI',
formatter_class=argparse.RawTextHelpFormatter,
epilog=
"""
Example 1: sonic-db-cli CONFIG_DB keys *
Example 2: sonic-db-cli APPL_DB HGETALL VLAN_TABLE:Vlan10
**sudo** needed for commands accesing a different namespace [-n], or using unixsocket connection [-s]

Example 1: sonic-db-cli -n asic0 CONFIG_DB keys \*
Example 2: sonic-db-cli -n asic2 APPL_DB HGETALL VLAN_TABLE:Vlan10
Example 3: sonic-db-cli APPL_DB HGET VLAN_TABLE:Vlan10 mtu
Example 4: sonic-db-cli APPL_DB EVAL "return {KEYS[1],KEYS[2],ARGV[1],ARGV[2]}" 2 k1 k2 v1 v2
Example 4: sonic-db-cli -n asic3 APPL_DB EVAL "return {KEYS[1],KEYS[2],ARGV[1],ARGV[2]}" 2 k1 k2 v1 v2
Example 5: sonic-db-cli PING
Example 6: sonic-db-cli -s PING
""")
parser.add_argument('-s', '--unixsocket', help="Override use of tcp_port and use unixsocket",action='store_true')
parser.add_argument('-n', '--namespace', type=str, help="Namespace string to use asic0/asic1.../asicn", default=None)
parser.add_argument('db_or_op', type=str, help='Database name Or Unary operation(only PING supported)')
parser.add_argument('cmd', nargs='*', type=str, help='Command to execute in database')

args = parser.parse_args()

if args.db_or_op:
if args.cmd:
execute_cmd(args.db_or_op, args.cmd)
execute_cmd(args.db_or_op, args.cmd, args.namespace, args.unixsocket)
elif args.db_or_op == 'PING':
ping_all_instances()
ping_all_instances(args.namespace, args.unixsocket)
# TODO next PR will support 'SAVE' and 'FLUSHALL'
# elif args.db_or_op == 'SAVE':
# elif args.db_or_op == 'FLUSHALL':
Expand Down
Loading