From c82b2b836b01c285576cc98ecf2b99ff3d8838ef Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Fri, 23 May 2025 10:31:18 -0500 Subject: configd: T7488: allow distinction of first-order error verify vs apply Leave hint if vyos-configd encounters an error in the generate/apply stages: this only detects 'first-order' differences, meaning those originating from the called config mode script, and not its dependencies. This is useful for supporting automatic rollback for certain cases of apply stage error. --- src/services/vyos-configd | 14 ++++++++++++-- src/shim/vyshim.c | 41 +++++++++++++++++++++++++++++++++-------- 2 files changed, 45 insertions(+), 10 deletions(-) diff --git a/src/services/vyos-configd b/src/services/vyos-configd index 28acccd2c..c45d492f9 100755 --- a/src/services/vyos-configd +++ b/src/services/vyos-configd @@ -68,6 +68,7 @@ class Response(Enum): ERROR_COMMIT = 2 ERROR_DAEMON = 4 PASS = 8 + ERROR_COMMIT_APPLY = 16 vyos_conf_scripts_dir = directories['conf_mode'] @@ -142,8 +143,6 @@ def run_script(script_name, config, args) -> tuple[Response, str]: try: c = script.get_config(config) script.verify(c) - script.generate(c) - script.apply(c) except ConfigError as e: logger.error(e) return Response.ERROR_COMMIT, str(e) @@ -152,6 +151,17 @@ def run_script(script_name, config, args) -> tuple[Response, str]: logger.error(tb) return Response.ERROR_COMMIT, tb + try: + script.generate(c) + script.apply(c) + except ConfigError as e: + logger.error(e) + return Response.ERROR_COMMIT_APPLY, str(e) + except Exception: + tb = traceback.format_exc() + logger.error(tb) + return Response.ERROR_COMMIT_APPLY, tb + return Response.SUCCESS, '' diff --git a/src/shim/vyshim.c b/src/shim/vyshim.c index 1eb653cbf..35f995419 100644 --- a/src/shim/vyshim.c +++ b/src/shim/vyshim.c @@ -18,8 +18,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -55,15 +57,17 @@ enum { SUCCESS = 1 << 0, ERROR_COMMIT = 1 << 1, ERROR_DAEMON = 1 << 2, - PASS = 1 << 3 + PASS = 1 << 3, + ERROR_COMMIT_APPLY = 1 << 4 }; volatile int init_alarm = 0; volatile int timeout = 0; -int initialization(void *); +int initialization(void *, char *); int pass_through(char **, int); void timer_handler(int); +void leave_hint(char *); double get_posix_clock_time(void); @@ -94,8 +98,17 @@ int main(int argc, char* argv[]) char *test = strstr(string_node_data, "VYOS_TAGNODE_VALUE"); ex_index = test ? 2 : 1; + char *env_tmp = getenv("VYATTA_CONFIG_TMP"); + if (env_tmp == NULL) { + fprintf(stderr, "Error: Environment variable VYATTA_CONFIG_TMP is not set.\n"); + exit(EXIT_FAILURE); + } + char *pid_str = strdup(env_tmp); + strsep(&pid_str, "_"); + debug_print("config session pid: %s\n", pid_str); + if (access(COMMIT_MARKER, F_OK) != -1) { - init_timeout = initialization(requester); + init_timeout = initialization(requester, pid_str); if (!init_timeout) remove(COMMIT_MARKER); } @@ -151,13 +164,19 @@ int main(int argc, char* argv[]) ret = -1; } + if (err & ERROR_COMMIT_APPLY) { + debug_print("Received ERROR_COMMIT_APPLY\n"); + leave_hint(pid_str); + ret = -1; + } + zmq_close(requester); zmq_ctx_destroy(context); return ret; } -int initialization(void* Requester) +int initialization(void* Requester, char* pid_val) { char *active_str = NULL; size_t active_len = 0; @@ -185,10 +204,6 @@ int initialization(void* Requester) double prev_time_value, time_value; double time_diff; - char *pid_val = getenv("VYATTA_CONFIG_TMP"); - strsep(&pid_val, "_"); - debug_print("config session pid: %s\n", pid_val); - char *sudo_user = getenv("SUDO_USER"); if (!sudo_user) { char nobody[] = "nobody"; @@ -338,6 +353,16 @@ void timer_handler(int signum) return; } +void leave_hint(char *pid_val) +{ + char tmp_str[16]; + mode_t omask = umask(0); + snprintf(tmp_str, sizeof(tmp_str), "/tmp/apply_%s", pid_val); + open(tmp_str, O_CREAT|O_RDWR|O_TRUNC, 0666); + chown(tmp_str, 1002, 102); + umask(omask); +} + #ifdef _POSIX_MONOTONIC_CLOCK double get_posix_clock_time(void) { -- cgit v1.2.3 From 00cb7fd19587771129c9923a781488929c03f3f8 Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Fri, 23 May 2025 10:40:40 -0500 Subject: T7488: add utility for automatic rollback of section on apply stage err --- python/vyos/configsession.py | 6 ++- src/helpers/reset_section.py | 102 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 1 deletion(-) create mode 100755 src/helpers/reset_section.py diff --git a/python/vyos/configsession.py b/python/vyos/configsession.py index f0d636b89..7af2cb333 100644 --- a/python/vyos/configsession.py +++ b/python/vyos/configsession.py @@ -146,7 +146,7 @@ class ConfigSession(object): The write API of VyOS. """ - def __init__(self, session_id, app=APP): + def __init__(self, session_id, app=APP, shared=False): """ Creates a new config session. @@ -187,7 +187,11 @@ class ConfigSession(object): else: self._vyconf_session = None + self.shared = shared + def __del__(self): + if self.shared: + return if self._vyconf_session is None: try: output = ( diff --git a/src/helpers/reset_section.py b/src/helpers/reset_section.py new file mode 100755 index 000000000..22b608f00 --- /dev/null +++ b/src/helpers/reset_section.py @@ -0,0 +1,102 @@ +#!/usr/bin/env python3 +# +# Copyright (C) 2025 VyOS maintainers and contributors +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 2 or later as +# published by the Free Software Foundation. +# +# This program 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 General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# +# + + +import argparse +import sys +import os + +from vyos.configsession import ConfigSession +from vyos.config import Config +from vyos.configdiff import get_config_diff +from vyos.xml_ref import is_leaf + + +def type_str_to_list(value): + if isinstance(value, str): + return value.split() + raise argparse.ArgumentTypeError('path must be a whitespace separated string') + + +parser = argparse.ArgumentParser() +parser.add_argument('path', type=type_str_to_list, help='section to reload/rollback') +parser.add_argument('--pid', help='pid of config session') + +group = parser.add_mutually_exclusive_group() +group.add_argument('--reload', action='store_true', help='retry proposed commit') +group.add_argument( + '--rollback', action='store_true', default=True, help='rollback to stable commit' +) + +args = parser.parse_args() + +path = args.path +reload = args.reload +rollback = args.rollback +pid = args.pid + +try: + if is_leaf(path): + sys.exit('path is leaf node: neither allowed nor useful') +except ValueError: + sys.exit('nonexistent path: neither allowed nor useful') + +test = Config() +if not test.in_session(): + sys.exit('reset_section not available outside of a config session') + +diff = get_config_diff(test) +if not diff.is_node_changed(path): + # No discrepancies at path after commit, hence no error to revert. + sys.exit() + +del diff +del test + + +session_id = int(pid) if pid else os.getppid() + +# check hint left by vyshim when ConfigError is from apply stage +hint_name = f'/tmp/apply_{session_id}' +if not os.path.exists(hint_name): + # no apply error; exit + sys.exit() +else: + # cleanup hint and continue with reset + os.unlink(hint_name) + +session = ConfigSession(session_id, shared=True) + +session_env = session.get_session_env() +config = Config(session_env) + +effective = not bool(reload) + +d = config.get_config_dict(path, effective=effective, get_first_key=True) + +session.discard() + +session.delete(path) +session.commit() + +if not d: + # nothing more to do in either case of reload/rollback + sys.exit() + +session.set_section(path, d) +session.commit() -- cgit v1.2.3 From 4c1bffe71454c7f15ecc1fa27e4a3ecb1176361f Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Mon, 26 May 2025 09:11:52 -0500 Subject: T7488: allow reloads outside of config session --- src/helpers/reset_section.py | 60 ++++++++++++++++++++++++++++---------------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/src/helpers/reset_section.py b/src/helpers/reset_section.py index 22b608f00..f1e4cfc6c 100755 --- a/src/helpers/reset_section.py +++ b/src/helpers/reset_section.py @@ -20,6 +20,7 @@ import argparse import sys import os +import grp from vyos.configsession import ConfigSession from vyos.config import Config @@ -27,6 +28,9 @@ from vyos.configdiff import get_config_diff from vyos.xml_ref import is_leaf +CFG_GROUP = 'vyattacfg' + + def type_str_to_list(value): if isinstance(value, str): return value.split() @@ -57,39 +61,52 @@ except ValueError: sys.exit('nonexistent path: neither allowed nor useful') test = Config() -if not test.in_session(): - sys.exit('reset_section not available outside of a config session') +in_session = test.in_session() -diff = get_config_diff(test) -if not diff.is_node_changed(path): - # No discrepancies at path after commit, hence no error to revert. - sys.exit() +if in_session: + if reload: + sys.exit('reset_section reload not available inside of a config session') + + diff = get_config_diff(test) + if not diff.is_node_changed(path): + # No discrepancies at path after commit, hence no error to revert. + sys.exit() + + del diff +else: + if not reload: + sys.exit('reset_section rollback not available outside of a config session') -del diff del test session_id = int(pid) if pid else os.getppid() -# check hint left by vyshim when ConfigError is from apply stage -hint_name = f'/tmp/apply_{session_id}' -if not os.path.exists(hint_name): - # no apply error; exit - sys.exit() -else: - # cleanup hint and continue with reset - os.unlink(hint_name) +if in_session: + # check hint left by vyshim when ConfigError is from apply stage + hint_name = f'/tmp/apply_{session_id}' + if not os.path.exists(hint_name): + # no apply error; exit + sys.exit() + else: + # cleanup hint and continue with reset + os.unlink(hint_name) + +cfg_group = grp.getgrnam(CFG_GROUP) +os.setgid(cfg_group.gr_gid) +os.umask(0o002) -session = ConfigSession(session_id, shared=True) +shared = not bool(reload) + +session = ConfigSession(session_id, shared=shared) session_env = session.get_session_env() config = Config(session_env) -effective = not bool(reload) - -d = config.get_config_dict(path, effective=effective, get_first_key=True) +d = config.get_config_dict(path, effective=True, get_first_key=True) -session.discard() +if in_session: + session.discard() session.delete(path) session.commit() @@ -99,4 +116,5 @@ if not d: sys.exit() session.set_section(path, d) -session.commit() +out = session.commit() +print(out) -- cgit v1.2.3 From 2ef19495e1a2750330a5a520f41601202ca2b703 Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Tue, 10 Jun 2025 11:06:05 -0500 Subject: T7488: exit silently if path doesn't exist, unless debug --- src/helpers/reset_section.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/helpers/reset_section.py b/src/helpers/reset_section.py index f1e4cfc6c..32857f650 100755 --- a/src/helpers/reset_section.py +++ b/src/helpers/reset_section.py @@ -29,6 +29,7 @@ from vyos.xml_ref import is_leaf CFG_GROUP = 'vyattacfg' +DEBUG = False def type_str_to_list(value): @@ -58,7 +59,10 @@ try: if is_leaf(path): sys.exit('path is leaf node: neither allowed nor useful') except ValueError: - sys.exit('nonexistent path: neither allowed nor useful') + if DEBUG: + sys.exit('nonexistent path: neither allowed nor useful') + else: + sys.exit() test = Config() in_session = test.in_session() -- cgit v1.2.3