Skip to content

[Draft] Revert "[sonic-bgpcfgd] Add ability to wait on rehydration for additional Loopback IPs during reloads"#26773

Closed
xwjiang-ms wants to merge 1 commit into
masterfrom
revert-26349-additional_los
Closed

[Draft] Revert "[sonic-bgpcfgd] Add ability to wait on rehydration for additional Loopback IPs during reloads"#26773
xwjiang-ms wants to merge 1 commit into
masterfrom
revert-26349-additional_los

Conversation

@xwjiang-ms

Copy link
Copy Markdown
Contributor

Reverts #26349

Copilot AI review requested due to automatic review settings April 15, 2026 00:24
@xwjiang-ms xwjiang-ms marked this pull request as draft April 15, 2026 00:24
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reverts the earlier change that added support for waiting on rehydration of additional loopback IPs (beyond the existing loopback requirements) during BGP reload handling in sonic-bgpcfgd.

Changes:

  • Removes the “additional loopbacks” discovery/init logic from the BGP peer manager and restores explicit Loopback0/Loopback4096 gating behavior.
  • Updates unit tests to stop asserting the removed informational log messages.
  • Deletes the internal additional_loopbacks.conf.j2 template that supported the reverted behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/sonic-bgpcfgd/tests/test_bgp.py Removes mocks/assertions tied to the reverted “additional loopbacks acquired” logging.
src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py Reverts additional-loopback initialization and restores direct Loopback0/Loopback4096 checks in add_peer.
dockers/docker-fpm-frr/frr/bgpd/templates/internal/additional_loopbacks.conf.j2 Removes the template used for additional loopback enumeration (now unused after revert).

Comment on lines 483 to 488
def get_lo_ipv4(self, loopback_str):
"""
Extract LoopbackX ipv4 address from the Directory
:return: ipv4 address for LoopbackX, None if nothing found
Extract Loopback0 ipv4 address from the Directory
:return: ipv4 address for Loopback0, None if nothing found
"""
loopback0_ipv4 = None

Copilot AI Apr 15, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The get_lo_ipv4() docstring now states it only extracts Loopback0 IPv4, but the function is used for other loopbacks as well (e.g., Loopback4096| in add_peer). Please update the docstring (and ideally the local variable name loopback0_ipv4) to reflect that this is a generic helper for any loopback prefix passed via loopback_str.

Copilot uses AI. Check for mistakes.
@yxieca

yxieca commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

/azpw ms_conflict

@yxieca

yxieca commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

/azpw ms_conflict

3 similar comments
@yxieca

yxieca commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

/azpw ms_conflict

@yxieca

yxieca commented Apr 17, 2026

Copy link
Copy Markdown
Contributor

/azpw ms_conflict

@yxieca

yxieca commented Apr 20, 2026

Copy link
Copy Markdown
Contributor

/azpw ms_conflict

@xwjiang-ms xwjiang-ms closed this Apr 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants