From 9c91636abb43a87ef514d66fad735b5c256a4ff3 Mon Sep 17 00:00:00 2001 From: junchao Date: Mon, 14 Mar 2022 18:06:09 +0800 Subject: [PATCH 1/6] [YANG] Fix issue: Non compliant leaf list in config_db schema --- src/sonic-yang-mgmt/sonic_yang.py | 4 +++ src/sonic-yang-mgmt/sonic_yang_ext.py | 39 +++++++++++++++++++++++---- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/src/sonic-yang-mgmt/sonic_yang.py b/src/sonic-yang-mgmt/sonic_yang.py index 16aeb8a9ff8a..f80225967978 100644 --- a/src/sonic-yang-mgmt/sonic_yang.py +++ b/src/sonic-yang-mgmt/sonic_yang.py @@ -45,6 +45,10 @@ def __init__(self, yang_dir, debug=False, print_log_enabled=True): # all yang modules, such as grouping. self.preProcessedYang = dict() + self.processingPath = [] + + self.leaf_list_with_string_value_set = set() + 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 aa36ee0a1951..8726a44da85b 100644 --- a/src/sonic-yang-mgmt/sonic_yang_ext.py +++ b/src/sonic-yang-mgmt/sonic_yang_ext.py @@ -407,6 +407,9 @@ def _yangConvert(val): # if it is a leaf-list do it for each element if leafDict[key]['__isleafList']: vValue = list() + if isinstance(value, str): + self.leaf_list_with_string_value_set.add(tuple(self.processingPath)) + value = (x.strip() for x in value.split(',')) for v in value: vValue.append(_yangConvert(v)) else: @@ -545,6 +548,7 @@ def _xlateList(self, model, yang, config, table, exceptionList): primaryKeys = list(config.keys()) for pkey in primaryKeys: try: + self.processingPath.append(pkey) vKey = None self.sysLog(syslog.LOG_DEBUG, "xlateList Extract pkey:{}".\ format(pkey)) @@ -552,9 +556,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.processingPath.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.processingPath.pop() yang.append(keyDict) # delete pkey from config, done to match one key with one list del config[pkey] @@ -566,6 +574,8 @@ def _xlateList(self, model, yang, config, table, exceptionList): exceptionList.append(str(e)) # with multilist, we continue matching other keys. continue + finally: + self.processingPath.pop() return @@ -596,8 +606,10 @@ def _xlateContainerInContainer(self, model, yang, configC, table): if not configC.get(ccontainer['@name']): return self.sysLog(msg="xlateProcessListOfContainer: {}".format(ccontainer['@name'])) + self.processingPath.append(ccontainer['@name']) self._xlateContainer(ccontainer, yang[ccontainer['@name']], \ configC[ccontainer['@name']], table) + self.processingPath.pop() # clean empty container if len(yang[ccontainer['@name']]) == 0: del yang[ccontainer['@name']] @@ -645,8 +657,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.processingPath.append(vKey) self.sysLog(syslog.LOG_DEBUG, "xlateContainer vkey {}".format(vKey)) yang[vKey] = self._findYangTypedValue(vKey, configC[vKey], leafDict) + self.processingPath.pop() # delete entry from copy of config del configC[vKey] @@ -676,8 +690,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.processingPath.append(table) self._xlateContainer(cmap['container'], yangJ[key][subkey], \ jIn[table], table) + self.processingPath = [] return @@ -734,9 +750,12 @@ 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 tuple(self.processingPath) in self.leaf_list_with_string_value_set: + vValue = ','.join(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: @@ -845,12 +864,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.processingPath.append(pkey) config[pkey]= dict() # fill rest of the entries for key in entry: if key not in pkeydict: + self.processingPath.append(key) config[pkey][key] = self._revFindYangTypedValue(key, \ entry[key], leafDict) + self.processingPath.pop() + self.processingPath.pop() return @@ -874,8 +897,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.processingPath.append(modelContainer['@name']) self._revXlateContainer(modelContainer, yang[modelContainer['@name']], \ config[modelContainer['@name']], table) + self.processingPath.pop() return """ @@ -907,7 +932,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.processingPath.append(vKey) config[vKey] = self._revFindYangTypedValue(vKey, yang[vKey], leafDict) + self.processingPath.pop() return @@ -935,8 +962,10 @@ def _revXlateYangtoConfigDB(self, yangJ, cDbJson): cDbJson[table] = dict() #print(key + "--" + subkey) self.sysLog(msg="revXlateYangtoConfigDB {}".format(table)) + self.processingPath.append(table) self._revXlateContainer(cmap['container'], yangJ[module_top][container], \ cDbJson[table], table) + self.processingPath = [] return From aa7bca44e85e641478d2fe7e7a90c6cb72478445 Mon Sep 17 00:00:00 2001 From: junchao Date: Tue, 15 Mar 2022 16:18:03 +0800 Subject: [PATCH 2/6] Fix issue found in test --- src/sonic-yang-mgmt/sonic_yang_ext.py | 2 +- .../tests/files/sample_config_db.json | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/sonic-yang-mgmt/sonic_yang_ext.py b/src/sonic-yang-mgmt/sonic_yang_ext.py index 8726a44da85b..63d0591a9554 100644 --- a/src/sonic-yang-mgmt/sonic_yang_ext.py +++ b/src/sonic-yang-mgmt/sonic_yang_ext.py @@ -751,7 +751,7 @@ def _revYangConvert(val): # if it is a leaf-list do it for each element if leafDict[key]['__isleafList']: if tuple(self.processingPath) in self.leaf_list_with_string_value_set: - vValue = ','.join(value) + vValue = ','.join((_revYangConvert(x) for x in value)) else: vValue = list() for v in value: 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 920181646c46..620158c61f02 100644 --- a/src/sonic-yang-models/tests/files/sample_config_db.json +++ b/src/sonic-yang-models/tests/files/sample_config_db.json @@ -432,7 +432,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", @@ -440,7 +443,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", From 59800d125b1d99ea2495c4204a788b96fdd71f9c Mon Sep 17 00:00:00 2001 From: junchao Date: Wed, 16 Mar 2022 15:16:17 +0800 Subject: [PATCH 3/6] Fix review comment --- src/sonic-yang-mgmt/sonic_yang.py | 2 +- src/sonic-yang-mgmt/sonic_yang_ext.py | 44 +++++++++++++-------------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/sonic-yang-mgmt/sonic_yang.py b/src/sonic-yang-mgmt/sonic_yang.py index f80225967978..ad38692b75ee 100644 --- a/src/sonic-yang-mgmt/sonic_yang.py +++ b/src/sonic-yang-mgmt/sonic_yang.py @@ -45,7 +45,7 @@ def __init__(self, yang_dir, debug=False, print_log_enabled=True): # all yang modules, such as grouping. self.preProcessedYang = dict() - self.processingPath = [] + self.elementPath = [] self.leaf_list_with_string_value_set = set() diff --git a/src/sonic-yang-mgmt/sonic_yang_ext.py b/src/sonic-yang-mgmt/sonic_yang_ext.py index 63d0591a9554..31dac74adc4f 100644 --- a/src/sonic-yang-mgmt/sonic_yang_ext.py +++ b/src/sonic-yang-mgmt/sonic_yang_ext.py @@ -408,7 +408,7 @@ def _yangConvert(val): if leafDict[key]['__isleafList']: vValue = list() if isinstance(value, str): - self.leaf_list_with_string_value_set.add(tuple(self.processingPath)) + self.leaf_list_with_string_value_set.add(tuple(self.elementPath)) value = (x.strip() for x in value.split(',')) for v in value: vValue.append(_yangConvert(v)) @@ -548,7 +548,7 @@ def _xlateList(self, model, yang, config, table, exceptionList): primaryKeys = list(config.keys()) for pkey in primaryKeys: try: - self.processingPath.append(pkey) + self.elementPath.append(pkey) vKey = None self.sysLog(syslog.LOG_DEBUG, "xlateList Extract pkey:{}".\ format(pkey)) @@ -556,13 +556,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.processingPath.append(vKey) + self.elementPath.append(vKey) self.sysLog(syslog.LOG_DEBUG, "xlateList vkey {}".format(vKey)) try: keyDict[vKey] = self._findYangTypedValue(vKey, \ config[pkey][vKey], leafDict) finally: - self.processingPath.pop() + self.elementPath.pop() yang.append(keyDict) # delete pkey from config, done to match one key with one list del config[pkey] @@ -575,7 +575,7 @@ def _xlateList(self, model, yang, config, table, exceptionList): # with multilist, we continue matching other keys. continue finally: - self.processingPath.pop() + self.elementPath.pop() return @@ -606,10 +606,10 @@ def _xlateContainerInContainer(self, model, yang, configC, table): if not configC.get(ccontainer['@name']): return self.sysLog(msg="xlateProcessListOfContainer: {}".format(ccontainer['@name'])) - self.processingPath.append(ccontainer['@name']) + self.elementPath.append(ccontainer['@name']) self._xlateContainer(ccontainer, yang[ccontainer['@name']], \ configC[ccontainer['@name']], table) - self.processingPath.pop() + self.elementPath.pop() # clean empty container if len(yang[ccontainer['@name']]) == 0: del yang[ccontainer['@name']] @@ -657,10 +657,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.processingPath.append(vKey) + self.elementPath.append(vKey) self.sysLog(syslog.LOG_DEBUG, "xlateContainer vkey {}".format(vKey)) yang[vKey] = self._findYangTypedValue(vKey, configC[vKey], leafDict) - self.processingPath.pop() + self.elementPath.pop() # delete entry from copy of config del configC[vKey] @@ -690,10 +690,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.processingPath.append(table) + self.elementPath.append(table) self._xlateContainer(cmap['container'], yangJ[key][subkey], \ jIn[table], table) - self.processingPath = [] + self.elementPath = [] return @@ -750,7 +750,7 @@ def _revYangConvert(val): # if it is a leaf-list do it for each element if leafDict[key]['__isleafList']: - if tuple(self.processingPath) in self.leaf_list_with_string_value_set: + if tuple(self.elementPath) in self.leaf_list_with_string_value_set: vValue = ','.join((_revYangConvert(x) for x in value)) else: vValue = list() @@ -864,16 +864,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.processingPath.append(pkey) + self.elementPath.append(pkey) config[pkey]= dict() # fill rest of the entries for key in entry: if key not in pkeydict: - self.processingPath.append(key) + self.elementPath.append(key) config[pkey][key] = self._revFindYangTypedValue(key, \ entry[key], leafDict) - self.processingPath.pop() - self.processingPath.pop() + self.elementPath.pop() + self.elementPath.pop() return @@ -897,10 +897,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.processingPath.append(modelContainer['@name']) + self.elementPath.append(modelContainer['@name']) self._revXlateContainer(modelContainer, yang[modelContainer['@name']], \ config[modelContainer['@name']], table) - self.processingPath.pop() + self.elementPath.pop() return """ @@ -932,9 +932,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.processingPath.append(vKey) + self.elementPath.append(vKey) config[vKey] = self._revFindYangTypedValue(vKey, yang[vKey], leafDict) - self.processingPath.pop() + self.elementPath.pop() return @@ -962,10 +962,10 @@ def _revXlateYangtoConfigDB(self, yangJ, cDbJson): cDbJson[table] = dict() #print(key + "--" + subkey) self.sysLog(msg="revXlateYangtoConfigDB {}".format(table)) - self.processingPath.append(table) + self.elementPath.append(table) self._revXlateContainer(cmap['container'], yangJ[module_top][container], \ cDbJson[table], table) - self.processingPath = [] + self.elementPath = [] return From 3f6cff580dbb60fc1171ee4badbf745642b313b9 Mon Sep 17 00:00:00 2001 From: junchao Date: Thu, 14 Apr 2022 16:38:50 +0800 Subject: [PATCH 4/6] Fix review comment --- src/sonic-yang-mgmt/sonic_yang.py | 6 ++--- src/sonic-yang-mgmt/sonic_yang_ext.py | 22 ++++++++++++++----- .../tests/files/sample_config_db.json | 9 +++----- 3 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/sonic-yang-mgmt/sonic_yang.py b/src/sonic-yang-mgmt/sonic_yang.py index ad38692b75ee..61f049eb9921 100644 --- a/src/sonic-yang-mgmt/sonic_yang.py +++ b/src/sonic-yang-mgmt/sonic_yang.py @@ -44,11 +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 = [] - - self.leaf_list_with_string_value_set = set() - 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 78cb9ab404a0..d340c4e4f37d 100644 --- a/src/sonic-yang-mgmt/sonic_yang_ext.py +++ b/src/sonic-yang-mgmt/sonic_yang_ext.py @@ -20,6 +20,19 @@ 'CABLE_LENGTH_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. """ @@ -407,9 +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): - self.leaf_list_with_string_value_set.add(tuple(self.elementPath)) - value = (x.strip() for x in value.split(',')) + if (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])])) for v in value: vValue.append(_yangConvert(v)) else: @@ -757,8 +769,8 @@ def _revYangConvert(val): # if it is a leaf-list do it for each element if leafDict[key]['__isleafList']: - if tuple(self.elementPath) in self.leaf_list_with_string_value_set: - vValue = ','.join((_revYangConvert(x) for x in value)) + if (self.elementPath[0], self.elementPath[-1]) in LEAF_LIST_WITH_STRING_VALUE_DICT: + 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: 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 affc20675415..e82a517139ff 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": { @@ -394,10 +394,7 @@ "NTP": { "global": { "vrf": "mgmt", - "src_intf": [ - "eth0", - "Loopback0" - ] + "src_intf": "eth0;Loopback0" } }, "NTP_SERVER": { From f1c888374e9dc76d58c5781a99087c5933a5a7f6 Mon Sep 17 00:00:00 2001 From: junchao Date: Mon, 18 Apr 2022 09:53:03 +0800 Subject: [PATCH 5/6] Fix vs test failure --- src/sonic-yang-mgmt/sonic_yang_ext.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sonic-yang-mgmt/sonic_yang_ext.py b/src/sonic-yang-mgmt/sonic_yang_ext.py index d340c4e4f37d..084588545adc 100644 --- a/src/sonic-yang-mgmt/sonic_yang_ext.py +++ b/src/sonic-yang-mgmt/sonic_yang_ext.py @@ -420,7 +420,7 @@ def _yangConvert(val): # if it is a leaf-list do it for each element if leafDict[key]['__isleafList']: vValue = list() - if (self.elementPath[0], self.elementPath[-1]) in LEAF_LIST_WITH_STRING_VALUE_DICT: + 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])])) for v in value: vValue.append(_yangConvert(v)) @@ -769,7 +769,7 @@ def _revYangConvert(val): # if it is a leaf-list do it for each element if leafDict[key]['__isleafList']: - if (self.elementPath[0], self.elementPath[-1]) in LEAF_LIST_WITH_STRING_VALUE_DICT: + if isinstance(value, list) and (self.elementPath[0], self.elementPath[-1]) in LEAF_LIST_WITH_STRING_VALUE_DICT: vValue = LEAF_LIST_WITH_STRING_VALUE_DICT[(self.elementPath[0], self.elementPath[-1])].join((_revYangConvert(x) for x in value)) else: vValue = list() From 40f3bf6a124892d1679644158d208c4928e2c7ae Mon Sep 17 00:00:00 2001 From: junchao Date: Fri, 29 Apr 2022 09:01:13 +0800 Subject: [PATCH 6/6] Fix review comment --- src/sonic-yang-mgmt/sonic_yang_ext.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/sonic-yang-mgmt/sonic_yang_ext.py b/src/sonic-yang-mgmt/sonic_yang_ext.py index 084588545adc..2bc5825cf19c 100644 --- a/src/sonic-yang-mgmt/sonic_yang_ext.py +++ b/src/sonic-yang-mgmt/sonic_yang_ext.py @@ -421,6 +421,9 @@ def _yangConvert(val): 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)) @@ -770,6 +773,8 @@ def _revYangConvert(val): # if it is a leaf-list do it for each element if leafDict[key]['__isleafList']: 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()