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.

1 files changed, 67 insertions, 3 deletions

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')