Skip to content

Commit 785508d

Browse files
ghoooyxieca
authored andcommitted
[GCU] Handling type1 lists (#2171)
#### What I did Fixes #2034 Which lists where handled before in ConfigDb to YANG conversion? - Dictionary of key to Object e.g. ``` "TACPLUS_SERVER": { "1.1.1.1": { <= Key: Object (i.e. {...}) "priority": "1", "tcp_port": "49" }, "1.1.1.2": { "priority": "1", "tcp_port": "49" }, "1.1.1.3": { "priority": "1", "tcp_port": "49" } } ``` - List of values ``` "VLAN": { "Vlan1000": { "vlanid": "1000", "dhcp_servers": [ <= Values "192.0.0.1", "192.0.0.2", "192.0.0.3", "192.0.0.4" ] } } ``` - But there is no handling of Dictionary of Key to Value ``` "DOT1P_TO_TC_MAP": { "Dot1p_to_tc_map1": { <= Key: Value "1": "1", "2": "2", "3": "1", "4": "2" } }, ``` Refer to sonic-net/sonic-buildimage#7375 for more info about how Type 1 list ConfigDb is getting converted to SonicYang. After checking how type1 is handled during ConfigDb to SonicYang conversion. Check the unit-tests here for converting ConfigDb Path to SonicYang xpath. Also added CABLE_LENGTH to ADD_RACK and REMOVE_RACK tests. #### How I did it When handling a list, check if it is of type1. If that's the case call type1 list handling. #### How to verify it unit-test #### Previous command output (if the output of a command-line utility has changed) #### New command output (if the output of a command-line utility has changed)
1 parent 56c2c6b commit 785508d

File tree

5 files changed

+181
-9
lines changed

5 files changed

+181
-9
lines changed

generic_config_updater/gu_common.py

+86-5
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import jsonpatch
33
from jsonpointer import JsonPointer
44
import sonic_yang
5+
import sonic_yang_ext
56
import subprocess
67
import yang as ly
78
import copy
@@ -155,14 +156,14 @@ def crop_tables_without_yang(self, config_db_as_json):
155156
sy._cropConfigDB()
156157

157158
return sy.jIn
158-
159+
159160
def get_empty_tables(self, config):
160161
empty_tables = []
161162
for key in config.keys():
162163
if not(config[key]):
163164
empty_tables.append(key)
164165
return empty_tables
165-
166+
166167
def remove_empty_tables(self, config):
167168
config_with_non_empty_tables = {}
168169
for table in config:
@@ -398,7 +399,7 @@ def find_ref_paths(self, path, config):
398399
Finds the paths referencing any line under the given 'path' within the given 'config'.
399400
Example:
400401
path: /PORT
401-
config:
402+
config:
402403
{
403404
"VLAN_MEMBER": {
404405
"Vlan1000|Ethernet0": {},
@@ -543,10 +544,25 @@ def _get_xpath_tokens_from_list(self, model, token_index, path_tokens, config):
543544
if len(path_tokens)-1 == token_index:
544545
return xpath_tokens
545546

547+
type_1_list_model = self._get_type_1_list_model(model)
548+
if type_1_list_model:
549+
new_xpath_tokens = self._get_xpath_tokens_from_type_1_list(type_1_list_model, token_index+1, path_tokens, config[path_tokens[token_index]])
550+
xpath_tokens.extend(new_xpath_tokens)
551+
return xpath_tokens
552+
546553
new_xpath_tokens = self._get_xpath_tokens_from_leaf(model, token_index+1, path_tokens,config[path_tokens[token_index]])
547554
xpath_tokens.extend(new_xpath_tokens)
548555
return xpath_tokens
549556

557+
def _get_xpath_tokens_from_type_1_list(self, model, token_index, path_tokens, config):
558+
type_1_list_name = model['@name']
559+
keyName = model['key']['@value']
560+
value = path_tokens[token_index]
561+
keyToken = f"[{keyName}='{value}']"
562+
itemToken = f"{type_1_list_name}{keyToken}"
563+
564+
return [itemToken]
565+
550566
def _get_xpath_tokens_from_leaf(self, model, token_index, path_tokens, config):
551567
token = path_tokens[token_index]
552568

@@ -580,7 +596,7 @@ def _get_xpath_tokens_from_leaf(self, model, token_index, path_tokens, config):
580596
# /module-name:container/leaf-list[.='val']
581597
# Source: Check examples in https://netopeer.liberouter.org/doc/libyang/master/html/howto_x_path.html
582598
return [f"{token}[.='{value}']"]
583-
599+
584600
# checking 'uses' statement
585601
if not isinstance(config[token], list): # leaf-list under uses is not supported yet in sonic_yang
586602
table = path_tokens[0]
@@ -608,7 +624,7 @@ def _extractKey(self, tableKey, keys):
608624
def _get_list_model(self, model, token_index, path_tokens):
609625
parent_container_name = path_tokens[token_index]
610626
clist = model.get('list')
611-
# Container contains a single list, just return it
627+
# Container contains a single list, just return it
612628
# TODO: check if matching also by name is necessary
613629
if isinstance(clist, dict):
614630
return clist
@@ -630,6 +646,15 @@ def _get_list_model(self, model, token_index, path_tokens):
630646

631647
return None
632648

649+
def _get_type_1_list_model(self, model):
650+
list_name = model['@name']
651+
if list_name not in sonic_yang_ext.Type_1_list_maps_model:
652+
return None
653+
654+
# Type 1 list is expected to have a single inner list model.
655+
# No need to check if it is a dictionary of list models.
656+
return model.get('list')
657+
633658
def convert_xpath_to_path(self, xpath, config, sy):
634659
"""
635660
Converts the given XPATH to JsonPatch path (i.e. JsonPointer).
@@ -711,10 +736,66 @@ def _get_path_tokens_from_list(self, model, token_index, xpath_tokens, config):
711736
if next_token in key_dict:
712737
return path_tokens
713738

739+
type_1_list_model = self._get_type_1_list_model(model)
740+
if type_1_list_model:
741+
new_path_tokens = self._get_path_tokens_from_type_1_list(type_1_list_model, token_index+1, xpath_tokens, config[path_token])
742+
path_tokens.extend(new_path_tokens)
743+
return path_tokens
744+
714745
new_path_tokens = self._get_path_tokens_from_leaf(model, token_index+1, xpath_tokens, config[path_token])
715746
path_tokens.extend(new_path_tokens)
716747
return path_tokens
717748

749+
def _get_path_tokens_from_type_1_list(self, model, token_index, xpath_tokens, config):
750+
type_1_inner_list_name = model['@name']
751+
752+
token = xpath_tokens[token_index]
753+
list_tokens = token.split("[", 1) # split once on the first '[', first element will be the inner list name
754+
inner_list_name = list_tokens[0]
755+
756+
if type_1_inner_list_name != inner_list_name:
757+
raise GenericConfigUpdaterError(f"Type 1 inner list name '{type_1_inner_list_name}' does match xpath inner list name '{inner_list_name}'.")
758+
759+
key_dict = self._extract_key_dict(token)
760+
761+
# If no keys specified return empty tokens, as we are already inside the correct table.
762+
# Also note that the type 1 inner list name in SonicYang has no correspondence in ConfigDb and is ignored.
763+
# Example where VLAN_MEMBER_LIST has no specific key/value:
764+
# xpath: /sonic-dot1p-tc-map:sonic-dot1p-tc-map/DOT1P_TO_TC_MAP/DOT1P_TO_TC_MAP_LIST[name='Dot1p_to_tc_map1']/DOT1P_TO_TC_MAP
765+
# path: /DOT1P_TO_TC_MAP/Dot1p_to_tc_map1
766+
if not(key_dict):
767+
return []
768+
769+
if len(key_dict) > 1:
770+
raise GenericConfigUpdaterError(f"Type 1 inner list should have only 1 key in xpath, {len(key_dict)} specified. Key dictionary: {key_dict}")
771+
772+
keyName = next(iter(key_dict.keys()))
773+
value = key_dict[keyName]
774+
775+
path_tokens = [value]
776+
777+
# If this is the last xpath token, return the path tokens we have built so far, no need for futher checks
778+
# Example:
779+
# xpath: /sonic-dot1p-tc-map:sonic-dot1p-tc-map/DOT1P_TO_TC_MAP/DOT1P_TO_TC_MAP_LIST[name='Dot1p_to_tc_map1']/DOT1P_TO_TC_MAP[dot1p='2']
780+
# path: /DOT1P_TO_TC_MAP/Dot1p_to_tc_map1/2
781+
if token_index+1 >= len(xpath_tokens):
782+
return path_tokens
783+
784+
# Checking if the next_token is actually a child leaf of the inner type 1 list, for which case
785+
# just ignore the token, and return the already created ConfigDb path pointing to the whole object
786+
# Example where the leaf specified is the key:
787+
# xpath: /sonic-dot1p-tc-map:sonic-dot1p-tc-map/DOT1P_TO_TC_MAP/DOT1P_TO_TC_MAP_LIST[name='Dot1p_to_tc_map1']/DOT1P_TO_TC_MAP[dot1p='2']/dot1p
788+
# path: /DOT1P_TO_TC_MAP/Dot1p_to_tc_map1/2
789+
# Example where the leaf specified is not the key:
790+
# xpath: /sonic-dot1p-tc-map:sonic-dot1p-tc-map/DOT1P_TO_TC_MAP/DOT1P_TO_TC_MAP_LIST[name='Dot1p_to_tc_map1']/DOT1P_TO_TC_MAP[dot1p='2']/tc
791+
# path: /DOT1P_TO_TC_MAP/Dot1p_to_tc_map1/2
792+
next_token = xpath_tokens[token_index+1]
793+
leaf_model = self._get_model(model.get('leaf'), next_token)
794+
if leaf_model:
795+
return path_tokens
796+
797+
raise GenericConfigUpdaterError(f"Type 1 inner list '{type_1_inner_list_name}' does not have a child leaf named '{next_token}'")
798+
718799
def _get_path_tokens_from_leaf(self, model, token_index, xpath_tokens, config):
719800
token = xpath_tokens[token_index]
720801

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
{
2+
"DOT1P_TO_TC_MAP": {
3+
"Dot1p_to_tc_map1": {
4+
"1": "1",
5+
"2": "2"
6+
},
7+
"Dot1p_to_tc_map2": {
8+
"3": "3",
9+
"4": "4"
10+
}
11+
},
12+
"EXP_TO_FC_MAP": {
13+
"Exp_to_fc_map1": {
14+
"1": "1",
15+
"2": "2"
16+
},
17+
"Exp_to_fc_map2": {
18+
"3": "3",
19+
"4": "4"
20+
}
21+
}
22+
}

tests/generic_config_updater/files/patch_sorter_test_success.json

+46-3
Original file line numberDiff line numberDiff line change
@@ -2673,8 +2673,10 @@
26732673
]
26742674
},
26752675
"ADDING_LOOPBACK0_VRF_NAME__DELETES_LOOPBACK0_AND_IPS_DOES_NOT_AFFECT_OTHER_TABLES": {
2676-
"desc": ["Adding loopback vrf name, deletes loopback0 and the associated ips. ",
2677-
"It does not affect other tables."],
2676+
"desc": [
2677+
"Adding loopback vrf name, deletes loopback0 and the associated ips. ",
2678+
"It does not affect other tables."
2679+
],
26782680
"current_config": {
26792681
"CABLE_LENGTH": {
26802682
"AZURE": {
@@ -2733,7 +2735,7 @@
27332735
"op": "add",
27342736
"path": "/LOOPBACK_INTERFACE",
27352737
"value": {
2736-
"Loopback0":{
2738+
"Loopback0": {
27372739
"vrf_name": "Vrf_01"
27382740
}
27392741
}
@@ -3538,6 +3540,15 @@
35383540
"profile": "egress_lossy_profile"
35393541
}
35403542
},
3543+
"CABLE_LENGTH": {
3544+
"AZURE": {
3545+
"Ethernet52": "40m",
3546+
"Ethernet56": "40m",
3547+
"Ethernet60": "40m",
3548+
"Ethernet68": "40m",
3549+
"Ethernet72": "40m"
3550+
}
3551+
},
35413552
"DEVICE_NEIGHBOR": {
35423553
"Ethernet52": {
35433554
"name": "ARISTA13T2",
@@ -3819,6 +3830,11 @@
38193830
"path": "/ACL_TABLE/EVERFLOW/ports/0",
38203831
"value": "Ethernet64"
38213832
},
3833+
{
3834+
"op": "add",
3835+
"path": "/CABLE_LENGTH/AZURE/Ethernet64",
3836+
"value": "40m"
3837+
},
38223838
{
38233839
"op": "add",
38243840
"path": "/INTERFACE/Ethernet64",
@@ -4584,6 +4600,13 @@
45844600
"value": "up"
45854601
}
45864602
],
4603+
[
4604+
{
4605+
"op": "add",
4606+
"path": "/CABLE_LENGTH/AZURE/Ethernet64",
4607+
"value": "40m"
4608+
}
4609+
],
45874610
[
45884611
{
45894612
"op": "replace",
@@ -5233,6 +5256,16 @@
52335256
"profile": "egress_lossy_profile"
52345257
}
52355258
},
5259+
"CABLE_LENGTH": {
5260+
"AZURE": {
5261+
"Ethernet52": "40m",
5262+
"Ethernet56": "40m",
5263+
"Ethernet60": "40m",
5264+
"Ethernet64": "40m",
5265+
"Ethernet68": "40m",
5266+
"Ethernet72": "40m"
5267+
}
5268+
},
52365269
"DEVICE_NEIGHBOR": {
52375270
"Ethernet52": {
52385271
"name": "ARISTA13T2",
@@ -5680,6 +5713,10 @@
56805713
"op": "remove",
56815714
"path": "/BGP_NEIGHBOR/fc00::7e/admin_status"
56825715
},
5716+
{
5717+
"op": "remove",
5718+
"path": "/CABLE_LENGTH/AZURE/Ethernet64"
5719+
},
56835720
{
56845721
"op": "remove",
56855722
"path": "/INTERFACE/Ethernet64"
@@ -6063,6 +6100,12 @@
60636100
"path": "/BGP_NEIGHBOR/fc00::a/admin_status"
60646101
}
60656102
],
6103+
[
6104+
{
6105+
"op": "remove",
6106+
"path": "/CABLE_LENGTH/AZURE/Ethernet64"
6107+
}
6108+
],
60666109
[
60676110
{
60686111
"op": "replace",

tests/generic_config_updater/gu_common_test.py

+27
Original file line numberDiff line numberDiff line change
@@ -855,6 +855,15 @@ def check(path, xpath, config=None):
855855
check(path="/BUFFER_PORT_EGRESS_PROFILE_LIST/Ethernet9/profile_list",
856856
xpath="/sonic-buffer-port-egress-profile-list:sonic-buffer-port-egress-profile-list/BUFFER_PORT_EGRESS_PROFILE_LIST/BUFFER_PORT_EGRESS_PROFILE_LIST_LIST[port='Ethernet9']/profile_list",
857857
config=Files.CONFIG_DB_WITH_PROFILE_LIST)
858+
check(path="/DOT1P_TO_TC_MAP/Dot1p_to_tc_map1",
859+
xpath="/sonic-dot1p-tc-map:sonic-dot1p-tc-map/DOT1P_TO_TC_MAP/DOT1P_TO_TC_MAP_LIST[name='Dot1p_to_tc_map1']",
860+
config=Files.CONFIG_DB_WITH_TYPE1_TABLES)
861+
check(path="/DOT1P_TO_TC_MAP/Dot1p_to_tc_map1/2",
862+
xpath="/sonic-dot1p-tc-map:sonic-dot1p-tc-map/DOT1P_TO_TC_MAP/DOT1P_TO_TC_MAP_LIST[name='Dot1p_to_tc_map1']/DOT1P_TO_TC_MAP[dot1p='2']",
863+
config=Files.CONFIG_DB_WITH_TYPE1_TABLES)
864+
check(path="/EXP_TO_FC_MAP/Exp_to_fc_map2/4",
865+
xpath="/sonic-exp-fc-map:sonic-exp-fc-map/EXP_TO_FC_MAP/EXP_TO_FC_MAP_LIST[name='Exp_to_fc_map2']/EXP_TO_FC_MAP[exp='4']",
866+
config=Files.CONFIG_DB_WITH_TYPE1_TABLES)
858867

859868
def test_convert_xpath_to_path(self):
860869
def check(xpath, path, config=None):
@@ -936,6 +945,24 @@ def check(xpath, path, config=None):
936945
check(xpath="/sonic-buffer-port-egress-profile-list:sonic-buffer-port-egress-profile-list/BUFFER_PORT_EGRESS_PROFILE_LIST/BUFFER_PORT_EGRESS_PROFILE_LIST_LIST[port='Ethernet9']/profile_list[.='egress_lossy_profile']",
937946
path="/BUFFER_PORT_EGRESS_PROFILE_LIST/Ethernet9/profile_list",
938947
config=Files.CONFIG_DB_WITH_PROFILE_LIST)
948+
check(xpath="/sonic-dot1p-tc-map:sonic-dot1p-tc-map/DOT1P_TO_TC_MAP/DOT1P_TO_TC_MAP_LIST[name='Dot1p_to_tc_map1']",
949+
path="/DOT1P_TO_TC_MAP/Dot1p_to_tc_map1",
950+
config=Files.CONFIG_DB_WITH_TYPE1_TABLES)
951+
check(xpath="/sonic-dot1p-tc-map:sonic-dot1p-tc-map/DOT1P_TO_TC_MAP/DOT1P_TO_TC_MAP_LIST[name='Dot1p_to_tc_map1']/DOT1P_TO_TC_MAP",
952+
path="/DOT1P_TO_TC_MAP/Dot1p_to_tc_map1",
953+
config=Files.CONFIG_DB_WITH_TYPE1_TABLES)
954+
check(xpath="/sonic-dot1p-tc-map:sonic-dot1p-tc-map/DOT1P_TO_TC_MAP/DOT1P_TO_TC_MAP_LIST[name='Dot1p_to_tc_map1']/DOT1P_TO_TC_MAP[dot1p='2']",
955+
path="/DOT1P_TO_TC_MAP/Dot1p_to_tc_map1/2",
956+
config=Files.CONFIG_DB_WITH_TYPE1_TABLES)
957+
check(xpath="/sonic-dot1p-tc-map:sonic-dot1p-tc-map/DOT1P_TO_TC_MAP/DOT1P_TO_TC_MAP_LIST[name='Dot1p_to_tc_map1']/DOT1P_TO_TC_MAP[dot1p='2']/dot1p",
958+
path="/DOT1P_TO_TC_MAP/Dot1p_to_tc_map1/2",
959+
config=Files.CONFIG_DB_WITH_TYPE1_TABLES)
960+
check(xpath="/sonic-dot1p-tc-map:sonic-dot1p-tc-map/DOT1P_TO_TC_MAP/DOT1P_TO_TC_MAP_LIST[name='Dot1p_to_tc_map1']/DOT1P_TO_TC_MAP[dot1p='2']/tc",
961+
path="/DOT1P_TO_TC_MAP/Dot1p_to_tc_map1/2",
962+
config=Files.CONFIG_DB_WITH_TYPE1_TABLES)
963+
check(xpath="/sonic-exp-fc-map:sonic-exp-fc-map/EXP_TO_FC_MAP/EXP_TO_FC_MAP_LIST[name='Exp_to_fc_map2']/EXP_TO_FC_MAP[exp='4']",
964+
path="/EXP_TO_FC_MAP/Exp_to_fc_map2/4",
965+
config=Files.CONFIG_DB_WITH_TYPE1_TABLES)
939966

940967
def test_has_path(self):
941968
def check(config, path, expected):

tests/generic_config_updater/patch_sorter_test.py

-1
Original file line numberDiff line numberDiff line change
@@ -3078,7 +3078,6 @@ def test_patch_sorter_success(self):
30783078
data = Files.PATCH_SORTER_TEST_SUCCESS
30793079
skip_exact_change_list_match = False
30803080
for test_case_name in data:
3081-
# TODO: Add CABLE_LENGTH to ADD_RACK and REMOVE_RACK tests https://github.com/Azure/sonic-utilities/issues/2034
30823081
with self.subTest(name=test_case_name):
30833082
self.run_single_success_case(data[test_case_name], skip_exact_change_list_match)
30843083

0 commit comments

Comments
 (0)