From 57a0333c423f74ef733619f57dbfc608e513aa56 Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Wed, 21 Aug 2024 19:40:52 -0500 Subject: xml: T5666: add with_tag keyword arg to owner --- python/vyos/xml_ref/__init__.py | 6 +++--- python/vyos/xml_ref/definition.py | 22 ++++++++++++++++------ 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/python/vyos/xml_ref/__init__.py b/python/vyos/xml_ref/__init__.py index 91ce394f7..99d8432d2 100644 --- a/python/vyos/xml_ref/__init__.py +++ b/python/vyos/xml_ref/__init__.py @@ -1,4 +1,4 @@ -# Copyright 2023 VyOS maintainers and contributors +# Copyright 2024 VyOS maintainers and contributors # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -54,8 +54,8 @@ def is_valueless(path: list) -> bool: def is_leaf(path: list) -> bool: return load_reference().is_leaf(path) -def owner(path: list) -> str: - return load_reference().owner(path) +def owner(path: list, with_tag=False) -> str: + return load_reference().owner(path, with_tag=with_tag) def priority(path: list) -> str: return load_reference().priority(path) diff --git a/python/vyos/xml_ref/definition.py b/python/vyos/xml_ref/definition.py index c85835ffd..5ff28daed 100644 --- a/python/vyos/xml_ref/definition.py +++ b/python/vyos/xml_ref/definition.py @@ -1,4 +1,4 @@ -# Copyright 2023 VyOS maintainers and contributors +# Copyright 2024 VyOS maintainers and contributors # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -139,28 +139,38 @@ class Xml: ref_path = path.copy() d = self.ref data = '' + tag = '' while ref_path and d: + tag_val = '' d = d.get(ref_path[0], {}) ref_path.pop(0) if self._is_tag_node(d) and ref_path: + tag_val = ref_path[0] ref_path.pop(0) if self._is_leaf_node(d) and ref_path: ref_path.pop(0) res = self._get_ref_node_data(d, name) if res is not None: data = res + tag = tag_val - return data + return data, tag - def owner(self, path: list) -> str: + def owner(self, path: list, with_tag=False) -> str: from pathlib import Path - data = self._least_upper_data(path, 'owner') + data, tag = self._least_upper_data(path, 'owner') + tag_ext = f'_{tag}' if tag else '' if data: - data = Path(data.split()[0]).name + if with_tag: + data = Path(data.split()[0]).stem + data = f'{data}{tag_ext}' + else: + data = Path(data.split()[0]).name return data def priority(self, path: list) -> str: - return self._least_upper_data(path, 'priority') + data, _ = self._least_upper_data(path, 'priority') + return data @staticmethod def _dict_get(d: dict, path: list) -> dict: -- cgit v1.2.3 From 5819fd88e7948572a65b62885ddcba8ebbb7371c Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Wed, 21 Aug 2024 20:18:48 -0500 Subject: configdiff: T5666: provide list of scripts scheduled for proposed commit --- python/vyos/configdiff.py | 37 +++++++++++++++++++++++++++++++++++++ python/vyos/utils/dict.py | 1 + 2 files changed, 38 insertions(+) diff --git a/python/vyos/configdiff.py b/python/vyos/configdiff.py index f975df45d..b6d4a5558 100644 --- a/python/vyos/configdiff.py +++ b/python/vyos/configdiff.py @@ -15,6 +15,7 @@ from enum import IntFlag from enum import auto +from itertools import chain from vyos.config import Config from vyos.configtree import DiffTree @@ -22,7 +23,10 @@ from vyos.configdict import dict_merge from vyos.utils.dict import get_sub_dict from vyos.utils.dict import mangle_dict_keys from vyos.utils.dict import dict_search_args +from vyos.utils.dict import dict_to_key_paths from vyos.xml_ref import get_defaults +from vyos.xml_ref import owner +from vyos.xml_ref import priority class ConfigDiffError(Exception): """ @@ -94,6 +98,39 @@ def get_config_diff(config, key_mangling=None): return ConfigDiff(config, key_mangling, diff_tree=diff_t, diff_dict=diff_d) +def get_commit_scripts(config) -> list: + """Return the list of config scripts to be executed by commit + + Return a list of the scripts to be called by commit for the proposed + config. The list is ordered by priority for reference, however, the + actual order of execution by the commit algorithm is not reflected + (delete vs. add queue), nor needed for current use. + """ + if not config or not isinstance(config, Config): + raise TypeError("argument must me a Config instance") + + if hasattr(config, 'commit_scripts'): + return getattr(config, 'commit_scripts') + + D = get_config_diff(config) + d = D._diff_dict + s = set() + for p in chain(dict_to_key_paths(d['sub']), dict_to_key_paths(d['add'])): + p_owner = owner(p, with_tag=True) + if not p_owner: + continue + p_priority = priority(p) + if not p_priority: + # default priority in legacy commit-algorithm + p_priority = 0 + p_priority = int(p_priority) + s.add((p_priority, p_owner)) + + res = [x[1] for x in sorted(s, key=lambda x: x[0])] + setattr(config, 'commit_scripts', res) + + return res + class ConfigDiff(object): """ The class of config changes as represented by comparison between the diff --git a/python/vyos/utils/dict.py b/python/vyos/utils/dict.py index 1eb6abcd5..1a7a6b96f 100644 --- a/python/vyos/utils/dict.py +++ b/python/vyos/utils/dict.py @@ -267,6 +267,7 @@ def dict_to_paths_values(conf: dict) -> dict: dict_of_options[path] = dict_search(path,conf) return dict_of_options + def dict_to_key_paths(d: dict) -> list: """ Generator to return list of key paths from dict of list[str]|str """ -- cgit v1.2.3 From d4b6bed84e5ac4214f2eae0e6ee7c1f4e0852222 Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Thu, 22 Aug 2024 10:44:44 -0500 Subject: configd: T6671: track scripts proposed and scripts called --- src/services/vyos-configd | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/services/vyos-configd b/src/services/vyos-configd index d797e90cf..3674d9627 100755 --- a/src/services/vyos-configd +++ b/src/services/vyos-configd @@ -30,6 +30,7 @@ from vyos.defaults import directories from vyos.utils.boot import boot_configuration_complete from vyos.configsource import ConfigSourceString from vyos.configsource import ConfigSourceError +from vyos.configdiff import get_commit_scripts from vyos.config import Config from vyos import ConfigError @@ -220,6 +221,12 @@ def initialization(socket): dependent_func: dict[str, list[typing.Callable]] = {} setattr(config, 'dependent_func', dependent_func) + commit_scripts = get_commit_scripts(config) + logger.debug(f'commit_scripts: {commit_scripts}') + + scripts_called = [] + setattr(config, 'scripts_called', scripts_called) + return config def process_node_data(config, data, last: bool = False) -> int: @@ -228,6 +235,7 @@ def process_node_data(config, data, last: bool = False) -> int: return R_ERROR_DAEMON script_name = None + os.environ['VYOS_TAGNODE_VALUE'] = '' args = [] config.dependency_list.clear() @@ -244,6 +252,12 @@ def process_node_data(config, data, last: bool = False) -> int: args = res.group(3).split() args.insert(0, f'{script_name}.py') + tag_value = os.getenv('VYOS_TAGNODE_VALUE', '') + tag_ext = f'_{tag_value}' if tag_value else '' + script_record = f'{script_name}{tag_ext}' + scripts_called = getattr(config, 'scripts_called', []) + scripts_called.append(script_record) + if script_name not in include_set: return R_PASS @@ -302,11 +316,12 @@ if __name__ == '__main__': socket.send(resp.encode()) config = initialization(socket) elif message["type"] == "node": - if message["last"]: - logger.debug(f'final element of priority queue') res = process_node_data(config, message["data"], message["last"]) response = res.to_bytes(1, byteorder=sys.byteorder) logger.debug(f"Sending response {res}") socket.send(response) + if message["last"] and config: + scripts_called = getattr(config, 'scripts_called', []) + logger.debug(f'scripts_called: {scripts_called}') else: logger.critical(f"Unexpected message: {message}") -- cgit v1.2.3 From 4f0e0265d87b01aafde39f2682d2d5099ac4e942 Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Thu, 22 Aug 2024 13:35:02 -0500 Subject: configdep: T6671: run dependency only if not scheduled by priority --- python/vyos/configdep.py | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/python/vyos/configdep.py b/python/vyos/configdep.py index e0fe1ddac..cf7c9d543 100644 --- a/python/vyos/configdep.py +++ b/python/vyos/configdep.py @@ -95,7 +95,8 @@ def get_dependency_dict(config: 'Config') -> dict: setattr(config, 'cached_dependency_dict', d) return d -def run_config_mode_script(script: str, config: 'Config'): +def run_config_mode_script(target: str, config: 'Config'): + script = target + '.py' path = os.path.join(directories['conf_mode'], script) name = canon_name(script) mod = load_as_module(name, path) @@ -109,15 +110,34 @@ def run_config_mode_script(script: str, config: 'Config'): except (VyOSError, ConfigError) as e: raise ConfigError(str(e)) from e +def run_conditionally(target: str, tagnode: str, config: 'Config'): + tag_ext = f'_{tagnode}' if tagnode else '' + script_name = f'{target}{tag_ext}' + + scripts_called = getattr(config, 'scripts_called', []) + commit_scripts = getattr(config, 'commit_scripts', []) + + debug_print(f'scripts_called: {scripts_called}') + debug_print(f'commit_scripts: {commit_scripts}') + + if script_name in commit_scripts and script_name not in scripts_called: + debug_print(f'dependency {script_name} deferred to priority') + return + + run_config_mode_script(target, config) + def def_closure(target: str, config: 'Config', tagnode: typing.Optional[str] = None) -> typing.Callable: - script = target + '.py' def func_impl(): + tag_value = '' if tagnode is not None: os.environ['VYOS_TAGNODE_VALUE'] = tagnode - run_config_mode_script(script, config) + tag_value = tagnode + run_conditionally(target, tag_value, config) + tag_ext = f'_{tagnode}' if tagnode is not None else '' func_impl.__name__ = f'{target}{tag_ext}' + return func_impl def set_dependents(case: str, config: 'Config', -- cgit v1.2.3 From 9ff620c50625c81733020f399c7f5a14e07c4d09 Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Sun, 25 Aug 2024 17:19:49 -0500 Subject: T6671: add smoketest for dependency deferred to priority --- smoketest/scripts/cli/test_config_dependency.py | 85 ++++++++++++++++++++++++- 1 file changed, 83 insertions(+), 2 deletions(-) diff --git a/smoketest/scripts/cli/test_config_dependency.py b/smoketest/scripts/cli/test_config_dependency.py index 14e88321a..99e807ac5 100755 --- a/smoketest/scripts/cli/test_config_dependency.py +++ b/smoketest/scripts/cli/test_config_dependency.py @@ -16,13 +16,39 @@ import unittest +from time import sleep -from base_vyostest_shim import VyOSUnitTestSHIM - +from vyos.utils.process import is_systemd_service_running +from vyos.utils.process import cmd from vyos.configsession import ConfigSessionError +from base_vyostest_shim import VyOSUnitTestSHIM + class TestConfigDep(VyOSUnitTestSHIM.TestCase): + @classmethod + def setUpClass(cls): + # smoketests are run without configd in 1.4; with configd in 1.5 + # the tests below check behavior under configd: + # test_configdep_error checks for regression under configd (T6559) + # test_configdep_prio_queue checks resolution under configd (T6671) + cls.running_state = is_systemd_service_running('vyos-configd.service') + + if not cls.running_state: + cmd('sudo systemctl start vyos-configd.service') + # allow time for init + sleep(1) + + super(TestConfigDep, cls).setUpClass() + + @classmethod + def tearDownClass(cls): + super(TestConfigDep, cls).tearDownClass() + + # return to running_state + if not cls.running_state: + cmd('sudo systemctl stop vyos-configd.service') + def test_configdep_error(self): address_group = 'AG' address = '192.168.137.5' @@ -45,5 +71,60 @@ class TestConfigDep(VyOSUnitTestSHIM.TestCase): self.cli_delete(['nat']) self.cli_commit() + def test_configdep_prio_queue(self): + # confirm that that a dependency (in this case, conntrack -> + # conntrack-sync) is not immediately called if the target is + # scheduled in the priority queue, indicating that it may require an + # intermediate activitation (bond0) + bonding_base = ['interfaces', 'bonding'] + bond_interface = 'bond0' + bond_address = '192.0.2.1/24' + vrrp_group_base = ['high-availability', 'vrrp', 'group'] + vrrp_sync_group_base = ['high-availability', 'vrrp', 'sync-group'] + vrrp_group = 'ETH2' + vrrp_sync_group = 'GROUP' + conntrack_sync_base = ['service', 'conntrack-sync'] + conntrack_peer = '192.0.2.77' + + # simple set to trigger in-session conntrack -> conntrack-sync + # dependency; note that this is triggered on boot in 1.4 due to + # default 'system conntrack modules' + self.cli_set(['system', 'conntrack', 'table-size', '524288']) + + self.cli_set(['interfaces', 'ethernet', 'eth2', 'address', + '198.51.100.2/24']) + + self.cli_set(bonding_base + [bond_interface, 'address', + bond_address]) + self.cli_set(bonding_base + [bond_interface, 'member', 'interface', + 'eth3']) + + self.cli_set(vrrp_group_base + [vrrp_group, 'address', + '198.51.100.200/24']) + self.cli_set(vrrp_group_base + [vrrp_group, 'hello-source-address', + '198.51.100.2']) + self.cli_set(vrrp_group_base + [vrrp_group, 'interface', 'eth2']) + self.cli_set(vrrp_group_base + [vrrp_group, 'priority', '200']) + self.cli_set(vrrp_group_base + [vrrp_group, 'vrid', '22']) + self.cli_set(vrrp_sync_group_base + [vrrp_sync_group, 'member', + vrrp_group]) + + self.cli_set(conntrack_sync_base + ['failover-mechanism', 'vrrp', + 'sync-group', vrrp_sync_group]) + + self.cli_set(conntrack_sync_base + ['interface', bond_interface, + 'peer', conntrack_peer]) + + self.cli_commit() + + # clean up + self.cli_delete(bonding_base) + self.cli_delete(vrrp_group_base) + self.cli_delete(vrrp_sync_group_base) + self.cli_delete(conntrack_sync_base) + self.cli_delete(['interfaces', 'ethernet', 'eth2', 'address']) + self.cli_delete(['system', 'conntrack', 'table-size']) + self.cli_commit() + if __name__ == '__main__': unittest.main(verbosity=2) -- cgit v1.2.3