From 4fc09b111268a46f74cc14263a1109e40d87ad39 Mon Sep 17 00:00:00 2001 From: Mohamed Ghoneim Date: Thu, 19 May 2022 17:01:09 -0700 Subject: [PATCH] [GCU] Handling non-compliant leaf-list with string values (#2174) #### What I did Fixes #2170 Converting xpath to path: - There is only 1 case where the path is referring to the list itself. Example: ``` 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", ``` - There is no other case as we the list is just a single string, and we cannot refer to individual elements in string in ConfigDb. Converting path to xpath, there are 2 cases: - Xpath is pointing to leaf-list as a whole, return path of whole 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", path="/BUFFER_PORT_EGRESS_PROFILE_LIST/Ethernet9/profile_list", ``` - Xpath is pointing to an element of the leaf-list, return path of whole list as well since it is not possible to point to specific element -- here we are pointing to the element `egress_lossy_profile` ``` 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", ``` #### How I did it Only a single change if the xpath is pointing to a specific element, just return whole list token from ConfigDb format. Please note this solution is future proof, if ConfigDb changes how a list is saved as a string or json list, we don't care. #### 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 | 13 ++++++ .../files/config_db_with_profile_list.json | 42 +++++++++++++++++++ .../generic_config_updater/gu_common_test.py | 22 ++++++++++ 3 files changed, 77 insertions(+) create mode 100644 tests/generic_config_updater/files/config_db_with_profile_list.json diff --git a/generic_config_updater/gu_common.py b/generic_config_updater/gu_common.py index 3a7e9346fc..c0206cf198 100644 --- a/generic_config_updater/gu_common.py +++ b/generic_config_updater/gu_common.py @@ -750,6 +750,19 @@ def _get_path_tokens_from_leaf(self, model, token_index, xpath_tokens, config): # leaf_list_name = match.group(1) leaf_list_value = match.group(1) list_config = config[leaf_list_name] + # Workaround for those fields who is defined as leaf-list in YANG model but have string value in config DB + # No need to lookup the item index in ConfigDb since the list is represented as a string, return path to string immediately + # Example: + # 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 + if isinstance(list_config, str): + return [leaf_list_name] + + if not isinstance(list_config, list): + raise ValueError(f"list_config is expected to be of type list or string. Found {type(list_config)}.\n " + \ + f"model: {model}\n token_index: {token_index}\n " + \ + f"xpath_tokens: {xpath_tokens}\n config: {config}") + list_idx = list_config.index(leaf_list_value) return [leaf_list_name, list_idx] diff --git a/tests/generic_config_updater/files/config_db_with_profile_list.json b/tests/generic_config_updater/files/config_db_with_profile_list.json new file mode 100644 index 0000000000..00195433b6 --- /dev/null +++ b/tests/generic_config_updater/files/config_db_with_profile_list.json @@ -0,0 +1,42 @@ +{ + "BUFFER_POOL": { + "egress_lossless_pool": { + "mode": "static", + "size": "33004032", + "xoff": "196608", + "type": "egress" + }, + "egress_lossy_pool": { + "size": "12766208", + "type": "egress", + "mode": "dynamic" + } + }, + "BUFFER_PROFILE": { + "egress_lossless_profile": { + "pool":"egress_lossless_pool", + "size":"1518", + "dynamic_th":"3" + }, + "egress_lossy_profile": { + "pool":"egress_lossy_pool", + "size":"1518", + "dynamic_th":"3" + } + }, + "BUFFER_PORT_EGRESS_PROFILE_LIST": { + "Ethernet9": { + "profile_list": "egress_lossless_profile,egress_lossy_profile" + } + }, + "PORT": { + "Ethernet9": { + "alias": "Eth3/2", + "lanes": "74", + "description": "", + "speed": "11100", + "tpid": "0x8100", + "admin_status": "up" + } + } +} \ No newline at end of file diff --git a/tests/generic_config_updater/gu_common_test.py b/tests/generic_config_updater/gu_common_test.py index 90702d1592..2beb2a2987 100644 --- a/tests/generic_config_updater/gu_common_test.py +++ b/tests/generic_config_updater/gu_common_test.py @@ -779,6 +779,19 @@ def test_find_ref_paths__does_not_remove_tables_without_yang(self): # Assert self.assertEqual(expected_config, config) + def test_find_ref_paths__ref_path_is_leaflist_in_yang_but_string_in_config_db__path_to_string_returned(self): + # Arrange + path = "/BUFFER_PROFILE/egress_lossless_profile" + expected = [ + "/BUFFER_PORT_EGRESS_PROFILE_LIST/Ethernet9/profile_list", + ] + + # Act + actual = self.path_addressing.find_ref_paths(path, Files.CONFIG_DB_WITH_PROFILE_LIST) + + # Assert + self.assertEqual(expected, actual) + def test_convert_path_to_xpath(self): def check(path, xpath, config=None): if not config: @@ -839,6 +852,9 @@ def check(path, xpath, config=None): check(path="/LLDP/GLOBAL/mode", xpath="/sonic-lldp:sonic-lldp/LLDP/GLOBAL/mode", config=Files.CONFIG_DB_WITH_LLDP) + 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) def test_convert_xpath_to_path(self): def check(xpath, path, config=None): @@ -914,6 +930,12 @@ def check(xpath, path, config=None): check(xpath="/sonic-lldp:sonic-lldp/LLDP/GLOBAL/mode", path="/LLDP/GLOBAL/mode", config=Files.CONFIG_DB_WITH_LLDP) + 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", + path="/BUFFER_PORT_EGRESS_PROFILE_LIST/Ethernet9/profile_list", + config=Files.CONFIG_DB_WITH_PROFILE_LIST) + 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) def test_has_path(self): def check(config, path, expected):