From 6a374e35562960acaeecee590ae9bd2978fb990b Mon Sep 17 00:00:00 2001 From: ghooo Date: Wed, 26 Jan 2022 15:08:15 -0800 Subject: [PATCH 1/2] [GCU] Supporting Groupings during path-xpath translation --- generic_config_updater/gu_common.py | 51 ++++++++++++++++++- .../files/config_db_with_bgp_neighbor.json | 20 ++++++++ .../generic_config_updater/gu_common_test.py | 12 +++++ 3 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 tests/generic_config_updater/files/config_db_with_bgp_neighbor.json diff --git a/generic_config_updater/gu_common.py b/generic_config_updater/gu_common.py index 6ed9699f26..c144ab8369 100644 --- a/generic_config_updater/gu_common.py +++ b/generic_config_updater/gu_common.py @@ -360,6 +360,9 @@ def create_xpath(self, tokens): return f"{PathAddressing.XPATH_SEPARATOR}{PathAddressing.XPATH_SEPARATOR.join(str(t) for t in tokens)}" + def _create_sonic_yang_with_loaded_models(self): + return self.config_wrapper.create_sonic_yang_with_loaded_models() + def find_ref_paths(self, path, config): """ Finds the paths referencing any line under the given 'path' within the given 'config'. @@ -401,7 +404,7 @@ def find_ref_paths(self, path, config): return self._find_leafref_paths(path, config) def _find_leafref_paths(self, path, config): - sy = self.config_wrapper.create_sonic_yang_with_loaded_models() + sy = self._create_sonic_yang_with_loaded_models() tmp_config = copy.deepcopy(config) @@ -547,6 +550,13 @@ 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] + uses_leaf_model = self._get_uses_leaf_model(model, table, token) + if uses_leaf_model: + return [token] raise ValueError("Token not found") @@ -712,6 +722,13 @@ def _get_path_tokens_from_leaf(self, model, token_index, xpath_tokens, config): list_idx = list_config.index(leaf_list_value) return [leaf_list_name, list_idx] + # checking 'uses' statement + if not isinstance(config[leaf_list_name], list): # leaf-list under uses is not supported yet in sonic_yang + table = xpath_tokens[1] + uses_leaf_model = self._get_uses_leaf_model(model, table, token) + if uses_leaf_model: + return [token] + raise Exception("no leaf") def _extract_key_dict(self, list_token): @@ -746,6 +763,38 @@ def _get_model(self, model, name): return None + def _get_uses_leaf_model(self, model, table, token): + """ + Getting leaf model in uses model matching the given token. + """ + uses_s = model.get('uses') + if not uses_s: + return None + + # a model can be a single dict or a list of dictionaries, unify to a list of dictionaries + if isinstance(uses_s, dict): + uses_s = [uses_s] + + sy = self._create_sonic_yang_with_loaded_models() + # find yang module for current table + table_module = sy.confDbYangMap[table]['yangModule'] + # uses Example: "@name": "bgpcmn:sonic-bgp-cmn" + for uses in uses_s: + # Assume ':' means reference to another module + if ':' in uses['@name']: + prefix = uses['@name'].split(':')[0].strip() + uses_module = sy._findYangModuleFromPrefix(prefix, table_module) + else: + uses_module = table_module + grouping = uses['@name'].split(':')[-1].strip() + leafs = sy.preProcessedYang['grouping'][uses_module][grouping] + + leaf_model = self._get_model(leafs, token) + if leaf_model: + return leaf_model + + return None + class TitledLogger(logger.Logger): def __init__(self, syslog_identifier, title, verbose, print_all_to_console): super().__init__(syslog_identifier) diff --git a/tests/generic_config_updater/files/config_db_with_bgp_neighbor.json b/tests/generic_config_updater/files/config_db_with_bgp_neighbor.json new file mode 100644 index 0000000000..d58368d053 --- /dev/null +++ b/tests/generic_config_updater/files/config_db_with_bgp_neighbor.json @@ -0,0 +1,20 @@ +{ + "BGP_NEIGHBOR": { + "1.2.3.4": { + "admin_status": "up", + "asn": "65000", + "holdtime": "10", + "keepalive": "3", + "local_addr": "10.0.0.10", + "name": "ARISTA03T1", + "nhopself": "0", + "rrclient": "0" + }, + "default|1.2.3.4": { + "local_asn": "65200", + "asn": "65100", + "name": "bgp peer 65100", + "ebgp_multihop_ttl": "3" + } + } +} \ 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 ff3b504dfb..4c46e7174f 100644 --- a/tests/generic_config_updater/gu_common_test.py +++ b/tests/generic_config_updater/gu_common_test.py @@ -735,6 +735,12 @@ def check(path, xpath, config=None): check(path="/PORTCHANNEL_INTERFACE/PortChannel0001|1.1.1.1~124", xpath="/sonic-portchannel:sonic-portchannel/PORTCHANNEL_INTERFACE/PORTCHANNEL_INTERFACE_IPPREFIX_LIST[name='PortChannel0001'][ip_prefix='1.1.1.1/24']", config=Files.CONFIG_DB_WITH_PORTCHANNEL_INTERFACE) + check(path="/BGP_NEIGHBOR/1.2.3.4/holdtime", + xpath="/sonic-bgp-neighbor:sonic-bgp-neighbor/BGP_NEIGHBOR/BGP_NEIGHBOR_TEMPLATE_LIST[neighbor='1.2.3.4']/holdtime", + config=Files.CONFIG_DB_WITH_BGP_NEIGHBOR) + check(path="/BGP_NEIGHBOR/default|1.2.3.4/asn", + xpath="/sonic-bgp-neighbor:sonic-bgp-neighbor/BGP_NEIGHBOR/BGP_NEIGHBOR_LIST[vrf_name='default'][neighbor='1.2.3.4']/asn", + config=Files.CONFIG_DB_WITH_BGP_NEIGHBOR) def test_convert_xpath_to_path(self): def check(xpath, path, config=None): @@ -798,6 +804,12 @@ def check(xpath, path, config=None): check(xpath="/sonic-portchannel:sonic-portchannel/PORTCHANNEL_INTERFACE/PORTCHANNEL_INTERFACE_IPPREFIX_LIST[name='PortChannel0001'][ip_prefix='1.1.1.1/24']", path="/PORTCHANNEL_INTERFACE/PortChannel0001|1.1.1.1~124", config=Files.CONFIG_DB_WITH_PORTCHANNEL_INTERFACE) + check(xpath="/sonic-bgp-neighbor:sonic-bgp-neighbor/BGP_NEIGHBOR/BGP_NEIGHBOR_TEMPLATE_LIST[neighbor='1.2.3.4']/holdtime", + path="/BGP_NEIGHBOR/1.2.3.4/holdtime", + config=Files.CONFIG_DB_WITH_BGP_NEIGHBOR) + check(xpath="/sonic-bgp-neighbor:sonic-bgp-neighbor/BGP_NEIGHBOR/BGP_NEIGHBOR_LIST[vrf_name='default'][neighbor='1.2.3.4']/asn", + path="/BGP_NEIGHBOR/default|1.2.3.4/asn", + config=Files.CONFIG_DB_WITH_BGP_NEIGHBOR) def test_has_path(self): def check(config, path, expected): From 21b748e177874669ff8239c296ab9d4961e94ffb Mon Sep 17 00:00:00 2001 From: ghooo Date: Thu, 27 Jan 2022 23:00:19 -0800 Subject: [PATCH 2/2] adding unit-tests, checking for exceptions --- generic_config_updater/gu_common.py | 19 +++++++++++++------ .../files/config_db_with_bgp_neighbor.json | 12 ++++++++++++ .../files/config_db_with_lldp.json | 14 ++++++++++++++ .../generic_config_updater/gu_common_test.py | 12 ++++++++++++ 4 files changed, 51 insertions(+), 6 deletions(-) create mode 100644 tests/generic_config_updater/files/config_db_with_lldp.json diff --git a/generic_config_updater/gu_common.py b/generic_config_updater/gu_common.py index c144ab8369..2bee402375 100644 --- a/generic_config_updater/gu_common.py +++ b/generic_config_updater/gu_common.py @@ -772,7 +772,7 @@ def _get_uses_leaf_model(self, model, table, token): return None # a model can be a single dict or a list of dictionaries, unify to a list of dictionaries - if isinstance(uses_s, dict): + if not isinstance(uses_s, list): uses_s = [uses_s] sy = self._create_sonic_yang_with_loaded_models() @@ -780,14 +780,21 @@ def _get_uses_leaf_model(self, model, table, token): table_module = sy.confDbYangMap[table]['yangModule'] # uses Example: "@name": "bgpcmn:sonic-bgp-cmn" for uses in uses_s: + if not isinstance(uses, dict): + raise GenericConfigUpdaterError(f"'uses' is expected to be a dictionary found '{type(uses)}'.\n" \ + f" uses: {uses}\n model: {model}\n table: {table}\n token: {token}") + # Assume ':' means reference to another module if ':' in uses['@name']: - prefix = uses['@name'].split(':')[0].strip() - uses_module = sy._findYangModuleFromPrefix(prefix, table_module) + name_parts = uses['@name'].split(':') + prefix = name_parts[0].strip() + uses_module_name = sy._findYangModuleFromPrefix(prefix, table_module) + grouping = name_parts[-1].strip() else: - uses_module = table_module - grouping = uses['@name'].split(':')[-1].strip() - leafs = sy.preProcessedYang['grouping'][uses_module][grouping] + uses_module_name = table_module['@name'] + grouping = uses['@name'] + + leafs = sy.preProcessedYang['grouping'][uses_module_name][grouping] leaf_model = self._get_model(leafs, token) if leaf_model: diff --git a/tests/generic_config_updater/files/config_db_with_bgp_neighbor.json b/tests/generic_config_updater/files/config_db_with_bgp_neighbor.json index d58368d053..04ed2992e2 100644 --- a/tests/generic_config_updater/files/config_db_with_bgp_neighbor.json +++ b/tests/generic_config_updater/files/config_db_with_bgp_neighbor.json @@ -16,5 +16,17 @@ "name": "bgp peer 65100", "ebgp_multihop_ttl": "3" } + }, + "BGP_MONITORS": { + "5.6.7.8": { + "admin_status": "up", + "asn": "65000", + "holdtime": "180", + "keepalive": "60", + "local_addr": "10.0.0.11", + "name": "BGPMonitor", + "nhopself": "0", + "rrclient": "0" + } } } \ No newline at end of file diff --git a/tests/generic_config_updater/files/config_db_with_lldp.json b/tests/generic_config_updater/files/config_db_with_lldp.json new file mode 100644 index 0000000000..5162881dba --- /dev/null +++ b/tests/generic_config_updater/files/config_db_with_lldp.json @@ -0,0 +1,14 @@ +{ + "LLDP": { + "GLOBAL": { + "mode": "TRANSMIT", + "enabled": "true", + "hello_time": "12", + "multiplier": "5", + "supp_mgmt_address_tlv": "true", + "supp_system_capabilities_tlv": "false", + "system_name": "sonic", + "system_description": "sonic-system" + } + } +} \ 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 4c46e7174f..423d95c780 100644 --- a/tests/generic_config_updater/gu_common_test.py +++ b/tests/generic_config_updater/gu_common_test.py @@ -741,6 +741,12 @@ def check(path, xpath, config=None): check(path="/BGP_NEIGHBOR/default|1.2.3.4/asn", xpath="/sonic-bgp-neighbor:sonic-bgp-neighbor/BGP_NEIGHBOR/BGP_NEIGHBOR_LIST[vrf_name='default'][neighbor='1.2.3.4']/asn", config=Files.CONFIG_DB_WITH_BGP_NEIGHBOR) + check(path="/BGP_MONITORS/5.6.7.8/name", + xpath="/sonic-bgp-monitor:sonic-bgp-monitor/BGP_MONITORS/BGP_MONITORS_LIST[addr='5.6.7.8']/name", + config=Files.CONFIG_DB_WITH_BGP_NEIGHBOR) + check(path="/LLDP/GLOBAL/mode", + xpath="/sonic-lldp:sonic-lldp/LLDP/GLOBAL/mode", + config=Files.CONFIG_DB_WITH_LLDP) def test_convert_xpath_to_path(self): def check(xpath, path, config=None): @@ -810,6 +816,12 @@ def check(xpath, path, config=None): check(xpath="/sonic-bgp-neighbor:sonic-bgp-neighbor/BGP_NEIGHBOR/BGP_NEIGHBOR_LIST[vrf_name='default'][neighbor='1.2.3.4']/asn", path="/BGP_NEIGHBOR/default|1.2.3.4/asn", config=Files.CONFIG_DB_WITH_BGP_NEIGHBOR) + check(xpath="/sonic-bgp-monitor:sonic-bgp-monitor/BGP_MONITORS/BGP_MONITORS_LIST[addr='5.6.7.8']/name", + path="/BGP_MONITORS/5.6.7.8/name", + config=Files.CONFIG_DB_WITH_BGP_NEIGHBOR) + check(xpath="/sonic-lldp:sonic-lldp/LLDP/GLOBAL/mode", + path="/LLDP/GLOBAL/mode", + config=Files.CONFIG_DB_WITH_LLDP) def test_has_path(self): def check(config, path, expected):