From 4e38b5d55f4f9a82903584554a02194c899faf1a Mon Sep 17 00:00:00 2001 From: Taoyu Li Date: Mon, 24 Jul 2017 20:20:10 +0000 Subject: [PATCH 1/5] Move BGP configuration into DB --- dockers/docker-fpm-frr/bgpd.conf.j2 | 6 +- dockers/docker-fpm-frr/isolate.j2 | 6 +- dockers/docker-fpm-frr/unisolate.j2 | 6 +- dockers/docker-fpm-gobgp/gobgpd.conf.j2 | 4 +- dockers/docker-fpm-gobgp/isolate.j2 | 6 +- dockers/docker-fpm-gobgp/unisolate.j2 | 6 +- dockers/docker-fpm-quagga/bgpcfgd | 16 +---- dockers/docker-fpm-quagga/bgpd.conf.j2 | 14 ++--- dockers/docker-fpm-quagga/isolate.j2 | 6 +- dockers/docker-fpm-quagga/start.sh | 6 +- dockers/docker-fpm-quagga/unisolate.j2 | 6 +- .../build_templates/sonic_debian_extension.j2 | 2 +- files/image_config/platform/rc.local | 10 ++++ files/image_config/updategraph/updategraph | 7 +++ src/sonic-config-engine/minigraph.py | 25 ++++---- src/sonic-config-engine/sonic-cfggen | 58 ++++++++----------- src/sonic-config-engine/tests/test_cfggen.py | 2 +- 17 files changed, 90 insertions(+), 96 deletions(-) diff --git a/dockers/docker-fpm-frr/bgpd.conf.j2 b/dockers/docker-fpm-frr/bgpd.conf.j2 index 9b8195943444..d964e1096458 100644 --- a/dockers/docker-fpm-frr/bgpd.conf.j2 +++ b/dockers/docker-fpm-frr/bgpd.conf.j2 @@ -18,7 +18,7 @@ log facility local4 ! ! bgp multiple-instance ! -router bgp {{ minigraph_bgp_asn }} +router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }} bgp log-neighbor-changes bgp bestpath as-path multipath-relax {# TODO: use lo[0] for backward compatibility, will revisit the case with multiple lo interfaces #} @@ -46,7 +46,7 @@ router bgp {{ minigraph_bgp_asn }} {% endfor %} {% endblock vlan_advertisement %} {% block bgp_sessions %} -{% for bgp_session in minigraph_bgp %} +{% for bgp_session in BGP_NEIGHBOR.values() %} {% if bgp_session['asn'] != 0 %} neighbor {{ bgp_session['addr'] }} remote-as {{ bgp_session['asn'] }} neighbor {{ bgp_session['addr'] }} description {{ bgp_session['name'] }} @@ -66,5 +66,5 @@ router bgp {{ minigraph_bgp_asn }} maximum-paths 64 ! route-map ISOLATE permit 10 -set as-path prepend {{ minigraph_bgp_asn }} +set as-path prepend {{ DEVICE_METADATA['localhost']['bgp_asn'] }} ! diff --git a/dockers/docker-fpm-frr/isolate.j2 b/dockers/docker-fpm-frr/isolate.j2 index 35ef5bbc0209..ad18160d1be1 100755 --- a/dockers/docker-fpm-frr/isolate.j2 +++ b/dockers/docker-fpm-frr/isolate.j2 @@ -8,13 +8,13 @@ exit $? ## vtysh script start from next line, which line number MUST eqaul in 'sed' command above configure terminal - router bgp {{ minigraph_bgp_asn }} -{% for bgp_session in minigraph_bgp %} + router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }} +{% for bgp_session in BGP_NEIGHBOR.values() %} neighbor {{ bgp_session['addr'] }} route-map ISOLATE out {% endfor %} exit exit -{% for bgp_session in minigraph_bgp %} +{% for bgp_session in BGP_NEIGHBOR.values() %} clear ip bgp {{ bgp_session['addr'] }} soft out {% endfor %} diff --git a/dockers/docker-fpm-frr/unisolate.j2 b/dockers/docker-fpm-frr/unisolate.j2 index c113a74fab45..a3f15b58f1b4 100755 --- a/dockers/docker-fpm-frr/unisolate.j2 +++ b/dockers/docker-fpm-frr/unisolate.j2 @@ -8,13 +8,13 @@ exit $? ## vtysh script start from next line, which line number MUST eqaul in 'sed' command above configure terminal - router bgp {{ minigraph_bgp_asn }} -{% for bgp_session in minigraph_bgp %} + router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }} +{% for bgp_session in BGP_NEIGHBOR.values() %} no neighbor {{ bgp_session['addr'] }} route-map ISOLATE out {% endfor %} exit exit -{% for bgp_session in minigraph_bgp %} +{% for bgp_session in BGP_NEIGHBOR.values() %} clear ip bgp {{ bgp_session['addr'] }} soft out {% endfor %} diff --git a/dockers/docker-fpm-gobgp/gobgpd.conf.j2 b/dockers/docker-fpm-gobgp/gobgpd.conf.j2 index adbc063cfc6c..b035ccd91875 100644 --- a/dockers/docker-fpm-gobgp/gobgpd.conf.j2 +++ b/dockers/docker-fpm-gobgp/gobgpd.conf.j2 @@ -1,7 +1,7 @@ [global.config] - as = {{ minigraph_bgp_asn }} + as = {{ DEVICE_METADATA['localhost']['bgp_asn'] }} router-id = "{{ minigraph_lo_interfaces[0]['addr'] }}" -{% for bgp_session in minigraph_bgp %} +{% for bgp_session in BGP_NEIGHBOR.values() %} {% if bgp_session['asn'] != 0 %} [[neighbors]] [neighbors.config] diff --git a/dockers/docker-fpm-gobgp/isolate.j2 b/dockers/docker-fpm-gobgp/isolate.j2 index e587623e984f..2f395b7b5b36 100755 --- a/dockers/docker-fpm-gobgp/isolate.j2 +++ b/dockers/docker-fpm-gobgp/isolate.j2 @@ -12,13 +12,13 @@ exit $? ## vtysh script start from next line, which line number MUST eqaul in 'sed' command above configure terminal - router bgp {{ minigraph_bgp_asn }} -{% for bgp_session in minigraph_bgp %} + router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }} +{% for bgp_session in BGP_NEIGHBOR.values() %} neighbor {{ bgp_session['addr'] }} route-map ISOLATE out {% endfor %} exit exit -{% for bgp_session in minigraph_bgp %} +{% for bgp_session in BGP_NEIGHBOR.values() %} clear ip bgp {{ bgp_session['addr'] }} soft out {% endfor %} diff --git a/dockers/docker-fpm-gobgp/unisolate.j2 b/dockers/docker-fpm-gobgp/unisolate.j2 index d1310114d908..9e2f7b53db5b 100755 --- a/dockers/docker-fpm-gobgp/unisolate.j2 +++ b/dockers/docker-fpm-gobgp/unisolate.j2 @@ -12,13 +12,13 @@ exit $? ## vtysh script start from next line, which line number MUST eqaul in 'sed' command above configure terminal - router bgp {{ minigraph_bgp_asn }} -{% for bgp_session in minigraph_bgp %} + router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }} +{% for bgp_session in BGP_NEIGHBOR.values() %} no neighbor {{ bgp_session['addr'] }} route-map ISOLATE out {% endfor %} exit exit -{% for bgp_session in minigraph_bgp %} +{% for bgp_session in BGP_NEIGHBOR.values() %} clear ip bgp {{ bgp_session['addr'] }} soft out {% endfor %} diff --git a/dockers/docker-fpm-quagga/bgpcfgd b/dockers/docker-fpm-quagga/bgpcfgd index 2bcecb13f905..db9599749e3f 100755 --- a/dockers/docker-fpm-quagga/bgpcfgd +++ b/dockers/docker-fpm-quagga/bgpcfgd @@ -6,18 +6,6 @@ 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 @@ -33,10 +21,10 @@ def bgp_config(asn, ip, config): def main(): sub = ConfigDBConnector() - bgp_asn = _get_bgp_asn_from_minigraph() + sub.connect() + bgp_asn = sub.get_entry('DEVICE_METADATA', 'localhost')['bgp_asn'] handler = lambda table, key, data: bgp_config(bgp_asn, key, data) sub.subscribe('BGP_NEIGHBOR', handler) - sub.connect() sub.listen() main() diff --git a/dockers/docker-fpm-quagga/bgpd.conf.j2 b/dockers/docker-fpm-quagga/bgpd.conf.j2 index ceb866edd02d..5e88197a5203 100644 --- a/dockers/docker-fpm-quagga/bgpd.conf.j2 +++ b/dockers/docker-fpm-quagga/bgpd.conf.j2 @@ -14,7 +14,7 @@ log facility local4 ! enable password {# {{ en_passwd }} TODO: param needed #} {% endblock system_init %} ! -{% if minigraph_bgp_asn is not none %} +{% if DEVICE_METADATA['localhost'].has_key('bgp_asn') %} {% block bgp_init %} ! ! bgp multiple-instance @@ -23,7 +23,7 @@ route-map FROM_BGP_SPEAKER_V4 permit 10 ! route-map TO_BGP_SPEAKER_V4 deny 10 ! -router bgp {{ minigraph_bgp_asn }} +router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }} bgp log-neighbor-changes bgp bestpath as-path multipath-relax {# Advertise graceful restart capability for ToR #} @@ -50,11 +50,11 @@ router bgp {{ minigraph_bgp_asn }} {% endfor %} {% endblock vlan_advertisement %} {% block bgp_sessions %} -{% for bgp_session in minigraph_bgp %} +{% for bgp_session in BGP_NEIGHBOR.values() %} {% if bgp_session['asn'] != 0 %} neighbor {{ bgp_session['addr'] }} remote-as {{ bgp_session['asn'] }} neighbor {{ bgp_session['addr'] }} description {{ bgp_session['name'] }} -{% if bgp_admin_state and bgp_admin_state.has_key(bgp_session['addr']) and bgp_admin_state[bgp_session['addr']]==False or bgp_admin_state and not bgp_admin_state.has_key(bgp_session['addr']) and bgp_admin_state.has_key('all') and bgp_admin_state['all']==False %} +{% if bgp_session.has_key('admin_status') and bgp_session['admin_status'] == 'down' or not bgp_session.has_key('admin_status') and DEVICE_METADATA['localhost'].has_key('default_bgp_status') and DEVICE_METADATA['localhost']['default_bgp_status'] == 'down' %} neighbor {{ bgp_session['addr'] }} shutdown {% endif %} {% if bgp_session['addr'] | ipv4 %} @@ -75,7 +75,7 @@ router bgp {{ minigraph_bgp_asn }} {% endfor %} {% endblock bgp_sessions %} {% block bgp_peers_with_range %} -{% for bgp_peer in minigraph_bgp_peers_with_range %} +{% for bgp_peer in BGP_PEER_RANGE.values() %} neighbor {{ bgp_peer['name'] }} peer-group neighbor {{ bgp_peer['name'] }} passive neighbor {{ bgp_peer['name'] }} remote-as {{deployment_id_asn_map[deployment_id] }} @@ -90,10 +90,10 @@ router bgp {{ minigraph_bgp_asn }} {% endfor %} {% endblock bgp_peers_with_range %} ! -{% if minigraph_bgp_asn is not none %} +{% if DEVICE_METADATA['localhost'].has_key('bgp_asn') %} maximum-paths 64 ! route-map ISOLATE permit 10 -set as-path prepend {{ minigraph_bgp_asn }} +set as-path prepend {{ DEVICE_METADATA['localhost']['bgp_asn'] }} {% endif %} ! diff --git a/dockers/docker-fpm-quagga/isolate.j2 b/dockers/docker-fpm-quagga/isolate.j2 index 35ef5bbc0209..ad18160d1be1 100755 --- a/dockers/docker-fpm-quagga/isolate.j2 +++ b/dockers/docker-fpm-quagga/isolate.j2 @@ -8,13 +8,13 @@ exit $? ## vtysh script start from next line, which line number MUST eqaul in 'sed' command above configure terminal - router bgp {{ minigraph_bgp_asn }} -{% for bgp_session in minigraph_bgp %} + router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }} +{% for bgp_session in BGP_NEIGHBOR.values() %} neighbor {{ bgp_session['addr'] }} route-map ISOLATE out {% endfor %} exit exit -{% for bgp_session in minigraph_bgp %} +{% for bgp_session in BGP_NEIGHBOR.values() %} clear ip bgp {{ bgp_session['addr'] }} soft out {% endfor %} diff --git a/dockers/docker-fpm-quagga/start.sh b/dockers/docker-fpm-quagga/start.sh index 892c1f6ea974..ca876efe184e 100755 --- a/dockers/docker-fpm-quagga/start.sh +++ b/dockers/docker-fpm-quagga/start.sh @@ -3,13 +3,13 @@ mkdir -p /etc/quagga 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 -d -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 +sonic-cfggen -m -d -t /usr/share/sonic/templates/isolate.j2 > /usr/sbin/bgp-isolate chown root:root /usr/sbin/bgp-isolate chmod 0755 /usr/sbin/bgp-isolate -sonic-cfggen -m /etc/sonic/minigraph.xml -t /usr/share/sonic/templates/unisolate.j2 > /usr/sbin/bgp-unisolate +sonic-cfggen -m -d -t /usr/share/sonic/templates/unisolate.j2 > /usr/sbin/bgp-unisolate chown root:root /usr/sbin/bgp-unisolate chmod 0755 /usr/sbin/bgp-unisolate diff --git a/dockers/docker-fpm-quagga/unisolate.j2 b/dockers/docker-fpm-quagga/unisolate.j2 index c113a74fab45..a3f15b58f1b4 100755 --- a/dockers/docker-fpm-quagga/unisolate.j2 +++ b/dockers/docker-fpm-quagga/unisolate.j2 @@ -8,13 +8,13 @@ exit $? ## vtysh script start from next line, which line number MUST eqaul in 'sed' command above configure terminal - router bgp {{ minigraph_bgp_asn }} -{% for bgp_session in minigraph_bgp %} + router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }} +{% for bgp_session in BGP_NEIGHBOR.values() %} no neighbor {{ bgp_session['addr'] }} route-map ISOLATE out {% endfor %} exit exit -{% for bgp_session in minigraph_bgp %} +{% for bgp_session in BGP_NEIGHBOR.values() %} clear ip bgp {{ bgp_session['addr'] }} soft out {% endfor %} diff --git a/files/build_templates/sonic_debian_extension.j2 b/files/build_templates/sonic_debian_extension.j2 index 3c0804f7abfa..fc84191e3cf1 100644 --- a/files/build_templates/sonic_debian_extension.j2 +++ b/files/build_templates/sonic_debian_extension.j2 @@ -132,7 +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\": { \"all\": false } }' >> $FILESYSTEM_ROOT/etc/sonic/config_db.json" +sudo bash -c "echo '{ \"DEVICE_METADATA\": { \"localhost\": { \"default_bgp_status\": \"down\" } } }' >> $FILESYSTEM_ROOT/etc/sonic/init_cfg.json" {% endif %} # Copy SNMP configuration files sudo cp $IMAGE_CONFIGS/snmp/snmp.yml $FILESYSTEM_ROOT/etc/sonic/ diff --git a/files/image_config/platform/rc.local b/files/image_config/platform/rc.local index dad2510ce756..2b4d420f26bc 100755 --- a/files/image_config/platform/rc.local +++ b/files/image_config/platform/rc.local @@ -35,8 +35,18 @@ if [ -f /host/image-$sonic_version/platform/firsttime ]; then mv -f /host/old_config/* /etc/sonic/ elif [ -f /host/minigraph.xml ]; then mv /host/minigraph.xml /etc/sonic/ + if [ -f /etc/sonic/init_cfg.json ]; then + sonic-cfggen -m -j /etc/sonic/init_cfg.json --print-data > /etc/sonic/config_db.json + else + sonic-cfggen -m --print-data > /etc/sonic/config_db.json + fi else cp /usr/share/sonic/device/$platform/minigraph.xml /etc/sonic/ + if [ -f /etc/sonic/init_cfg.json ]; then + sonic-cfggen -m -j /etc/sonic/init_cfg.json --print-data > /etc/sonic/config_db.json + else + sonic-cfggen -m --print-data > /etc/sonic/config_db.json + fi fi if [ -d /host/image-$sonic_version/platform/$platform ]; then diff --git a/files/image_config/updategraph/updategraph b/files/image_config/updategraph/updategraph index 7240d913c76d..4e9846fba024 100755 --- a/files/image_config/updategraph/updategraph +++ b/files/image_config/updategraph/updategraph @@ -68,6 +68,13 @@ while true; do sleep 5 done +echo "Regenerating config DB from minigraph..." +if [ -f /etc/sonic/init_cfg.json ]; then + sonic-cfggen -m -j /etc/sonic/init_cfg.json --print-data > /etc/sonic/config_db.json +else + sonic-cfggen -m --print-data > /etc/sonic/config_db.json +fi + # Mark as disabled after graph is successfully downloaded sed -i "/enabled=/d" /etc/sonic/updategraph.conf echo "enabled=false" >> /etc/sonic/updategraph.conf diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index 65738a0508f5..e5039f0f3358 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -249,9 +249,9 @@ def parse_dpg(dpg, hname): def parse_cpg(cpg, hname): - bgp_sessions = [] + bgp_sessions = {} myasn = None - bgp_peers_with_range = [] + bgp_peers_with_range = {} for child in cpg: tag = child.tag if tag == str(QName(ns, "PeeringSessions")): @@ -261,17 +261,17 @@ def parse_cpg(cpg, hname): end_router = session.find(str(QName(ns, "EndRouter"))).text end_peer = session.find(str(QName(ns, "EndPeer"))).text if end_router == hname: - bgp_sessions.append({ + bgp_sessions[start_peer] = { 'name': start_router, 'addr': start_peer, 'peer_addr': end_peer - }) + } else: - bgp_sessions.append({ + bgp_sessions[end_peer] = { 'name': end_router, 'addr': end_peer, 'peer_addr': start_peer - }) + } elif child.tag == str(QName(ns, "Routers")): for router in child.findall(str(QName(ns1, "BGPRouterDeclaration"))): asn = router.find(str(QName(ns1, "ASN"))).text @@ -285,12 +285,13 @@ def parse_cpg(cpg, hname): name = bgpPeer.find(str(QName(ns1, "Name"))).text ip_range = bgpPeer.find(str(QName(ns1, "PeersRange"))).text ip_range_group = ip_range.split(';') if ip_range and ip_range != "" else [] - bgp_peers_with_range.append({ + bgp_peers_with_range[name] = { 'name': name, 'ip_range': ip_range_group - }) + } else: - for bgp_session in bgp_sessions: + for peer in bgp_sessions: + bgp_session = bgp_sessions[peer] if hostname == bgp_session['name']: bgp_session['asn'] = int(asn) @@ -447,9 +448,9 @@ def parse_xml(filename, platform=None, port_config_file=None): # sorting by lambdas are not easily done without custom filters. # TODO: add jinja2 filter to accept a lambda to sort a list of dictionaries by attribute. # TODO: alternatively (preferred), implement class containers for multiple-attribute entries, enabling sort by attr - results['minigraph_bgp'] = sorted(bgp_sessions, key=lambda x: x['addr']) - results['minigraph_bgp_asn'] = bgp_asn - results['minigraph_bgp_peers_with_range'] = bgp_peers_with_range + results['BGP_NEIGHBOR'] = bgp_sessions + results['DEVICE_METADATA'] = {'localhost': { 'bgp_asn': bgp_asn }} + results['BGP_PEER_RANGE'] = bgp_peers_with_range # TODO: sort does not work properly on all interfaces of varying lengths. Need to sort by integer group(s). phyport_intfs = [] diff --git a/src/sonic-config-engine/sonic-cfggen b/src/sonic-config-engine/sonic-cfggen index d93b4a6a41dc..070b3100ea5a 100755 --- a/src/sonic-config-engine/sonic-cfggen +++ b/src/sonic-config-engine/sonic-cfggen @@ -73,37 +73,25 @@ TODO(taoyl): Current version of config db only supports BGP admin states. """ @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 + return db_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' + for table_name in output_data: + if table_name == 'BGP_NEIGHBOR' or table_name == 'BGP_PEER_RANGE' or table_name == 'DEVICE_METADATA': + db_data[table_name] = output_data[table_name] return db_data +def deep_update(dst, src): + for key, value in src.iteritems(): + if isinstance(value, dict): + node = dst.setdefault(key, {}) + deep_update(node, value) + else: + dst[key] = value + return dst + def main(): parser=argparse.ArgumentParser(description="Render configuration file from minigraph data and jinja2 template.") @@ -126,7 +114,7 @@ def main(): data = {} machine_info = get_machine_info() if machine_info != None: - data.update(machine_info) + deep_update(data, machine_info) platform_info = get_platform_info(machine_info) if platform_info != None: data['platform'] = platform_info @@ -135,34 +123,34 @@ def main(): minigraph = args.minigraph if data.has_key('platform'): if args.port_config != None: - data.update(parse_xml(minigraph, data['platform'], args.port_config)) + deep_update(data, parse_xml(minigraph, data['platform'], args.port_config)) else: - data.update(parse_xml(minigraph, data['platform'])) + deep_update(data, parse_xml(minigraph, data['platform'])) else: if args.port_config != None: - data.update(parse_xml(minigraph, port_config_file=args.port_config)) + deep_update(data, parse_xml(minigraph, port_config_file=args.port_config)) else: - data.update(parse_xml(minigraph)) + deep_update(data, parse_xml(minigraph)) if args.device_description != None: - data.update(parse_device_desc_xml(args.device_description)) + deep_update(data, parse_device_desc_xml(args.device_description)) for yaml_file in args.yaml: with open(yaml_file, 'r') as stream: additional_data = yaml.load(stream) - data.update(additional_data) + deep_update(data, additional_data) for json_file in args.json: with open(json_file, 'r') as stream: - data.update(json.load(stream)) + deep_update(data, json.load(stream)) if args.additional_data != None: - data.update(json.loads(args.additional_data)) + deep_update(data, json.loads(args.additional_data)) if args.from_db: configdb = ConfigDBConnector() configdb.connect() - data.update(FormatConverter.db_to_output(configdb.get_config())) + deep_update(data, FormatConverter.db_to_output(configdb.get_config())) if args.template != None: template_file = os.path.abspath(args.template) diff --git a/src/sonic-config-engine/tests/test_cfggen.py b/src/sonic-config-engine/tests/test_cfggen.py index 6835c60e27fd..d3ff3e8b9f4e 100644 --- a/src/sonic-config-engine/tests/test_cfggen.py +++ b/src/sonic-config-engine/tests/test_cfggen.py @@ -111,7 +111,7 @@ def test_minigraph_neighbors(self): self.assertEqual(output.strip(), "{'Ethernet116': {'name': 'ARISTA02T1', 'port': 'Ethernet1/1'}, 'Ethernet124': {'name': 'ARISTA04T1', 'port': 'Ethernet1/1'}, 'Ethernet112': {'name': 'ARISTA01T1', 'port': 'Ethernet1/1'}, 'Ethernet120': {'name': 'ARISTA03T1', 'port': 'Ethernet1/1'}}") def test_minigraph_peers_with_range(self): - argument = '-m "' + self.sample_graph_bgp_speaker + '" -p "' + self.port_config + '" -v minigraph_bgp_peers_with_range' + argument = '-m "' + self.sample_graph_bgp_speaker + '" -p "' + self.port_config + '" -v BGP_PEER_RANGE.values\(\)' output = self.run_script(argument) self.assertEqual(output.strip(), "[{'name': 'BGPSLBPassive', 'ip_range': ['10.10.10.10/26', '100.100.100.100/26']}]") From 0ee51ac85fb084e67a57f4b82189c7656f9e1a3d Mon Sep 17 00:00:00 2001 From: Taoyu Li Date: Thu, 27 Jul 2017 17:19:40 +0000 Subject: [PATCH 2/5] [bgp cfg] Use neighbor addr from table key instead of attribute --- dockers/docker-fpm-frr/bgpd.conf.j2 | 12 ++++++------ dockers/docker-fpm-frr/isolate.j2 | 8 ++++---- dockers/docker-fpm-frr/unisolate.j2 | 8 ++++---- dockers/docker-fpm-gobgp/gobgpd.conf.j2 | 6 +++--- dockers/docker-fpm-gobgp/isolate.j2 | 20 -------------------- dockers/docker-fpm-gobgp/unisolate.j2 | 19 ------------------- dockers/docker-fpm-quagga/bgpd.conf.j2 | 18 +++++++++--------- dockers/docker-fpm-quagga/isolate.j2 | 8 ++++---- dockers/docker-fpm-quagga/unisolate.j2 | 8 ++++---- src/sonic-config-engine/minigraph.py | 2 -- 10 files changed, 34 insertions(+), 75 deletions(-) diff --git a/dockers/docker-fpm-frr/bgpd.conf.j2 b/dockers/docker-fpm-frr/bgpd.conf.j2 index d964e1096458..d04f6ac623e0 100644 --- a/dockers/docker-fpm-frr/bgpd.conf.j2 +++ b/dockers/docker-fpm-frr/bgpd.conf.j2 @@ -46,16 +46,16 @@ router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }} {% endfor %} {% endblock vlan_advertisement %} {% block bgp_sessions %} -{% for bgp_session in BGP_NEIGHBOR.values() %} +{% for neighbor_addr, bgp_session in BGP_NEIGHBOR.iteritems() %} {% if bgp_session['asn'] != 0 %} - neighbor {{ bgp_session['addr'] }} remote-as {{ bgp_session['asn'] }} - neighbor {{ bgp_session['addr'] }} description {{ bgp_session['name'] }} + neighbor {{ neighbor_addr }} remote-as {{ bgp_session['asn'] }} + neighbor {{ neighbor_addr }} description {{ bgp_session['name'] }} {% if minigraph_devices[inventory_hostname]['type'] == 'ToRRouter' %} - neighbor {{ bgp_session['addr'] }} allowas-in 1 + neighbor {{ neighbor_addr }} allowas-in 1 {% endif %} -{% if bgp_session['addr'] | ipv6 %} +{% if neighbor_addr | ipv6 %} address-family ipv6 - neighbor {{ bgp_session['addr'] }} activate + neighbor {{ neighbor_addr }} activate maximum-paths 64 exit-address-family {% endif %} diff --git a/dockers/docker-fpm-frr/isolate.j2 b/dockers/docker-fpm-frr/isolate.j2 index ad18160d1be1..7b7fecf9fa9e 100755 --- a/dockers/docker-fpm-frr/isolate.j2 +++ b/dockers/docker-fpm-frr/isolate.j2 @@ -9,12 +9,12 @@ exit $? configure terminal router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }} -{% for bgp_session in BGP_NEIGHBOR.values() %} - neighbor {{ bgp_session['addr'] }} route-map ISOLATE out +{% for neighbor_addr in BGP_NEIGHBOR %} + neighbor {{ neighbor_addr }} route-map ISOLATE out {% endfor %} exit exit -{% for bgp_session in BGP_NEIGHBOR.values() %} -clear ip bgp {{ bgp_session['addr'] }} soft out +{% for neighbor_addr in BGP_NEIGHBOR %} +clear ip bgp {{ neighbor_addr }} soft out {% endfor %} diff --git a/dockers/docker-fpm-frr/unisolate.j2 b/dockers/docker-fpm-frr/unisolate.j2 index a3f15b58f1b4..f2129556747b 100755 --- a/dockers/docker-fpm-frr/unisolate.j2 +++ b/dockers/docker-fpm-frr/unisolate.j2 @@ -9,12 +9,12 @@ exit $? configure terminal router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }} -{% for bgp_session in BGP_NEIGHBOR.values() %} - no neighbor {{ bgp_session['addr'] }} route-map ISOLATE out +{% for neighbor_ip in BGP_NEIGHBOR %} + no neighbor {{ neighbor_ip }} route-map ISOLATE out {% endfor %} exit exit -{% for bgp_session in BGP_NEIGHBOR.values() %} -clear ip bgp {{ bgp_session['addr'] }} soft out +{% for neighbor_ip in BGP_NEIGHBOR %} +clear ip bgp {{ neighbor_ip }} soft out {% endfor %} diff --git a/dockers/docker-fpm-gobgp/gobgpd.conf.j2 b/dockers/docker-fpm-gobgp/gobgpd.conf.j2 index b035ccd91875..c6ec7a4fd166 100644 --- a/dockers/docker-fpm-gobgp/gobgpd.conf.j2 +++ b/dockers/docker-fpm-gobgp/gobgpd.conf.j2 @@ -1,17 +1,17 @@ [global.config] as = {{ DEVICE_METADATA['localhost']['bgp_asn'] }} router-id = "{{ minigraph_lo_interfaces[0]['addr'] }}" -{% for bgp_session in BGP_NEIGHBOR.values() %} +{% for neighbor_addr, bgp_session in BGP_NEIGHBOR.iteritems() %} {% if bgp_session['asn'] != 0 %} [[neighbors]] [neighbors.config] peer-as = {{ bgp_session['asn'] }} - neighbor-address = "{{ bgp_session['addr'] }}" + neighbor-address = "{{ neighbor_addr }}" [neighbors.graceful-restart.config] enabled = true [[neighbors.afi-safis]] [neighbors.afi-safis.config] -{% if bgp_session['addr'] | ipv6 %} +{% if neighbor_addr | ipv6 %} afi-safi-name = "ipv6-unicast" {% else %} afi-safi-name = "ipv4-unicast" diff --git a/dockers/docker-fpm-gobgp/isolate.j2 b/dockers/docker-fpm-gobgp/isolate.j2 index 2f395b7b5b36..4502ff895777 100755 --- a/dockers/docker-fpm-gobgp/isolate.j2 +++ b/dockers/docker-fpm-gobgp/isolate.j2 @@ -2,23 +2,3 @@ echo Not implemented yet exit - -## vtysh only accepts script in stdin, so cannot be directly used in shebang -## Cut the tail of this script and feed vtysh stdin -sed -n -e '9,$p' < "$0" | vtysh "$@" -## Exit with vtysh return code -exit $? - -## vtysh script start from next line, which line number MUST eqaul in 'sed' command above - -configure terminal - router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }} -{% for bgp_session in BGP_NEIGHBOR.values() %} - neighbor {{ bgp_session['addr'] }} route-map ISOLATE out -{% endfor %} - exit -exit - -{% for bgp_session in BGP_NEIGHBOR.values() %} -clear ip bgp {{ bgp_session['addr'] }} soft out -{% endfor %} diff --git a/dockers/docker-fpm-gobgp/unisolate.j2 b/dockers/docker-fpm-gobgp/unisolate.j2 index 9e2f7b53db5b..69172744bae7 100755 --- a/dockers/docker-fpm-gobgp/unisolate.j2 +++ b/dockers/docker-fpm-gobgp/unisolate.j2 @@ -3,22 +3,3 @@ echo Not implemented yet exit -## vtysh only accepts script in stdin, so cannot be directly used in shebang -## Cut the tail of this script and feed vtysh stdin -sed -n -e '9,$p' < "$0" | vtysh "$@" -## Exit with vtysh return code -exit $? - -## vtysh script start from next line, which line number MUST eqaul in 'sed' command above - -configure terminal - router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }} -{% for bgp_session in BGP_NEIGHBOR.values() %} - no neighbor {{ bgp_session['addr'] }} route-map ISOLATE out -{% endfor %} - exit -exit - -{% for bgp_session in BGP_NEIGHBOR.values() %} -clear ip bgp {{ bgp_session['addr'] }} soft out -{% endfor %} diff --git a/dockers/docker-fpm-quagga/bgpd.conf.j2 b/dockers/docker-fpm-quagga/bgpd.conf.j2 index 5e88197a5203..a5c317aa2e80 100644 --- a/dockers/docker-fpm-quagga/bgpd.conf.j2 +++ b/dockers/docker-fpm-quagga/bgpd.conf.j2 @@ -50,24 +50,24 @@ router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }} {% endfor %} {% endblock vlan_advertisement %} {% block bgp_sessions %} -{% for bgp_session in BGP_NEIGHBOR.values() %} +{% for neighbor_addr, bgp_session in BGP_NEIGHBOR.iteritems() %} {% if bgp_session['asn'] != 0 %} - neighbor {{ bgp_session['addr'] }} remote-as {{ bgp_session['asn'] }} - neighbor {{ bgp_session['addr'] }} description {{ bgp_session['name'] }} + neighbor {{ neighbor_addr }} remote-as {{ bgp_session['asn'] }} + neighbor {{ neighbor_addr }} description {{ bgp_session['name'] }} {% if bgp_session.has_key('admin_status') and bgp_session['admin_status'] == 'down' or not bgp_session.has_key('admin_status') and DEVICE_METADATA['localhost'].has_key('default_bgp_status') and DEVICE_METADATA['localhost']['default_bgp_status'] == 'down' %} - neighbor {{ bgp_session['addr'] }} shutdown + neighbor {{ neighbor_addr }} shutdown {% endif %} -{% if bgp_session['addr'] | ipv4 %} +{% if neighbor_addr | ipv4 %} {% if minigraph_devices[inventory_hostname]['type'] == 'ToRRouter' %} - neighbor {{ bgp_session['addr'] }} allowas-in 1 + neighbor {{ neighbor_addr }} allowas-in 1 {% endif %} {% endif %} -{% if bgp_session['addr'] | ipv6 %} +{% if neighbor_addr | ipv6 %} address-family ipv6 {% if minigraph_devices[inventory_hostname]['type'] == 'ToRRouter' %} - neighbor {{ bgp_session['addr'] }} allowas-in 1 + neighbor {{ neighbor_addr }} allowas-in 1 {% endif %} - neighbor {{ bgp_session['addr'] }} activate + neighbor {{ neighbor_addr }} activate maximum-paths 64 exit-address-family {% endif %} diff --git a/dockers/docker-fpm-quagga/isolate.j2 b/dockers/docker-fpm-quagga/isolate.j2 index ad18160d1be1..7b7fecf9fa9e 100755 --- a/dockers/docker-fpm-quagga/isolate.j2 +++ b/dockers/docker-fpm-quagga/isolate.j2 @@ -9,12 +9,12 @@ exit $? configure terminal router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }} -{% for bgp_session in BGP_NEIGHBOR.values() %} - neighbor {{ bgp_session['addr'] }} route-map ISOLATE out +{% for neighbor_addr in BGP_NEIGHBOR %} + neighbor {{ neighbor_addr }} route-map ISOLATE out {% endfor %} exit exit -{% for bgp_session in BGP_NEIGHBOR.values() %} -clear ip bgp {{ bgp_session['addr'] }} soft out +{% for neighbor_addr in BGP_NEIGHBOR %} +clear ip bgp {{ neighbor_addr }} soft out {% endfor %} diff --git a/dockers/docker-fpm-quagga/unisolate.j2 b/dockers/docker-fpm-quagga/unisolate.j2 index a3f15b58f1b4..f2129556747b 100755 --- a/dockers/docker-fpm-quagga/unisolate.j2 +++ b/dockers/docker-fpm-quagga/unisolate.j2 @@ -9,12 +9,12 @@ exit $? configure terminal router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }} -{% for bgp_session in BGP_NEIGHBOR.values() %} - no neighbor {{ bgp_session['addr'] }} route-map ISOLATE out +{% for neighbor_ip in BGP_NEIGHBOR %} + no neighbor {{ neighbor_ip }} route-map ISOLATE out {% endfor %} exit exit -{% for bgp_session in BGP_NEIGHBOR.values() %} -clear ip bgp {{ bgp_session['addr'] }} soft out +{% for neighbor_ip in BGP_NEIGHBOR %} +clear ip bgp {{ neighbor_ip }} soft out {% endfor %} diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index e5039f0f3358..b225a11a4342 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -263,13 +263,11 @@ def parse_cpg(cpg, hname): if end_router == hname: bgp_sessions[start_peer] = { 'name': start_router, - 'addr': start_peer, 'peer_addr': end_peer } else: bgp_sessions[end_peer] = { 'name': end_router, - 'addr': end_peer, 'peer_addr': start_peer } elif child.tag == str(QName(ns, "Routers")): From 6baffa465ac438ea1b5dfee90f6037b00010b1ae Mon Sep 17 00:00:00 2001 From: Taoyu Li Date: Thu, 27 Jul 2017 19:53:24 +0000 Subject: [PATCH 3/5] [bgp cfg] support incremental config for entire BGP_NEIGHBOR table --- dockers/docker-fpm-quagga/bgpcfgd | 65 ++++++++++++++++++++++++------- 1 file changed, 50 insertions(+), 15 deletions(-) diff --git a/dockers/docker-fpm-quagga/bgpcfgd b/dockers/docker-fpm-quagga/bgpcfgd index db9599749e3f..012a766c20b9 100755 --- a/dockers/docker-fpm-quagga/bgpcfgd +++ b/dockers/docker-fpm-quagga/bgpcfgd @@ -6,25 +6,60 @@ import subprocess import syslog from swsssdk import ConfigDBConnector -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) - +class BGPConfigDaemon: + + def __init__(self): + self.config_db = ConfigDBConnector() + self.config_db.connect() + self.bgp_asn = self.config_db.get_entry('DEVICE_METADATA', 'localhost')['bgp_asn'] + self.bgp_neighbor = self.config_db.get_table('BGP_NEIGHBOR') + + def __run_command(self, command): +# print command 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 metadata_handler(self, key, data): + if key == 'localhost' and data.has_key('bgp_asn'): + if data['bgp_asn'] != self.bgp_asn: + syslog.syslog(syslog.LOG_INFO, '[bgp cfgd] ASN changed to {} from {}, restart BGP...'.format(data['bgp_asn'], self.bgp_asn)) + self.__run_command("supervisorctl restart start.sh") + self.__run_command("service quagga restart") + self.bgp_asn = data['bgp_asn'] + + def bgp_handler(self, key, data): + syslog.syslog(syslog.LOG_INFO, '[bgp cfgd] value for {} changed to {}'.format(key, data)) + if not data: + # Neighbor is deleted + command = "vtysh -c 'configure terminal' -c 'router bgp {}' -c 'no neighbor {}'".format(self.bgp_asn, key) + self.__run_command(command) + self.bgp_neighbor.pop(key) + else: + command = "vtysh -c 'configure terminal' -c 'router bgp {}' -c 'neighbor {} remote-as {}'".format(self.bgp_asn, key, data['asn']) + self.__run_command(command) + if data.has_key('name'): + command = "vtysh -c 'configure terminal' -c 'router bgp {}' -c 'neighbor {} description {}'".format(self.bgp_asn, key, data['name']) + self.__run_command(command) + if data.has_key('admin_status'): + command_mod = 'no ' if data['admin_status'] == 'up' else '' + command = "vtysh -c 'configure terminal' -c 'router bgp {}' -c '{}neighbor {} shutdown'".format(self.bgp_asn, command_mod, key) + self.__run_command(command) + self.bgp_neighbor[key] = data + + def start(self): + self.config_db.subscribe('BGP_NEIGHBOR', + lambda table, key, data: self.bgp_handler(key, data)) + self.config_db.subscribe('DEVICE_METADATA', + lambda table, key, data: self.metadata_handler(key, data)) + self.config_db.listen() + + def main(): - sub = ConfigDBConnector() - sub.connect() - bgp_asn = sub.get_entry('DEVICE_METADATA', 'localhost')['bgp_asn'] - handler = lambda table, key, data: bgp_config(bgp_asn, key, data) - sub.subscribe('BGP_NEIGHBOR', handler) - sub.listen() - -main() + daemon = BGPConfigDaemon() + daemon.start() + +if __name__ == "__main__": + main() From 05826b4ca8db67431093b9e757584a9f15c0c726 Mon Sep 17 00:00:00 2001 From: Taoyu Li Date: Wed, 2 Aug 2017 02:06:51 +0000 Subject: [PATCH 4/5] Update sonic-utilities pointer --- src/sonic-utilities | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sonic-utilities b/src/sonic-utilities index 6c7e22362fbc..e4f7161b055a 160000 --- a/src/sonic-utilities +++ b/src/sonic-utilities @@ -1 +1 @@ -Subproject commit 6c7e22362fbc05ba455e7e336e2a88430de0de18 +Subproject commit e4f7161b055a345813424004f17b116a055d590a From da12361104c004ffd20e6efd62122da3dadcbade Mon Sep 17 00:00:00 2001 From: Taoyu Li Date: Tue, 8 Aug 2017 21:46:50 +0000 Subject: [PATCH 5/5] Resolve CR comments --- files/image_config/platform/rc.local | 2 ++ src/sonic-config-engine/minigraph.py | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/files/image_config/platform/rc.local b/files/image_config/platform/rc.local index 2b4d420f26bc..49b2ed6e1b69 100755 --- a/files/image_config/platform/rc.local +++ b/files/image_config/platform/rc.local @@ -35,6 +35,8 @@ if [ -f /host/image-$sonic_version/platform/firsttime ]; then mv -f /host/old_config/* /etc/sonic/ elif [ -f /host/minigraph.xml ]; then mv /host/minigraph.xml /etc/sonic/ + # Combine information in minigraph and init_cfg.json to form initiate config DB dump file. + # TODO: After moving all information from minigraph to DB, sample config DB dump should be provide if [ -f /etc/sonic/init_cfg.json ]; then sonic-cfggen -m -j /etc/sonic/init_cfg.json --print-data > /etc/sonic/config_db.json else diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index b225a11a4342..6dbf0d570d47 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -263,12 +263,12 @@ def parse_cpg(cpg, hname): if end_router == hname: bgp_sessions[start_peer] = { 'name': start_router, - 'peer_addr': end_peer + 'local_addr': end_peer } else: bgp_sessions[end_peer] = { 'name': end_router, - 'peer_addr': start_peer + 'local_addr': start_peer } elif child.tag == str(QName(ns, "Routers")): for router in child.findall(str(QName(ns1, "BGPRouterDeclaration"))):