From b8c9a8e59310fcaa93ce4921a558f9853c606403 Mon Sep 17 00:00:00 2001 From: Alex Kudentsov <43482574+alexk37@users.noreply.github.com> Date: Fri, 2 Oct 2026 14:46:50 +0700 Subject: config-mgmt: T9399: read the committed config once in the post-commit hooks 01vyos-commit-revision built a full Config() in ConfigMgmt() (active and working config, the working one read through the session's unionfs-fuse) and then ran vyos-save-config.py, which built a second full Config() to write the same active config out. On a config with a 2,000-entry prefix-list the hook took 17 s of every commit. The hooks run after the active config has been written, so read it once from the active store, and write the archive copy and config.json from the tree already held. The read is made as ConfigSourceSession makes it (stdout only, unmodified), and as there, no running config is passed until the boot configuration is complete, so the boot commit's hooks see no effective values and 02vyos-commit-archive uploads nothing at boot. After a commit that does not end in COMMIT_STATUS=SUCCESS (PARTIAL in the tests; a conf_mode script failure gives PARTIAL, and FAILURE takes the same branch) the hooks read the config the stock way (a full Config() of the working config); only the archive copy is still written from that tree in-process instead of by vyos-save-config.py. vyatta-cfg's discard removes the session's changes underneath the unionfs-fuse mount, and a set within about a second of it fails while the kernel still caches the removed directories; the stock hook takes longer than that and so hides it. A faster hook after a failed commit made smoketest test_bgp_27_route_reflector_client fail; on stock it fails the same way with 01vyos-commit-revision removed. confirm and initialize_revision use the same archive helper, so they also write the archive copy from the tree already held instead of starting vyos-save-config.py. --- python/vyos/config_mgmt.py | 70 ++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 67 insertions(+), 3 deletions(-) (limited to 'python') diff --git a/python/vyos/config_mgmt.py b/python/vyos/config_mgmt.py index 06cfb3d78..d75122ddb 100644 --- a/python/vyos/config_mgmt.py +++ b/python/vyos/config_mgmt.py @@ -27,6 +27,7 @@ from textwrap import dedent from pathlib import Path from shutil import copy, chown from subprocess import Popen +from subprocess import PIPE from subprocess import DEVNULL from urllib.parse import urlsplit from urllib.parse import urlunsplit @@ -45,11 +46,14 @@ from vyos.version import get_full_version_data from vyos.utils.io import ask_yes_no from vyos.utils.io import catch_broken_pipe from vyos.utils.boot import boot_configuration_complete +from vyos.utils.backend import vyconf_backend from vyos.utils.process import is_systemd_service_active from vyos.utils.process import rc_cmd from vyos.defaults import DEFAULT_COMMIT_CONFIRM_MINUTES from vyos.component_version import append_system_version +from vyos.component_version import add_system_version_string from vyos.utils.file import file_compare +from vyos.utils.file import write_file SAVE_CONFIG = '/usr/libexec/vyos/vyos-save-config.py' config_json = '/run/vyatta/config/config.json' @@ -625,7 +629,7 @@ Proceed ?""" cmp_saved = f'/tmp/config.boot.{ext}' if boot_configuration_complete(): - save_config(cmp_saved, json_out=config_json) + self._save_active_config(cmp_saved) else: copy(config_file, cmp_saved) @@ -657,6 +661,26 @@ Proceed ?""" return True + def _save_active_config(self, target): + """Write the active config and its JSON the way vyos-save-config.py + does, from the tree this object already holds: running that script + built a second full Config() for the same content.""" + ct = self.active_config + if ct is None: + save_config(target, json_out=config_json) + return + try: + write_file(target, add_system_version_string(ct.to_string())) + except OSError as e: + # as save_config() does when vyos-save-config.py fails + logger.critical(f'save config failed: {e}') + return + try: + with open(config_json, 'w') as f: + f.write(ct.to_json()) + except OSError as e: + logger.warning(f'cannot write {config_json}: {e}') + @staticmethod def _update_archive(): cmd = f'sudo logrotate -f -s {logrotate_state} {logrotate_conf}' @@ -802,16 +826,54 @@ Proceed ?""" os.umask(mask) +def _committed_config(): + """Config for the post-commit hooks. + + The hooks run after the commit has written the active config, so the + working config is the same tree. Read it once, from the active store. + Config() reads it twice, and its working-config read goes through the + session's unionfs-fuse, which takes seconds on a large config. + """ + from vyos.configsource import ConfigSourceString + + # after a failed or partial commit read the config the stock way (full + # Config()): a faster read here lets a scripted discard + set land inside + # vyatta-cfg's unionfs-fuse cache window after discard, where the set fails + if os.environ.get('COMMIT_STATUS', 'SUCCESS') != 'SUCCESS': + return None + + # with the vyconf backend Config() reads the vyconf session, not the + # legacy store: keep that path + if vyconf_backend() and boot_configuration_complete(): + return None + + # read stdout only, unmodified, as ConfigSourceSession does + p = Popen(['/bin/cli-shell-api', '--show-active-only', '--show-show-defaults', + '--show-ignore-edit', 'showConfig'], stdout=PIPE) + text = p.communicate()[0].decode() + if p.returncode != 0 or not text: + return None + # as ConfigSourceSession: no running config until the boot config is + # loaded, so the boot commit's hooks see no effective values (and do not + # upload the archive) + running = text if boot_configuration_complete() else '' + return Config(config_source=ConfigSourceString(running_config_text=running, + session_config_text=text)) + + # entry_point for console script # @catch_broken_pipe def run(): from argparse import ArgumentParser, REMAINDER - config_mgmt = ConfigMgmt() - for s in list(commit_hooks): if sys.argv[0].replace('-', '_').endswith(s): + try: + config = _committed_config() + except Exception: # pylint: disable=broad-exception-caught + config = None # ConfigMgmt() falls back to a full Config() + config_mgmt = ConfigMgmt(config=config) func = getattr(config_mgmt, s) try: func() @@ -819,6 +881,8 @@ def run(): print(f'{s}: {e}') sys.exit(0) + config_mgmt = ConfigMgmt() + parser = ArgumentParser() subparsers = parser.add_subparsers(dest='subcommand') -- cgit v1.2.3