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

[YANG] Fix issue: Non compliant leaf list in config_db schema #10291

Merged
merged 8 commits into from
May 5, 2022

Conversation

Junchao-Mellanox
Copy link
Collaborator

@Junchao-Mellanox Junchao-Mellanox commented Mar 21, 2022

Why I did it

Fix issue: Non compliant leaf list in config_db schema: #9801

How I did it

The basic flow of DPB is like:

  1. Transfer config db json value to YANG json value, name it “yangIn”
  2. Validate “yangIn” by libyang
  3. Generate a YANG json value to represent the target configuration, name it “yangTarget”
  4. Do diff between “yangIn” and “yangTarget”
  5. Apply the diff to CONFIG DB json and save it back to DB

The fix:
• For step #1, If value of a leaf-list field string type, transfer it to a list by splitting it with “,” the purpose here is to make step#2 happy. We also need to save <table_name>..<field_name> to a set named “leaf_list_with_string_value_set”.
• For step#5, loop “leaf_list_with_string_value_set” and change those fields back to a string.

How to verify it

1.Manual test
2. Changed sample config DB and unit test passed

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

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

Description for the changelog

Link to config_db schema for YANG module changes

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

dgsudharsan
dgsudharsan previously approved these changes Mar 21, 2022
@Junchao-Mellanox
Copy link
Collaborator Author

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Junchao-Mellanox
Copy link
Collaborator Author

Hi @qiluo-msft and @praveen-li , could you please review?

@zhangyanzhao zhangyanzhao added the YANG YANG model related changes label Mar 24, 2022
@zhangyanzhao
Copy link
Collaborator

@praveen-li would you please help to review this PR?

@liat-grozovik
Copy link
Collaborator

Hi @qiluo-msft and @praveen-li kindly reminder to review. As this is also needed for 202111 we need to urge you on that. sorry.

@zhangyanzhao
Copy link
Collaborator

@praveen-li would you please help to review and approve this PR? Thanks.

@liat-grozovik
Copy link
Collaborator

@praveen-li kindly reminder. this fix is must for 202111

@praveen-li
Copy link
Collaborator

praveen-li commented Apr 4, 2022 via email

src/sonic-yang-mgmt/sonic_yang_ext.py Show resolved Hide resolved
src/sonic-yang-mgmt/sonic_yang_ext.py Show resolved Hide resolved
src/sonic-yang-mgmt/sonic_yang_ext.py Outdated Show resolved Hide resolved
src/sonic-yang-mgmt/sonic_yang_ext.py Outdated Show resolved Hide resolved
src/sonic-yang-mgmt/sonic_yang_ext.py Show resolved Hide resolved
@Junchao-Mellanox
Copy link
Collaborator Author

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@wen587
Copy link
Contributor

wen587 commented Apr 15, 2022

Hi I saw testcase fail on GCU generic_config_updater/test_bgp_prefix.py
It seems the changes doesn't pass yang validation with below configuration.

admin@vlab-01:~/hijack/test$ cat add1.load
{
    "BGP_ALLOWED_PREFIXES": {
        "DEPLOYMENT_ID|0": {
            "prefixes_v4": [
                "10.20.0.0/16"
            ],
            "prefixes_v6": [
                "fc02:20::/64"
            ]
        }
    }
}

admin@vlab-01:~/hijack/test$ sudo config load -y add1.load
Running command: /usr/local/bin/sonic-cfggen -j add1.load --write-to-db

admin@vlab-01:~/hijack/test$ cd ~
admin@vlab-01:~$ cat verify_configDB_yang.py
import subprocess
import sonic_yang
import json
#import pytest
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:~$ python verify_configDB_yang.py
sonic_yang(6):Note: Below table(s) have no YANG models: CONSOLE_SWITCH, DEVICE_NEIGHBOR_METADATA, DHCP_SERVER, KDUMP, RESTAPI, SNMP, SNMP_COMMUNITY, TELEMETRY
sonic_yang(3):All Keys are not parsed in BGP_ALLOWED_PREFIXES
dict_keys(['DEPLOYMENT_ID|0'])
sonic_yang(3):exceptionList:['Value not found for deployment id community in DEPLOYMENT_ID|0', "'list' object has no attribute 'split'"]
sonic_yang(3):Data Loading Failed:All Keys are not parsed in BGP_ALLOWED_PREFIXES
dict_keys(['DEPLOYMENT_ID|0'])
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/dist-packages/sonic_yang_ext.py", line 1107, in loadData
    self._xlateConfigDB(xlateFile=xlateFile)
  File "/usr/local/lib/python3.9/dist-packages/sonic_yang_ext.py", line 727, in _xlateConfigDB
    self._xlateConfigDBtoYang(jIn, yangJ)
  File "/usr/local/lib/python3.9/dist-packages/sonic_yang_ext.py", line 713, in _xlateConfigDBtoYang
    self._xlateContainer(cmap['container'], yangJ[key][subkey], \
  File "/usr/local/lib/python3.9/dist-packages/sonic_yang_ext.py", line 692, in _xlateContainer
    raise(Exception("All Keys are not parsed in {}\n{}".format(table, \
Exception: All Keys are not parsed in BGP_ALLOWED_PREFIXES
dict_keys(['DEPLOYMENT_ID|0'])

During handling of the above exception, another exception occurred:

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

@liat-grozovik
Copy link
Collaborator

@praveen-li could you please help to review following comments provided?

@liat-grozovik
Copy link
Collaborator

@praveen-li kindly reminder. this must get in 2021111 so appreciate if we can close the loop. thanks.

Copy link
Collaborator

@praveen-li praveen-li left a comment

Choose a reason for hiding this comment

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

Looks over all good to me.

@@ -407,6 +420,8 @@ def _yangConvert(val):
# if it is a leaf-list do it for each element
if leafDict[key]['__isleafList']:
vValue = list()
if isinstance(value, str) and (self.elementPath[0], self.elementPath[-1]) in LEAF_LIST_WITH_STRING_VALUE_DICT:
value = (x.strip() for x in value.split(LEAF_LIST_WITH_STRING_VALUE_DICT[(self.elementPath[0], self.elementPath[-1])]))
Copy link
Collaborator

Choose a reason for hiding this comment

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

plz explain with example this step. This has to break down the values from string to list. Please explain how it is working in this case. Thanks

@zhangyanzhao
Copy link
Collaborator

Once Praveen's comment is addressed, he will approve and merge.

@Junchao-Mellanox
Copy link
Collaborator Author

Hi @praveen-li , I have fixed the comments, could you please review and sign-off?

@Junchao-Mellanox
Copy link
Collaborator Author

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Collaborator

@praveen-li praveen-li left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for fixing it

@zhangyanzhao
Copy link
Collaborator

Qi will merge

@qiluo-msft qiluo-msft merged commit f850122 into sonic-net:master May 5, 2022
@Junchao-Mellanox Junchao-Mellanox deleted the fix-yang-leaf-list branch May 6, 2022 01:06
@Junchao-Mellanox
Copy link
Collaborator Author

no clean cherry-pick, i will create separate PR for 202111.

@keboliu keboliu removed the Request for 202111 Branch For PRs being requested for 202111 branch label May 6, 2022
Junchao-Mellanox added a commit to Junchao-Mellanox/sonic-buildimage that referenced this pull request May 6, 2022
…net#10291)

Fix issue: Non compliant leaf list in config_db schema: sonic-net#9801

The basic flow of DPB is like:
1.	Transfer config db json value to YANG json value, name it “yangIn”
2.	Validate “yangIn” by libyang
3.	Generate a YANG json value to represent the target configuration, name it “yangTarget”
4.	Do diff between “yangIn” and “yangTarget”
5.	Apply the diff to CONFIG DB json and save it back to DB

The fix:
•	For step #1, If value of a leaf-list field string type, transfer it to a list by splitting it with “,” the purpose here is to make step#2 happy. We also need to save <table_name>.<key>.<field_name> to a set named “leaf_list_with_string_value_set”.
•	For step#5, loop “leaf_list_with_string_value_set” and change those fields back to a string.

1. Manual test
2. Changed sample config DB and unit test passed

Conflicts:
	src/sonic-yang-mgmt/sonic_yang_ext.py
judyjoseph pushed a commit that referenced this pull request May 9, 2022
#10768)

Fix issue: Non compliant leaf list in config_db schema: #9801

The basic flow of DPB is like:
1.	Transfer config db json value to YANG json value, name it “yangIn”
2.	Validate “yangIn” by libyang
3.	Generate a YANG json value to represent the target configuration, name it “yangTarget”
4.	Do diff between “yangIn” and “yangTarget”
5.	Apply the diff to CONFIG DB json and save it back to DB

The fix:
•	For step #1, If value of a leaf-list field string type, transfer it to a list by splitting it with “,” the purpose here is to make step#2 happy. We also need to save <table_name>.<key>.<field_name> to a set named “leaf_list_with_string_value_set”.
•	For step#5, loop “leaf_list_with_string_value_set” and change those fields back to a string.

1. Manual test
2. Changed sample config DB and unit test passed

Conflicts:
	src/sonic-yang-mgmt/sonic_yang_ext.py
liushilongbuaa pushed a commit to liushilongbuaa/sonic-buildimage that referenced this pull request Jun 20, 2022
…net#10291)

#### Why I did it

Fix issue: Non compliant leaf list in config_db schema: sonic-net#9801

#### How I did it

The basic flow of DPB is like:
1.	Transfer config db json value to YANG json value, name it “yangIn”
2.	Validate “yangIn” by libyang
3.	Generate a YANG json value to represent the target configuration, name it “yangTarget”
4.	Do diff between “yangIn” and “yangTarget”
5.	Apply the diff to CONFIG DB json and save it back to DB

The fix:
•	For step #1, If value of a leaf-list field string type, transfer it to a list by splitting it with “,” the purpose here is to make step#2 happy. We also need to save <table_name>.<key>.<field_name> to a set named “leaf_list_with_string_value_set”.
•	For step#5, loop “leaf_list_with_string_value_set” and change those fields back to a string.

#### How to verify it

1. Manual test
2. Changed sample config DB and unit test passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🐛 YANG YANG model related changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants