sonic-buildimage
sonic-buildimage copied to clipboard
[multi-asic] Use asic_id/asic_name for APIs to get port config data
Signed-off-by: Anand Mehra [email protected]
Fixes: https://github.com/sonic-net/sonic-buildimage/issues/18431
Why I did it
sonic-cfggen misses handling for namespace at couple of locations. Due to this, when port config is fetched, it returns data from global config and not the namespace.
The function calls for get_path_to_port_config_file get_port_config
needs to be fixed to handle namespace for multi-asic.
Work item tracking
- Microsoft ADO (number only):
How I did it
1.Pass asic_id as argument when calling get_path_to_port_config_file() For single asic, asic_id is None and the function returns the correct port_config.ini file path. For multi-asic, using asic_id provides the correct file path for the asic.
- Pass asic_name as argument when calling get_port_config()
How to verify it
sonic-cfggen -H --print -n -k This command should return the correct port config for the namespace.
Which release branch to backport (provide reason below if selected)
- [ ] 201811
- [ ] 201911
- [ ] 202006
- [ ] 202012
- [ ] 202106
- [ ] 202111
- [ ] 202205
- [] 202211
- [x] 202305
Tested branch (Please provide the tested image version)
- [ ]
- [ ]
Description for the changelog
Link to config_db schema for YANG module changes
A picture of a cute animal (not mandatory but encouraged)
Hi @abdosi , please review.
Hi @abdosi , @rlhui , please help with the review. Thanks
I raised a PR to fix this https://github.com/sonic-net/sonic-buildimage/pull/18452 Please help to review.
Please help with the review. Thanks
@SuvarnaMeenakshi : can you help with review of this.
Thank you for the fix. Seems like was missed in this PR https://github.com/sonic-net/sonic-buildimage/pull/7632 and issue will NOT be seen if sonic-cfggen -m <minigraph.xml> is used. Can we add a unit-test in sonic-config-engine multi-asic unit-test, so that if minigraph.xml is not passed, then the expected PORT from namespace is present in the output?
Thank you for the fix. Seems like was missed in this PR #7632 and issue will NOT be seen if sonic-cfggen -m <minigraph.xml> is used. Can we add a unit-test in sonic-config-engine multi-asic unit-test, so that if minigraph.xml is not passed, then the expected PORT from namespace is present in the output?
Thanks for review @SuvarnaMeenakshi ! May I use another PR to add UT? Busy with a couple of other activities. This PR needs to help with nightly T2 runs.
Thank you for the fix. Seems like was missed in this PR #7632 and issue will NOT be seen if sonic-cfggen -m <minigraph.xml> is used. Can we add a unit-test in sonic-config-engine multi-asic unit-test, so that if minigraph.xml is not passed, then the expected PORT from namespace is present in the output?
Thanks for review @SuvarnaMeenakshi ! May I use another PR to add UT? Busy with a couple of other activities. This PR needs to help with nightly T2 runs.
It might fall through later, it appears to be a simple UT can do the test in this case. similar to test done here: https://github.com/sonic-net/sonic-buildimage/blob/master/src/sonic-config-engine/tests/test_multinpu_cfggen.py#L209, without -m option and could check for presence of BP interface. Without this current fix, the BP interface would not be present in the result as it would pick from global config_db.
Hi @SuvarnaMeenakshi , added the test case, please review. Thanks
Hi @SuvarnaMeenakshi , @abdosi , please suggest what to be done for ms_conflict. Thanks
@anamehra please help resolve conflicts
@anamehra please help resolve conflicts
Hi @rlhui , I am not able to find details of the failure and what needs to be done. Waiting for inputs from @abdosi . Thanks
/azpw ms_conflict -f
Hi @abdosi , please merge. Thanks
@qiluo-msft : can you help with merge of this.