summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorChristian Breunig <christian@breunig.cc>2024-08-26 07:23:59 +0200
committerGitHub <noreply@github.com>2024-08-26 07:23:59 +0200
commit42191a8f6a4efc248e973c1bc3a974474835b15e (patch)
tree80fa64b91d8a3dcf16e286178433232be5b4f46c
parentda64a7246e9b12d5bd84287517cfbfa59e364c28 (diff)
parent9ff620c50625c81733020f399c7f5a14e07c4d09 (diff)
downloadvyos-1x-42191a8f6a4efc248e973c1bc3a974474835b15e.tar.gz
vyos-1x-42191a8f6a4efc248e973c1bc3a974474835b15e.zip
Merge pull request #4015 from jestabro/configdep-prio
T6671: defer config dependency if scheduled in priority queue
-rw-r--r--python/vyos/configdep.py26
-rw-r--r--python/vyos/configdiff.py37
-rw-r--r--python/vyos/utils/dict.py1
-rw-r--r--python/vyos/xml_ref/__init__.py6
-rw-r--r--python/vyos/xml_ref/definition.py22
-rwxr-xr-xsmoketest/scripts/cli/test_config_dependency.py85
-rwxr-xr-xsrc/services/vyos-configd19
7 files changed, 180 insertions, 16 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',
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
"""
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 <maintainers@vyos.io>
+# Copyright 2024 VyOS maintainers and contributors <maintainers@vyos.io>
#
# 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 <maintainers@vyos.io>
+# Copyright 2024 VyOS maintainers and contributors <maintainers@vyos.io>
#
# 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:
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)
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}")