From 97d998af087afec9b96fc963378b90b64e58f2ba Mon Sep 17 00:00:00 2001 From: Junchao-Mellanox <57339448+Junchao-Mellanox@users.noreply.github.com> Date: Fri, 6 May 2022 01:11:35 +0800 Subject: [PATCH] [YANG] Fix issue: Non compliant leaf list in config_db schema (#10291) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix issue: Non compliant leaf list in config_db schema: https://github.com/Azure/sonic-buildimage/issues/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 .. 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 --- src/sonic-yang-mgmt/sonic_yang.py | 4 +- src/sonic-yang-mgmt/sonic_yang_ext.py | 56 +++++++++++++++++-- .../tests/files/sample_config_db.json | 19 ++++--- 3 files changed, 65 insertions(+), 14 deletions(-) diff --git a/src/sonic-yang-mgmt/sonic_yang.py b/src/sonic-yang-mgmt/sonic_yang.py index 16aeb8a9ff8a..61f049eb9921 100644 --- a/src/sonic-yang-mgmt/sonic_yang.py +++ b/src/sonic-yang-mgmt/sonic_yang.py @@ -44,7 +44,9 @@ def __init__(self, yang_dir, debug=False, print_log_enabled=True): # below dict will store preProcessed yang objects, which may be needed by # all yang modules, such as grouping. self.preProcessedYang = dict() - + # element path for CONFIG DB. An example for this list could be: + # ['PORT', 'Ethernet0', 'speed'] + self.elementPath = [] try: self.ctx = ly.Context(yang_dir) except Exception as e: diff --git a/src/sonic-yang-mgmt/sonic_yang_ext.py b/src/sonic-yang-mgmt/sonic_yang_ext.py index 54f336f4f16a..1cd15911e0e1 100644 --- a/src/sonic-yang-mgmt/sonic_yang_ext.py +++ b/src/sonic-yang-mgmt/sonic_yang_ext.py @@ -15,6 +15,19 @@ 'MAP_PFC_PRIORITY_TO_QUEUE_LIST', 'PFC_PRIORITY_TO_PRIORITY_GROUP_MAP_LIST'] +# Workaround for those fields who is defined as leaf-list in YANG model but have string value in config DB. +# Dictinary structure key = (, ), value = seperator +LEAF_LIST_WITH_STRING_VALUE_DICT = { + ('MIRROR_SESSION', 'src_ip'): ',', + ('NTP', 'src_intf'): ';', + ('BGP_ALLOWED_PREFIXES', 'prefixes_v4'): ',', + ('BGP_ALLOWED_PREFIXES', 'prefixes_v6'): ',', + ('BUFFER_PORT_EGRESS_PROFILE_LIST', 'profile_list'): ',', + ('BUFFER_PORT_INGRESS_PROFILE_LIST', 'profile_list'): ',', + ('PORT', 'adv_speeds'): ',', + ('PORT', 'adv_interface_types'): ',', +} + """ This is the Exception thrown out of all public function of this class. """ @@ -402,6 +415,11 @@ 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: + # For field defined as leaf-list but has string value in CONFIG DB, need do special handling here. For exampe: + # port.adv_speeds in CONFIG DB has value "100,1000,10000", it shall be transferred to [100,1000,10000] as YANG value here to + # make it align with its YANG definition. + value = (x.strip() for x in value.split(LEAF_LIST_WITH_STRING_VALUE_DICT[(self.elementPath[0], self.elementPath[-1])])) for v in value: vValue.append(_yangConvert(v)) else: @@ -540,6 +558,7 @@ def _xlateList(self, model, yang, config, table, exceptionList): primaryKeys = list(config.keys()) for pkey in primaryKeys: try: + self.elementPath.append(pkey) vKey = None self.sysLog(syslog.LOG_DEBUG, "xlateList Extract pkey:{}".\ format(pkey)) @@ -547,9 +566,13 @@ def _xlateList(self, model, yang, config, table, exceptionList): keyDict = self._extractKey(pkey, listKeys) # fill rest of the values in keyDict for vKey in config[pkey]: + self.elementPath.append(vKey) self.sysLog(syslog.LOG_DEBUG, "xlateList vkey {}".format(vKey)) - keyDict[vKey] = self._findYangTypedValue(vKey, \ - config[pkey][vKey], leafDict) + try: + keyDict[vKey] = self._findYangTypedValue(vKey, \ + config[pkey][vKey], leafDict) + finally: + self.elementPath.pop() yang.append(keyDict) # delete pkey from config, done to match one key with one list del config[pkey] @@ -561,6 +584,8 @@ def _xlateList(self, model, yang, config, table, exceptionList): exceptionList.append(str(e)) # with multilist, we continue matching other keys. continue + finally: + self.elementPath.pop() return @@ -591,8 +616,10 @@ def _xlateContainerInContainer(self, model, yang, configC, table): if not configC.get(ccontainer['@name']): return self.sysLog(msg="xlateProcessListOfContainer: {}".format(ccontainer['@name'])) + self.elementPath.append(ccontainer['@name']) self._xlateContainer(ccontainer, yang[ccontainer['@name']], \ configC[ccontainer['@name']], table) + self.elementPath.pop() # clean empty container if len(yang[ccontainer['@name']]) == 0: del yang[ccontainer['@name']] @@ -640,8 +667,10 @@ def _xlateContainer(self, model, yang, config, table): for vKey in vKeys: #vkey must be a leaf\leaf-list\choice in container if leafDict.get(vKey): + self.elementPath.append(vKey) self.sysLog(syslog.LOG_DEBUG, "xlateContainer vkey {}".format(vKey)) yang[vKey] = self._findYangTypedValue(vKey, configC[vKey], leafDict) + self.elementPath.pop() # delete entry from copy of config del configC[vKey] @@ -671,8 +700,10 @@ def _xlateConfigDBtoYang(self, jIn, yangJ): yangJ[key] = dict() if yangJ.get(key) is None else yangJ[key] yangJ[key][subkey] = dict() self.sysLog(msg="xlateConfigDBtoYang {}:{}".format(key, subkey)) + self.elementPath.append(table) self._xlateContainer(cmap['container'], yangJ[key][subkey], \ jIn[table], table) + self.elementPath = [] return @@ -729,9 +760,14 @@ def _revYangConvert(val): # if it is a leaf-list do it for each element if leafDict[key]['__isleafList']: - vValue = list() - for v in value: - vValue.append(_revYangConvert(v)) + if isinstance(value, list) and (self.elementPath[0], self.elementPath[-1]) in LEAF_LIST_WITH_STRING_VALUE_DICT: + # For field defined as leaf-list but has string value in CONFIG DB, we need do special handling here: + # e.g. port.adv_speeds is [10,100,1000] in YANG, need to convert it into a string for CONFIG DB: "10,100,1000" + vValue = LEAF_LIST_WITH_STRING_VALUE_DICT[(self.elementPath[0], self.elementPath[-1])].join((_revYangConvert(x) for x in value)) + else: + vValue = list() + for v in value: + vValue.append(_revYangConvert(v)) elif leafDict[key]['type']['@name'] == 'boolean': vValue = 'true' if value else 'false' else: @@ -840,12 +876,16 @@ def _revXlateList(self, model, yang, config, table): # create key of config DB table pkey, pkeydict = self._createKey(entry, listKeys) self.sysLog(syslog.LOG_DEBUG, "revXlateList pkey:{}".format(pkey)) + self.elementPath.append(pkey) config[pkey]= dict() # fill rest of the entries for key in entry: if key not in pkeydict: + self.elementPath.append(key) config[pkey][key] = self._revFindYangTypedValue(key, \ entry[key], leafDict) + self.elementPath.pop() + self.elementPath.pop() return @@ -869,8 +909,10 @@ def _revXlateContainerInContainer(self, model, yang, config, table): if yang.get(modelContainer['@name']): config[modelContainer['@name']] = dict() self.sysLog(msg="revXlateContainerInContainer {}".format(modelContainer['@name'])) + self.elementPath.append(modelContainer['@name']) self._revXlateContainer(modelContainer, yang[modelContainer['@name']], \ config[modelContainer['@name']], table) + self.elementPath.pop() return """ @@ -902,7 +944,9 @@ def _revXlateContainer(self, model, yang, config, table): #vkey must be a leaf\leaf-list\choice in container if leafDict.get(vKey): self.sysLog(syslog.LOG_DEBUG, "revXlateContainer vkey {}".format(vKey)) + self.elementPath.append(vKey) config[vKey] = self._revFindYangTypedValue(vKey, yang[vKey], leafDict) + self.elementPath.pop() return @@ -930,8 +974,10 @@ def _revXlateYangtoConfigDB(self, yangJ, cDbJson): cDbJson[table] = dict() #print(key + "--" + subkey) self.sysLog(msg="revXlateYangtoConfigDB {}".format(table)) + self.elementPath.append(table) self._revXlateContainer(cmap['container'], yangJ[module_top][container], \ cDbJson[table], table) + self.elementPath = [] return diff --git a/src/sonic-yang-models/tests/files/sample_config_db.json b/src/sonic-yang-models/tests/files/sample_config_db.json index 4b477abec043..db8db156767e 100644 --- a/src/sonic-yang-models/tests/files/sample_config_db.json +++ b/src/sonic-yang-models/tests/files/sample_config_db.json @@ -65,12 +65,12 @@ }, "BUFFER_PORT_INGRESS_PROFILE_LIST": { "Ethernet9": { - "profile_list": ["ingress_lossy_profile"] + "profile_list": "ingress_lossy_profile" } }, "BUFFER_PORT_EGRESS_PROFILE_LIST": { "Ethernet9": { - "profile_list": ["egress_lossless_profile", "egress_lossy_profile"] + "profile_list": "egress_lossless_profile,egress_lossy_profile" } }, "PORTCHANNEL": { @@ -363,10 +363,7 @@ "NTP": { "global": { "vrf": "mgmt", - "src_intf": [ - "eth0", - "Loopback0" - ] + "src_intf": "eth0;Loopback0" } }, "NTP_SERVER": { @@ -397,7 +394,10 @@ "description": "", "speed": "11100", "tpid": "0x8100", - "admin_status": "up" + "admin_status": "up", + "autoneg": "on", + "adv_speeds": "100000,50000", + "adv_interface_types": "CR,CR4" }, "Ethernet2": { "alias": "Eth1/3", @@ -405,7 +405,10 @@ "description": "", "speed": "11100", "tpid": "0x8100", - "admin_status": "up" + "admin_status": "up", + "autoneg": "on", + "adv_speeds": "all", + "adv_interface_types": "all" }, "Ethernet3": { "alias": "Eth1/4",