[Fix]: Allow empty object lists in high-frequency-telemetry YANG model#23917
[Fix]: Allow empty object lists in high-frequency-telemetry YANG model#23917qiluo-msft merged 3 commits intosonic-net:masterfrom
Conversation
- Remove mandatory constraint for object_names list - Add test cases for empty lists and validation scenarios - Include high-frequency telemetry model Signed-off-by: Ze Gan <[email protected]>
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
src/sonic-yang-models/setup.py
Outdated
| 'sonic-smart-switch.yang', | ||
| 'sonic-spanning-tree.yang', | ||
| 'sonic-srv6.yang', | ||
| 'sonic-high-frequency-telemetry.yang', |
| ] | ||
| } | ||
| }, | ||
| "sonic-high-frequency-telemetry:sonic-high-frequency-telemetry": { |
There was a problem hiding this comment.
could you add a test for empty object list? I don't think current YANG allowed empty list becasue there are still constraints here if group_name is PORT:
leaf-list object_names {
type string;
must "( ../group_name = 'PORT' and current() = /port:sonic-port/port:PORT/port:PORT_LIST/port:name )"
+ " or ( ../group_name = 'BUFFER_POOL' and current() = /bpl:sonic-buffer-pool/bpl:BUFFER_POOL/bpl:BUFFER_POOL_LIST/bpl:name )"
+ " or ( ../group_name = 'INGRESS_PRIORITY_GROUP' and substring-before(current(), '|') = /bpg:sonic-buffer-pg/bpg:BUFFER_PG/bpg:BUFFER_PG_LIST/bpg:port and re-match(substring-after(current(), '|'), '[0-9]+') )"
+ " or ( ../group_name = 'QUEUE' and substring-before(current(), '|') = /bql:sonic-buffer-queue/bql:BUFFER_QUEUE/bql:BUFFER_QUEUE_LIST/bql:port and re-match(substring-after(current(), '|'), '[0-9]+') )";
}
There was a problem hiding this comment.
There was a problem hiding this comment.
The test in the link still have elements in object_names
"profile_name": "partial_objects",
"group_name": "PORT",
"object_names": [
"Ethernet0"
],
I mean empty object_names what ndm will generates. such as:
"profile_name": "partial_objects",
"group_name": "PORT",
"object_names": [],
or
"profile_name": "partial_objects",
"group_name": "PORT",
"object_names": "",
There was a problem hiding this comment.
It seems that our SONiC YANG parser doesn't support the structure like:
"object_names": [],
It will raise an error from Yang parser.
libyang[0]: Invalid JSON data (unexpected value). (path: /sonic-high-frequency-telemetry:sonic-high-frequency-telemetry/HIGH_FREQUENCY_TELEMETRY_GROUP/HIGH_FREQUENCY_TELEMETRY_GROUP_LIST[profile_name='dummy_profile'][group_name='PORT']/object_names)
Exception >Invalid JSON data (unexpected value). in not empty< in /sonic/src/sonic-yang-models/tests/yang_model_tests/test_yang_model.py:278
IMHO, its schema is defined by us, so we can define we only support a json without these fields.
There was a problem hiding this comment.
Hi @Pterosaur , right. That's what i am concerned. e.g. if NDM liquid provides "" for object_names and object_counters. Such config will still fail yang validation after we remove must "count(object_names) > 0"; .
liquid is using "" for field value:
https://msazure.visualstudio.com/One/_git/Networking-Metadata?path=/src/data/Network/DeviceConfigTemplates/Sonic_HIGH_FREQUENCY_TELEMETRY_GROUP_Template.liquid&version=GBmaster&_a=contents
To support such field for NDM, should we remove the constraints? such as
leaf-list object_names {
type string;
must "( ../group_name = 'PORT' and current() = /port:sonic-port/port:PORT/port:PORT_LIST/port:name )"
I am not sure if "" applies to leaf-list. or we should pick [] instead for NDM
There was a problem hiding this comment.
Gotch, I add two negative test cases, please check it. We need to update the NDM code later.
|
|
||
| must "count(object_names) > 0"; | ||
|
|
||
| leaf-list object_counters { |
There was a problem hiding this comment.
please also include such config in sample_config_db.json
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Ze Gan <[email protected]>
6333f53 to
424d147
Compare
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: Ze Gan <[email protected]>
|
/azp run Azure.sonic-buildimage |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| + " or ( ../group_name = 'QUEUE' and substring-before(current(), '|') = /bql:sonic-buffer-queue/bql:BUFFER_QUEUE/bql:BUFFER_QUEUE_LIST/bql:port and re-match(substring-after(current(), '|'), '[0-9]+') )"; | ||
| } | ||
|
|
||
| must "count(object_names) > 0"; |
There was a problem hiding this comment.
There was a problem hiding this comment.
Use min-elements for what? I removed this condition because I want to support no explicate fields in HFT_GROUP object. such as:
"dummy_table|PORT": {}
#1593) Origin PR: sonic-net/sonic-buildimage#23917 #### Why I did it We must have a dummy HFT table to support dynamically configuring HFT. This Yang model change is to support the dummy table. ##### Work item tracking - Microsoft ADO **(number only)**: #### How I did it - Remove mandatory constraint for object_names list - Add test cases for empty lists and validation scenarios - Include high-frequency telemetry model - #### How to verify it Check Azp status. This PR includes its test. #### Which release branch to backport (provide reason below if selected) - [x] 202412 #### Tested branch (Please provide the tested image version) master
#1593) Origin PR: sonic-net/sonic-buildimage#23917 #### Why I did it We must have a dummy HFT table to support dynamically configuring HFT. This Yang model change is to support the dummy table. ##### Work item tracking - Microsoft ADO **(number only)**: #### How I did it - Remove mandatory constraint for object_names list - Add test cases for empty lists and validation scenarios - Include high-frequency telemetry model - #### How to verify it Check Azp status. This PR includes its test. #### Which release branch to backport (provide reason below if selected) - [x] 202412 #### Tested branch (Please provide the tested image version) master
sonic-net#23917) Why I did it We must have a dummy HFT table to support dynamically configuring HFT. This Yang model change is to support the dummy table. How I did it Remove mandatory constraint for object_names list Add test cases for empty lists and validation scenarios Include high-frequency telemetry model How to verify it Check Azp status. This PR includes its test. Signed-off-by: Feng Pan <[email protected]>
Why I did it
We must have a dummy HFT table to support dynamically configuring HFT. This Yang model change is to support the dummy table.
Work item tracking
How I did it
How to verify it
Check Azp status. This PR includes its test.
Which release branch to backport (provide reason below if selected)
Tested branch (Please provide the tested image version)
master