sonic-yang-mgmt: uses clause with leaf-list, choice not honored#21907
Conversation
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
5e93791 to
0a069b5
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
f7775f7 to
1c8718c
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
bradh352
changed the title
|
@wen587, @venkatmahalingam you approved the PR that this is replacing. Can you review and approve this instead? |
1c8718c to
363be19
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
363be19 to
15ef7ab
Compare
|
@adyeung just wanted to see if we could move this forward. Thanks. |
|
@qiluo-msft @dgsudharsan can you please help to take a look at this issue? Thanks. |
|
Hi @vaibhavhd - can you find a reviewer for this one? |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes handling of YANG uses clauses by including leaf-list and choice entries and honoring refine, and adds corresponding test cases and sample config updates for BGP route-map validation.
- Extend grouping preprocessing to capture
leaf-listandchoicenodes. - Introduce
_compileUsesClauseto inlineusesclauses into the schema. - Add tests and sample config entries for valid BGP neighbor/peergroup route-map references.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/sonic-yang-models/tests/yang_model_tests/tests_config/bgp.json | Add valid BGP_NEIGHBOR_AF_ROUTE_MAP_VALID and BGP_PEERGROUP_AF_ROUTE_MAP_VALID configs |
| src/sonic-yang-models/tests/yang_model_tests/tests/bgp.json | Define new test cases for valid route-map references |
| src/sonic-yang-models/tests/files/sample_config_db.json | Populate route_map_in / route_map_out entries in sample DB |
| src/sonic-yang-mgmt/sonic_yang_ext.py | Add _compileUsesClause* methods, update grouping preprocessor |
Comments suppressed due to low confidence (2)
src/sonic-yang-mgmt/sonic_yang_ext.py:167
- This new helper lacks a docstring. Consider adding a brief description of its purpose, parameters (
refine,model), and effects to aid future maintainers.
def _compileUsesClauseRefineApply(self, refine, model):
src/sonic-yang-mgmt/sonic_yang_ext.py:217
- The new
compileUsesClause*methods introduce significant logic but lack direct unit tests. Consider adding focused tests for_compileUsesClauseModel(e.g., grouping withleaf-list,choice, andrefine) to ensure correct behavior and prevent regressions.
def _compileUsesClauseModel(self, module, model):
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
another spurious test failure ... rebased to get a clean build |
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
When a uses clause imports a grouping, it was only processing leaf entries and ignoring leaf-list and choice clauses. That means that for instance in bgp route maps, route_map_in and route_map_out validations would fail. Honoring the uses `refine` clause is now also honored which is depended upon in sonic-utilities. This now precompiles the uses clause and integrates it as if the uses clause was not part of the schema as multiple end users were having to do this additional processing. This fixes that behavior and adds test cases to ensure it doesn't regress in the future. Signed-off-by: Brad House (@bradh352)
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
and the last build failed due to mellanox build failure related to some nfs mount failure on the build system ... then also a test timeout. Rebasing again to force another rebuild. None of the failure are related to the content in this PR. |
|
All checks passed. @qiluo-msft can you merge? |
|
Hi @qiluo-msft , could you help to suggest whether this change should goto 202505 branch? |
|
I consider this as a new feature for use case like BGP_NEIGHBOR_AF. No need to backport. |
@qiluo-msft It fixed #21078, which affects 202505 and older versions. Please reconsider backporting. |
|
Can you please get it merged in 202505 also |
|
Cherry-pick PR to msft-202412: Azure/sonic-buildimage-msft#1731 |
Why I did it
When a uses clause imports a grouping, it was only processing leaf entries and ignoring leaf-list and choice clauses. That means that for instance in bgp route maps, route_map_in and route_map_out validations would fail.
Honoring the uses
refineclause is now also honored which is depended upon in sonic-utilities.This now precompiles the uses clause and integrates it as if the uses clause was not part of the schema as multiple end users were having to do this additional processing.
This fixes that behavior and adds test cases to ensure it doesn't regress in the future.
This is the proper fix, replacing #21078 that just worked around it.
Removal of
useslogic in sonic-utilities here: sonic-net/sonic-utilities#3814Fixes #22382
Work item tracking
How I did it
Added leaf-list lookup.
How to verify it
See test cases pass
Which release branch to backport (provide reason below if selected)
Tested branch (Please provide the tested image version)
master as of 20250304
Description for the changelog
sonic-yang-mgmt: uses clause with leaf-list, choice not honored
Link to config_db schema for YANG module changes
N/A
A picture of a cute animal (not mandatory but encouraged)
Signed-off-by: Brad House (@bradh352)