Merge pull request #475 from mssonicbld/sonicbld/202305-merge

[code sync] Merge code from sonic-net/sonic-buildimage:202305 to 202305
Azure · Dec 10, 2024 · a206101 · a206101
2 parents ec9a859 + 5167267
commit a206101
Show file tree

Hide file tree

Showing 2 changed files with 85 additions and 18 deletions.
diff --git 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
@@ -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):