frrender: T9442: cache the FRR configuration only after it was applied

The configuration was stored as the cached one before FRR had reloaded
it. If the reload failed, or FRR was down, the next commit found the
configuration unchanged, skipped the reload and reported success while
FRR kept running the old configuration.

Keep the rendered configuration aside and make it the cached one only
after the reload succeeded. A render which does not reach the reload is
not cached either.

If the reload fails, FRR may run a part of the new configuration. The
cache is dropped then, so even a return to the previous configuration
reloads FRR. A reload that succeeds on the last try no longer raises an
error.

1 files changed, 27 insertions, 4 deletions

diff --git a/python/vyos/frrender.py b/python/vyos/frrender.py
index 6a8124325..e4b8dc1ab 100644
--- a/python/vyos/frrender.py
+++ b/python/vyos/frrender.py
@@ -788,10 +788,14 @@ def get_dhcp_route_interfaces(config_dict) -> set:
     return interfaces
 
 class FRRender:
+    # Configuration of the last successful apply(), see generate()
     cached_config_dict = {}
     cached_dhcp_gateways = {}
     def __init__(self):
         self._frr_conf = frr_config_file
+        # Configuration rendered by generate(), which apply() has not
+        # confirmed yet
+        self._pending = None
 
     def generate(self, config_dict) -> None:
         """
@@ -812,6 +816,9 @@ class FRRender:
             for interface in get_dhcp_route_interfaces(config_dict)
         }
 
+        # A render which did not reach apply() must not count as applied
+        self._pending = None
+
         if (
             self.cached_config_dict == config_dict
             and self.cached_dhcp_gateways == dhcp_gateways
@@ -819,9 +826,6 @@ class FRRender:
             debug('FRR:        NO CHANGES DETECTED')
             return False
 
-        self.cached_config_dict = config_dict
-        self.cached_dhcp_gateways = dhcp_gateways
-
         def inline_helper(config_dict) -> str:
             output = '!\n'
             if 'babel' in config_dict and 'deleted' not in config_dict['babel']:
@@ -950,6 +954,11 @@ class FRRender:
         debug(output)
         write_file(self._frr_conf, output)
         debug('FRR:        RENDERING CONFIG COMPLETE')
+
+        # Only apply() makes this the cached configuration. If FRR does not
+        # take it, the next commit must render and apply it again instead of
+        # reporting no changes (T9442).
+        self._pending = (config_dict, dhcp_gateways)
         return True
 
     def apply(self, count_max=5):
@@ -962,6 +971,7 @@ class FRRender:
 
         count = 0
         emsg = ''
+        reloaded = False
         while count < count_max:
             count += 1
             debug(f'FRR: reloading configuration - tries: {count} | Python class ID: {id(self)}')
@@ -974,9 +984,16 @@ class FRRender:
                 continue
             debug(emsg)
             debug('FRR: configuration reload complete')
+            reloaded = True
             break
 
-        if count >= count_max:
+        if not reloaded:
+            # frr-reload.py applies its changes through vtysh in several
+            # steps, FRR can be left with a part of the new configuration.
+            # Forget the cached one, so that the next commit reloads FRR even
+            # if it returns to the previously applied configuration.
+            self.cached_config_dict = None
+            self.cached_dhcp_gateways = {}
             raise ConfigError(emsg)
 
         write_file(frr_applied_config_file, read_file(self._frr_conf))
@@ -988,3 +1005,9 @@ class FRRender:
         # --daemon. Repeating it asks every daemon for its configuration once
         # more, and a bgpd busy with a policy walk then holds the commit until
         # the walk ends.
+
+        # FRR runs the rendered configuration now, remember it for the change
+        # detection of generate()
+        if self._pending is not None:
+            self.cached_config_dict, self.cached_dhcp_gateways = self._pending
+            self._pending = None