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]: Split one bgp mega-template to chunks. #4143

Merged
merged 34 commits into from
Apr 23, 2020

Conversation

pavel-shirshov
Copy link
Contributor

@pavel-shirshov pavel-shirshov commented Feb 12, 2020

The one big bgp configuration template was splitted into chunks.

Currently we have three types of bgp neighbor peers:

  • general bgp peers. They are represented by CONFIG_DB::BGP_NEIGHBOR table entries
  • dynamic bgp peers. They are represented by CONFIG_DB::BGP_PEER_RANGE table entries
  • monitors bgp peers. They are represented by CONFIG_DB::BGP_MONITORS table entries

This PR introduces three templates for each peer type:

  • bgp policies: represent policieas that will be applied to the bgp peer-group (ip prefix-lists, route-maps, etc)
  • bgp peer-group: represent bgp peer group which has common configuration for the bgp peer type and uses bgp routing policy from the previous item
  • bgp peer-group instance: represent bgp configuration, which will be used to instatiate a bgp peer-group for the bgp peer-type. Usually this one is simple, consist of the referral to the bgp peer-group, bgp peer description and bgp peer ip address.

This PR redefined constant.yml file. Now this file has a setting for to use or don't use bgp_neighbor metadata. This file has more parameters for now, which are not used. They will be used in the next iteration of bgpcfgd.

Currently all tests have been disabled. I'm going to create next PR with the tests right after this PR is merged.

I'm going to introduce better bgpcfgd in a short time. It will include support of dynamic changes for the templates.

FIX:: #4231

@pavel-shirshov pavel-shirshov marked this pull request as ready for review February 12, 2020 22:40
@pavel-shirshov pavel-shirshov changed the title New bgpcfgd [bgpcfgd]: Split one bgp mega-template to chunks. Feb 12, 2020
@pavel-shirshov pavel-shirshov self-assigned this Feb 12, 2020
@pavel-shirshov
Copy link
Contributor Author

retest vsimage please

4 similar comments
@pavel-shirshov
Copy link
Contributor Author

retest vsimage please

@pavel-shirshov
Copy link
Contributor Author

retest vsimage please

@lguohan
Copy link
Collaborator

lguohan commented Mar 9, 2020

retest vsimage please

@lguohan
Copy link
Collaborator

lguohan commented Mar 21, 2020

retest vsimage please

@lguohan
Copy link
Collaborator

lguohan commented Mar 23, 2020

can you look at the test failure? I think there is a real issue here.

@pavel-shirshov
Copy link
Contributor Author

@lguohan Yes. I'm currently working on it.

@lguohan
Copy link
Collaborator

lguohan commented Apr 10, 2020

can you update the config db entry that this pr support?

@pavel-shirshov
Copy link
Contributor Author

I'm still working on the changes.

@lgtm-com
Copy link

lgtm-com bot commented Apr 14, 2020

This pull request introduces 2 alerts and fixes 3 when merging cd06c31 into e04eb80 - view on LGTM.com

new alerts:

  • 2 for Except block handles 'BaseException'

fixed alerts:

  • 1 for Unused local variable
  • 1 for Except block handles 'BaseException'
  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Apr 14, 2020

This pull request fixes 7 alerts when merging e9dbfa3 into a02255e - view on LGTM.com

fixed alerts:

  • 5 for Except block handles 'BaseException'
  • 1 for Unused local variable
  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Apr 15, 2020

This pull request fixes 7 alerts when merging fc51718 into 89fb105 - view on LGTM.com

fixed alerts:

  • 5 for Except block handles 'BaseException'
  • 1 for Unused local variable
  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Apr 21, 2020

This pull request introduces 2 alerts and fixes 7 when merging ea88cfd into bddd0d1 - view on LGTM.com

new alerts:

  • 2 for Unreachable code

fixed alerts:

  • 5 for Except block handles 'BaseException'
  • 1 for Unused local variable
  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Apr 22, 2020

This pull request introduces 2 alerts and fixes 7 when merging d44926e into 3a82ade - view on LGTM.com

new alerts:

  • 2 for Unreachable code

fixed alerts:

  • 5 for Except block handles 'BaseException'
  • 1 for Unused local variable
  • 1 for Unused import

@lguohan
Copy link
Collaborator

lguohan commented Apr 22, 2020

please merge with proper commit messages.

Copy link
Collaborator

@lguohan lguohan left a comment

Choose a reason for hiding this comment

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

as comments

@lgtm-com
Copy link

lgtm-com bot commented Apr 22, 2020

This pull request introduces 2 alerts and fixes 7 when merging 1b9def0 into 3a82ade - view on LGTM.com

new alerts:

  • 2 for Unreachable code

fixed alerts:

  • 5 for Except block handles 'BaseException'
  • 1 for Unused local variable
  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Apr 22, 2020

This pull request introduces 2 alerts and fixes 7 when merging 1272cd4 into c5e9844 - view on LGTM.com

new alerts:

  • 2 for Unreachable code

fixed alerts:

  • 5 for Except block handles 'BaseException'
  • 1 for Unused local variable
  • 1 for Unused import

@pavel-shirshov
Copy link
Contributor Author

retest vsimage please

@lguohan
Copy link
Collaborator

lguohan commented Apr 23, 2020

Signed-off-by: Guohan Lu <[email protected]>
@lguohan lguohan merged commit 057ced0 into sonic-net:master Apr 23, 2020
@sonic-net sonic-net deleted a comment from paulnice Apr 23, 2020
@pavel-shirshov
Copy link
Contributor Author

Thanks

@pavel-shirshov pavel-shirshov deleted the pavelsh/new branch April 23, 2020 18:15
lguohan pushed a commit that referenced this pull request Apr 25, 2020
The one big bgp configuration template was splitted into chunks.

Currently we have three types of bgp neighbor peers:

general bgp peers. They are represented by CONFIG_DB::BGP_NEIGHBOR table entries
dynamic bgp peers. They are represented by CONFIG_DB::BGP_PEER_RANGE table entries
monitors bgp peers. They are represented by CONFIG_DB::BGP_MONITORS table entries
This PR introduces three templates for each peer type:

bgp policies: represent policieas that will be applied to the bgp peer-group (ip prefix-lists, route-maps, etc)
bgp peer-group: represent bgp peer group which has common configuration for the bgp peer type and uses bgp routing policy from the previous item
bgp peer-group instance: represent bgp configuration, which will be used to instatiate a bgp peer-group for the bgp peer-type. Usually this one is simple, consist of the referral to the bgp peer-group, bgp peer description and bgp peer ip address.
This PR redefined constant.yml file. Now this file has a setting for to use or don't use bgp_neighbor metadata. This file has more parameters for now, which are not used. They will be used in the next iteration of bgpcfgd.

Currently all tests have been disabled. I'm going to create next PR with the tests right after this PR is merged.

I'm going to introduce better bgpcfgd in a short time. It will include support of dynamic changes for the templates.

FIX:: #4231
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.

BGP neighbor configurations are not pushed to frr when DEVICE_NEIGHBOR_METADATA is not present
3 participants