[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)

generic_config_updater/gu_common.py

@@ -2,6 +2,7 @@
 import jsonpatch
 from jsonpointer import JsonPointer
 import sonic_yang
+import sonic_yang_ext
 import subprocess
 import yang as ly
 import copy
@@ -155,14 +156,14 @@ def crop_tables_without_yang(self, config_db_as_json):
         sy._cropConfigDB()
 
         return sy.jIn
-    
+
     def get_empty_tables(self, config):
         empty_tables = []
         for key in config.keys():
             if not(config[key]):
                 empty_tables.append(key)
         return empty_tables
-        
+
     def remove_empty_tables(self, config):
         config_with_non_empty_tables = {}
         for table in config:
@@ -398,7 +399,7 @@ def find_ref_paths(self, path, config):
         Finds the paths referencing any line under the given 'path' within the given 'config'.
         Example:
           path: /PORT
-          config: 
+          config:
             {
                 "VLAN_MEMBER": {
                     "Vlan1000|Ethernet0": {},
@@ -543,10 +544,25 @@ def _get_xpath_tokens_from_list(self, model, token_index, path_tokens, config):
         if len(path_tokens)-1 == token_index:
             return xpath_tokens
 
+        type_1_list_model = self._get_type_1_list_model(model)
+        if type_1_list_model:
+            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]])
+            xpath_tokens.extend(new_xpath_tokens)
+            return xpath_tokens
+
         new_xpath_tokens = self._get_xpath_tokens_from_leaf(model, token_index+1, path_tokens,config[path_tokens[token_index]])
         xpath_tokens.extend(new_xpath_tokens)
         return xpath_tokens
 
+    def _get_xpath_tokens_from_type_1_list(self, model, token_index, path_tokens, config):
+        type_1_list_name = model['@name']
+        keyName = model['key']['@value']
+        value = path_tokens[token_index]
+        keyToken = f"[{keyName}='{value}']"
+        itemToken = f"{type_1_list_name}{keyToken}"
+
+        return [itemToken]
+
     def _get_xpath_tokens_from_leaf(self, model, token_index, path_tokens, config):
         token = path_tokens[token_index]
 
@@ -580,7 +596,7 @@ def _get_xpath_tokens_from_leaf(self, model, token_index, path_tokens, config):
             #   /module-name:container/leaf-list[.='val']
             # Source: Check examples in https://netopeer.liberouter.org/doc/libyang/master/html/howto_x_path.html
             return [f"{token}[.='{value}']"]
-        
+
         # checking 'uses' statement
         if not isinstance(config[token], list): # leaf-list under uses is not supported yet in sonic_yang
             table = path_tokens[0]
@@ -608,7 +624,7 @@ def _extractKey(self, tableKey, keys):
     def _get_list_model(self, model, token_index, path_tokens):
         parent_container_name = path_tokens[token_index]
         clist = model.get('list')
-        # Container contains a single list, just return it 
+        # Container contains a single list, just return it
         # TODO: check if matching also by name is necessary
         if isinstance(clist, dict):
             return clist
@@ -630,6 +646,15 @@ def _get_list_model(self, model, token_index, path_tokens):
 
         return None
 
+    def _get_type_1_list_model(self, model):
+        list_name = model['@name']
+        if list_name not in sonic_yang_ext.Type_1_list_maps_model:
+            return None
+
+        # Type 1 list is expected to have a single inner list model.
+        # No need to check if it is a dictionary of list models.
+        return model.get('list')
+
     def convert_xpath_to_path(self, xpath, config, sy):
         """
         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):
         if next_token in key_dict:
             return path_tokens
 
+        type_1_list_model = self._get_type_1_list_model(model)
+        if type_1_list_model:
+            new_path_tokens = self._get_path_tokens_from_type_1_list(type_1_list_model, token_index+1, xpath_tokens, config[path_token])
+            path_tokens.extend(new_path_tokens)
+            return path_tokens
+
         new_path_tokens = self._get_path_tokens_from_leaf(model, token_index+1, xpath_tokens, config[path_token])
         path_tokens.extend(new_path_tokens)
         return path_tokens
 
+    def _get_path_tokens_from_type_1_list(self, model, token_index, xpath_tokens, config):
+        type_1_inner_list_name = model['@name']
+
+        token = xpath_tokens[token_index]
+        list_tokens = token.split("[", 1) # split once on the first '[', first element will be the inner list name
+        inner_list_name = list_tokens[0]
+
+        if type_1_inner_list_name != inner_list_name:
+            raise GenericConfigUpdaterError(f"Type 1 inner list name '{type_1_inner_list_name}' does match xpath inner list name '{inner_list_name}'.")
+
+        key_dict = self._extract_key_dict(token)
+
+        # If no keys specified return empty tokens, as we are already inside the correct table.
+        # Also note that the type 1 inner list name in SonicYang has no correspondence in ConfigDb and is ignored.
+        # Example where VLAN_MEMBER_LIST has no specific key/value:
+        #   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
+        #   path: /DOT1P_TO_TC_MAP/Dot1p_to_tc_map1
+        if not(key_dict):
+            return []
+
+        if len(key_dict) > 1:
+            raise GenericConfigUpdaterError(f"Type 1 inner list should have only 1 key in xpath, {len(key_dict)} specified. Key dictionary: {key_dict}")
+
+        keyName = next(iter(key_dict.keys()))
+        value = key_dict[keyName]
+
+        path_tokens = [value]
+
+        # If this is the last xpath token, return the path tokens we have built so far, no need for futher checks
+        # Example:
+        #   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']
+        #   path: /DOT1P_TO_TC_MAP/Dot1p_to_tc_map1/2
+        if token_index+1 >= len(xpath_tokens):
+            return path_tokens
+
+        # Checking if the next_token is actually a child leaf of the inner type 1 list, for which case
+        # just ignore the token, and return the already created ConfigDb path pointing to the whole object
+        # Example where the leaf specified is the key:
+        #   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
+        #   path: /DOT1P_TO_TC_MAP/Dot1p_to_tc_map1/2
+        # Example where the leaf specified is not the key:
+        #   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
+        #   path: /DOT1P_TO_TC_MAP/Dot1p_to_tc_map1/2
+        next_token = xpath_tokens[token_index+1]
+        leaf_model = self._get_model(model.get('leaf'), next_token)
+        if leaf_model:
+            return path_tokens
+
+        raise GenericConfigUpdaterError(f"Type 1 inner list '{type_1_inner_list_name}' does not have a child leaf named '{next_token}'")
+
     def _get_path_tokens_from_leaf(self, model, token_index, xpath_tokens, config):
         token = xpath_tokens[token_index]
 

tests/generic_config_updater/files/config_db_with_type1_tables.json

@@ -0,0 +1,22 @@
+{
+    "DOT1P_TO_TC_MAP": {
+        "Dot1p_to_tc_map1": { 
+           "1": "1",
+           "2": "2"
+         }, 
+        "Dot1p_to_tc_map2": { 
+           "3": "3",
+           "4": "4"
+         }
+     },
+     "EXP_TO_FC_MAP": {
+        "Exp_to_fc_map1": {
+           "1": "1",
+           "2": "2"
+        },
+        "Exp_to_fc_map2": {
+           "3": "3",
+           "4": "4"
+         }
+    }
+}

tests/generic_config_updater/files/patch_sorter_test_success.json

@@ -2673,8 +2673,10 @@
         ]
     },
     "ADDING_LOOPBACK0_VRF_NAME__DELETES_LOOPBACK0_AND_IPS_DOES_NOT_AFFECT_OTHER_TABLES": {
-        "desc": ["Adding loopback vrf name, deletes loopback0 and the associated ips. ",
-                 "It does not affect other tables."],
+        "desc": [
+            "Adding loopback vrf name, deletes loopback0 and the associated ips. ",
+            "It does not affect other tables."
+        ],
         "current_config": {
             "CABLE_LENGTH": {
                 "AZURE": {
@@ -2733,7 +2735,7 @@
                     "op": "add",
                     "path": "/LOOPBACK_INTERFACE",
                     "value": {
-                        "Loopback0":{
+                        "Loopback0": {
                             "vrf_name": "Vrf_01"
                         }
                     }
@@ -3538,6 +3540,15 @@
                     "profile": "egress_lossy_profile"
                 }
             },
+            "CABLE_LENGTH": {
+                "AZURE": {
+                    "Ethernet52": "40m",
+                    "Ethernet56": "40m",
+                    "Ethernet60": "40m",
+                    "Ethernet68": "40m",
+                    "Ethernet72": "40m"
+                }
+            },
             "DEVICE_NEIGHBOR": {
                 "Ethernet52": {
                     "name": "ARISTA13T2",
@@ -3819,6 +3830,11 @@
                 "path": "/ACL_TABLE/EVERFLOW/ports/0",
                 "value": "Ethernet64"
             },
+            {
+                "op": "add",
+                "path": "/CABLE_LENGTH/AZURE/Ethernet64",
+                "value": "40m"
+            },
             {
                 "op": "add",
                 "path": "/INTERFACE/Ethernet64",
@@ -4584,6 +4600,13 @@
                     "value": "up"
                 }
             ],
+            [
+                {
+                    "op": "add",
+                    "path": "/CABLE_LENGTH/AZURE/Ethernet64",
+                    "value": "40m"
+                }
+            ],
             [
                 {
                     "op": "replace",
@@ -5233,6 +5256,16 @@
                     "profile": "egress_lossy_profile"
                 }
             },
+            "CABLE_LENGTH": {
+                "AZURE": {
+                    "Ethernet52": "40m",
+                    "Ethernet56": "40m",
+                    "Ethernet60": "40m",
+                    "Ethernet64": "40m",
+                    "Ethernet68": "40m",
+                    "Ethernet72": "40m"
+                }
+            },
             "DEVICE_NEIGHBOR": {
                 "Ethernet52": {
                     "name": "ARISTA13T2",
@@ -5680,6 +5713,10 @@
                 "op": "remove",
                 "path": "/BGP_NEIGHBOR/fc00::7e/admin_status"
             },
+            {
+                "op": "remove",
+                "path": "/CABLE_LENGTH/AZURE/Ethernet64"
+            },
             {
                 "op": "remove",
                 "path": "/INTERFACE/Ethernet64"
@@ -6063,6 +6100,12 @@
                     "path": "/BGP_NEIGHBOR/fc00::a/admin_status"
                 }
             ],
+            [
+                {
+                    "op": "remove",
+                    "path": "/CABLE_LENGTH/AZURE/Ethernet64"
+                }
+            ],
             [
                 {
                     "op": "replace",

tests/generic_config_updater/gu_common_test.py

@@ -855,6 +855,15 @@ def check(path, xpath, config=None):
         check(path="/BUFFER_PORT_EGRESS_PROFILE_LIST/Ethernet9/profile_list",
               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",
               config=Files.CONFIG_DB_WITH_PROFILE_LIST)
+        check(path="/DOT1P_TO_TC_MAP/Dot1p_to_tc_map1",
+              xpath="/sonic-dot1p-tc-map:sonic-dot1p-tc-map/DOT1P_TO_TC_MAP/DOT1P_TO_TC_MAP_LIST[name='Dot1p_to_tc_map1']",
+              config=Files.CONFIG_DB_WITH_TYPE1_TABLES)
+        check(path="/DOT1P_TO_TC_MAP/Dot1p_to_tc_map1/2",
+              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']",
+              config=Files.CONFIG_DB_WITH_TYPE1_TABLES)
+        check(path="/EXP_TO_FC_MAP/Exp_to_fc_map2/4",
+              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']",
+              config=Files.CONFIG_DB_WITH_TYPE1_TABLES)
 
     def test_convert_xpath_to_path(self):
         def check(xpath, path, config=None):
@@ -936,6 +945,24 @@ def check(xpath, path, config=None):
         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']",
               path="/BUFFER_PORT_EGRESS_PROFILE_LIST/Ethernet9/profile_list",
               config=Files.CONFIG_DB_WITH_PROFILE_LIST)
+        check(xpath="/sonic-dot1p-tc-map:sonic-dot1p-tc-map/DOT1P_TO_TC_MAP/DOT1P_TO_TC_MAP_LIST[name='Dot1p_to_tc_map1']",
+              path="/DOT1P_TO_TC_MAP/Dot1p_to_tc_map1",
+              config=Files.CONFIG_DB_WITH_TYPE1_TABLES)
+        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",
+              path="/DOT1P_TO_TC_MAP/Dot1p_to_tc_map1",
+              config=Files.CONFIG_DB_WITH_TYPE1_TABLES)
+        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']",
+              path="/DOT1P_TO_TC_MAP/Dot1p_to_tc_map1/2",
+              config=Files.CONFIG_DB_WITH_TYPE1_TABLES)
+        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",
+              path="/DOT1P_TO_TC_MAP/Dot1p_to_tc_map1/2",
+              config=Files.CONFIG_DB_WITH_TYPE1_TABLES)
+        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",
+              path="/DOT1P_TO_TC_MAP/Dot1p_to_tc_map1/2",
+              config=Files.CONFIG_DB_WITH_TYPE1_TABLES)
+        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']",
+              path="/EXP_TO_FC_MAP/Exp_to_fc_map2/4",
+              config=Files.CONFIG_DB_WITH_TYPE1_TABLES)
 
     def test_has_path(self):
         def check(config, path, expected):

tests/generic_config_updater/patch_sorter_test.py

@@ -3078,7 +3078,6 @@ def test_patch_sorter_success(self):
         data = Files.PATCH_SORTER_TEST_SUCCESS
         skip_exact_change_list_match = False
         for test_case_name in data:
-            # TODO: Add CABLE_LENGTH to ADD_RACK and REMOVE_RACK tests https://github.com/Azure/sonic-utilities/issues/2034
             with self.subTest(name=test_case_name):
                 self.run_single_success_case(data[test_case_name], skip_exact_change_list_match)