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