Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bgpcfgd] Remove unnecessary dependency for StaticRouteMgr #8037

Merged
merged 2 commits into from
Jul 9, 2021

Conversation

shi-su
Copy link
Contributor

@shi-su shi-su commented Jul 2, 2021

Why I did it

Static route configuration should not depend on BGP_ASN. Remove the dependency on BGP_ASN for StaticRouteMgr.
Fix #8027

How I did it

Check if BGP_ASN field before configuring static route redistribution and wait until BGP_ASN is available to enable static route redistribution.

How to verify it

Add unit test to cover the scenario and verify the functionality on a virtual switch.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106

Description for the changelog

A picture of a cute animal (not mandatory but encouraged)

@shi-su shi-su requested a review from lguohan as a code owner July 2, 2021 00:38
@shi-su shi-su requested a review from prsunny July 2, 2021 00:41
@liat-grozovik
Copy link
Collaborator

@vivekreddynv could you please review and provide feedback that indeed it is fixed following the issue you raised?
@shi-su i suggest to have it in 202012 and 202106. Please check it can be taken to these branches and set proper label.

@vivekrnv
Copy link
Contributor

vivekrnv commented Jul 6, 2021

Verified.

admin@sonic:~$ sudo config route add prefix 20.0.0.0/24 nexthop 30.0.0.2
admin@sonic:~$ show ip route 20.0.0.0/24
Routing entry for 20.0.0.0/24
  Known via "static", distance 1, metric 0, best
  Last update 00:00:08 ago
    30.0.0.2 (recursive), weight 1
       10.210.24.1, via eth0, weight 1

admin@sonic:~$ show logging -f | grep bgpcfgd
Jul  6 06:03:15.525457 sonic DEBUG bgp#bgpcfgd: Received message : '('20.0.0.0/24', 'SET', (('blackhole', 'false'), ('distance', '0'), ('ifname', ''), ('nexthop', '30.0.0.2'), ('nexthop-vrf', '')))'
Jul  6 06:03:15.525550 sonic DEBUG bgp#bgpcfgd: Static route 20.0.0.0/24 is scheduled for updates
Jul  6 06:03:15.525887 sonic DEBUG bgp#bgpcfgd: execute command '['vtysh', '-f', '/tmp/tmp_7p8s_gz']'.

@shi-su shi-su merged commit c85abcb into sonic-net:master Jul 9, 2021
qiluo-msft pushed a commit that referenced this pull request Jul 13, 2021
Why I did it
Static route configuration should not depend on BGP_ASN. Remove the dependency on BGP_ASN for StaticRouteMgr.
Fix #8027

How I did it
Check if BGP_ASN field before configuring static route redistribution and wait until BGP_ASN is available to enable static route redistribution.

How to verify it
Add unit test to cover the scenario and verify the functionality on a virtual switch.
judyjoseph pushed a commit that referenced this pull request Jul 13, 2021
Why I did it
Static route configuration should not depend on BGP_ASN. Remove the dependency on BGP_ASN for StaticRouteMgr.
Fix #8027

How I did it
Check if BGP_ASN field before configuring static route redistribution and wait until BGP_ASN is available to enable static route redistribution.

How to verify it
Add unit test to cover the scenario and verify the functionality on a virtual switch.
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
…#8037)

Why I did it
Static route configuration should not depend on BGP_ASN. Remove the dependency on BGP_ASN for StaticRouteMgr.
Fix sonic-net#8027

How I did it
Check if BGP_ASN field before configuring static route redistribution and wait until BGP_ASN is available to enable static route redistribution.

How to verify it
Add unit test to cover the scenario and verify the functionality on a virtual switch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bgpcfgd] Couldn't configure a Static Route without BGP_ASN field
6 participants