From 4d3a4c640f77d7ee2363946e9e368191b24b9ef9 Mon Sep 17 00:00:00 2001 From: Tejaswini Chadaga Date: Fri, 24 Jun 2022 10:41:20 +0000 Subject: [PATCH 01/12] TSA persistence changes for reload --- dockers/docker-fpm-frr/base_image_files/TS | 19 ++- dockers/docker-fpm-frr/base_image_files/TSA | 24 ++-- dockers/docker-fpm-frr/base_image_files/TSB | 21 ++-- files/build_templates/init_cfg.json.j2 | 5 + src/sonic-bgpcfgd/bgpcfgd/main.py | 3 + src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py | 8 +- .../bgpcfgd/managers_device_global.py | 111 ++++++++++++++++++ .../yang-models/sonic-bgp-device-global.yang | 36 ++++++ 8 files changed, 207 insertions(+), 20 deletions(-) create mode 100644 src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py create mode 100644 src/sonic-yang-models/yang-models/sonic-bgp-device-global.yang diff --git a/dockers/docker-fpm-frr/base_image_files/TS b/dockers/docker-fpm-frr/base_image_files/TS index de1e50b7a306..e7c76306e848 100755 --- a/dockers/docker-fpm-frr/base_image_files/TS +++ b/dockers/docker-fpm-frr/base_image_files/TS @@ -5,6 +5,12 @@ PLATFORM=${PLATFORM:-`sonic-cfggen -H -v DEVICE_METADATA.localhost.platform`} +if [[ $1 == "TSA" ]]; then + TSA_STATE_UPDATE='{"BGP_DEVICE_GLOBAL":{"STATE":{"tsa_enabled": "True"}}}' +elif [[ $1 == "TSB" ]]; then + TSA_STATE_UPDATE='{"BGP_DEVICE_GLOBAL":{"STATE":{"tsa_enabled": "False"}}}' +fi + # Parse the device specific asic conf file, if it exists ASIC_CONF=/usr/share/sonic/device/$PLATFORM/asic.conf [ -f $ASIC_CONF ] && . $ASIC_CONF @@ -20,10 +26,19 @@ if [[ ($NUM_ASIC -gt 1) ]]; then if [ $sub_role == 'FrontEnd' ] then echo -e "BGP"$asic" : \c" - docker exec -i bgp$asic /usr/bin/$1 + if [[ -n "$TSA_STATE_UPDATE" ]]; then + sonic-cfggen -a "$TSA_STATE_UPDATE" -w -n $NAMESPACE_PREFIX$asic + else + docker exec -i bgp$asic /usr/bin/$1 + fi fi asic=$[$asic+1] done else - docker exec -i bgp /usr/bin/$1 + if [[ -n "$TSA_STATE_UPDATE" ]]; then + sonic-cfggen -a "$TSA_STATE_UPDATE" -w + else + docker exec -i bgp /usr/bin/$1 + fi fi + diff --git a/dockers/docker-fpm-frr/base_image_files/TSA b/dockers/docker-fpm-frr/base_image_files/TSA index 6b2ddb264834..737e048c465f 100755 --- a/dockers/docker-fpm-frr/base_image_files/TSA +++ b/dockers/docker-fpm-frr/base_image_files/TSA @@ -1,12 +1,20 @@ #!/bin/bash -# toggle the mux to standby if dualtor and any mux active -if -[[ "$(sonic-cfggen -d -v DEVICE_METADATA.localhost.subtype | tr [:upper:] [:lower:])" == *"dualtor"* ]] && -[[ $(show mux status | grep active | wc -l) > 0 ]]; -then - logger -t TSA -p user.info "Toggle all mux mode to standby" - sudo config mux mode standby all +if [[ "$(sonic-cfggen -d -v BGP_DEVICE_GLOBAL.STATE.tsa_enabled)" == "True" ]]; then + echo "System is already in Maintenance" +else + # toggle the mux to standby if dualtor and any mux active + if + [[ "$(sonic-cfggen -d -v DEVICE_METADATA.localhost.subtype | tr [:upper:] [:lower:])" == *"dualtor"* ]] && + [[ $(show mux status | grep active | wc -l) > 0 ]]; + then + logger -t TSA -p user.info "Toggle all mux mode to standby" + sudo config mux mode standby all + fi + + /usr/bin/TS TSA + echo "System Mode: Normal -> Maintenance" + echo "" + echo "Please execute 'config save' to preserve System mode in Maintenance after reboot or config reload" fi -/usr/bin/TS TSA diff --git a/dockers/docker-fpm-frr/base_image_files/TSB b/dockers/docker-fpm-frr/base_image_files/TSB index 3fed7bb644f5..517bc89087f7 100755 --- a/dockers/docker-fpm-frr/base_image_files/TSB +++ b/dockers/docker-fpm-frr/base_image_files/TSB @@ -1,10 +1,17 @@ #!/bin/bash -# toggle the mux to auto if dualtor -if [[ "$(sonic-cfggen -d -v DEVICE_METADATA.localhost.subtype | tr [:upper:] [:lower:])" == *"dualtor"* ]]; -then - logger -t TSB -p user.info "Toggle all mux mode to auto" - sudo config mux mode auto all -fi +if [[ "$(sonic-cfggen -d -v BGP_DEVICE_GLOBAL.STATE.tsa_enabled)" == "False" ]]; then + echo "System is already in Normal mode" +else + # toggle the mux to auto if dualtor + if [[ "$(sonic-cfggen -d -v DEVICE_METADATA.localhost.subtype | tr [:upper:] [:lower:])" == *"dualtor"* ]]; + then + logger -t TSB -p user.info "Toggle all mux mode to auto" + sudo config mux mode auto all + fi -/usr/bin/TS TSB + /usr/bin/TS TSB + echo "System Mode: Maintenance -> Normal" + echo "" + echo "Please execute 'config save' to preserve System mode in Normal state after reboot or config reload" +fi diff --git a/files/build_templates/init_cfg.json.j2 b/files/build_templates/init_cfg.json.j2 index ca5a0756f954..fd20d16cfd6e 100644 --- a/files/build_templates/init_cfg.json.j2 +++ b/files/build_templates/init_cfg.json.j2 @@ -27,6 +27,11 @@ "POLL_INTERVAL": "10000" } }, + "BGP_DEVICE_GLOBAL": { + "STATE": { + "tsa_enabled": "False" + } + }, {%- set features = [("bgp", "enabled", false, "enabled"), ("database", "always_enabled", false, "always_enabled"), ("lldp", "enabled", true, "enabled"), diff --git a/src/sonic-bgpcfgd/bgpcfgd/main.py b/src/sonic-bgpcfgd/bgpcfgd/main.py index 7b4291b4d4a3..20488e297510 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/main.py +++ b/src/sonic-bgpcfgd/bgpcfgd/main.py @@ -18,6 +18,7 @@ from .managers_setsrc import ZebraSetSrc from .managers_static_rt import StaticRouteMgr from .managers_rm import RouteMapMgr +from .managers_device_global import DeviceGlobalCfgMgr from .runner import Runner, signal_handler from .template import TemplateFabric from .utils import read_constants @@ -64,6 +65,8 @@ def do_work(): # Route Advertisement Managers AdvertiseRouteMgr(common_objs, "STATE_DB", swsscommon.STATE_ADVERTISE_NETWORK_TABLE_NAME), RouteMapMgr(common_objs, "APPL_DB", swsscommon.APP_BGP_PROFILE_TABLE_NAME), + # Device Global Manager + DeviceGlobalCfgMgr(common_objs, "CONFIG_DB", "BGP_DEVICE_GLOBAL"), ] runner = Runner(common_objs['cfg_mgr']) for mgr in managers: diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py b/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py index c787ae2abe69..b153440c17d5 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py @@ -8,7 +8,7 @@ from .manager import Manager from .template import TemplateFabric from .utils import run_command - +from .managers_device_global import DeviceGlobalCfgMgr class BGPPeerGroupMgr(object): """ This class represents peer-group and routing policy for the peer_type """ @@ -23,6 +23,7 @@ def __init__(self, common_objs, base_template): tf = common_objs['tf'] self.policy_template = tf.from_file(base_template + "policies.conf.j2") self.peergroup_template = tf.from_file(base_template + "peer-group.conf.j2") + self.device_global_cfgmgr = DeviceGlobalCfgMgr(common_objs, "CONFIG_DB", "BGP_DEVICE_GLOBAL") def update(self, name, **kwargs): """ @@ -56,14 +57,15 @@ def update_pg(self, name, **kwargs): """ try: pg = self.peergroup_template.render(**kwargs) + tsa_rm = self.device_global_cfgmgr.check_state_and_update_tsa_routemaps(pg) except jinja2.TemplateError as e: log_err("Can't render peer-group template: '%s': %s" % (name, str(e))) return False if kwargs['vrf'] == 'default': - cmd = ('router bgp %s\n' % kwargs['bgp_asn']) + pg + cmd = ('router bgp %s\n' % kwargs['bgp_asn']) + pg + tsa_rm else: - cmd = ('router bgp %s vrf %s\n' % (kwargs['bgp_asn'], kwargs['vrf'])) + pg + cmd = ('router bgp %s vrf %s\n' % (kwargs['bgp_asn'], kwargs['vrf'])) + pg + tsa_rm self.update_entity(cmd, "Peer-group for peer '%s'" % name) return True diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py b/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py new file mode 100644 index 000000000000..602f72a54651 --- /dev/null +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py @@ -0,0 +1,111 @@ +from .manager import Manager +from .log import log_err, log_debug +from swsscommon import swsscommon +import subprocess +import re + +class DeviceGlobalCfgMgr(Manager): + """This class responds to change in device-specific state""" + + def __init__(self, common_objs, db, table): + """ + Initialize the object + :param common_objs: common object dictionary + :param db: name of the db + :param table: name of the table in the db + """ + self.directory = common_objs['directory'] + self.cfg_mgr = common_objs['cfg_mgr'] + self.constants = common_objs['constants'] + self.tsa_template = common_objs['tf'].from_file("bgpd/tsa/bgpd.tsa.isolate.conf.j2") + self.tsb_template = common_objs['tf'].from_file("bgpd/tsa/bgpd.tsa.unisolate.conf.j2") + super(DeviceGlobalCfgMgr, self).__init__( + common_objs, + [], + db, + table, + ) + + def set_handler(self, key, data): + log_debug("DeviceGlobalCfgMgr:: set handler") + """ Handle device tsa_enabled state change """ + if not data: + log_err("DeviceGlobalCfgMgr:: data is None") + return False + + if "tsa_enabled" in data: + self.cfg_mgr.commit() + self.isolate_unisolate_device(data["tsa_enabled"]) + self.directory.put(self.db_name, self.table_name, "tsa_enabled", data["tsa_enabled"]) + return True + return False + + def del_handler(self, key): + log_debug("DeviceGlobalCfgMgr:: del handler") + return True + + def check_state_and_get_tsa_routemaps(self, cmds): + """ API to get TSA route-maps if device is isolated""" + cmd = "" + if self.directory.path_exist("CONFIG_DB", "BGP_DEVICE_GLOBAL", "tsa_enabled"): + tsa_status = self.directory.get_slot("CONFIG_DB", "BGP_DEVICE_GLOBAL")["tsa_enabled"] + if tsa_status == "True": + log_debug("DeviceGlobalCfgMgr:: Device is isolated. Applying TSA route-maps") + cmd = self.get_tsa_routemaps(cmds) + return cmd + + def isolate_unisolate_device(self, tsa_status): + """ API to get TSA/TSB route-maps and apply configuration""" + cmd = "\n" + if tsa_status == "True": + log_debug("DeviceGlobalCfgMgr:: Device isolated. Executing TSA") + cmd += self.get_tsa_routemaps(self.cfg_mgr.get_text()) + else: + log_debug("DeviceGlobalCfgMgr:: Device un-isolated. Executing TSB") + cmd += self.get_tsb_routemaps(self.cfg_mgr.get_text()) + + self.cfg_mgr.push(cmd) + log_debug("DeviceGlobalCfgMgr::Done") + + def get_tsa_routemaps(self, cfg): + route_map_names = self.__extract_out_route_map_names(cfg) + return self.__generate_routemaps_from_template(route_map_names, self.tsa_template) + + def get_tsb_routemaps(self, cfg): + route_map_names = self.__extract_out_route_map_names(cfg) + return self.__generate_routemaps_from_template(route_map_names, self.tsb_template) + + def __generate_routemaps_from_template(self, route_map_names, template): + cmd = "\n" + for rm in route_map_names: + if "_INTERNAL_" in rm: + continue + if "V4" in rm: + ipv="V4" ; ipp="ip" + elif "V6" in rm: + ipv="V6" ; ipp="ipv6" + else: + continue + cmd += template.render(route_map_name=rm,ip_version=ipv,ip_protocol=ipp, constants=self.constants) + log_debug("DeviceGlobalCfgMgr::get_tsa_routemaps:: Done") + return cmd + + def __extract_out_route_map_names(self, cfg): + route_map_names = [] + out_route_map = re.compile(r'^\s*neighbor \S+ route-map (\S+) out$') + for line in cfg.replace("#012", "\n").split("\n"): + result = out_route_map.match(line) + if result: + route_map_names.append(result.group(1)) + return route_map_names + + def __update_traffic_shift_using_cmd(self, tsa_status): + if tsa_status == "True": + log_debug("DeviceGlobalCfgMgr:: Device isolated. Executing TSA") + subprocess.run(['TSA']) + else: + log_debug("DeviceGlobalCfgMgr:: Device un-isolated. Executing TSB") + subprocess.run(['TSB']) + log_debug("DeviceGlobalCfgMgr::Done") + + diff --git a/src/sonic-yang-models/yang-models/sonic-bgp-device-global.yang b/src/sonic-yang-models/yang-models/sonic-bgp-device-global.yang new file mode 100644 index 000000000000..b0c9215bf590 --- /dev/null +++ b/src/sonic-yang-models/yang-models/sonic-bgp-device-global.yang @@ -0,0 +1,36 @@ +module sonic-bgp-device-global { + namespace "http://github.com/Azure/sonic-bgp-device-global"; + prefix bgp_device_global; + yang-version 1.1; + + organization + "SONiC"; + + contact + "SONiC"; + + description + "SONIC Device-specific BGP global data"; + + revision 2022-06-07 { + description + "Initial revision"; + } + + container sonic-bgp-device-global { + container BGP_DEVICE_GLOBAL { + container STATE { + description: "BGP device-specific global data" + leaf tsa_enabled { + type boolean; + default "False"; + description + "When set to true, Traffic is shifted away (TSA), i.e, BGP routes are not advertised to neighboring routers"; + } + } /* end of STATE container */ + } /* end of BGP_DEVICE_GLOBAL container */ + + } /* end of top level container */ + +} /* end of module sonic-bgp-device-global */ + From 0ae596f9ad58dc78a0a0b2e890478bbb39a5ac1f Mon Sep 17 00:00:00 2001 From: Tejaswini Chadaga Date: Sat, 25 Jun 2022 02:39:41 +0000 Subject: [PATCH 02/12] added unit test code and other fixes --- dockers/docker-fpm-frr/base_image_files/TS | 4 +- dockers/docker-fpm-frr/base_image_files/TSA | 2 +- dockers/docker-fpm-frr/base_image_files/TSB | 2 +- files/build_templates/init_cfg.json.j2 | 2 +- src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py | 2 +- .../bgpcfgd/managers_device_global.py | 15 +-- .../peer-group.conf/result_all_isolate.conf | 33 +++++++ .../peer-group.conf/result_all_unisolate.conf | 29 ++++++ .../peer-group.conf/result_isolate.conf | 11 +++ .../peer-group.conf/result_unisolate.conf | 7 ++ src/sonic-bgpcfgd/tests/test_device_global.py | 93 +++++++++++++++++++ .../yang-models/sonic-bgp-device-global.yang | 4 +- 12 files changed, 184 insertions(+), 20 deletions(-) create mode 100644 src/sonic-bgpcfgd/tests/data/general/peer-group.conf/result_all_isolate.conf create mode 100644 src/sonic-bgpcfgd/tests/data/general/peer-group.conf/result_all_unisolate.conf create mode 100644 src/sonic-bgpcfgd/tests/data/general/peer-group.conf/result_isolate.conf create mode 100644 src/sonic-bgpcfgd/tests/data/general/peer-group.conf/result_unisolate.conf create mode 100644 src/sonic-bgpcfgd/tests/test_device_global.py diff --git a/dockers/docker-fpm-frr/base_image_files/TS b/dockers/docker-fpm-frr/base_image_files/TS index e7c76306e848..e4e917850408 100755 --- a/dockers/docker-fpm-frr/base_image_files/TS +++ b/dockers/docker-fpm-frr/base_image_files/TS @@ -6,9 +6,9 @@ PLATFORM=${PLATFORM:-`sonic-cfggen -H -v DEVICE_METADATA.localhost.platform`} if [[ $1 == "TSA" ]]; then - TSA_STATE_UPDATE='{"BGP_DEVICE_GLOBAL":{"STATE":{"tsa_enabled": "True"}}}' + TSA_STATE_UPDATE='{"BGP_DEVICE_GLOBAL":{"STATE":{"tsa_enabled": "true"}}}' elif [[ $1 == "TSB" ]]; then - TSA_STATE_UPDATE='{"BGP_DEVICE_GLOBAL":{"STATE":{"tsa_enabled": "False"}}}' + TSA_STATE_UPDATE='{"BGP_DEVICE_GLOBAL":{"STATE":{"tsa_enabled": "false"}}}' fi # Parse the device specific asic conf file, if it exists diff --git a/dockers/docker-fpm-frr/base_image_files/TSA b/dockers/docker-fpm-frr/base_image_files/TSA index 737e048c465f..7c49cb56de04 100755 --- a/dockers/docker-fpm-frr/base_image_files/TSA +++ b/dockers/docker-fpm-frr/base_image_files/TSA @@ -1,6 +1,6 @@ #!/bin/bash -if [[ "$(sonic-cfggen -d -v BGP_DEVICE_GLOBAL.STATE.tsa_enabled)" == "True" ]]; then +if [[ "$(sonic-cfggen -d -v BGP_DEVICE_GLOBAL.STATE.tsa_enabled)" == "true" ]]; then echo "System is already in Maintenance" else # toggle the mux to standby if dualtor and any mux active diff --git a/dockers/docker-fpm-frr/base_image_files/TSB b/dockers/docker-fpm-frr/base_image_files/TSB index 517bc89087f7..15ff96c1a4d2 100755 --- a/dockers/docker-fpm-frr/base_image_files/TSB +++ b/dockers/docker-fpm-frr/base_image_files/TSB @@ -1,6 +1,6 @@ #!/bin/bash -if [[ "$(sonic-cfggen -d -v BGP_DEVICE_GLOBAL.STATE.tsa_enabled)" == "False" ]]; then +if [[ "$(sonic-cfggen -d -v BGP_DEVICE_GLOBAL.STATE.tsa_enabled)" == "false" ]]; then echo "System is already in Normal mode" else # toggle the mux to auto if dualtor diff --git a/files/build_templates/init_cfg.json.j2 b/files/build_templates/init_cfg.json.j2 index fd20d16cfd6e..c4e54f306b4e 100644 --- a/files/build_templates/init_cfg.json.j2 +++ b/files/build_templates/init_cfg.json.j2 @@ -29,7 +29,7 @@ }, "BGP_DEVICE_GLOBAL": { "STATE": { - "tsa_enabled": "False" + "tsa_enabled": "false" } }, {%- set features = [("bgp", "enabled", false, "enabled"), diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py b/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py index b153440c17d5..9657c32d5aea 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py @@ -57,7 +57,7 @@ def update_pg(self, name, **kwargs): """ try: pg = self.peergroup_template.render(**kwargs) - tsa_rm = self.device_global_cfgmgr.check_state_and_update_tsa_routemaps(pg) + tsa_rm = self.device_global_cfgmgr.check_state_and_get_tsa_routemaps(pg) except jinja2.TemplateError as e: log_err("Can't render peer-group template: '%s': %s" % (name, str(e))) return False diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py b/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py index 602f72a54651..3a60e2cbfb87 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py @@ -49,7 +49,7 @@ def check_state_and_get_tsa_routemaps(self, cmds): cmd = "" if self.directory.path_exist("CONFIG_DB", "BGP_DEVICE_GLOBAL", "tsa_enabled"): tsa_status = self.directory.get_slot("CONFIG_DB", "BGP_DEVICE_GLOBAL")["tsa_enabled"] - if tsa_status == "True": + if tsa_status == "true": log_debug("DeviceGlobalCfgMgr:: Device is isolated. Applying TSA route-maps") cmd = self.get_tsa_routemaps(cmds) return cmd @@ -57,7 +57,7 @@ def check_state_and_get_tsa_routemaps(self, cmds): def isolate_unisolate_device(self, tsa_status): """ API to get TSA/TSB route-maps and apply configuration""" cmd = "\n" - if tsa_status == "True": + if tsa_status == "true": log_debug("DeviceGlobalCfgMgr:: Device isolated. Executing TSA") cmd += self.get_tsa_routemaps(self.cfg_mgr.get_text()) else: @@ -87,6 +87,7 @@ def __generate_routemaps_from_template(self, route_map_names, template): else: continue cmd += template.render(route_map_name=rm,ip_version=ipv,ip_protocol=ipp, constants=self.constants) + cmd += "\n" log_debug("DeviceGlobalCfgMgr::get_tsa_routemaps:: Done") return cmd @@ -99,13 +100,3 @@ def __extract_out_route_map_names(self, cfg): route_map_names.append(result.group(1)) return route_map_names - def __update_traffic_shift_using_cmd(self, tsa_status): - if tsa_status == "True": - log_debug("DeviceGlobalCfgMgr:: Device isolated. Executing TSA") - subprocess.run(['TSA']) - else: - log_debug("DeviceGlobalCfgMgr:: Device un-isolated. Executing TSB") - subprocess.run(['TSB']) - log_debug("DeviceGlobalCfgMgr::Done") - - diff --git a/src/sonic-bgpcfgd/tests/data/general/peer-group.conf/result_all_isolate.conf b/src/sonic-bgpcfgd/tests/data/general/peer-group.conf/result_all_isolate.conf new file mode 100644 index 000000000000..a078dadd6f04 --- /dev/null +++ b/src/sonic-bgpcfgd/tests/data/general/peer-group.conf/result_all_isolate.conf @@ -0,0 +1,33 @@ +! +! template: bgpd/templates/general/peer-group.conf.j2 +! + neighbor PEER_V4 peer-group + neighbor PEER_V6 peer-group + address-family ipv4 + neighbor PEER_V4 allowas-in 1 + neighbor PEER_V4 soft-reconfiguration inbound + neighbor PEER_V4 route-map FROM_BGP_PEER_V4 in + neighbor PEER_V4 route-map TO_BGP_PEER_V4 out + exit-address-family + address-family ipv6 + neighbor PEER_V6 allowas-in 1 + neighbor PEER_V6 soft-reconfiguration inbound + neighbor PEER_V6 route-map FROM_BGP_PEER_V6 in + neighbor PEER_V6 route-map TO_BGP_PEER_V6 out + exit-address-family +! +! end of template: bgpd/templates/general/peer-group.conf.j2 +! + + +route-map TO_BGP_PEER_V4 permit 20 + match ip address prefix-list PL_LoopbackV4 + set community 12345:12345 +route-map TO_BGP_PEER_V4 deny 30 +! +route-map TO_BGP_PEER_V6 permit 20 + match ipv6 address prefix-list PL_LoopbackV6 + set community 12345:12345 +route-map TO_BGP_PEER_V6 deny 30 +! + diff --git a/src/sonic-bgpcfgd/tests/data/general/peer-group.conf/result_all_unisolate.conf b/src/sonic-bgpcfgd/tests/data/general/peer-group.conf/result_all_unisolate.conf new file mode 100644 index 000000000000..1cd4442f4f3d --- /dev/null +++ b/src/sonic-bgpcfgd/tests/data/general/peer-group.conf/result_all_unisolate.conf @@ -0,0 +1,29 @@ +! +! template: bgpd/templates/general/peer-group.conf.j2 +! + neighbor PEER_V4 peer-group + neighbor PEER_V6 peer-group + address-family ipv4 + neighbor PEER_V4 allowas-in 1 + neighbor PEER_V4 soft-reconfiguration inbound + neighbor PEER_V4 route-map FROM_BGP_PEER_V4 in + neighbor PEER_V4 route-map TO_BGP_PEER_V4 out + exit-address-family + address-family ipv6 + neighbor PEER_V6 allowas-in 1 + neighbor PEER_V6 soft-reconfiguration inbound + neighbor PEER_V6 route-map FROM_BGP_PEER_V6 in + neighbor PEER_V6 route-map TO_BGP_PEER_V6 out + exit-address-family +! +! end of template: bgpd/templates/general/peer-group.conf.j2 +! + + +no route-map TO_BGP_PEER_V4 permit 20 +no route-map TO_BGP_PEER_V4 deny 30 +! +no route-map TO_BGP_PEER_V6 permit 20 +no route-map TO_BGP_PEER_V6 deny 30 +! + diff --git a/src/sonic-bgpcfgd/tests/data/general/peer-group.conf/result_isolate.conf b/src/sonic-bgpcfgd/tests/data/general/peer-group.conf/result_isolate.conf new file mode 100644 index 000000000000..902b8cfcdab9 --- /dev/null +++ b/src/sonic-bgpcfgd/tests/data/general/peer-group.conf/result_isolate.conf @@ -0,0 +1,11 @@ + +route-map TO_BGP_PEER_V4 permit 20 + match ip address prefix-list PL_LoopbackV4 + set community 12345:12345 +route-map TO_BGP_PEER_V4 deny 30 +! +route-map TO_BGP_PEER_V6 permit 20 + match ipv6 address prefix-list PL_LoopbackV6 + set community 12345:12345 +route-map TO_BGP_PEER_V6 deny 30 +! diff --git a/src/sonic-bgpcfgd/tests/data/general/peer-group.conf/result_unisolate.conf b/src/sonic-bgpcfgd/tests/data/general/peer-group.conf/result_unisolate.conf new file mode 100644 index 000000000000..8fd9fde7f759 --- /dev/null +++ b/src/sonic-bgpcfgd/tests/data/general/peer-group.conf/result_unisolate.conf @@ -0,0 +1,7 @@ + +no route-map TO_BGP_PEER_V4 permit 20 +no route-map TO_BGP_PEER_V4 deny 30 +! +no route-map TO_BGP_PEER_V6 permit 20 +no route-map TO_BGP_PEER_V6 deny 30 +! diff --git a/src/sonic-bgpcfgd/tests/test_device_global.py b/src/sonic-bgpcfgd/tests/test_device_global.py new file mode 100644 index 000000000000..da06d425c791 --- /dev/null +++ b/src/sonic-bgpcfgd/tests/test_device_global.py @@ -0,0 +1,93 @@ +from unittest.mock import MagicMock, patch + +import os +from bgpcfgd.directory import Directory +from bgpcfgd.template import TemplateFabric +from . import swsscommon_test +from .util import load_constants +import bgpcfgd.managers_device_global + +TEMPLATE_PATH = os.path.abspath('../../dockers/docker-fpm-frr/frr') +BASE_PATH = os.path.abspath('../sonic-bgpcfgd/tests/data/general/peer-group.conf/') + +def constructor(): + cfg_mgr = MagicMock() + def get_text(): + return cfg_mgr.changes + def update(): + cfg_mgr.changes = get_string_from_file("/result_all.conf") + def push(cfg): + cfg_mgr.changes += cfg + "\n" + cfg_mgr.get_text = get_text + cfg_mgr.update = update + cfg_mgr.push = push + + constants = load_constants()['constants'] + common_objs = { + 'directory': Directory(), + 'cfg_mgr': cfg_mgr, + 'tf': TemplateFabric(TEMPLATE_PATH), + 'constants': constants + } + mgr = bgpcfgd.managers_device_global.DeviceGlobalCfgMgr(common_objs, "CONFIG_DB", "BGP_DEVICE_GLOBAL") + cfg_mgr.update() + return mgr + + +@patch('bgpcfgd.managers_device_global.log_debug') +def test_isolate_device(mocked_log_info): + m = constructor() + res = m.set_handler("STATE", {"tsa_enabled": "true"}) + assert res, "Expect True return value for set_handler" + mocked_log_info.assert_called_with("DeviceGlobalCfgMgr::Done") + assert m.cfg_mgr.get_text() == get_string_from_file("/result_all_isolate.conf") + +@patch('bgpcfgd.managers_device_global.log_debug') +def test_unisolate_device(mocked_log_info): + m = constructor() + res = m.set_handler("STATE", {"tsa_enabled": "false"}) + assert res, "Expect True return value for set_handler" + mocked_log_info.assert_called_with("DeviceGlobalCfgMgr::Done") + assert m.cfg_mgr.get_text() == get_string_from_file("/result_all_unisolate.conf") + +def test_check_state_and_get_tsa_routemaps(): + m = constructor() + m.set_handler("STATE", {"tsa_enabled": "true"}) + res = m.check_state_and_get_tsa_routemaps(m.cfg_mgr.get_text()) + assert res == get_string_from_file("/result_isolate.conf") + + m.set_handler("STATE", {"tsa_enabled": "false"}) + res = m.check_state_and_get_tsa_routemaps(m.cfg_mgr.get_text()) + assert res == "" + +def test_get_tsa_routemaps(): + m = constructor() + res = m.get_tsa_routemaps(m.cfg_mgr.get_text()) + expected_res = get_string_from_file("/result_isolate.conf") + assert res == expected_res + +def test_get_tsb_routemaps(): + m = constructor() + res = m.get_tsb_routemaps(m.cfg_mgr.get_text()) + expected_res = get_string_from_file("/result_unisolate.conf") + assert res == expected_res + +def get_string_from_file(filename): + fp = open(BASE_PATH + filename, "r") + cfg = fp.read() + fp.close() + + return cfg + +@patch('bgpcfgd.managers_device_global.log_err') +def test_set_handler_failure_case(mocked_log_info): + m = constructor() + res = m.set_handler("STATE", {}) + assert res == False, "Expect False return value for invalid data passed to set_handler" + mocked_log_info.assert_called_with("DeviceGlobalCfgMgr:: data is None") + +def test_del_handler(): + m = constructor() + res = m.del_handler("STATE") + assert res, "Expect True return value for del_handler" + diff --git a/src/sonic-yang-models/yang-models/sonic-bgp-device-global.yang b/src/sonic-yang-models/yang-models/sonic-bgp-device-global.yang index b0c9215bf590..29d45b55268e 100644 --- a/src/sonic-yang-models/yang-models/sonic-bgp-device-global.yang +++ b/src/sonic-yang-models/yang-models/sonic-bgp-device-global.yang @@ -20,10 +20,10 @@ module sonic-bgp-device-global { container sonic-bgp-device-global { container BGP_DEVICE_GLOBAL { container STATE { - description: "BGP device-specific global data" + description "BGP device-specific global data"; leaf tsa_enabled { type boolean; - default "False"; + default "false"; description "When set to true, Traffic is shifted away (TSA), i.e, BGP routes are not advertised to neighboring routers"; } From 1c66abaf14062273982f29ae9aac49ffcba3b68c Mon Sep 17 00:00:00 2001 From: Tejaswini Chadaga Date: Sat, 25 Jun 2022 19:20:54 +0000 Subject: [PATCH 03/12] Timing issue fix --- src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py | 1 + .../bgpcfgd/managers_device_global.py | 35 +++++++++++-------- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py b/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py index 9657c32d5aea..c475db1ca179 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py @@ -106,6 +106,7 @@ def __init__(self, common_objs, db_name, table_name, peer_type, check_neig_meta) deps = [ ("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost/bgp_asn"), ("CONFIG_DB", swsscommon.CFG_LOOPBACK_INTERFACE_TABLE_NAME, "Loopback0"), + ("CONFIG_DB", "BGP_DEVICE_GLOBAL", ""), ("LOCAL", "local_addresses", ""), ("LOCAL", "interfaces", ""), ] diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py b/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py index 3a60e2cbfb87..a67fd82c71c6 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py @@ -1,7 +1,5 @@ from .manager import Manager from .log import log_err, log_debug -from swsscommon import swsscommon -import subprocess import re class DeviceGlobalCfgMgr(Manager): @@ -34,7 +32,7 @@ def set_handler(self, key, data): return False if "tsa_enabled" in data: - self.cfg_mgr.commit() + self.cfg_mgr.update() self.isolate_unisolate_device(data["tsa_enabled"]) self.directory.put(self.db_name, self.table_name, "tsa_enabled", data["tsa_enabled"]) return True @@ -44,12 +42,13 @@ def del_handler(self, key): log_debug("DeviceGlobalCfgMgr:: del handler") return True - def check_state_and_get_tsa_routemaps(self, cmds): + def check_state_and_get_tsa_routemaps(self, cfg): """ API to get TSA route-maps if device is isolated""" cmd = "" if self.directory.path_exist("CONFIG_DB", "BGP_DEVICE_GLOBAL", "tsa_enabled"): tsa_status = self.directory.get_slot("CONFIG_DB", "BGP_DEVICE_GLOBAL")["tsa_enabled"] if tsa_status == "true": + cmds = cfg.replace("#012", "\n").split("\n") log_debug("DeviceGlobalCfgMgr:: Device is isolated. Applying TSA route-maps") cmd = self.get_tsa_routemaps(cmds) return cmd @@ -67,12 +66,18 @@ def isolate_unisolate_device(self, tsa_status): self.cfg_mgr.push(cmd) log_debug("DeviceGlobalCfgMgr::Done") - def get_tsa_routemaps(self, cfg): - route_map_names = self.__extract_out_route_map_names(cfg) + def get_tsa_routemaps(self, cmds): + if not cmds: + return "" + + route_map_names = self.__extract_out_route_map_names(cmds) return self.__generate_routemaps_from_template(route_map_names, self.tsa_template) - def get_tsb_routemaps(self, cfg): - route_map_names = self.__extract_out_route_map_names(cfg) + def get_tsb_routemaps(self, cmds): + if not cmds: + return "" + + route_map_names = self.__extract_out_route_map_names(cmds) return self.__generate_routemaps_from_template(route_map_names, self.tsb_template) def __generate_routemaps_from_template(self, route_map_names, template): @@ -91,12 +96,14 @@ def __generate_routemaps_from_template(self, route_map_names, template): log_debug("DeviceGlobalCfgMgr::get_tsa_routemaps:: Done") return cmd - def __extract_out_route_map_names(self, cfg): + def __extract_out_route_map_names(self, cmds): + if not cmds: + return "" + route_map_names = [] - out_route_map = re.compile(r'^\s*neighbor \S+ route-map (\S+) out$') - for line in cfg.replace("#012", "\n").split("\n"): - result = out_route_map.match(line) + out_route_map = re.compile(r'^\s*neighbor \S+ route-map (\S+) out$') + for line in cmds: + result = out_route_map.match(line) if result: - route_map_names.append(result.group(1)) + route_map_names.append(result.group(1)) return route_map_names - From 2481aa7b9d68f42a27ff871aab5bb9d923f64b25 Mon Sep 17 00:00:00 2001 From: Tejaswini Chadaga Date: Sun, 26 Jun 2022 03:28:56 +0000 Subject: [PATCH 04/12] Fixes --- .../bgpcfgd/managers_device_global.py | 3 -- src/sonic-bgpcfgd/tests/test_device_global.py | 28 +++++++++++++------ 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py b/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py index a67fd82c71c6..8c69accfa898 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py @@ -97,9 +97,6 @@ def __generate_routemaps_from_template(self, route_map_names, template): return cmd def __extract_out_route_map_names(self, cmds): - if not cmds: - return "" - route_map_names = [] out_route_map = re.compile(r'^\s*neighbor \S+ route-map (\S+) out$') for line in cmds: diff --git a/src/sonic-bgpcfgd/tests/test_device_global.py b/src/sonic-bgpcfgd/tests/test_device_global.py index da06d425c791..bec57d172f4f 100644 --- a/src/sonic-bgpcfgd/tests/test_device_global.py +++ b/src/sonic-bgpcfgd/tests/test_device_global.py @@ -13,15 +13,24 @@ def constructor(): cfg_mgr = MagicMock() def get_text(): - return cfg_mgr.changes + text = [] + for line in cfg_mgr.changes.split('\n'): + if line.lstrip().startswith('!'): + continue + text.append(line) + text += [" "] + return text def update(): cfg_mgr.changes = get_string_from_file("/result_all.conf") def push(cfg): cfg_mgr.changes += cfg + "\n" + def get_config(): + return cfg_mgr.changes cfg_mgr.get_text = get_text cfg_mgr.update = update cfg_mgr.push = push - + cfg_mgr.get_config = get_config + constants = load_constants()['constants'] common_objs = { 'directory': Directory(), @@ -30,7 +39,7 @@ def push(cfg): 'constants': constants } mgr = bgpcfgd.managers_device_global.DeviceGlobalCfgMgr(common_objs, "CONFIG_DB", "BGP_DEVICE_GLOBAL") - cfg_mgr.update() + cfg_mgr.update() return mgr @@ -40,7 +49,7 @@ def test_isolate_device(mocked_log_info): res = m.set_handler("STATE", {"tsa_enabled": "true"}) assert res, "Expect True return value for set_handler" mocked_log_info.assert_called_with("DeviceGlobalCfgMgr::Done") - assert m.cfg_mgr.get_text() == get_string_from_file("/result_all_isolate.conf") + assert m.cfg_mgr.get_config() == get_string_from_file("/result_all_isolate.conf") @patch('bgpcfgd.managers_device_global.log_debug') def test_unisolate_device(mocked_log_info): @@ -48,26 +57,30 @@ def test_unisolate_device(mocked_log_info): res = m.set_handler("STATE", {"tsa_enabled": "false"}) assert res, "Expect True return value for set_handler" mocked_log_info.assert_called_with("DeviceGlobalCfgMgr::Done") - assert m.cfg_mgr.get_text() == get_string_from_file("/result_all_unisolate.conf") + assert m.cfg_mgr.get_config() == get_string_from_file("/result_all_unisolate.conf") def test_check_state_and_get_tsa_routemaps(): m = constructor() m.set_handler("STATE", {"tsa_enabled": "true"}) - res = m.check_state_and_get_tsa_routemaps(m.cfg_mgr.get_text()) + res = m.check_state_and_get_tsa_routemaps(m.cfg_mgr.get_config()) assert res == get_string_from_file("/result_isolate.conf") m.set_handler("STATE", {"tsa_enabled": "false"}) - res = m.check_state_and_get_tsa_routemaps(m.cfg_mgr.get_text()) + res = m.check_state_and_get_tsa_routemaps(m.cfg_mgr.get_config()) assert res == "" def test_get_tsa_routemaps(): m = constructor() + assert m.get_tsa_routemaps([]) == "" + res = m.get_tsa_routemaps(m.cfg_mgr.get_text()) expected_res = get_string_from_file("/result_isolate.conf") assert res == expected_res def test_get_tsb_routemaps(): m = constructor() + assert m.get_tsb_routemaps([]) == "" + res = m.get_tsb_routemaps(m.cfg_mgr.get_text()) expected_res = get_string_from_file("/result_unisolate.conf") assert res == expected_res @@ -90,4 +103,3 @@ def test_del_handler(): m = constructor() res = m.del_handler("STATE") assert res, "Expect True return value for del_handler" - From 32772c0f7f16cd416877e9d257eeab4a652b6a0d Mon Sep 17 00:00:00 2001 From: Tejaswini Chadaga Date: Sun, 26 Jun 2022 03:34:29 +0000 Subject: [PATCH 05/12] Fixes --- src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py b/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py index c475db1ca179..c3987bf78117 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py @@ -10,6 +10,7 @@ from .utils import run_command from .managers_device_global import DeviceGlobalCfgMgr + class BGPPeerGroupMgr(object): """ This class represents peer-group and routing policy for the peer_type """ def __init__(self, common_objs, base_template): From b92f4da61224ef6d5af5803c9c1a1c0d3d05d27f Mon Sep 17 00:00:00 2001 From: tchadaga Date: Sun, 26 Jun 2022 07:10:44 +0000 Subject: [PATCH 06/12] yang test files --- src/sonic-yang-models/setup.py | 1 + src/sonic-yang-models/tests/files/sample_config_db.json | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/src/sonic-yang-models/setup.py b/src/sonic-yang-models/setup.py index 4d61be960c3b..616047aad3eb 100644 --- a/src/sonic-yang-models/setup.py +++ b/src/sonic-yang-models/setup.py @@ -80,6 +80,7 @@ def run(self): ('yang-models', ['./yang-models/sonic-acl.yang', './yang-models/sonic-auto_techsupport.yang', './yang-models/sonic-bgp-common.yang', + './yang-models/sonic-bgp-device-global.yang', './yang-models/sonic-bgp-global.yang', './yang-models/sonic-bgp-monitor.yang', './yang-models/sonic-bgp-internal-neighbor.yang', diff --git a/src/sonic-yang-models/tests/files/sample_config_db.json b/src/sonic-yang-models/tests/files/sample_config_db.json index d8f90284091a..93226ef5d6bf 100644 --- a/src/sonic-yang-models/tests/files/sample_config_db.json +++ b/src/sonic-yang-models/tests/files/sample_config_db.json @@ -1299,6 +1299,11 @@ "default|ipv4_unicast|21.0.0.0/8": { } }, + "BGP_DEVICE_GLOBAL": { + "STATE": { + "tsa_enabled": "false" + } + }, "BGP_PEER_RANGE": { "BGPSLBPassive": { "ip_range": [ From 2209fc0f826c26e4f954735d2703cb0fa21c5c20 Mon Sep 17 00:00:00 2001 From: tchadaga Date: Sun, 26 Jun 2022 07:12:48 +0000 Subject: [PATCH 07/12] yang file update --- src/sonic-yang-models/yang-models/sonic-bgp-device-global.yang | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sonic-yang-models/yang-models/sonic-bgp-device-global.yang b/src/sonic-yang-models/yang-models/sonic-bgp-device-global.yang index 29d45b55268e..728714c7d51f 100644 --- a/src/sonic-yang-models/yang-models/sonic-bgp-device-global.yang +++ b/src/sonic-yang-models/yang-models/sonic-bgp-device-global.yang @@ -12,7 +12,7 @@ module sonic-bgp-device-global { description "SONIC Device-specific BGP global data"; - revision 2022-06-07 { + revision 2022-06-26 { description "Initial revision"; } From 8ebc5b1dadefc7472af58353f33370d187d9c6f0 Mon Sep 17 00:00:00 2001 From: tchadaga Date: Tue, 28 Jun 2022 09:14:40 +0000 Subject: [PATCH 08/12] Remove bringup dependency --- src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py | 1 - src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py b/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py index c3987bf78117..813fef34c7f7 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py @@ -107,7 +107,6 @@ def __init__(self, common_objs, db_name, table_name, peer_type, check_neig_meta) deps = [ ("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost/bgp_asn"), ("CONFIG_DB", swsscommon.CFG_LOOPBACK_INTERFACE_TABLE_NAME, "Loopback0"), - ("CONFIG_DB", "BGP_DEVICE_GLOBAL", ""), ("LOCAL", "local_addresses", ""), ("LOCAL", "interfaces", ""), ] diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py b/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py index 8c69accfa898..ccbf3ba9198e 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py @@ -32,7 +32,7 @@ def set_handler(self, key, data): return False if "tsa_enabled" in data: - self.cfg_mgr.update() + self.cfg_mgr.commit() self.isolate_unisolate_device(data["tsa_enabled"]) self.directory.put(self.db_name, self.table_name, "tsa_enabled", data["tsa_enabled"]) return True From 72540eabefe88bc5637ab72a7718e7d685fb14cd Mon Sep 17 00:00:00 2001 From: Tejaswini Chadaga Date: Sat, 2 Jul 2022 00:13:22 +0000 Subject: [PATCH 09/12] Fixed minor issue, addressed review comments --- dockers/docker-fpm-frr/base_image_files/TS | 4 +++ dockers/docker-fpm-frr/base_image_files/TSA | 3 ++ dockers/docker-fpm-frr/base_image_files/TSB | 2 ++ .../bgpcfgd/managers_device_global.py | 14 ++++---- src/sonic-yang-models/setup.py | 1 - .../tests/files/sample_config_db.json | 5 --- .../yang-models/sonic-bgp-device-global.yang | 36 ------------------- 7 files changed, 16 insertions(+), 49 deletions(-) delete mode 100644 src/sonic-yang-models/yang-models/sonic-bgp-device-global.yang diff --git a/dockers/docker-fpm-frr/base_image_files/TS b/dockers/docker-fpm-frr/base_image_files/TS index e4e917850408..3394d59ed28e 100755 --- a/dockers/docker-fpm-frr/base_image_files/TS +++ b/dockers/docker-fpm-frr/base_image_files/TS @@ -28,7 +28,9 @@ if [[ ($NUM_ASIC -gt 1) ]]; then echo -e "BGP"$asic" : \c" if [[ -n "$TSA_STATE_UPDATE" ]]; then sonic-cfggen -a "$TSA_STATE_UPDATE" -w -n $NAMESPACE_PREFIX$asic + logger -t $1 -p user.info "ConfigDB updated" else + # If TSC is executed, invoke FRR script to check installed route-maps docker exec -i bgp$asic /usr/bin/$1 fi fi @@ -37,7 +39,9 @@ if [[ ($NUM_ASIC -gt 1) ]]; then else if [[ -n "$TSA_STATE_UPDATE" ]]; then sonic-cfggen -a "$TSA_STATE_UPDATE" -w + logger -t $1 -p user.info "ConfigDB updated" else + # If TSC is executed, invoke FRR script to check installed route-maps docker exec -i bgp /usr/bin/$1 fi fi diff --git a/dockers/docker-fpm-frr/base_image_files/TSA b/dockers/docker-fpm-frr/base_image_files/TSA index 7c49cb56de04..2fea99028fd4 100755 --- a/dockers/docker-fpm-frr/base_image_files/TSA +++ b/dockers/docker-fpm-frr/base_image_files/TSA @@ -1,7 +1,9 @@ #!/bin/bash + if [[ "$(sonic-cfggen -d -v BGP_DEVICE_GLOBAL.STATE.tsa_enabled)" == "true" ]]; then echo "System is already in Maintenance" + logger -t TSA -p user.info "System is already in Maintenance" else # toggle the mux to standby if dualtor and any mux active if @@ -13,6 +15,7 @@ else fi /usr/bin/TS TSA + logger -t TSA -p user.info "System Mode: Normal -> Maintenance" echo "System Mode: Normal -> Maintenance" echo "" echo "Please execute 'config save' to preserve System mode in Maintenance after reboot or config reload" diff --git a/dockers/docker-fpm-frr/base_image_files/TSB b/dockers/docker-fpm-frr/base_image_files/TSB index 15ff96c1a4d2..cddbdd9fe1dd 100755 --- a/dockers/docker-fpm-frr/base_image_files/TSB +++ b/dockers/docker-fpm-frr/base_image_files/TSB @@ -2,6 +2,7 @@ if [[ "$(sonic-cfggen -d -v BGP_DEVICE_GLOBAL.STATE.tsa_enabled)" == "false" ]]; then echo "System is already in Normal mode" + logger -t TSB -p user.info "System is already in Normal mode" else # toggle the mux to auto if dualtor if [[ "$(sonic-cfggen -d -v DEVICE_METADATA.localhost.subtype | tr [:upper:] [:lower:])" == *"dualtor"* ]]; @@ -11,6 +12,7 @@ else fi /usr/bin/TS TSB + logger -t TSB -p user.info "System Mode: Maintenance -> Normal" echo "System Mode: Maintenance -> Normal" echo "" echo "Please execute 'config save' to preserve System mode in Normal state after reboot or config reload" diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py b/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py index ccbf3ba9198e..5577a14ee058 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py @@ -1,5 +1,5 @@ from .manager import Manager -from .log import log_err, log_debug +from .log import log_err, log_debug, log_notice import re class DeviceGlobalCfgMgr(Manager): @@ -33,6 +33,7 @@ def set_handler(self, key, data): if "tsa_enabled" in data: self.cfg_mgr.commit() + self.cfg_mgr.update() self.isolate_unisolate_device(data["tsa_enabled"]) self.directory.put(self.db_name, self.table_name, "tsa_enabled", data["tsa_enabled"]) return True @@ -49,7 +50,7 @@ def check_state_and_get_tsa_routemaps(self, cfg): tsa_status = self.directory.get_slot("CONFIG_DB", "BGP_DEVICE_GLOBAL")["tsa_enabled"] if tsa_status == "true": cmds = cfg.replace("#012", "\n").split("\n") - log_debug("DeviceGlobalCfgMgr:: Device is isolated. Applying TSA route-maps") + log_notice("DeviceGlobalCfgMgr:: Device is isolated. Applying TSA route-maps") cmd = self.get_tsa_routemaps(cmds) return cmd @@ -57,10 +58,10 @@ def isolate_unisolate_device(self, tsa_status): """ API to get TSA/TSB route-maps and apply configuration""" cmd = "\n" if tsa_status == "true": - log_debug("DeviceGlobalCfgMgr:: Device isolated. Executing TSA") + log_notice("DeviceGlobalCfgMgr:: Device isolated. Executing TSA") cmd += self.get_tsa_routemaps(self.cfg_mgr.get_text()) else: - log_debug("DeviceGlobalCfgMgr:: Device un-isolated. Executing TSB") + log_notice("DeviceGlobalCfgMgr:: Device un-isolated. Executing TSB") cmd += self.get_tsb_routemaps(self.cfg_mgr.get_text()) self.cfg_mgr.push(cmd) @@ -93,14 +94,13 @@ def __generate_routemaps_from_template(self, route_map_names, template): continue cmd += template.render(route_map_name=rm,ip_version=ipv,ip_protocol=ipp, constants=self.constants) cmd += "\n" - log_debug("DeviceGlobalCfgMgr::get_tsa_routemaps:: Done") return cmd def __extract_out_route_map_names(self, cmds): - route_map_names = [] + route_map_names = set() out_route_map = re.compile(r'^\s*neighbor \S+ route-map (\S+) out$') for line in cmds: result = out_route_map.match(line) if result: - route_map_names.append(result.group(1)) + route_map_names.add(result.group(1)) return route_map_names diff --git a/src/sonic-yang-models/setup.py b/src/sonic-yang-models/setup.py index 616047aad3eb..4d61be960c3b 100644 --- a/src/sonic-yang-models/setup.py +++ b/src/sonic-yang-models/setup.py @@ -80,7 +80,6 @@ def run(self): ('yang-models', ['./yang-models/sonic-acl.yang', './yang-models/sonic-auto_techsupport.yang', './yang-models/sonic-bgp-common.yang', - './yang-models/sonic-bgp-device-global.yang', './yang-models/sonic-bgp-global.yang', './yang-models/sonic-bgp-monitor.yang', './yang-models/sonic-bgp-internal-neighbor.yang', diff --git a/src/sonic-yang-models/tests/files/sample_config_db.json b/src/sonic-yang-models/tests/files/sample_config_db.json index 93226ef5d6bf..d8f90284091a 100644 --- a/src/sonic-yang-models/tests/files/sample_config_db.json +++ b/src/sonic-yang-models/tests/files/sample_config_db.json @@ -1299,11 +1299,6 @@ "default|ipv4_unicast|21.0.0.0/8": { } }, - "BGP_DEVICE_GLOBAL": { - "STATE": { - "tsa_enabled": "false" - } - }, "BGP_PEER_RANGE": { "BGPSLBPassive": { "ip_range": [ diff --git a/src/sonic-yang-models/yang-models/sonic-bgp-device-global.yang b/src/sonic-yang-models/yang-models/sonic-bgp-device-global.yang deleted file mode 100644 index 728714c7d51f..000000000000 --- a/src/sonic-yang-models/yang-models/sonic-bgp-device-global.yang +++ /dev/null @@ -1,36 +0,0 @@ -module sonic-bgp-device-global { - namespace "http://github.com/Azure/sonic-bgp-device-global"; - prefix bgp_device_global; - yang-version 1.1; - - organization - "SONiC"; - - contact - "SONiC"; - - description - "SONIC Device-specific BGP global data"; - - revision 2022-06-26 { - description - "Initial revision"; - } - - container sonic-bgp-device-global { - container BGP_DEVICE_GLOBAL { - container STATE { - description "BGP device-specific global data"; - leaf tsa_enabled { - type boolean; - default "false"; - description - "When set to true, Traffic is shifted away (TSA), i.e, BGP routes are not advertised to neighboring routers"; - } - } /* end of STATE container */ - } /* end of BGP_DEVICE_GLOBAL container */ - - } /* end of top level container */ - -} /* end of module sonic-bgp-device-global */ - From 37dbaf646016fd757a8acc712289fa53441f7c21 Mon Sep 17 00:00:00 2001 From: Tejaswini Chadaga Date: Sat, 2 Jul 2022 00:36:08 +0000 Subject: [PATCH 10/12] Fixed json formatting --- files/build_templates/init_cfg.json.j2 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/files/build_templates/init_cfg.json.j2 b/files/build_templates/init_cfg.json.j2 index c4e54f306b4e..7de0ad977807 100644 --- a/files/build_templates/init_cfg.json.j2 +++ b/files/build_templates/init_cfg.json.j2 @@ -29,8 +29,8 @@ }, "BGP_DEVICE_GLOBAL": { "STATE": { - "tsa_enabled": "false" - } + "tsa_enabled": "false" + } }, {%- set features = [("bgp", "enabled", false, "enabled"), ("database", "always_enabled", false, "always_enabled"), From 2ed1d5cd4d4fe30927c4c32988542f48ee13a8d0 Mon Sep 17 00:00:00 2001 From: Tejaswini Chadaga Date: Sat, 2 Jul 2022 02:27:20 +0000 Subject: [PATCH 11/12] Fix flaky unit test --- src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py b/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py index 5577a14ee058..42adfb038990 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py @@ -83,7 +83,7 @@ def get_tsb_routemaps(self, cmds): def __generate_routemaps_from_template(self, route_map_names, template): cmd = "\n" - for rm in route_map_names: + for rm in sorted(route_map_names): if "_INTERNAL_" in rm: continue if "V4" in rm: From ba688690374cdd5eb261ccba2ae3d51cdc4b9e6f Mon Sep 17 00:00:00 2001 From: Tejaswini Chadaga Date: Thu, 7 Jul 2022 23:26:57 +0000 Subject: [PATCH 12/12] Minor updates based on comments --- dockers/docker-fpm-frr/base_image_files/TS | 6 +++-- dockers/docker-fpm-frr/base_image_files/TSA | 3 --- dockers/docker-fpm-frr/base_image_files/TSB | 4 +--- src/sonic-bgpcfgd/bgpcfgd/main.py | 3 ++- src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py | 2 +- .../bgpcfgd/managers_device_global.py | 23 ++++++++----------- src/sonic-bgpcfgd/tests/test_device_global.py | 12 ++++++---- 7 files changed, 24 insertions(+), 29 deletions(-) diff --git a/dockers/docker-fpm-frr/base_image_files/TS b/dockers/docker-fpm-frr/base_image_files/TS index 3394d59ed28e..4ee085282be8 100755 --- a/dockers/docker-fpm-frr/base_image_files/TS +++ b/dockers/docker-fpm-frr/base_image_files/TS @@ -28,7 +28,8 @@ if [[ ($NUM_ASIC -gt 1) ]]; then echo -e "BGP"$asic" : \c" if [[ -n "$TSA_STATE_UPDATE" ]]; then sonic-cfggen -a "$TSA_STATE_UPDATE" -w -n $NAMESPACE_PREFIX$asic - logger -t $1 -p user.info "ConfigDB updated" + logger -t $1 -p user.info "BGP$asic: System Mode: Normal -> Maintenance" + echo "BGP$asic: System Mode: Normal -> Maintenance" else # If TSC is executed, invoke FRR script to check installed route-maps docker exec -i bgp$asic /usr/bin/$1 @@ -39,7 +40,8 @@ if [[ ($NUM_ASIC -gt 1) ]]; then else if [[ -n "$TSA_STATE_UPDATE" ]]; then sonic-cfggen -a "$TSA_STATE_UPDATE" -w - logger -t $1 -p user.info "ConfigDB updated" + logger -t $1 -p user.info "System Mode: Normal -> Maintenance" + echo "System Mode: Normal -> Maintenance" else # If TSC is executed, invoke FRR script to check installed route-maps docker exec -i bgp /usr/bin/$1 diff --git a/dockers/docker-fpm-frr/base_image_files/TSA b/dockers/docker-fpm-frr/base_image_files/TSA index 2fea99028fd4..8c37525ef0a9 100755 --- a/dockers/docker-fpm-frr/base_image_files/TSA +++ b/dockers/docker-fpm-frr/base_image_files/TSA @@ -15,9 +15,6 @@ else fi /usr/bin/TS TSA - logger -t TSA -p user.info "System Mode: Normal -> Maintenance" - echo "System Mode: Normal -> Maintenance" - echo "" echo "Please execute 'config save' to preserve System mode in Maintenance after reboot or config reload" fi diff --git a/dockers/docker-fpm-frr/base_image_files/TSB b/dockers/docker-fpm-frr/base_image_files/TSB index cddbdd9fe1dd..5f8d90160fcb 100755 --- a/dockers/docker-fpm-frr/base_image_files/TSB +++ b/dockers/docker-fpm-frr/base_image_files/TSB @@ -12,8 +12,6 @@ else fi /usr/bin/TS TSB - logger -t TSB -p user.info "System Mode: Maintenance -> Normal" - echo "System Mode: Maintenance -> Normal" - echo "" echo "Please execute 'config save' to preserve System mode in Normal state after reboot or config reload" fi + diff --git a/src/sonic-bgpcfgd/bgpcfgd/main.py b/src/sonic-bgpcfgd/bgpcfgd/main.py index 20488e297510..f777867eaacc 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/main.py +++ b/src/sonic-bgpcfgd/bgpcfgd/main.py @@ -66,7 +66,7 @@ def do_work(): AdvertiseRouteMgr(common_objs, "STATE_DB", swsscommon.STATE_ADVERTISE_NETWORK_TABLE_NAME), RouteMapMgr(common_objs, "APPL_DB", swsscommon.APP_BGP_PROFILE_TABLE_NAME), # Device Global Manager - DeviceGlobalCfgMgr(common_objs, "CONFIG_DB", "BGP_DEVICE_GLOBAL"), + DeviceGlobalCfgMgr(common_objs, "CONFIG_DB", swsscommon.CFG_BGP_DEVICE_GLOBAL_TABLE_NAME), ] runner = Runner(common_objs['cfg_mgr']) for mgr in managers: @@ -99,3 +99,4 @@ def main(): sys.exit(rc) except SystemExit: os._exit(rc) + diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py b/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py index 813fef34c7f7..55a16a273993 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py @@ -24,7 +24,7 @@ def __init__(self, common_objs, base_template): tf = common_objs['tf'] self.policy_template = tf.from_file(base_template + "policies.conf.j2") self.peergroup_template = tf.from_file(base_template + "peer-group.conf.j2") - self.device_global_cfgmgr = DeviceGlobalCfgMgr(common_objs, "CONFIG_DB", "BGP_DEVICE_GLOBAL") + self.device_global_cfgmgr = DeviceGlobalCfgMgr(common_objs, "CONFIG_DB", swsscommon.CFG_BGP_DEVICE_GLOBAL_TABLE_NAME) def update(self, name, **kwargs): """ diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py b/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py index 42adfb038990..1d30a5b94a64 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py @@ -1,6 +1,7 @@ from .manager import Manager from .log import log_err, log_debug, log_notice import re +from swsscommon import swsscommon class DeviceGlobalCfgMgr(Manager): """This class responds to change in device-specific state""" @@ -46,12 +47,12 @@ def del_handler(self, key): def check_state_and_get_tsa_routemaps(self, cfg): """ API to get TSA route-maps if device is isolated""" cmd = "" - if self.directory.path_exist("CONFIG_DB", "BGP_DEVICE_GLOBAL", "tsa_enabled"): - tsa_status = self.directory.get_slot("CONFIG_DB", "BGP_DEVICE_GLOBAL")["tsa_enabled"] + if self.directory.path_exist("CONFIG_DB", swsscommon.CFG_BGP_DEVICE_GLOBAL_TABLE_NAME, "tsa_enabled"): + tsa_status = self.directory.get_slot("CONFIG_DB", swsscommon.CFG_BGP_DEVICE_GLOBAL_TABLE_NAME)["tsa_enabled"] if tsa_status == "true": cmds = cfg.replace("#012", "\n").split("\n") log_notice("DeviceGlobalCfgMgr:: Device is isolated. Applying TSA route-maps") - cmd = self.get_tsa_routemaps(cmds) + cmd = self.get_ts_routemaps(cmds, self.tsa_template) return cmd def isolate_unisolate_device(self, tsa_status): @@ -59,27 +60,20 @@ def isolate_unisolate_device(self, tsa_status): cmd = "\n" if tsa_status == "true": log_notice("DeviceGlobalCfgMgr:: Device isolated. Executing TSA") - cmd += self.get_tsa_routemaps(self.cfg_mgr.get_text()) + cmd += self.get_ts_routemaps(self.cfg_mgr.get_text(), self.tsa_template) else: log_notice("DeviceGlobalCfgMgr:: Device un-isolated. Executing TSB") - cmd += self.get_tsb_routemaps(self.cfg_mgr.get_text()) + cmd += self.get_ts_routemaps(self.cfg_mgr.get_text(), self.tsb_template) self.cfg_mgr.push(cmd) log_debug("DeviceGlobalCfgMgr::Done") - def get_tsa_routemaps(self, cmds): + def get_ts_routemaps(self, cmds, ts_template): if not cmds: return "" route_map_names = self.__extract_out_route_map_names(cmds) - return self.__generate_routemaps_from_template(route_map_names, self.tsa_template) - - def get_tsb_routemaps(self, cmds): - if not cmds: - return "" - - route_map_names = self.__extract_out_route_map_names(cmds) - return self.__generate_routemaps_from_template(route_map_names, self.tsb_template) + return self.__generate_routemaps_from_template(route_map_names, ts_template) def __generate_routemaps_from_template(self, route_map_names, template): cmd = "\n" @@ -104,3 +98,4 @@ def __extract_out_route_map_names(self, cmds): if result: route_map_names.add(result.group(1)) return route_map_names + diff --git a/src/sonic-bgpcfgd/tests/test_device_global.py b/src/sonic-bgpcfgd/tests/test_device_global.py index bec57d172f4f..eae1ff424e1f 100644 --- a/src/sonic-bgpcfgd/tests/test_device_global.py +++ b/src/sonic-bgpcfgd/tests/test_device_global.py @@ -6,6 +6,7 @@ from . import swsscommon_test from .util import load_constants import bgpcfgd.managers_device_global +from swsscommon import swsscommon TEMPLATE_PATH = os.path.abspath('../../dockers/docker-fpm-frr/frr') BASE_PATH = os.path.abspath('../sonic-bgpcfgd/tests/data/general/peer-group.conf/') @@ -38,7 +39,7 @@ def get_config(): 'tf': TemplateFabric(TEMPLATE_PATH), 'constants': constants } - mgr = bgpcfgd.managers_device_global.DeviceGlobalCfgMgr(common_objs, "CONFIG_DB", "BGP_DEVICE_GLOBAL") + mgr = bgpcfgd.managers_device_global.DeviceGlobalCfgMgr(common_objs, "CONFIG_DB", swsscommon.CFG_BGP_DEVICE_GLOBAL_TABLE_NAME) cfg_mgr.update() return mgr @@ -71,17 +72,17 @@ def test_check_state_and_get_tsa_routemaps(): def test_get_tsa_routemaps(): m = constructor() - assert m.get_tsa_routemaps([]) == "" + assert m.get_ts_routemaps([], m.tsa_template) == "" - res = m.get_tsa_routemaps(m.cfg_mgr.get_text()) + res = m.get_ts_routemaps(m.cfg_mgr.get_text(), m.tsa_template) expected_res = get_string_from_file("/result_isolate.conf") assert res == expected_res def test_get_tsb_routemaps(): m = constructor() - assert m.get_tsb_routemaps([]) == "" + assert m.get_ts_routemaps([], m.tsb_template) == "" - res = m.get_tsb_routemaps(m.cfg_mgr.get_text()) + res = m.get_ts_routemaps(m.cfg_mgr.get_text(), m.tsb_template) expected_res = get_string_from_file("/result_unisolate.conf") assert res == expected_res @@ -103,3 +104,4 @@ def test_del_handler(): m = constructor() res = m.del_handler("STATE") assert res, "Expect True return value for del_handler" +