From 655ad2e9a5890b31cb906a17a3f9ea9937a6883f Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Sun, 14 Jul 2024 16:29:18 -0500 Subject: configdep: T6559: drop global redundancy removal to fix error reporting (cherry picked from commit 7249d10f1fbb3f90a4bdbcd0223926d0380ddd3a) --- python/vyos/configdep.py | 45 ++++++++++++++++++++++++--------------------- src/services/vyos-configd | 18 ++++++------------ 2 files changed, 30 insertions(+), 33 deletions(-) diff --git a/python/vyos/configdep.py b/python/vyos/configdep.py index 73bd9ea96..141c7baf7 100644 --- a/python/vyos/configdep.py +++ b/python/vyos/configdep.py @@ -33,10 +33,9 @@ if typing.TYPE_CHECKING: dependency_dir = os.path.join(directories['data'], 'config-mode-dependencies') -local_dependent_func: dict[str, list[typing.Callable]] = {} +dependent_func: dict[str, list[typing.Callable]] = {} DEBUG = False -FORCE_LOCAL = False def debug_print(s: str): if DEBUG: @@ -50,7 +49,8 @@ def canon_name_of_path(path: str) -> str: return canon_name(script) def caller_name() -> str: - return stack()[2].filename + filename = stack()[2].filename + return canon_name_of_path(filename) def name_of(f: typing.Callable) -> str: return f.__name__ @@ -107,46 +107,49 @@ def run_config_mode_script(script: str, config: 'Config'): mod.generate(c) mod.apply(c) except (VyOSError, ConfigError) as e: - raise ConfigError(repr(e)) + raise ConfigError(str(e)) from e def def_closure(target: str, config: 'Config', tagnode: typing.Optional[str] = None) -> typing.Callable: script = target + '.py' def func_impl(): - if tagnode: + if tagnode is not None: os.environ['VYOS_TAGNODE_VALUE'] = tagnode run_config_mode_script(script, 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', tagnode: typing.Optional[str] = None): + global dependent_func + d = get_dependency_dict(config) - k = canon_name_of_path(caller_name()) - tag_ext = f'_{tagnode}' if tagnode is not None else '' - if hasattr(config, 'dependent_func') and not FORCE_LOCAL: + k = caller_name() + + if hasattr(config, 'dependent_func'): dependent_func = getattr(config, 'dependent_func') - l = dependent_func.setdefault('vyos_configd', []) - else: - dependent_func = local_dependent_func - l = dependent_func.setdefault(k, []) + + l = dependent_func.setdefault(k, []) + for target in d[k][case]: func = def_closure(target, config, tagnode) - func.__name__ = f'{target}{tag_ext}' append_uniq(l, func) + debug_print(f'set_dependents: caller {k}, dependents {names_of(l)}') -def call_dependents(dependent_func: dict = None): - k = canon_name_of_path(caller_name()) - if dependent_func is None or FORCE_LOCAL: - dependent_func = local_dependent_func - l = dependent_func.get(k, []) - else: - l = dependent_func.get('vyos_configd', []) +def call_dependents(): + k = caller_name() + l = dependent_func.get(k, []) debug_print(f'call_dependents: caller {k}, dependents {names_of(l)}') while l: f = l.pop(0) debug_print(f'calling: {f.__name__}') - f() + try: + f() + except ConfigError as e: + s = f'dependent {f.__name__}: {str(e)}' + raise ConfigError(s) from e def called_as_dependent() -> bool: st = stack()[1:] diff --git a/src/services/vyos-configd b/src/services/vyos-configd index c89c486e5..f4d9b81c4 100755 --- a/src/services/vyos-configd +++ b/src/services/vyos-configd @@ -30,7 +30,6 @@ 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.configdep import call_dependents from vyos.config import Config from vyos import ConfigError @@ -134,7 +133,8 @@ def explicit_print(path, mode, msg): except OSError: logger.critical("error explicit_print") -def run_script(script, config, args) -> int: +def run_script(script_name, config, args) -> int: + script = conf_mode_scripts[script_name] script.argv = args config.set_level([]) try: @@ -143,8 +143,9 @@ def run_script(script, config, args) -> int: script.generate(c) script.apply(c) except ConfigError as e: - logger.critical(e) - explicit_print(session_out, session_mode, str(e)) + s = f'{script_name}: {repr(e)}' + logger.error(s) + explicit_print(session_out, session_mode, s) return R_ERROR_COMMIT except Exception as e: logger.critical(e) @@ -227,17 +228,10 @@ def process_node_data(config, data, last: bool = False) -> int: args.insert(0, f'{script_name}.py') if script_name not in include_set: - # call dependents now if last element of prio queue is run - # independent of configd - if last: - call_dependents(dependent_func=config.dependent_func) return R_PASS with stdout_redirected(session_out, session_mode): - result = run_script(conf_mode_scripts[script_name], config, args) - - if last and result == R_SUCCESS: - call_dependents(dependent_func=config.dependent_func) + result = run_script(script_name, config, args) return result -- cgit v1.2.3 From 08f524a62f4a805a63503af64ed1b26fa48c6337 Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Sun, 14 Jul 2024 16:31:58 -0500 Subject: configdep: T6559: use single dependency list with reset under configd (cherry picked from commit 52d08b1ec5b2943744daac7123e35fd415f85db2) --- python/vyos/config.py | 1 + python/vyos/configdep.py | 18 ++++++++---------- src/services/vyos-configd | 1 + 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/python/vyos/config.py b/python/vyos/config.py index cca65f0eb..b7ee606a9 100644 --- a/python/vyos/config.py +++ b/python/vyos/config.py @@ -140,6 +140,7 @@ class Config(object): self._level = [] self._dict_cache = {} + self.dependency_list = [] (self._running_config, self._session_config) = self._config_source.get_configtree_tuple() diff --git a/python/vyos/configdep.py b/python/vyos/configdep.py index 141c7baf7..e0fe1ddac 100644 --- a/python/vyos/configdep.py +++ b/python/vyos/configdep.py @@ -33,7 +33,7 @@ if typing.TYPE_CHECKING: dependency_dir = os.path.join(directories['data'], 'config-mode-dependencies') -dependent_func: dict[str, list[typing.Callable]] = {} +dependency_list: list[typing.Callable] = [] DEBUG = False @@ -122,26 +122,24 @@ def def_closure(target: str, config: 'Config', def set_dependents(case: str, config: 'Config', tagnode: typing.Optional[str] = None): - global dependent_func + global dependency_list + + dependency_list = config.dependency_list d = get_dependency_dict(config) k = caller_name() - - if hasattr(config, 'dependent_func'): - dependent_func = getattr(config, 'dependent_func') - - l = dependent_func.setdefault(k, []) + l = dependency_list for target in d[k][case]: func = def_closure(target, config, tagnode) append_uniq(l, func) - debug_print(f'set_dependents: caller {k}, dependents {names_of(l)}') + debug_print(f'set_dependents: caller {k}, current dependents {names_of(l)}') def call_dependents(): k = caller_name() - l = dependent_func.get(k, []) - debug_print(f'call_dependents: caller {k}, dependents {names_of(l)}') + l = dependency_list + debug_print(f'call_dependents: caller {k}, remaining dependents {names_of(l)}') while l: f = l.pop(0) debug_print(f'calling: {f.__name__}') diff --git a/src/services/vyos-configd b/src/services/vyos-configd index f4d9b81c4..a7306afd1 100755 --- a/src/services/vyos-configd +++ b/src/services/vyos-configd @@ -213,6 +213,7 @@ def process_node_data(config, data, last: bool = False) -> int: script_name = None args = [] + config.dependency_list.clear() res = re.match(r'^(VYOS_TAGNODE_VALUE=[^/]+)?.*\/([^/]+).py(.*)', data) if res.group(1): -- cgit v1.2.3 From 65d4ee33269f18a5a4ebab34e6e3866623a6a043 Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Sun, 14 Jul 2024 17:59:54 -0500 Subject: configdep: T6559: add smoketest of dependency script error (cherry picked from commit ad43aa885a8ef689da212088d6ebb37c32d72883) --- smoketest/scripts/cli/test_config_dependency.py | 49 +++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100755 smoketest/scripts/cli/test_config_dependency.py diff --git a/smoketest/scripts/cli/test_config_dependency.py b/smoketest/scripts/cli/test_config_dependency.py new file mode 100755 index 000000000..14e88321a --- /dev/null +++ b/smoketest/scripts/cli/test_config_dependency.py @@ -0,0 +1,49 @@ +#!/usr/bin/env python3 +# 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 +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this library. If not, see . + + +import unittest + +from base_vyostest_shim import VyOSUnitTestSHIM + +from vyos.configsession import ConfigSessionError + + +class TestConfigDep(VyOSUnitTestSHIM.TestCase): + def test_configdep_error(self): + address_group = 'AG' + address = '192.168.137.5' + nat_base = ['nat', 'source', 'rule', '10'] + interface = 'eth1' + + self.cli_set(['firewall', 'group', 'address-group', address_group, + 'address', address]) + self.cli_set(nat_base + ['outbound-interface', 'name', interface]) + self.cli_set(nat_base + ['source', 'group', 'address-group', address_group]) + self.cli_set(nat_base + ['translation', 'address', 'masquerade']) + self.cli_commit() + + self.cli_delete(['firewall']) + # check error in call to dependent script (nat) + with self.assertRaises(ConfigSessionError): + self.cli_commit() + + # clean up remaining + self.cli_delete(['nat']) + self.cli_commit() + +if __name__ == '__main__': + unittest.main(verbosity=2) -- cgit v1.2.3