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

[frrcfgd] Support for FRR configuration based on config DB events #5142

Merged
merged 33 commits into from
Jan 25, 2021

Conversation

zhaozhenhong
Copy link
Contributor

@zhaozhenhong zhaozhenhong commented Aug 10, 2020

- Why I did it

  1. Support for non-template based FRR configurations (BGP, route-map, OSPF, static route..etc) using config DB schema.
  2. Support for save & restore - Jinja template based config-DB data read and apply to FRR during startup

- How I did it

Added a new script and corresponding service. Based on the settings in DEVICE_METADATA table, new service will be started along with the bgp container start. And when user changed the BGP or other related table entries in config DB, the daemon will run corresponding VTYSH commands to program on FRR.
Also added jinja template to generate FRR config file to be used by FRR daemons while bgp container restarted

- How to verify it

  1. Add/delete data on config DB and then run VTYSH "show running-config" command to check if FRR configuration changed.
  2. Restart bgp container and check if generated FRR config file is correct and run VTYSH "show running-config" command to check if FRR configuration is consistent with attributes in config DB

- Description for the changelog

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

@ghost
Copy link

ghost commented Aug 10, 2020

CLA assistant check
All CLA requirements met.

@zhaozhenhong zhaozhenhong changed the title Initial commit for adding non-template bgpcfgd script [bgpcfgd] Support for FRR configuration based on config DB events Aug 10, 2020
@zhaozhenhong
Copy link
Contributor Author

HLD: sonic-net/SONiC#544

@lguohan lguohan requested a review from pavel-shirshov August 14, 2020 04:27
@lgtm-com
Copy link

lgtm-com bot commented Aug 14, 2020

This pull request introduces 13 alerts when merging 288986c into c3202d8 - view on LGTM.com

new alerts:

  • 4 for Unused local variable
  • 2 for __eq__ not overridden when adding attributes
  • 2 for Inconsistent equality and inequality
  • 2 for Variable defined multiple times
  • 1 for Unused import
  • 1 for Duplicate key in dict literal
  • 1 for Inconsistent equality and hashing

@pavel-shirshov
Copy link
Contributor

@zhaozhenhong
Can you please make LGTM happy? You can find the report above.

@lgtm-com
Copy link

lgtm-com bot commented Aug 24, 2020

This pull request introduces 13 alerts when merging 2a5c58e into f3feb56 - view on LGTM.com

new alerts:

  • 4 for Unused local variable
  • 2 for __eq__ not overridden when adding attributes
  • 2 for Inconsistent equality and inequality
  • 2 for Variable defined multiple times
  • 1 for Unused import
  • 1 for Duplicate key in dict literal
  • 1 for Inconsistent equality and hashing

@zhaozhenhong
Copy link
Contributor Author

@zhaozhenhong
Can you please make LGTM happy? You can find the report above.

Fix has been added with latest commit

@lgtm-com
Copy link

lgtm-com bot commented Aug 25, 2020

This pull request introduces 3 alerts when merging e942575 into f3feb56 - view on LGTM.com

new alerts:

  • 2 for Comparison of identical values
  • 1 for Inconsistent equality and hashing

@venkatmahalingam
Copy link
Collaborator

venkatmahalingam commented Nov 4, 2020

@lguohan @ben-gale Please let us know the next step on this PR.
Summary:
In DEVICE_METADATA table, field bgp_template_config has been introduced to differentiate whether the config model is template based (e.g current bgpcfgd) or non-template based (new one which has all configs in config-DB and based on events from config-DB, FRR is updated thru VTYSH commands).

@ben-gale
Copy link
Collaborator

ben-gale commented Nov 4, 2020

@lguohan @ben-gale Please let us know the next step on this PR.
Summary:
In DEVICE_METADATA table, field bgp_template_config has been introduced to differentiate whether the config model is template based (e.g current bgpcfgd) or non-template based (new one which has all configs in config-DB and based on events from config-DB, FRR is updated thru VTYSH commands).

I think this is the right approach if MSFT wants to partition out some of the changes from their world. Somewhere else I saw a proposal to define cloud vs. enterprise builds, and use this as a selector, but I think this is too broad, and will create arguments (i.e. what should be in cloud vs. enterprise?).

tahmed-dev
tahmed-dev previously approved these changes Jan 15, 2021
tahmed-dev
tahmed-dev previously approved these changes Jan 19, 2021
@venkatmahalingam
Copy link
Collaborator

@lguohan @tahmed-dev Can you please review the recent commit and merge the PR if everything looks good? also, label "Request for 202012 branch" this PR for 202012 release.

@zhaozhenhong
Copy link
Contributor Author

retest Azure.sonic-buildimage please

@lguohan lguohan merged commit a171e6c into sonic-net:master Jan 25, 2021
@lguohan lguohan changed the title [bgpcfgd] Support for FRR configuration based on config DB events [frrcfgd] Support for FRR configuration based on config DB events Jan 25, 2021
lguohan pushed a commit that referenced this pull request Jan 25, 2021
…ork_config is true (#5142)

- Support for non-template based FRR configurations (BGP, route-map, OSPF, static route..etc) using config DB schema.
- Support for save & restore - Jinja template based config-DB data read and apply to FRR during startup

**- How I did it**

- add frrcfgd service
- when frr_mgmg_framework_config is set, frrcfgd starts in bgp container
- when user changed the BGP or other related table entries in config DB, frrcfgd will run corresponding VTYSH commands to program on FRR.
- add jinja template to generate FRR config file to be used by FRR daemons while bgp container restarted

**- How to verify it**
1. Add/delete data on config DB and then run VTYSH "show running-config" command to check if FRR configuration changed.
1. Restart bgp container and check if generated FRR config file is correct and run VTYSH "show running-config" command to check if FRR configuration is consistent with attributes in config DB

Co-authored-by: Zhenhong Zhao <[email protected]>
@sameerdell
Copy link

@lguohan @pavel-shirshov @tahmed-dev - to get these changes in 202012 branch need help with the label. Dell doesn't have permissions to add label.

@tahmed-dev
Copy link
Contributor

@lguohan @pavel-shirshov @tahmed-dev - to get these changes in 202012 branch need help with the label. Dell doesn't have permissions to add label.

Done!

@legolasCc
Copy link

@zhaozhenhong seems frrcfgd.py have problem in dependency processing between different config db tables. For example: step1 initial data with (BGP_GLOBALS, BGP_NEIGHBOR). step2 delete asn from BGP_GLOBLAS. result to BGP_NEIGHBOR data aslo removed from bgpd.conf. step3, restore asn again. router bgp {} vrf {} is success, while BGP_NEIGHBOR not.

bradh352 added a commit to bradh352/sonic-buildimage that referenced this pull request Dec 5, 2024
Cannot configured unified bgp for vxlan evpn without specifying
advertise-all-vpn.  The setting appears to have been introduced
as part of PR sonic-net#5142

Signed-off-by: Brad House (@bradh352)
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.

9 participants