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

[sonic-cfggen]: Fix generated deployment_id #10154

Merged
merged 2 commits into from
Mar 8, 2022

Conversation

ganglyu
Copy link
Contributor

@ganglyu ganglyu commented Mar 4, 2022

Signed-off-by: Gang Lv ganglv@microsoft.com

Why I did it

Config db schema generated by minigraph can’t pass yang validation, deployment_id can’t be none for yang validation.

How I did it

Update minigraph.py, skip deployment_id with None value

How to verify it

Run UT for sonic-config-enginue.
Run command 'sonic-cfggen -m tests/multi_npu_data/sample-minigraph-noportchannel.xml -p tests/multi_npu_data/sample_port_config-3.ini -n asic3 --print-data'.

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111

Description for the changelog

Fix #9592

Link to config_db schema for YANG module changes

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

Signed-off-by: Gang Lv ganglv@microsoft.com
@ganglyu ganglyu added the sonic-cfggen SONiC Configuration Generator Tool label Mar 4, 2022
@ganglyu ganglyu requested a review from qiluo-msft March 4, 2022 06:20
@ganglyu ganglyu requested a review from lguohan as a code owner March 4, 2022 06:20
Copy link
Contributor

@SuvarnaMeenakshi SuvarnaMeenakshi left a comment

Choose a reason for hiding this comment

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

lgtm

@SuvarnaMeenakshi
Copy link
Contributor

@arlakshm fyi

@@ -1293,6 +1292,9 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
}
}

if deployment_id:
results['DEVICE_METADATA']['localhost']['deployment_id'] = deployment_id
Copy link
Collaborator

@qiluo-msft qiluo-msft Mar 7, 2022

Choose a reason for hiding this comment

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

deployment_id

Please add a test case for this field missing. #Closed

@@ -1293,6 +1292,9 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
}
}

if deployment_id:
Copy link
Collaborator

@qiluo-msft qiluo-msft Mar 7, 2022

Choose a reason for hiding this comment

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

deployment_id

It is working with current code. I am worried about if anyone will accidently modify the implementation of parse_meta(), changing deployment_id from str to int. And if the deployment_id == 0, it will be skipped in result.

Suggest check deployment_id is not None. #Closed

Signed-off-by: Gang Lv ganglv@microsoft.com
Copy link
Contributor

@arlakshm arlakshm left a comment

Choose a reason for hiding this comment

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

lgtm

@ganglyu ganglyu merged commit 29f6b01 into sonic-net:master Mar 8, 2022
@ganglyu ganglyu added the Request for 202111 Branch For PRs being requested for 202111 branch label Apr 14, 2022
judyjoseph pushed a commit that referenced this pull request Apr 18, 2022
Why I did it
Config db schema generated by minigraph can’t pass yang validation, deployment_id can’t be none for yang validation.

How I did it
Update minigraph.py, skip deployment_id with None value

How to verify it
Run UT for sonic-config-enginue.
Run command 'sonic-cfggen -m tests/multi_npu_data/sample-minigraph-noportchannel.xml -p tests/multi_npu_data/sample_port_config-3.ini -n asic3 --print-data'.

Signed-off-by: Gang Lv ganglv@microsoft.com
abdosi added a commit that referenced this pull request Jul 18, 2022
)

What I did:
Added Support for deployment_id parsing for Device Asic metadata.

Why I did:-
Deployment Id is used in BGP docker for FRR template generation. For multi-asic platforms running in namespace without deployment id as key in DEVICE_METADATA FRR template generation fails. This change is needed after this #10154 where if deployment_id is none we don't update DEVICE_METADA dictionary.

How I verify:-
Added unit-test.
yxieca pushed a commit that referenced this pull request Jul 28, 2022
)

What I did:
Added Support for deployment_id parsing for Device Asic metadata.

Why I did:-
Deployment Id is used in BGP docker for FRR template generation. For multi-asic platforms running in namespace without deployment id as key in DEVICE_METADATA FRR template generation fails. This change is needed after this #10154 where if deployment_id is none we don't update DEVICE_METADA dictionary.

How I verify:-
Added unit-test.
skbarista pushed a commit to skbarista/sonic-buildimage that referenced this pull request Aug 17, 2022
…ic-net#11454)

What I did:
Added Support for deployment_id parsing for Device Asic metadata.

Why I did:-
Deployment Id is used in BGP docker for FRR template generation. For multi-asic platforms running in namespace without deployment id as key in DEVICE_METADATA FRR template generation fails. This change is needed after this sonic-net#10154 where if deployment_id is none we don't update DEVICE_METADA dictionary.

How I verify:-
Added unit-test.
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 sonic-cfggen SONiC Configuration Generator Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[yang-models] device_metadata validation issue
5 participants