[TSA] Reliable TSA: Addressing pizza box issues

dockers/docker-fpm-frr/base_image_files/TS

@@ -4,17 +4,31 @@
 [ -f /etc/sonic/sonic-environment ] && . /etc/sonic/sonic-environment
 
 PLATFORM=${PLATFORM:-`sonic-cfggen -H -v DEVICE_METADATA.localhost.platform`}
+switch_type=`sonic-db-cli CONFIG_DB hget 'DEVICE_METADATA|localhost' 'switch_type'`
+TSA_CHASSIS_STATE=false
+
+if [[ $switch_type == 'voq' ]]; then
+ TSA_CHASSIS_STATE="$(sonic-db-cli CHASSIS_APP_DB HGET "BGP_DEVICE_GLOBAL|STATE" tsa_enabled)"
+fi
 
 if [[ $1 == "TSA" ]]; then
     TSA_STATE_UPDATE='{"BGP_DEVICE_GLOBAL":{"STATE":{"tsa_enabled": "true"}}}'
-    log_msg='System Mode: Normal -> Maintenance'
+    if [[ $TSA_CHASSIS_STATE == true ]]; then
+        log_msg='System Mode: Maintenance -> Maintenance'
+    else
+        log_msg='System Mode: Normal -> Maintenance'
+    fi
     err_msg='System is already in Maintenance'
-    desired_tsa_state=true 
+    desired_tsa_state=true
 elif [[ $1 == "TSB" ]]; then
     TSA_STATE_UPDATE='{"BGP_DEVICE_GLOBAL":{"STATE":{"tsa_enabled": "false"}}}'
-    log_msg='System Mode: Maintenance -> Normal'
+    if [[ $TSA_CHASSIS_STATE == true ]]; then
+        log_msg='System Mode: Maintenance -> Maintenance'
+    else
+        log_msg='System Mode: Maintenance -> Normal'
+    fi
     err_msg='System is already in Normal mode'
-    desired_tsa_state=false    
+    desired_tsa_state=false
 fi
 
 # Parse the device specific asic conf file, if it exists

dockers/docker-fpm-frr/base_image_files/TSA

@@ -7,7 +7,18 @@ if [ "$EUID" -ne 0 ] ; then
 fi
 
 if [ -f /etc/sonic/chassisdb.conf ]; then
-  rexec all -c "sudo TSA chassis"
+  CHASSIS_TSA_STATE_UPDATE="CHASSIS_APP_DB HMSET "BGP_DEVICE_GLOBAL\|STATE" tsa_enabled "true""
+  CONFIG_DB_TSA_STATE_UPDATE='{"BGP_DEVICE_GLOBAL":{"STATE":{"tsa_enabled": "true"}}}'
+  current_tsa_state="$(sonic-cfggen -d -v BGP_DEVICE_GLOBAL.STATE.tsa_enabled)"
+  if [[ $current_tsa_state  == true ]]; then
+    echo "Chassis is already in Maintenance"
+    logger -t TSA -p user.info "Chassis is already in Maintenance"
+  else
+    sonic-db-cli $CHASSIS_TSA_STATE_UPDATE
+    sonic-cfggen -a "$CONFIG_DB_TSA_STATE_UPDATE" -w
+    echo "Chassis Mode: Normal -> Maintenance"
+    logger -t TSA -p user.info "Chassis Mode: Normal -> Maintenance"
+  fi
   echo "Please execute \"rexec all -c 'sudo config save -y'\" to preserve System mode in Maintenance after reboot\
   or config reload on all linecards"
   exit 0

dockers/docker-fpm-frr/base_image_files/TSB

@@ -6,11 +6,21 @@ if [ "$EUID" -ne 0 ] ; then
   exit 1
 fi
 
-# If run on supervisor of chassis, trigger remote execution of TSB on all linecards
 if [ -f /etc/sonic/chassisdb.conf ]; then
-  rexec all -c "sudo TSB chassis"
+  CHASSIS_TSA_STATE_UPDATE="CHASSIS_APP_DB HMSET "BGP_DEVICE_GLOBAL\|STATE" tsa_enabled "false""
+  CONFIG_DB_TSA_STATE_UPDATE='{"BGP_DEVICE_GLOBAL":{"STATE":{"tsa_enabled": "false"}}}'
+  current_tsa_state="$(sonic-cfggen -d -v BGP_DEVICE_GLOBAL.STATE.tsa_enabled)"
+  if [[ $current_tsa_state  == false ]]; then
+    echo "Chassis is already in Normal mode"
+    logger -t TSB -p user.info "Chassis is already in Normal mode"
+  else
+    sonic-db-cli $CHASSIS_TSA_STATE_UPDATE
+    sonic-cfggen -a "$CONFIG_DB_TSA_STATE_UPDATE" -w
+    echo "Chassis Mode: Maintenance -> Normal"
+    logger -t TSB -p user.info "Chassis Mode: Maintenance -> Normal"
+  fi
   echo "Please execute \"rexec all -c 'sudo config save -y'\" to preserve System mode in Normal state after reboot\
- or config reload on all linecards"
+  or config reload on all linecards"
   exit 0
 fi
 

files/build_templates/docker_image_ctl.j2

@@ -269,6 +269,14 @@ function postStartAction()
             $SONIC_DB_CLI CONFIG_DB SET "CONFIG_DB_INITIALIZED" "1"
         fi
 
+        # In SUP, enforce CHASSIS_APP_DB.tsa_enabled to be in sync with BGP_DEVICE_GLOBAL.STATE.tsa_enabled
+        if [[ -z "$DEV" ]] && [[ -f /etc/sonic/chassisdb.conf ]]; then
+           tsa_cfg="$($SONIC_DB_CLI CONFIG_DB HGET "BGP_DEVICE_GLOBAL|STATE" "tsa_enabled")"
+           if [[ -n "$tsa_cfg" ]]; then
+              docker exec -i ${DOCKERNAME} $SONIC_DB_CLI CHASSIS_APP_DB HMSET "BGP_DEVICE_GLOBAL|STATE" tsa_enabled ${tsa_cfg}
+           fi
+        fi
+
         # Add redis UDS to the redis group and give read/write access to the group
         REDIS_SOCK="/var/run/redis${DEV}/redis.sock"
     else

files/image_config/config-setup/config-setup

@@ -330,6 +330,12 @@ do_db_migration()
         /usr/local/bin/db_migrator.py -o migrate
     fi
     sonic-db-cli CONFIG_DB SET "CONFIG_DB_INITIALIZED" "1"
+
+    #Enforce CHASSIS_APP_DB.tsa_enabled to be in sync with BGP_DEVICE_GLOBAL.STATE.tsa_enabled
+    if [[ -f /etc/sonic/chassisdb.conf ]]; then
+       tsa_cfg="$(sonic-db-cli CONFIG_DB HGET "BGP_DEVICE_GLOBAL|STATE" "tsa_enabled")"
+       sonic-db-cli CHASSIS_APP_DB HMSET "BGP_DEVICE_GLOBAL|STATE" tsa_enabled ${tsa_cfg}
+    fi
 }
 
 # Perform configuration migration from backup copy.

src/sonic-bgpcfgd/bgpcfgd/main.py

@@ -6,6 +6,7 @@
 import traceback
 
 from swsscommon import swsscommon
+from sonic_py_common import device_info
 
 from .config import ConfigMgr
 from .directory import Directory
@@ -20,6 +21,7 @@
 from .managers_static_rt import StaticRouteMgr
 from .managers_rm import RouteMapMgr
 from .managers_device_global import DeviceGlobalCfgMgr
+from .managers_chassis_app_db import ChassisAppDbMgr
 from .static_rt_timer import StaticRouteTimer
 from .runner import Runner, signal_handler
 from .template import TemplateFabric
@@ -74,6 +76,10 @@ def do_work():
         # Device Global Manager
         DeviceGlobalCfgMgr(common_objs, "CONFIG_DB", swsscommon.CFG_BGP_DEVICE_GLOBAL_TABLE_NAME),
     ]
+
+    if device_info.get_platform_info().get('switch_type') == "voq":
+        managers.append(ChassisAppDbMgr(common_objs, "CHASSIS_APP_DB", "BGP_DEVICE_GLOBAL"))
+
     runner = Runner(common_objs['cfg_mgr'])
     for mgr in managers:
         runner.add_manager(mgr)

src/sonic-bgpcfgd/bgpcfgd/managers_chassis_app_db.py

@@ -0,0 +1,50 @@
+from .manager import Manager
+from .managers_device_global import DeviceGlobalCfgMgr
+from .log import log_err, log_debug, log_notice
+import re
+from swsscommon import swsscommon
+
+class ChassisAppDbMgr(Manager):
+    """This class responds to change in tsa_enabled state of the supervisor"""
+
+    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.lc_tsa = ""
+        self.directory = common_objs['directory']
+        self.dev_cfg_mgr = DeviceGlobalCfgMgr(common_objs, "CONFIG_DB", swsscommon.CFG_BGP_DEVICE_GLOBAL_TABLE_NAME)
+        self.directory.subscribe([("CONFIG_DB", swsscommon.CFG_BGP_DEVICE_GLOBAL_TABLE_NAME, "tsa_enabled"),], self.on_lc_tsa_status_change)
+        super(ChassisAppDbMgr, self).__init__(
+            common_objs,
+            [],
+            db,
+            table,
+        )
+
+    def on_lc_tsa_status_change(self):
+        if self.directory.path_exist("CONFIG_DB", swsscommon.CFG_BGP_DEVICE_GLOBAL_TABLE_NAME, "tsa_enabled"):
+            self.lc_tsa = self.directory.get_slot("CONFIG_DB", swsscommon.CFG_BGP_DEVICE_GLOBAL_TABLE_NAME)["tsa_enabled"]
+        log_debug("ChassisAppDbMgr:: LC TSA update handler status %s" % self.lc_tsa)
+
+    def set_handler(self, key, data):
+        log_debug("ChassisAppDbMgr:: set handler")
+
+        if not data:
+            log_err("ChassisAppDbMgr:: data is None")
+            return False
+
+        if "tsa_enabled" in data:
+            if self.lc_tsa == "false":
+                self.dev_cfg_mgr.cfg_mgr.commit()
+                self.dev_cfg_mgr.cfg_mgr.update()
+                self.dev_cfg_mgr.isolate_unisolate_device(data["tsa_enabled"])
+            return True
+        return False
+
+    def del_handler(self, key):
+        log_debug("ChassisAppDbMgr:: del handler")
+        return True

src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py

@@ -20,6 +20,7 @@ def __init__(self, common_objs, db, table):
         :param table: name of the table in the db
         """
         self.switch_type = ""
+        self.chassis_tsa = ""
         self.directory = common_objs['directory']
         self.cfg_mgr = common_objs['cfg_mgr']
         self.constants = common_objs['constants']
@@ -97,11 +98,16 @@ def configure_tsa(self, data=None):
             if "tsa_enabled" in data:
                 state = data["tsa_enabled"]
 
-        if self.is_update_required("tsa_enabled", state):
+        self.chassis_tsa = self.get_chassis_tsa_status()
+        requires_update = self.is_update_required("tsa_enabled", state)
+
+        if state in ["true", "false"] and self.directory.path_exist(self.db_name, self.table_name, "tsa_enabled"):
+            self.directory.put(self.db_name, self.table_name, "tsa_enabled", state)
+
+        if requires_update and self.chassis_tsa == "false":
             self.cfg_mgr.commit()
             self.cfg_mgr.update()
-            if self.isolate_unisolate_device(state):
-                self.directory.put(self.db_name, self.table_name, "tsa_enabled", state)
+            self.isolate_unisolate_device(state)
         else:
             log_notice("DeviceGlobalCfgMgr:: TSA configuration is up-to-date")
 
@@ -167,7 +173,9 @@ def check_state_and_get_tsa_routemaps(self, cfg):
         cmd = ""
         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":
+            chassis_tsa = self.get_chassis_tsa_status()
+
+            if tsa_status == "true" or chassis_tsa == "true":
                 cmds = cfg.replace("#012", "\n").split("\n")
                 log_notice("DeviceGlobalCfgMgr:: Device is isolated. Applying TSA route-maps")
                 cmd = self.get_ts_routemaps(cmds, self.tsa_template)
@@ -228,6 +236,17 @@ def __extract_out_route_map_names(self, cmds):
                 route_map_names.add(result.group(1))
         return route_map_names
 
+    def get_chassis_tsa_status(self):
+        chassis_tsa_status = "false"
+        try:
+            ch = swsscommon.SonicV2Connector(use_unix_socket_path=False)
+            ch.connect(ch.CHASSIS_APP_DB, False)
+            chassis_tsa_status = ch.get(ch.CHASSIS_APP_DB, "BGP_DEVICE_GLOBAL|STATE", 'tsa_enabled')
+        except Exception:
+            pass
+
+        return chassis_tsa_status
+
     def downstream_isolate_unisolate(self, idf_isolation_state):
         """ API to apply IDF configuration """
 

src/sonic-bgpcfgd/bgpcfgd/runner.py

@@ -39,7 +39,10 @@ def add_manager(self, manager):
         table_name = manager.get_table_name()
         db = swsscommon.SonicDBConfig.getDbId(db_name)
         if db not in self.db_connectors:
-            self.db_connectors[db] = swsscommon.DBConnector(db_name, 0)
+            if db_name == "CHASSIS_APP_DB":
+                self.db_connectors[db] = swsscommon.DBConnector(db_name, 0, True, '')
+            else:
+                self.db_connectors[db] = swsscommon.DBConnector(db_name, 0)
 
         if table_name not in self.callbacks[db]:
             conn = self.db_connectors[db]

src/sonic-bgpcfgd/tests/test_chassis_app_db.py

@@ -0,0 +1,142 @@
+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_chassis_app_db
+import bgpcfgd.managers_device_global
+from swsscommon import swsscommon
+from copy import deepcopy
+
+TEMPLATE_PATH = os.path.abspath('../../dockers/docker-fpm-frr/frr')
+BASE_PATH = os.path.abspath('../sonic-bgpcfgd/tests/data/general/peer-group.conf/')
+INTERNAL_BASE_PATH = os.path.abspath('../sonic-bgpcfgd/tests/data/internal/peer-group.conf/')
+global_constants = {
+    "bgp":  {
+        "traffic_shift_community" :"12345:12345",
+        "internal_community_match_tag" : "1001"
+    }
+}
+
+def constructor(check_internal=False):
+    cfg_mgr = MagicMock()
+    def get_text():
+        text = []
+        for line in cfg_mgr.changes.split('\n'):
+            if line.lstrip().startswith('!'):
+                continue
+            text.append(line)
+        text += ["     "]
+        return text
+    def update():
+        if check_internal:
+            cfg_mgr.changes = get_string_from_file("/result_chasiss_packet.conf", INTERNAL_BASE_PATH)
+        else:
+            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 = deepcopy(global_constants)
+    common_objs = {
+        'directory': Directory(),
+        'cfg_mgr':   cfg_mgr,
+        'tf':        TemplateFabric(TEMPLATE_PATH),
+        'constants': constants
+    }
+    mgr = bgpcfgd.managers_chassis_app_db.ChassisAppDbMgr(common_objs, "CHASSIS_APP_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()
+
+    m.lc_tsa = "false"
+    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_config() == get_string_from_file("/result_all_isolate.conf")
+
+    curr_cfg = m.cfg_mgr.get_config()
+    m.lc_tsa = "true"
+    res = m.set_handler("STATE", {"tsa_enabled": "true"})
+    assert res, "Expect True return value for set_handler"
+    assert m.cfg_mgr.get_config() == curr_cfg
+
+@patch('bgpcfgd.managers_device_global.log_debug')
+def test_isolate_device_internal_session(mocked_log_info):
+    m = constructor(check_internal=True)
+
+    m.lc_tsa = "false"
+    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_config() == get_string_from_file("/result_chassis_packet_isolate.conf", INTERNAL_BASE_PATH)
+
+    curr_cfg = m.cfg_mgr.get_config()
+    m.lc_tsa = "true"
+    res = m.set_handler("STATE", {"tsa_enabled": "true"})
+    assert res, "Expect True return value for set_handler"
+    assert m.cfg_mgr.get_config() == curr_cfg
+
+
+@patch('bgpcfgd.managers_device_global.log_debug')
+def test_unisolate_device(mocked_log_info):
+    m = constructor()
+
+    m.lc_tsa = "false"
+    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_config() == get_string_from_file("/result_all_unisolate.conf")
+
+    curr_cfg = m.cfg_mgr.get_config()
+    m.lc_tsa = "true"
+    res = m.set_handler("STATE", {"tsa_enabled": "false"})
+    assert res, "Expect True return value for set_handler"
+    assert m.cfg_mgr.get_config() == curr_cfg
+
+@patch('bgpcfgd.managers_device_global.log_debug')
+def test_unisolate_device_internal_session(mocked_log_info):
+    m = constructor(check_internal=True)
+
+    m.lc_tsa = "false"
+    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_config() == get_string_from_file("/result_chassis_packet_unisolate.conf", INTERNAL_BASE_PATH)
+
+    curr_cfg = m.cfg_mgr.get_config()
+    m.lc_tsa = "true"
+    res = m.set_handler("STATE", {"tsa_enabled": "false"})
+    assert res, "Expect True return value for set_handler"
+    assert m.cfg_mgr.get_config() == curr_cfg
+
+
+def get_string_from_file(filename, base_path=BASE_PATH):
+    fp = open(base_path + filename, "r")
+    cfg = fp.read()
+    fp.close()
+
+    return cfg
+
+@patch('bgpcfgd.managers_chassis_app_db.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("ChassisAppDbMgr:: data is None")
+
+def test_del_handler():
+    m = constructor()
+    res = m.del_handler("STATE")
+    assert res, "Expect True return value for del_handler"