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.

Sign up for GitHub

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

Jump to bottom

[yang] validating TACPLUS with empty “global” fails, while non-empty “global” does not fail #9746

Closed
wen587 opened this issue Jan 12, 2022 · 12 comments · Fixed by #10431
Closed

[yang] validating TACPLUS with empty “global” fails, while non-empty “global” does not fail #9746

wen587 opened this issue Jan 12, 2022 · 12 comments · Fixed by #10431
Assignees
@praveen-li
Labels
P0 Priority of the issue Triaged this issue has been triaged YANG YANG model related changes

Comments

@wen587
Copy link
Contributor

wen587 commented Jan 12, 2022 •
edited
Loading

Description

yang validation fail when global is empty, but it succeeds when nonempty.

Background of tacacs table.
In t0 topo, tacacs has default value while configDB doesn't contain those table.
After sudo config tacacs value, its table will appear in configDB.
Then the table will contains empty value after set back to default.

Steps to reproduce the issue:

  1. config tacacs and let TACPLUS table appear in configDB
admin@vlab-01:~$ show run all | grep -w TACPLUS
admin@vlab-01:~$ sudo config tacacs timeout 3
admin@vlab-01:~$ show run all | grep -w TACPLUS -A3
    "TACPLUS": {
        "global": {
            "timeout": "3"
        }
  1. Verify by python script. Pass. (Some yang is outdated so we ignore them)
admin@vlab-01:~$ cd /usr/local/yang-models/
admin@vlab-01:/usr/local/yang-models$ sudo mkdir ignore
admin@vlab-01:/usr/local/yang-models$ sudo mv sonic-queue.yang sonic-scheduler.yang sonic-feature.yang ignore/
admin@vlab-01:/usr/local/yang-models$ cd -
/home/admin

admin@vlab-01:~$ cat verify_configDB_yang.py
import subprocess
import sonic_yang
import json
def get_config_db_json():
    cmd = "show runningconfiguration all"
    result = subprocess.Popen(cmd, shell=True, text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
    text, err = result.communicate()
    return json.loads(text)
configDbJson = get_config_db_json()
sy = sonic_yang.SonicYang("/usr/local/yang-models")
sy.loadYangModel()
sy.loadData(configDbJson )
sy.validate_data_tree()
admin@vlab-01:~$

admin@vlab-01:~$ python verify_configDB_yang.py
Note: Below table(s) have no YANG models:
BGP_PEER_RANGE, CONSOLE_SWITCH, DEVICE_NEIGHBOR_METADATA, DHCP_SERVER, FEATURE, KDUMP, QUEUE, RESTAPI, SCHEDULER, SNMP, SNMP_COMMUNITY, TELEMETRY,
admin@vlab-01:~$
  1. set tacacs back to default and run python script again. It fails.
admin@vlab-01:~$ sudo config tacacs default timeout
admin@vlab-01:~$ show run all | grep -w TACPLUS -A3
    "TACPLUS": {
        "global": {}
    },
    "TACPLUS_SERVER": {
admin@vlab-01:~$ python verify_configDB_yang.py
Note: Below table(s) have no YANG models:
BGP_PEER_RANGE, CONSOLE_SWITCH, DEVICE_NEIGHBOR_METADATA, DHCP_SERVER, FEATURE, KDUMP, QUEUE, RESTAPI, SCHEDULER, SNMP, SNMP_COMMUNITY, TELEMETRY,
sonic_yang(3):All Keys are not parsed in TACPLUS
dict_keys(['global'])
sonic_yang(3):exceptionList:[]
sonic_yang(3):Data Loading Failed:All Keys are not parsed in TACPLUS
dict_keys(['global'])
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/dist-packages/sonic_yang_ext.py", line 1062, in loadData
    self._xlateConfigDB(xlateFile=xlateFile)
  File "/usr/local/lib/python3.9/dist-packages/sonic_yang_ext.py", line 695, in _xlateConfigDB
    self._xlateConfigDBtoYang(jIn, yangJ)
  File "/usr/local/lib/python3.9/dist-packages/sonic_yang_ext.py", line 682, in _xlateConfigDBtoYang
    self._xlateContainer(cmap['container'], yangJ[key][subkey], \
  File "/usr/local/lib/python3.9/dist-packages/sonic_yang_ext.py", line 662, in _xlateContainer
    raise(Exception("All Keys are not parsed in {}\n{}".format(table, \
Exception: All Keys are not parsed in TACPLUS
dict_keys(['global'])

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/admin/verify_configDB_yang.py", line 12, in <module>
    sy.loadData(configDbJson )
  File "/usr/local/lib/python3.9/dist-packages/sonic_yang_ext.py", line 1072, in loadData
    raise SonicYangException("Data Loading Failed\n{}".format(str(e)))
sonic_yang_ext.SonicYangException: Data Loading Failed
All Keys are not parsed in TACPLUS
dict_keys(['global'])

Describe the results you received:

See above.

Describe the results you expected:

Action should succeed because it should be valid configDB.

Output of show version:

admin@vlab-01:~/tacacs$ show ver

SONiC Software Version: SONiC.master.58770-6402a0226
Distribution: Debian 11.1
Kernel: 5.10.0-8-2-amd64
Build commit: 6402a0226
Build date: Mon Dec 13 09:02:14 UTC 2021
Built by: AzDevOps@sonic-build-workers-000Z9S

Platform: x86_64-kvm_x86_64-r0
HwSKU: Force10-S6000
ASIC: vs
ASIC Count: 1
Serial Number: N/A
Model Number: N/A
Hardware Revision: N/A
Uptime: 09:54:13 up  4:11,  2 users,  load average: 0.14, 0.14, 0.15

Output of show techsupport:

(paste your output here or download and attach the file here )

Additional information you deem important (e.g. issue happens only occasionally):
@wen587 wen587 added the YANG YANG model related changes label Jan 12, 2022
@ghooo ghooo mentioned this issue Jan 12, 2022
Closed
@zhangyanzhao
Copy link
Collaborator

@ArthiSivanantham can you please help to take a look at this issue? Venkat will follow-up with you. Thanks.

@zhangyanzhao
Copy link
Collaborator

Dell team is working on this.

@zhangyanzhao zhangyanzhao added the Triaged this issue has been triaged label Jan 20, 2022
@venkatmahalingam
Copy link
Collaborator

We'll update our findings today.

@ArthiSivanantham
Copy link
Contributor

For config validation, in sonic_yang_ext.py "_xlateContainer()" method, translates a dictionary in config DB to a Yang JSON container using yang model. A copy of config keys are deleted after every match. When all the keys are being processed, an empty dictionary was expected,

https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-mgmt/sonic_yang_ext.py#L654

Working case:

admin@testsonic5:~$ sudo config tacacs timeout 3

configC: {'global': {'timeout': '3'}}
model: OrderedDict([('@name', 'global'), ('leaf', OrderedDict([('@name', 'timeout'), ('type', OrderedDict([('@name', 'uint16'), ('range', OrderedDict([('@value', '1..60'), ('error-message', OrderedDict([('value', 'TACACS timeout must be 1..60')]))]))])), ('default', OrderedDict([('@value', '5')]))]))])

configC.keys was empty after processing the dict at the above mentioned point. (#654)

Non-working case:

There is an early return when processing a container inside a container case ( "_xlateContainerInContainer()" method in sonic_yang_ext.py). If the config has an empty dict as a value, it's key is not being cleaned up and returned from that function,

https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-mgmt/sonic_yang_ext.py#L597

admin@testsonic5:~$ sudo config tacacs default timeout

configC: {'global': {}}
model: OrderedDict([('@name', 'global'), ('leaf', OrderedDict([('@name', 'timeout'), ('type', OrderedDict([('@name', 'uint16'), ('range', OrderedDict([('@value', '1..60'), ('error-message', OrderedDict([('value', 'TACACS timeout must be 1..60')]))]))])), ('default', OrderedDict([('@value', '5')]))]))])

configC : {'global': {}} was not empty and the validation failed as,

Exception: All Keys are not parsed in TACPLUS
dict_keys(['global'])

https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-mgmt/sonic_yang_ext.py#L659

Requesting the script owner @praveen-li to re-visit the logic of clearing dictionary keys (for empty value).

@wen587
Copy link
Contributor Author

wen587 commented Mar 16, 2022

any update for this one?

@wen587
Copy link
Contributor Author

wen587 commented Mar 22, 2022

Is there an ETA for this one?

@wen587 wen587 added the P0 Priority of the issue label Mar 22, 2022
@qiluo-msft
Copy link
Collaborator

@praveen-li Could you help check "re-visit the logic of clearing dictionary keys (for empty value)"?

@praveen-li
Copy link
Collaborator

praveen-li commented Mar 23, 2022 via email

@praveen-li
Copy link
Collaborator

praveen-li commented Mar 23, 2022 via email

@wen587
Copy link
Contributor Author

wen587 commented Mar 23, 2022

sonic-system-tacacs.yang
Right, global is container and timeout is leaf.

@praveen-li
Copy link
Collaborator

praveen-li commented Apr 1, 2022 •
edited
Loading

Kindly add below, it should fix this issue. Let me know if not. Thx.

if not configC.get(ccontainer['@name']):
# Empty container, clean config and return
del configC[ccontainer['@name']]
return

@wen587
Copy link
Contributor Author

wen587 commented Apr 1, 2022

Hi @praveen-li , Could you elaberate where to add those lines?

@wen587 wen587 mentioned this issue Apr 1, 2022
Merged
6 tasks
wen587 added a commit that referenced this issue Apr 7, 2022
@wen587
[yang] Fix yang validation failure when table contains empty value (#…
d83ae1e 
…10431)

Why I did it
Fix #9746

How I did it
Split the check condition based on non-exist and zero length.

How to verify it
Run verification script when table contains empty value
liushilongbuaa pushed a commit to liushilongbuaa/sonic-buildimage that referenced this issue Jun 20, 2022
@yxieca
Merged PR 6004325: Manual merge github master to internal
19c4542 
Related work items: #49, #58, #107, sonic-net#247, sonic-net#249, sonic-net#277, sonic-net#593, sonic-net#597, sonic-net#1035, sonic-net#2130, sonic-net#2150, sonic-net#2165, sonic-net#2169, sonic-net#2178, sonic-net#2179, sonic-net#2187, sonic-net#2188, sonic-net#2191, sonic-net#2195, sonic-net#2197, sonic-net#2198, sonic-net#2200, sonic-net#2202, sonic-net#2206, sonic-net#2209, sonic-net#2211, sonic-net#2216, sonic-net#7909, sonic-net#8927, sonic-net#9681, sonic-net#9733, sonic-net#9746, sonic-net#9850, sonic-net#9967, sonic-net#10104, sonic-net#10152, sonic-net#10168, sonic-net#10228, sonic-net#10266, sonic-net#10288, sonic-net#10294, sonic-net#10313, sonic-net#10394, sonic-net#10403, sonic-net#10404, sonic-net#10421, sonic-net#10431, sonic-net#10437, sonic-net#10445, sonic-net#10457, sonic-net#10458, sonic-net#10465, sonic-net#10467, sonic-net#10469, sonic-net#10470, sonic-net#10474, sonic-net#10477, sonic-net#10478, sonic-net#10482, sonic-net#10485, sonic-net#10488, sonic-net#10489, sonic-net#10492, sonic-net#10494, sonic-net#10498, sonic-net#10501, sonic-net#10509, sonic-net#10512, sonic-net#10514, sonic-net#10516, sonic-net#10517, sonic-net#10523, sonic-net#10525, sonic-net#10531, sonic-net#10532, sonic-net#10538, sonic-net#10555, sonic-net#10557, sonic-net#10559, sonic-net#10561, sonic-net#10565, sonic-net#10572, sonic-net#10574, sonic-net#10576, sonic-net#10578, sonic-net#10581, sonic-net#10585, sonic-net#10587, sonic-net#10599, sonic-net#10607, sonic-net#10611, sonic-net#10616, sonic-net#10618, sonic-net#10619, sonic-net#10623, sonic-net#10624, sonic-net#10633, sonic-net#10646, sonic-net#10655, sonic-net#10660, sonic-net#10664, sonic-net#10680, sonic-net#10683
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@praveen-li praveen-li

Labels
P0 Priority of the issue Triaged this issue has been triaged YANG YANG model related changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix yang validation failure when table contains empty value wen587/sonic-buildimage
6 participants
@zhangyanzhao @wen587 @qiluo-msft @ArthiSivanantham @venkatmahalingam @praveen-li