From bfb2e85951409aa7f4ea891daa7fc05b1e5e5558 Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Fri, 27 Jun 2025 18:08:20 -0500 Subject: T7588: move blocking function to threadpool to allow async Refactor the blocking code in config handlers (endpoints /configure, /config-file) to run in a threadpool; this allows defining the handler as async. Consequently, the handler itself runs in the main thread, avoiding the need for thread registration for libvyosconfig function calls. --- src/services/api/rest/routers.py | 83 +++++++++++++++++++++++++++++----------- 1 file changed, 61 insertions(+), 22 deletions(-) diff --git a/src/services/api/rest/routers.py b/src/services/api/rest/routers.py index dfd24c0d5..48eca8f15 100644 --- a/src/services/api/rest/routers.py +++ b/src/services/api/rest/routers.py @@ -34,6 +34,7 @@ from fastapi import HTTPException from fastapi import APIRouter from fastapi import BackgroundTasks from fastapi.routing import APIRoute +from fastapi.concurrency import run_in_threadpool from starlette.datastructures import FormData from starlette.formparsers import FormParser from starlette.formparsers import MultiPartParser @@ -308,6 +309,7 @@ def call_commit_confirm(s: SessionState): env['IN_COMMIT_CONFIRM'] = 't' try: s.session.commit() + s.session.commit_confirm(minutes=s.confirm_time) except ConfigSessionError as e: s.session.discard() if s.debug: @@ -318,7 +320,29 @@ def call_commit_confirm(s: SessionState): del env['IN_COMMIT_CONFIRM'] -def _configure_op( +def run_commit(s: SessionState): + try: + out = s.session.commit() + return out, None + except Exception as e: + return None, e + + +def run_commit_confirm(s: SessionState): + env = s.session.get_session_env() + env['IN_COMMIT_CONFIRM'] = 't' + try: + out_c = s.session.commit() + out_cc = s.session.commit_confirm(minutes=s.confirm_time) + out = out_c + '\n' + out_cc + return out, None + except Exception as e: + return None, e + finally: + del env['IN_COMMIT_CONFIRM'] + + +async def _configure_op( data: Union[ ConfirmModel, ConfigureModel, @@ -420,22 +444,26 @@ def _configure_op( config = Config(session_env=env) d = get_config_diff(config) - if confirm_time: - out = session.commit_confirm(minutes=confirm_time) - msg = msg + out if msg else out - env['IN_COMMIT_CONFIRM'] = 't' + state.confirm_time = confirm_time if confirm_time else 0 - if d.is_node_changed(['service', 'https']): + if not d.is_node_changed(['service', 'https']): + if confirm_time: + out, err = await run_in_threadpool(run_commit_confirm, state) + if err: + raise err + msg = msg + out if msg else out + else: + out, err = await run_in_threadpool(run_commit, state) + if err: + raise err + msg = msg + out if msg else out + else: if confirm_time: background_tasks.add_task(call_commit_confirm, state) else: background_tasks.add_task(call_commit, state) out = self_ref_msg msg = msg + out if msg else out - else: - # capture non-fatal warnings - out = session.commit() - msg = msg + out if msg else out LOG.info(f"Configuration modified via HTTP API using key '{state.id}'") except ConfigSessionError as e: @@ -472,12 +500,14 @@ def create_path_import_pki_no_prompt(path): @router.post('/configure') -def configure_op( +async def configure_op( data: Union[ConfigureModel, ConfigureListModel, ConfirmModel], request: Request, background_tasks: BackgroundTasks, ): - return _configure_op(data, request, background_tasks) + out = await _configure_op(data, request, background_tasks) + + return out @router.post('/configure-section') @@ -534,13 +564,16 @@ async def retrieve_op(data: RetrieveModel): @router.post('/config-file') -def config_file_op(data: ConfigFileModel, background_tasks: BackgroundTasks): +async def config_file_op(data: ConfigFileModel, background_tasks: BackgroundTasks): state = SessionState() session = state.session env = session.get_session_env() op = data.op msg = None + # A non-zero confirm_time will start commit-confirm timer on commit + confirm_time = data.confirm_time + lock.acquire() try: @@ -569,21 +602,27 @@ def config_file_op(data: ConfigFileModel, background_tasks: BackgroundTasks): config = Config(session_env=env) d = get_config_diff(config) - if data.confirm_time: - out = session.commit_confirm(minutes=data.confirm_time) - msg = msg + out if msg else out - env['IN_COMMIT_CONFIRM'] = 't' + state.confirm_time = confirm_time if confirm_time else 0 - if d.is_node_changed(['service', 'https']): - if data.confirm_time: + if not d.is_node_changed(['service', 'https']): + if confirm_time: + out, err = await run_in_threadpool(run_commit_confirm, state) + if err: + raise err + msg = msg + out if msg else out + else: + out, err = await run_in_threadpool(run_commit, state) + if err: + raise err + msg = msg + out if msg else out + else: + if confirm_time: background_tasks.add_task(call_commit_confirm, state) else: background_tasks.add_task(call_commit, state) out = self_ref_msg msg = msg + out if msg else out - else: - out = session.commit() - msg = msg + out if msg else out + elif op == 'confirm': msg = session.confirm() else: -- cgit v1.2.3 From 9895031eb87a1c37d75dca21636ee712af57b413 Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Sat, 28 Jun 2025 19:11:35 -0500 Subject: T7588: restart vyos-commitd, http-api, after setting vyconf_backend --- python/vyos/utils/backend.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/python/vyos/utils/backend.py b/python/vyos/utils/backend.py index 1234e9aa4..d302a2efd 100644 --- a/python/vyos/utils/backend.py +++ b/python/vyos/utils/backend.py @@ -22,6 +22,7 @@ from pathlib import Path from vyos.utils.io import ask_yes_no from vyos.utils.process import call +from vyos.utils.process import is_systemd_service_active VYCONF_SENTINEL = '/run/vyconf_backend' @@ -69,6 +70,8 @@ def vyconf_backend() -> bool: def set_vyconf_backend(value: bool, no_prompt: bool = False): vyconfd_service = 'vyconfd.service' + commitd_service = 'vyos-commitd.service' + http_api_service = 'vyos-http-api.service' match value: case True: if vyconf_backend(): @@ -78,6 +81,9 @@ def set_vyconf_backend(value: bool, no_prompt: bool = False): Path(VYCONF_SENTINEL).touch() chattri(VYCONF_SENTINEL, True) call(f'systemctl restart {vyconfd_service}') + call(f'systemctl restart {commitd_service}') + if is_systemd_service_active(http_api_service): + call(f'systemctl restart {http_api_service}') case False: if not vyconf_backend(): return -- cgit v1.2.3 From 94a6154013e766f786a6e4b6eed86c6d9c6d73de Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Sat, 28 Jun 2025 23:06:46 -0500 Subject: T7588: add missing path arg --- python/vyos/configsession.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/vyos/configsession.py b/python/vyos/configsession.py index af87d83a0..216069992 100644 --- a/python/vyos/configsession.py +++ b/python/vyos/configsession.py @@ -333,7 +333,7 @@ class ConfigSession(object): if self._vyconf_session is None: config_data = self.__run_command(SHOW_CONFIG + path) else: - config_data, _ = self._vyconf_session.show_config() + config_data, _ = self._vyconf_session.show_config(path) if format == 'raw': return config_data -- cgit v1.2.3 From 67f8a713c862d200a4c69b18fed81cadeee8c4bf Mon Sep 17 00:00:00 2001 From: John Estabrook Date: Wed, 2 Jul 2025 13:39:26 -0500 Subject: T7588: detach commit-confirm-notify from calling process As we now await the call to commit-confirm, do not run commit-confirm-notify from a subshell. --- python/vyos/config_mgmt.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/python/vyos/config_mgmt.py b/python/vyos/config_mgmt.py index 67d03e76c..51c6f2241 100644 --- a/python/vyos/config_mgmt.py +++ b/python/vyos/config_mgmt.py @@ -25,10 +25,12 @@ from filecmp import cmp from datetime import datetime from textwrap import dedent from pathlib import Path -from tabulate import tabulate from shutil import copy, chown +from subprocess import Popen +from subprocess import DEVNULL from urllib.parse import urlsplit from urllib.parse import urlunsplit +from tabulate import tabulate from vyos.config import Config from vyos.configtree import ConfigTree @@ -231,7 +233,14 @@ Proceed ?""" else: cmd = f'sudo -b /usr/libexec/vyos/commit-confirm-notify.py {minutes}' - os.system(cmd) + Popen( + cmd.split(), + stdout=DEVNULL, + stderr=DEVNULL, + stdin=DEVNULL, + close_fds=True, + preexec_fn=os.setsid, + ) if self.reboot_unconfirmed: msg = f'Initialized commit-confirm; {minutes} minutes to confirm before reboot' -- cgit v1.2.3