-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[cfggen] Support reading from and writing to configdb #808
Changes from 5 commits
4f53ec1
41bfa76
419e036
79f1e17
76b888e
49f7143
6b4063e
16685db
6e92563
e092256
93c3495
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
#!/usr/bin/env bash | ||
|
||
supervisorctl start redis-server | ||
|
||
# If there is a config db dump file, load it | ||
if [ -r /etc/sonic/config_db.json ]; then | ||
# Wait until redis starts | ||
while true; do | ||
if [ -e /var/run/redis/redis.sock ]; then | ||
break | ||
fi | ||
sleep 1 | ||
done | ||
|
||
sonic-cfggen -j /etc/sonic/config_db.json -D | ||
fi | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,10 +3,19 @@ logfile_maxbytes=1MB | |
logfile_backups=2 | ||
nodaemon=true | ||
|
||
[program:redis-server] | ||
command=/usr/bin/redis-server /etc/redis/redis.conf | ||
[program:start.sh] | ||
command=/usr/bin/start.sh | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do not why start redis-server inside start.sh, why can you make start.sh independent of redis-server start. maybe rename it as configdb-load.sh? #Resolved There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's just because we are following the similar design - start.sh controls all - in other dockers. As redis-server in docker-database does not have any dependency, I can decouple it with start.sh. In reply to: 130537997 [](ancestors = 130537997) |
||
priority=1 | ||
autostart=true | ||
autorestart=false | ||
stdout_logfile=syslog | ||
stderr_logfile=syslog | ||
|
||
[program:redis-server] | ||
command=/usr/bin/redis-server /etc/redis/redis.conf | ||
priority=2 | ||
autostart=false | ||
autorestart=false | ||
stdout_logfile=syslog | ||
stderr_logfile=syslog | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -132,8 +132,7 @@ sudo bash -c "echo dhcp_as_static=true >> $FILESYSTEM_ROOT/etc/sonic/updategraph | |
sudo bash -c "echo enabled=false > $FILESYSTEM_ROOT/etc/sonic/updategraph.conf" | ||
{% endif %} | ||
{% if shutdown_bgp_on_start == "y" %} | ||
sudo bash -c "echo bgp_admin_state: > $FILESYSTEM_ROOT/etc/sonic/bgp_admin.yml" | ||
sudo bash -c "echo ' all: off' >> $FILESYSTEM_ROOT/etc/sonic/bgp_admin.yml" | ||
sudo bash -c "echo '{ \"bgp_admin_state\": { \"all\": false } }' >> $FILESYSTEM_ROOT/etc/sonic/config_db.json" | ||
{% endif %} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when the option is n, we should probably put bgp_admin_state to true. the reason is that when bgp docker start, it needs to read the bgp_admin_state, there is chance that the configuration is loaded to the config_db yet. In this case, how do you know whether config is already loaded but the state is true versus the config is not yet loaded? #ByDesign There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it's a good idea to tell whether configuration is loaded or not by looking at the content of config. Instead, we should find other ways to ensure initial config loading is finished before other container consume DB data, either by systemd service sequence or by setting flag in DB. In reply to: 128321125 [](ancestors = 128321125) |
||
# Copy SNMP configuration files | ||
sudo cp $IMAGE_CONFIGS/snmp/snmp.yml $FILESYSTEM_ROOT/etc/sonic/ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,19 @@ | ||
#!/usr/bin/env python | ||
"""sonic-cfggen | ||
|
||
A tool to read SONiC config data from one or more of the following sources: | ||
minigraph file, config DB, json file(s), yaml files(s), command line input, | ||
and write the data into DB, print as json, or render a jinja2 config template. | ||
|
||
Examples: | ||
Render template with minigraph: | ||
sonic-cfggen -m -t /usr/share/template/bgpd.conf.j2 | ||
Dump config DB content into json file: | ||
sonic-cfggen -d --print-data > db_dump.json | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
if we assume by default it will load configdb, then we do not need this option. #WontFix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Before we fully remove minigraph dependency, let's keep -m and -d options. In reply to: 128328361 [](ancestors = 128328361) |
||
Load content of json file into config DB: | ||
sonic-cfggen -j db_dump.json --write-to-db | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
sonic-cfggen --import db_dump.json #WontFix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's keep the current option setting for dev purposes. I'll later add a "config load" and "config save" command to CLI for users. In reply to: 128328144 [](ancestors = 128328144) |
||
See usage string for detail description for arguments. | ||
""" | ||
|
||
import sys | ||
import os.path | ||
|
@@ -12,6 +27,7 @@ from minigraph import parse_xml | |
from minigraph import parse_device_desc_xml | ||
from sonic_platform import get_machine_info | ||
from sonic_platform import get_platform_info | ||
from swsssdk import ConfigDBConnector | ||
|
||
def is_ipv4(value): | ||
if not value: | ||
|
@@ -46,19 +62,65 @@ def unique_name(l): | |
new_list.append(item) | ||
return new_list | ||
|
||
|
||
class FormatConverter: | ||
"""Convert config DB based schema to legacy minigraph based schema for backward capability. | ||
We will move to DB schema and remove this class when the config templates are modified. | ||
|
||
TODO(taoyl): Current version of config db only supports BGP admin states. | ||
All other configuration are still loaded from minigraph. Plan to remove | ||
minigraph and move everything into config db in a later commit. | ||
""" | ||
@staticmethod | ||
def db_to_output(db_data): | ||
data_bgp_admin = {} | ||
for table_name, content in db_data.iteritems(): | ||
if table_name == 'BGP_NEIGHBOR': | ||
for key, value in content.iteritems(): | ||
if value.has_key('admin_status'): | ||
data_bgp_admin[key] = (value['admin_status'] == 'up') | ||
elif table_name == 'DEVICE_METADATA': | ||
if content['localhost'].has_key('bgp_default_status'): | ||
data_bgp_admin['all'] = (content['localhost']['bgp_default_status'] == 'up') | ||
|
||
output_data = {'bgp_admin_state': data_bgp_admin} if data_bgp_admin else {} | ||
return output_data | ||
|
||
@staticmethod | ||
def output_to_db(output_data): | ||
db_data = {} | ||
for key, value in output_data.iteritems(): | ||
if key == 'bgp_admin_state': | ||
for neighbor, state in value.iteritems(): | ||
if neighbor == 'all': | ||
if not db_data.has_key('DEVICE_METADATA'): | ||
db_data['DEVICE_METADATA'] = {'localhost': {}} | ||
db_data['DEVICE_METADATA']['localhost']['bgp_default_status'] = 'up' if state else 'down' | ||
else: | ||
if not db_data.has_key('BGP_NEIGHBOR'): | ||
db_data['BGP_NEIGHBOR'] = {} | ||
if not db_data['BGP_NEIGHBOR'].has_key(neighbor): | ||
db_data['BGP_NEIGHBOR'][neighbor] = {} | ||
db_data['BGP_NEIGHBOR'][neighbor]['admin_status'] = 'up' if state else 'down' | ||
return db_data | ||
|
||
|
||
def main(): | ||
parser=argparse.ArgumentParser(description="Render configuration file from minigraph data and jinja2 template.") | ||
group = parser.add_mutually_exclusive_group() | ||
group.add_argument("-m", "--minigraph", help="minigraph xml file") | ||
group.add_argument("-m", "--minigraph", help="minigraph xml file", nargs='?', const='/etc/sonic/minigraph.xml') | ||
group.add_argument("-M", "--device-description", help="device description xml file") | ||
parser.add_argument("-p", "--port-config", help="port config file, used with -m") | ||
parser.add_argument("-y", "--yaml", help="yaml file that contains addtional variables", action='append', default=[]) | ||
parser.add_argument("-y", "--yaml", help="yaml file that contains additional variables", action='append', default=[]) | ||
parser.add_argument("-j", "--json", help="json file that contains additional variables", action='append', default=[]) | ||
parser.add_argument("-a", "--additional-data", help="addition data, in json string") | ||
parser.add_argument("-d", "--from-db", help="read config from configdb", action='store_true') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we need this option, we should always read from the configdb. can we make it as default? #WontFix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll suggest keep the -d option during migration from minigraph to db. We can come back to revisit this after we move all data to DB. In reply to: 128322480 [](ancestors = 128322480) |
||
group = parser.add_mutually_exclusive_group() | ||
group.add_argument("-t", "--template", help="render the data with the template file") | ||
group.add_argument("-v", "--var", help="print the value of a variable, support jinja2 expression") | ||
group.add_argument("--var-json", help="print the value of a variable, in json format") | ||
group.add_argument("--print-data", help="print all data", action='store_true') | ||
group.add_argument("-D", "--write-to-db", help="write config into configdb", action='store_true') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
--import : import json into configdb #WontFix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's keep the current option setting for dev purposes. I'll later add a "config load" and "config save" command to CLI for users. In reply to: 128327833 [](ancestors = 128327833) |
||
group.add_argument("-P", "--print-data", help="print all data", action='store_true') | ||
args = parser.parse_args() | ||
|
||
data = {} | ||
|
@@ -90,8 +152,17 @@ def main(): | |
additional_data = yaml.load(stream) | ||
data.update(additional_data) | ||
|
||
for json_file in args.json: | ||
with open(json_file, 'r') as stream: | ||
data.update(json.load(stream)) | ||
|
||
if args.additional_data != None: | ||
data.update(json.loads(args.additional_data)) | ||
|
||
if args.from_db: | ||
configdb = ConfigDBConnector() | ||
configdb.connect() | ||
data.update(FormatConverter.db_to_output(configdb.get_config())) | ||
|
||
if args.template != None: | ||
template_file = os.path.abspath(args.template) | ||
|
@@ -109,8 +180,14 @@ def main(): | |
if args.var_json != None: | ||
print json.dumps(data[args.var_json], indent=4, cls=minigraph_encoder) | ||
|
||
if args.write_to_db: | ||
configdb = ConfigDBConnector() | ||
configdb.connect() | ||
configdb.set_config(FormatConverter.output_to_db(data)) | ||
|
||
if args.print_data: | ||
print json.dumps(data, indent=4, cls=minigraph_encoder) | ||
|
||
|
||
if __name__ == "__main__": | ||
main() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while true; do if [ "$(redis-cli ping)" == "PONG" ]; then break; fi; sleep 1; done
is more reliable then just check existence of redis.sock #Resolved