diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_bbr.py b/src/sonic-bgpcfgd/bgpcfgd/managers_bbr.py index 0e82d0a4b6..6e1a33e894 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_bbr.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_bbr.py @@ -45,26 +45,38 @@ def del_handler(self, key): def __init(self): """ Initialize BBRMgr. Extracted from constructor """ - if not 'bgp' in self.constants: - log_err("BBRMgr::Disabled: 'bgp' key is not found in constants") - return - if 'bbr' in self.constants['bgp'] \ - and 'enabled' in self.constants['bgp']['bbr'] \ - and self.constants['bgp']['bbr']['enabled']: + # Check BGP_BBR table from config_db first + bbr_status_from_config_db = self.get_bbr_status_from_config_db() + + if bbr_status_from_config_db is None: + if not 'bgp' in self.constants: + log_err("BBRMgr::Disabled: 'bgp' key is not found in constants") + return + if 'bbr' in self.constants['bgp'] \ + and 'enabled' in self.constants['bgp']['bbr'] \ + and self.constants['bgp']['bbr']['enabled']: + self.bbr_enabled_pgs = self.__read_pgs() + if self.bbr_enabled_pgs: + self.enabled = True + if 'default_state' in self.constants['bgp']['bbr'] \ + and self.constants['bgp']['bbr']['default_state'] == 'enabled': + default_status = "enabled" + else: + default_status = "disabled" + self.directory.put(self.db_name, self.table_name, 'status', default_status) + log_info("BBRMgr::Initialized and enabled from constants. Default state: '%s'" % default_status) + else: + log_info("BBRMgr::Disabled: no BBR enabled peers") + else: + log_info("BBRMgr::Disabled: no bgp.bbr.enabled in the constants") + else: self.bbr_enabled_pgs = self.__read_pgs() if self.bbr_enabled_pgs: self.enabled = True - if 'default_state' in self.constants['bgp']['bbr'] \ - and self.constants['bgp']['bbr']['default_state'] == 'enabled': - default_status = "enabled" - else: - default_status = "disabled" - self.directory.put(self.db_name, self.table_name, 'status', default_status) - log_info("BBRMgr::Initialized and enabled. Default state: '%s'" % default_status) + self.directory.put(self.db_name, self.table_name, 'status', bbr_status_from_config_db) + log_info("BBRMgr::Initialized and enabled from config_db. Default state: '%s'" % bbr_status_from_config_db) else: log_info("BBRMgr::Disabled: no BBR enabled peers") - else: - log_info("BBRMgr::Disabled: no bgp.bbr.enabled in the constants") def __read_pgs(self): """ @@ -82,6 +94,35 @@ def __read_pgs(self): res[pg_name] = pg_afs return res + def get_bbr_status_from_config_db(self): + """ + Read BBR status from CONFIG_DB + :return: BBR status from CONFIG_DB or None if not found + """ + try: + config_db = swsscommon.ConfigDBConnector() + if config_db is None: + log_info("BBRMgr::Failed to connect to CONFIG_DB, get BBR default state from constants.yml") + return None + config_db.connect() + except Exception as e: + log_info("BBRMgr::Failed to connect to CONFIG_DB with exception %s, get BBR default state from constants.yml" % str(e)) + return None + + try: + bbr_table_data = config_db.get_table(self.table_name) + if bbr_table_data and 'all' in bbr_table_data and 'status' in bbr_table_data["all"]: + if bbr_table_data["all"]["status"] == "enabled": + return "enabled" + else: + return "disabled" + else: + log_info("BBRMgr::BBR status is not found in CONFIG_DB, get BBR default state from constants.yml") + return None + except Exception as e: + log_info("BBRMgr::Failed to read BBR status from CONFIG_DB with exception %s, get BBR default state from constants.yml" % str(e)) + return None + def __set_validation(self, key, data): """ Validate set-command arguments :param key: key of 'set' command diff --git a/src/sonic-bgpcfgd/tests/test_bbr.py b/src/sonic-bgpcfgd/tests/test_bbr.py index b11277bae7..8bb67e9982 100644 --- a/src/sonic-bgpcfgd/tests/test_bbr.py +++ b/src/sonic-bgpcfgd/tests/test_bbr.py @@ -111,6 +111,7 @@ def __init_common(constants, 'tf': TemplateFabric(), 'constants': constants, } + m = BBRMgr(common_objs, "CONFIG_DB", "BGP_BBR") m._BBRMgr__init() assert m.bbr_enabled_pgs == expected_bbr_enabled_pgs @@ -156,7 +157,7 @@ def test___init_6(): "bbr": expected_bbr_entries, } } - __init_common(constants, "BBRMgr::Initialized and enabled. Default state: 'disabled'", None, expected_bbr_entries, "disabled") + __init_common(constants, "BBRMgr::Initialized and enabled from constants. Default state: 'disabled'", None, expected_bbr_entries, "disabled") def test___init_7(): expected_bbr_entries = { @@ -170,7 +171,7 @@ def test___init_7(): "bbr": expected_bbr_entries, } } - __init_common(constants, "BBRMgr::Initialized and enabled. Default state: 'disabled'", None, expected_bbr_entries, "disabled") + __init_common(constants, "BBRMgr::Initialized and enabled from constants. Default state: 'disabled'", None, expected_bbr_entries, "disabled") def test___init_8(): expected_bbr_entries = { @@ -184,7 +185,32 @@ def test___init_8(): "bbr": expected_bbr_entries, } } - __init_common(constants, "BBRMgr::Initialized and enabled. Default state: 'enabled'", None, expected_bbr_entries, "enabled") + __init_common(constants, "BBRMgr::Initialized and enabled from constants. Default state: 'enabled'", None, expected_bbr_entries, "enabled") + +@patch('bgpcfgd.managers_bbr.BBRMgr.get_bbr_status_from_config_db', return_value='disabled') +def test___init_with_config_db_overwirte_constants(mocked_get_bbr_status_from_config_db): + expected_bbr_entries = { + "PEER_V4": ["ipv4"], + "PEER_V6": ["ipv6"], + } + constants = deepcopy(global_constants) + constants["bgp"]["bbr"] = {"enabled": True, "default_state": "enabled"} + constants["bgp"]["peers"] = { + "general": { + "bbr": expected_bbr_entries, + } + } + + # BBR status from config_db should be prioritized over constants + __init_common(constants, "BBRMgr::Initialized and enabled from config_db. Default state: 'disabled'", None, expected_bbr_entries, "disabled") + +@patch('bgpcfgd.managers_bbr.BBRMgr.get_bbr_status_from_config_db', return_value='enabled') +def test___init_with_config_db_no_peers(mocked_get_bbr_status_from_config_db): + + constants = deepcopy(global_constants) + constants["bgp"]["bbr"] = {"enabled": True} + + __init_common(constants, "BBRMgr::Disabled: no BBR enabled peers", None, {}, "disabled") @patch('bgpcfgd.managers_bbr.log_info') def read_pgs_common(constants, expected_log_info, expected_bbr_enabled_pgs, mocked_log_info):