Skip to content

[sonic-cfggen]: Add bgp asn for yang validation #9640

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

Merged
merged 2 commits into from
Jan 12, 2022

Conversation

ganglyu
Copy link
Contributor

@ganglyu ganglyu commented Dec 23, 2021

Signed-off-by: Gang Lv [email protected]

Why I did it

Config db schema generated by minigraph can’t pass yang validation, bgp_asn must not be None.

How I did it

Update sampe-voq-graph.xml to add bgp_asn.

How to verify it

Build sonic-config-engine.
Run command 'sonic-cfggen -m tests/sample-voq-graph.xml -p tests/voq-sample-port-config.ini --print-data', and check bgp_asn.

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106

Description for the changelog

Fix #9576

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

@ganglyu ganglyu requested a review from qiluo-msft December 23, 2021 06:36
@ganglyu ganglyu requested a review from lguohan as a code owner December 23, 2021 06:36
@qiluo-msft qiluo-msft requested a review from arlakshm December 24, 2021 18:20
@qiluo-msft
Copy link
Collaborator

qiluo-msft commented Dec 24, 2021

@arlakshm @shi-su Could you help review?

@qiluo-msft qiluo-msft requested a review from shi-su December 24, 2021 19:10
@qiluo-msft
Copy link
Collaborator

My understanding of currently SONiC behaviors:

  1. supporting missing "DEVICE_METADATA" - "bgp_asn"
  2. supporting "DEVICE_METADATA" - "bgp_asn" with a value "None"
    • The case appears if /etc/sonic/minigraph.xml miss
    • /etc/frr/bgpd.conf will have a router bgp None line, which is not valid
  3. supporting "DEVICE_METADATA" - "bgp_asn" with a int value
    • The normal L3 switch config

So right now, we need to fix sonic-device_metadata.yang to support above behaviors. In future, ideally we need to improve the first 2 cases, let /etc/frr/bgpd.conf handles them gracefully.

<a:ASN>65100</a:ASN>
<a:Hostname>linecard-1</a:Hostname>
<a:Peers>
<BGPPeer>
Copy link
Collaborator

Choose a reason for hiding this comment

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

do you need all these peers or we can just add asn and that is enough?

@lguohan lguohan added the YANG YANG model related changes label Jan 5, 2022
@ganglyu ganglyu requested a review from lguohan January 6, 2022 05:44
@ganglyu ganglyu merged commit c11ab02 into sonic-net:master Jan 12, 2022
@ganglyu ganglyu added the Request for 202111 Branch For PRs being requested for 202111 branch label Jan 20, 2022
judyjoseph pushed a commit that referenced this pull request Jan 23, 2022
Why I did it
Config db schema generated by minigraph can’t pass yang validation, bgp_asn must not be None.

How I did it
Update sampe-voq-graph.xml to add bgp_asn.

How to verify it
Build sonic-config-engine.
Run command 'sonic-cfggen -m tests/sample-voq-graph.xml -p tests/voq-sample-port-config.ini --print-data', and check bgp_asn.

Signed-off-by: Gang Lv [email protected]
abdosi added a commit to abdosi/sonic-buildimage that referenced this pull request Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Included in 202111 Branch Request for 202111 Branch For PRs being requested for 202111 branch YANG YANG model related changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[yang-models] device_metadata validation issue
4 participants