Skip to content
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

Merged
merged 11 commits into from
Aug 2, 2017
3 changes: 2 additions & 1 deletion dockers/docker-database/Dockerfile.j2
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM docker-base
FROM docker-config-engine

## Make apt-get non-interactive
ENV DEBIAN_FRONTEND=noninteractive
Expand Down Expand Up @@ -33,5 +33,6 @@ RUN sed -ri 's/^(save .*$)/# \1/g;
' /etc/redis/redis.conf

COPY ["supervisord.conf", "/etc/supervisor/conf.d/"]
COPY ["start.sh", "/usr/bin/"]

ENTRYPOINT ["/usr/bin/supervisord"]
19 changes: 19 additions & 0 deletions dockers/docker-database/start.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#!/usr/bin/env bash

supervisorctl start redis-server

# Wait until redis starts
while true; do
if [ `redis-cli ping` == "PONG" ]; then
break
fi
sleep 1
done

# If there is a config db dump file, load it
if [ -r /etc/sonic/config_db.json ]; then
sonic-cfggen -j /etc/sonic/config_db.json --write-to-db
fi

echo -en "SELECT 4\nSET CONFIG_DB_INITIALIZED true" | redis-cli

13 changes: 11 additions & 2 deletions dockers/docker-database/supervisord.conf
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator

@lguohan lguohan Aug 1, 2017

Choose a reason for hiding this comment

The 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

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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

1 change: 1 addition & 0 deletions dockers/docker-fpm-quagga/Dockerfile.j2
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ RUN apt-get clean -y; apt-get autoclean -y; apt-get autoremove -y
RUN rm -rf /debs

COPY ["start.sh", "/usr/bin/"]
COPY ["bgpcfgd", "/usr/bin/"]
COPY ["supervisord.conf", "/etc/supervisor/conf.d/"]
COPY ["*.j2", "/usr/share/sonic/templates/"]
COPY ["daemons", "/etc/quagga/"]
Expand Down
42 changes: 42 additions & 0 deletions dockers/docker-fpm-quagga/bgpcfgd
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
#!/usr/bin/env python

import sys
import redis
import subprocess
import syslog
from swsssdk import ConfigDBConnector

# Returns BGP ASN as a string
def _get_bgp_asn_from_minigraph():
# Get BGP ASN from minigraph
proc = subprocess.Popen(
['sonic-cfggen', '-m', '/etc/sonic/minigraph.xml', '-v', 'minigraph_bgp_asn'],
stdout=subprocess.PIPE,
shell=False,
stderr=subprocess.STDOUT)
stdout = proc.communicate()[0]
proc.wait()
return stdout.rstrip('\n')

def bgp_config(asn, ip, config):
syslog.syslog(syslog.LOG_INFO, '[bgp cfgd] value for {} changed to {}'.format(ip, config))
# Currently dynamic config is supported only for bgp admin status
if config.has_key('admin_status'):
command_mod = 'no ' if config['admin_status'] == 'up' else ''
command = "vtysh -c 'configure terminal' -c 'router bgp {}' -c '{}neighbor {} shutdown'".format(asn, command_mod, ip)

p = subprocess.Popen(command, shell=True, stdout=subprocess.PIPE)
stdout = p.communicate()[0]
p.wait()
if p.returncode != 0:
syslog.syslog(syslog.LOG_ERR, '[bgp cfgd] command execution returned {}. Command: "{}", stdout: "{}"'.format(p.returncode, command, stdout))

def main():
sub = ConfigDBConnector()
bgp_asn = _get_bgp_asn_from_minigraph()
handler = lambda table, key, data: bgp_config(bgp_asn, key, data)
sub.subscribe('BGP_NEIGHBOR', handler)
sub.connect()
sub.listen()

main()
9 changes: 4 additions & 5 deletions dockers/docker-fpm-quagga/start.sh
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
#!/usr/bin/env bash

mkdir -p /etc/quagga
if [ -f /etc/sonic/bgp_admin.yml ]; then
sonic-cfggen -m /etc/sonic/minigraph.xml -y /etc/sonic/bgp_admin.yml -y /etc/sonic/deployment_id_asn_map.yml -t /usr/share/sonic/templates/bgpd.conf.j2 > /etc/quagga/bgpd.conf
else
sonic-cfggen -m /etc/sonic/minigraph.xml -y /etc/sonic/deployment_id_asn_map.yml -t /usr/share/sonic/templates/bgpd.conf.j2 > /etc/quagga/bgpd.conf
fi
sonic-cfggen -m -d -y /etc/sonic/deployment_id_asn_map.yml -t /usr/share/sonic/templates/bgpd.conf.j2 > /etc/quagga/bgpd.conf

sonic-cfggen -m /etc/sonic/minigraph.xml -t /usr/share/sonic/templates/zebra.conf.j2 > /etc/quagga/zebra.conf

sonic-cfggen -m /etc/sonic/minigraph.xml -t /usr/share/sonic/templates/isolate.j2 > /usr/sbin/bgp-isolate
Expand All @@ -21,6 +18,8 @@ echo "# Config files managed by sonic-config-engine" > /var/sonic/config_status

rm -f /var/run/rsyslogd.pid

supervisorctl start bgpcfgd
Copy link
Collaborator

@lguohan lguohan Aug 1, 2017

Choose a reason for hiding this comment

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

probably need this for docker-fpm-frr too. #WontFix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will resolve in later PRs.


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


supervisorctl start rsyslogd

# Quagga has its own monitor process, 'watchquagga'
Expand Down
9 changes: 9 additions & 0 deletions dockers/docker-fpm-quagga/supervisord.conf
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,12 @@ autostart=false
autorestart=false
stdout_logfile=syslog
stderr_logfile=syslog

[program:bgpcfgd]
command=/usr/bin/bgpcfgd
priority=4
autostart=false
autorestart=false
stdout_logfile=syslog
stderr_logfile=syslog

3 changes: 1 addition & 2 deletions files/build_templates/sonic_debian_extension.j2
Original file line number Diff line number Diff line change
Expand Up @@ -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 %}
Copy link
Collaborator

@lguohan lguohan Jul 19, 2017

Choose a reason for hiding this comment

The 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

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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/
Expand Down
1 change: 1 addition & 0 deletions rules/docker-config-engine.mk
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

DOCKER_CONFIG_ENGINE = docker-config-engine.gz
$(DOCKER_CONFIG_ENGINE)_PATH = $(DOCKERS_PATH)/docker-config-engine
$(DOCKER_CONFIG_ENGINE)_PYTHON_WHEELS += $(SWSSSDK_PY2)
$(DOCKER_CONFIG_ENGINE)_PYTHON_WHEELS += $(SONIC_CONFIG_ENGINE)
$(DOCKER_CONFIG_ENGINE)_LOAD_DOCKERS += $(DOCKER_BASE)
SONIC_DOCKER_IMAGES += $(DOCKER_CONFIG_ENGINE)
3 changes: 2 additions & 1 deletion rules/docker-database.mk
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@
DOCKER_DATABASE = docker-database.gz
$(DOCKER_DATABASE)_PATH = $(DOCKERS_PATH)/docker-database
$(DOCKER_DATABASE)_DEPENDS += $(REDIS_SERVER) $(REDIS_TOOLS)
$(DOCKER_DATABASE)_LOAD_DOCKERS += $(DOCKER_BASE)
$(DOCKER_DATABASE)_LOAD_DOCKERS += $(DOCKER_CONFIG_ENGINE)
SONIC_DOCKER_IMAGES += $(DOCKER_DATABASE)
SONIC_INSTALL_DOCKER_IMAGES += $(DOCKER_DATABASE)

$(DOCKER_DATABASE)_CONTAINER_NAME = database
$(DOCKER_DATABASE)_RUN_OPT += --net=host --privileged -t
$(DOCKER_DATABASE)_RUN_OPT += -v /etc/sonic:/etc/sonic:ro

$(DOCKER_DATABASE)_BASE_IMAGE_FILES += redis-cli:/usr/bin/redis-cli
1 change: 0 additions & 1 deletion rules/docker-platform-monitor.mk
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
DOCKER_PLATFORM_MONITOR = docker-platform-monitor.gz
$(DOCKER_PLATFORM_MONITOR)_PATH = $(DOCKERS_PATH)/docker-platform-monitor
$(DOCKER_PLATFORM_MONITOR)_DEPENDS += $(SONIC_LEDD)
$(DOCKER_PLATFORM_MONITOR)_PYTHON_WHEELS += $(SWSSSDK_PY2)
$(DOCKER_PLATFORM_MONITOR)_LOAD_DOCKERS = $(DOCKER_CONFIG_ENGINE)

SONIC_DOCKER_IMAGES += $(DOCKER_PLATFORM_MONITOR)
Expand Down
1 change: 1 addition & 0 deletions rules/sonic-config.mk
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@

SONIC_CONFIG_ENGINE = sonic_config_engine-1.0-py2-none-any.whl
$(SONIC_CONFIG_ENGINE)_SRC_PATH = $(SRC_PATH)/sonic-config-engine
$(SONIC_CONFIG_ENGINE)_DEPENDS += $(SWSSSDK_PY2)
$(SONIC_CONFIG_ENGINE)_PYTHON_VERSION = 2
SONIC_PYTHON_WHEELS += $(SONIC_CONFIG_ENGINE)
81 changes: 79 additions & 2 deletions src/sonic-config-engine/sonic-cfggen
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
Copy link
Collaborator

@lguohan lguohan Jul 19, 2017

Choose a reason for hiding this comment

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

-d [](start = 21, length = 3)

if we assume by default it will load configdb, then we do not need this option. #WontFix

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

@lguohan lguohan Jul 19, 2017

Choose a reason for hiding this comment

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

onic-cfggen -j db_dump.json --write-to-db [](start = 9, length = 41)

sonic-cfggen --import db_dump.json #WontFix

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Expand All @@ -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:
Expand Down Expand Up @@ -46,18 +62,64 @@ 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')
Copy link
Collaborator

@lguohan lguohan Jul 19, 2017

Choose a reason for hiding this comment

The 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

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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("--write-to-db", help="write config into configdb", action='store_true')
group.add_argument("--print-data", help="print all data", action='store_true')
args = parser.parse_args()

Expand Down Expand Up @@ -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)
Expand All @@ -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(False)
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()
2 changes: 1 addition & 1 deletion src/sonic-py-swsssdk
2 changes: 1 addition & 1 deletion src/sonic-utilities