Prioritize configuration from config_db over constants.yml during BBR status initialization (#19590)

Why I did it
There are two places which we can configure the BBR disabled/enabled, one is constants.yml, the other is by config_db. During run time, config_db win. But at the init stage, constants.yml win. This is a wrong logic, constants.yml only win when there is no such config in the config_db.

Work item tracking
Microsoft ADO (number only): 28302790
How I did it
Initialize BBR status from constants.yml only when config_db doesn't have BBR entry.

How to verify it
Build image and run the following:

# bbr status in constants.yml is set to disabled
# change the bbr status in config_db to enabled and reload config
admin@str2-7050cx3-acs-01:~$ redis-cli -n 4 hset "BGP_BBR|all" "status" "enabled"
(integer) 0
admin@str2-7050cx3-acs-01:~$ redis-cli -n 4 hget "BGP_BBR|all" "status"
"enabled"
admin@str2-7050cx3-acs-01:~$ vtysh -c 'show run' | grep 'allowas'
  neighbor PEER_V4 allowas-in 1
  neighbor PEER_V6 allowas-in 1
admin@str2-7050cx3-acs-01:~$ sudo config save -y
Running command: /usr/local/bin/sonic-cfggen -d --print-data > /etc/sonic/config_db.json
admin@str2-7050cx3-acs-01:~$ sudo config reload -y

# check bgpcfgd log, bbr default status is enabled
2024 Aug 14 05:42:47.653216 str2-7050cx3-acs-01 INFO bgp#bgpcfgd: BBRMgr::Initialized and enabled from config_db. Default state: 'enabled'

Author: Gfrom2016

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

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):