From 2867bdc49cef5ccfa617a55018fba9abb12b37eb Mon Sep 17 00:00:00 2001 From: Sonic Build Admin Date: Tue, 7 Apr 2026 01:01:53 +0000 Subject: [PATCH] [sonic-bgpcfgd] Add ability to wait on rehydration for additional Loopback IPs during reloads #### Why I did it If there are additional Loopback IPs defined in config db that are being advertised out via frr template files(using bgp network statements) and their config has not replayed in sonic-bgpcfgd, then these loopback ips transiently fail to be advertised out by the templates. ##### Work item tracking - Microsoft ADO **(number only)**: #### How I did it We introduce the ability to check for and fail peer addition requests if a additional loopbacks, as specified via a per-peer J2 file(additional_loopbacks.j2), are not replayed to bgpcfgd yet. #### How to verify it bgpcfgd unit test, and real device test in lab #### Which release branch to backport (provide reason below if selected) - [ ] 202305 - [ ] 202311 - [ ] 202405 - [ ] 202411 - [ ] 202505 - [x] 202511 #### Tested branch (Please provide the tested image version) - [ ] - [ ] #### Description for the changelog #### Link to config_db schema for YANG module changes Signed-off-by: Sonic Build Admin #### A picture of a cute animal (not mandatory but encouraged) --- .../internal/additional_loopbacks.conf.j2 | 1 + src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py | 57 ++++++++++++++----- src/sonic-bgpcfgd/tests/test_bgp.py | 10 +++- 3 files changed, 51 insertions(+), 17 deletions(-) create mode 100644 dockers/docker-fpm-frr/frr/bgpd/templates/internal/additional_loopbacks.conf.j2 diff --git a/dockers/docker-fpm-frr/frr/bgpd/templates/internal/additional_loopbacks.conf.j2 b/dockers/docker-fpm-frr/frr/bgpd/templates/internal/additional_loopbacks.conf.j2 new file mode 100644 index 0000000000..745178e489 --- /dev/null +++ b/dockers/docker-fpm-frr/frr/bgpd/templates/internal/additional_loopbacks.conf.j2 @@ -0,0 +1 @@ +Loopback4096 diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py b/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py index b9d3f514a8..b727400d97 100644 --- a/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py +++ b/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py @@ -97,6 +97,8 @@ def __init__(self, common_objs, db_name, table_name, peer_type, check_neig_meta) self.constants = self.common_objs["constants"] self.fabric = common_objs['tf'] self.peer_type = peer_type + self.loopbacks = ["Loopback0"] + self.post_dependencies_init_complete = False base_template = "bgpd/templates/" + self.constants["bgp"]["peers"][peer_type]["template_dir"] + "/" self.templates = { @@ -175,23 +177,20 @@ def add_peer(self, vrf, nbr, data): :param data: associated data :return: True if this adding was successful, False otherwise """ - print_data = vrf, nbr, data - bgp_asn = self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME)["localhost"]["bgp_asn"] - # - lo0_ipv4 = self.get_lo_ipv4("Loopback0|") - if (lo0_ipv4 is None and "bgp_router_id" - not in self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME)["localhost"]): - log_warn("Loopback0 ipv4 address is not presented yet and bgp_router_id not configured") - return False - # - if self.peer_type == 'internal': - lo4096_ipv4 = self.get_lo_ipv4("Loopback4096|") - if (lo4096_ipv4 is None and "bgp_router_id" + if not self.post_dependencies_init_complete: + self.post_dependencies_init() + + for loopback in self.loopbacks: + lo_ipv4 = self.get_lo_ipv4(loopback + "|") + if (lo_ipv4 is None and "bgp_router_id" not in self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME)["localhost"]): - log_warn("Loopback4096 ipv4 address is not presented yet and bgp_router_id not configured") + log_warn(loopback + " ipv4 address is not presented yet and bgp_router_id not configured") return False + print_data = vrf, nbr, data + bgp_asn = self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME)["localhost"]["bgp_asn"] + if "local_addr" not in data: log_warn("Peer %s. Missing attribute 'local_addr'" % nbr) else: @@ -213,6 +212,8 @@ def add_peer(self, vrf, nbr, data): 'CONFIG_DB__LOOPBACK_INTERFACE':{ tuple(key.split('|')) : {} for key in self.directory.get_slot("CONFIG_DB", swsscommon.CFG_LOOPBACK_INTERFACE_TABLE_NAME) if '|' in key } } + + lo0_ipv4 = self.get_lo_ipv4("Loopback0|") if lo0_ipv4 is not None: kwargs['loopback0_ipv4'] = lo0_ipv4 if self.check_neig_meta: @@ -241,6 +242,32 @@ def add_peer(self, vrf, nbr, data): self.directory.put(self.db_name, self.table_name, vrf + '|' + nbr, data) return True + def post_dependencies_init(self): + self.post_dependencies_init_complete = True #Skip retrying template render failures to not impact existing workflow + orig_lo_list_len = len(self.loopbacks) + base_template = "bgpd/templates/" + self.constants["bgp"]["peers"][self.peer_type]["template_dir"] + "/" + if (os.path.exists(self.fabric.env.loader.searchpath[0] + "/" + base_template + "additional_loopbacks.conf.j2")): + kwargs = { + 'CONFIG_DB__DEVICE_METADATA': self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME) + } + try: + rendered_loopbacks = self.fabric.from_file(base_template + "additional_loopbacks.conf.j2").render(**kwargs) + except jinja2.TemplateError as e: + msg = "Error in rendering the template " + base_template + "additional_loopbacks.conf.j2" + log_err("%s: %s" % (msg, str(e))) + return + + for loopback in rendered_loopbacks.splitlines(): + loopback_name = loopback.strip() + if loopback_name and loopback_name not in self.loopbacks: + self.loopbacks.append(loopback_name) + + if len(self.loopbacks) > orig_lo_list_len: + log_info("Additional loopbacks acquired for peer %s, loopback list %s" % (self.peer_type, self.loopbacks)) + else: + log_info("No additional loopbacks acquired for peer %s, loopback list %s" % (self.peer_type, self.loopbacks)) + + def update_state_db(self, vrf, nbr, data, op): """ Update the database with the new data @@ -482,8 +509,8 @@ def apply_op(self, cmd, vrf): def get_lo_ipv4(self, loopback_str): """ - Extract Loopback0 ipv4 address from the Directory - :return: ipv4 address for Loopback0, None if nothing found + Extract LoopbackX ipv4 address from the Directory + :return: ipv4 address for LoopbackX, None if nothing found """ loopback0_ipv4 = None for loopback in self.directory.get_slot("CONFIG_DB", swsscommon.CFG_LOOPBACK_INTERFACE_TABLE_NAME).keys(): diff --git a/src/sonic-bgpcfgd/tests/test_bgp.py b/src/sonic-bgpcfgd/tests/test_bgp.py index e66c5621c4..28bda36ca4 100644 --- a/src/sonic-bgpcfgd/tests/test_bgp.py +++ b/src/sonic-bgpcfgd/tests/test_bgp.py @@ -115,11 +115,13 @@ def test_add_peer_internal(): res = m.set_handler("30.30.30.1", {'asn': '65200', 'holdtime': '180', 'keepalive': '60', 'local_addr': '30.30.30.30', 'name': 'TOR', 'nhopself': '0', 'rrclient': '0'}) assert res, "Expect True return value" -def test_add_peer_internal_no_router_id_no_lo4096(): +@patch('bgpcfgd.managers_bgp.log_info') +def test_add_peer_internal_no_router_id_no_lo4096(mocked_log_info): for constant in load_constant_files(): m = constructor(constant, peer_type="internal") res = m.set_handler("30.30.30.1", {'asn': '65200', 'holdtime': '180', 'keepalive': '60', 'local_addr': '30.30.30.30', 'name': 'TOR', 'nhopself': '0', 'rrclient': '0'}) assert not res, "Expect False return value" + mocked_log_info.assert_called_with("Additional loopbacks acquired for peer internal, loopback list ['Loopback0', 'Loopback4096']") def test_add_peer_internal_router_id(): for constant in load_constant_files(): @@ -137,11 +139,15 @@ def test_add_peer_router_id(): res = m.set_handler("30.30.30.1", {'asn': '65200', 'holdtime': '180', 'keepalive': '60', 'local_addr': '30.30.30.30', 'name': 'TOR', 'nhopself': '0', 'rrclient': '0'}) assert res, "Expect True return value" -def test_add_peer_without_lo_ipv4(): +@patch('bgpcfgd.managers_bgp.log_info') +@patch('bgpcfgd.managers_bgp.log_warn') +def test_add_peer_without_lo_ipv4(mocked_log_warn, mocked_log_info): for constant in load_constant_files(): m = constructor(constant, with_lo0_ipv4=False) res = m.set_handler("30.30.30.1", {'asn': '65200', 'holdtime': '180', 'keepalive': '60', 'local_addr': '30.30.30.30', 'name': 'TOR', 'nhopself': '0', 'rrclient': '0'}) assert not res, "Expect False return value" + mocked_log_info.assert_called_with("No additional loopbacks acquired for peer general, loopback list ['Loopback0']") + mocked_log_warn.assert_called_with("Loopback0 ipv4 address is not presented yet and bgp_router_id not configured") def test_add_peer_without_lo_ipv4_router_id(): for constant in load_constant_files():