Merge pull request #5446 from c-po/dhcp-static-route-flakiness

static: T9278: reconcile FRR config after every DHCP lease event

3 files changed, 61 insertions, 3 deletions

diff --git a/python/vyos/defaults.py b/python/vyos/defaults.py
index 854f95ff0..23243346d 100644
--- a/python/vyos/defaults.py
+++ b/python/vyos/defaults.py
@@ -64,6 +64,7 @@ config_files = {
 config_status = '/tmp/vyos-config-status'
 api_config_state = '/run/http-api-state'
 frr_debug_enable = '/tmp/vyos.frr.debug'
+dhclient_debug_enable = '/tmp/vyos.dhclient.debug'
 static_route_dhcp_interfaces_path = '/tmp/static_dhcp_interfaces'
 vyos_configd_socket_path = 'ipc:///run/vyos-configd.sock'
 
diff --git a/python/vyos/frrender.py b/python/vyos/frrender.py
index 266da3d3d..1ea49abe4 100644
--- a/python/vyos/frrender.py
+++ b/python/vyos/frrender.py
@@ -31,10 +31,8 @@ from vyos.config import config_dict_merge
 from vyos.configdict import get_dhcp_interfaces
 from vyos.configdict import get_pppoe_interfaces
 from vyos.defaults import frr_debug_enable
-from vyos.defaults import static_route_dhcp_interfaces_path
 from vyos.utils.dict import dict_search
 from vyos.utils.dict import dict_set_nested
-from vyos.utils.file import read_file
 from vyos.utils.file import write_file
 from vyos.utils.process import cmdl
 from vyos.utils.process import rc_cmd
@@ -692,6 +690,52 @@ def get_frrender_dict(conf: Config, argv=None) -> dict:
 
     return dict
 
+def get_dhcp_route_interfaces(config_dict) -> set:
+    """Collect all interfaces whose static route configuration is derived from a
+    DHCP lease. Two CLI constructs end up reading the DHCP lease file via the
+    "get_dhcp_router" Jinja filter and thus depend on the current lease:
+
+    - "protocols static route <prefix> dhcp-interface <ifname>", either directly
+      or below "protocols static table <id>"
+    - the default route implied by "interfaces <type> <ifname> address dhcp"
+
+    Both variants exist in the default VRF as well as inside any named VRF.
+    """
+    interfaces = set()
+
+    def _add_from_routes(route_conf):
+        if not isinstance(route_conf, dict):
+            return
+        for prefix_options in route_conf.values():
+            interfaces.update(prefix_options.get('dhcp_interface', []))
+
+    def _add_from_static(static_conf):
+        if not isinstance(static_conf, dict):
+            return
+        _add_from_routes(static_conf.get('route'))
+        # Routes in a dedicated routing table use the same "dhcp-interface"
+        # node, thus they depend on the DHCP lease in the very same way
+        for table_config in static_conf.get('table', {}).values():
+            if isinstance(table_config, dict):
+                _add_from_routes(table_config.get('route'))
+        for ifname, if_config in static_conf.get('dhcp', {}).items():
+            # An interface explicitly opting out of the default route does not
+            # contribute a route, thus a lease change is irrelevant for it
+            if dict_search('dhcp_options.no_default_route', if_config) != None:
+                continue
+            interfaces.add(ifname)
+
+    if not isinstance(config_dict, dict):
+        return interfaces
+
+    _add_from_static(config_dict.get('static'))
+    for vrf_name in dict_search('vrf.name', config_dict) or {}:
+        _add_from_static(
+            dict_search(f'vrf.name.{vrf_name}.protocols.static', config_dict)
+        )
+
+    return interfaces
+
 class FRRender:
     cached_config_dict = {}
     cached_dhcp_gateways = {}
@@ -707,9 +751,14 @@ class FRRender:
             tmp = type(config_dict)
             raise ValueError(f'Config must be of type "dict" and not "{tmp}"!')
 
+        # T8465: the rendered configuration embeds the current DHCP gateway,
+        # which changes independently of the CLI configuration. The interface
+        # list must be derived from the configuration itself - the DHCP hook
+        # list on disk is only written by protocols_static.py, which does not
+        # run on an interface-only commit.
         dhcp_gateways = {
             interface: get_dhcp_router(interface)
-            for interface in read_file(static_route_dhcp_interfaces_path, '').split()
+            for interface in get_dhcp_route_interfaces(config_dict)
         }
 
         if (
diff --git a/python/vyos/template.py b/python/vyos/template.py
index 83805a73c..f5efdb7ec 100755
--- a/python/vyos/template.py
+++ b/python/vyos/template.py
@@ -431,6 +431,14 @@ def get_dhcp_router(interface):
 
     Returns None if no router is found, returns the IP address as string if
     a router is found.
+
+    The file read here is not the dhclient lease database (dhclient_<if>.leases)
+    but the per event dump written by
+    /etc/dhcp/dhclient-exit-hooks.d/03-vyos-dhclient-hook. It is intentionally
+    not removed when the DHCP client is stopped - the RELEASE/STOP event
+    rewrites it with an empty "new_routers", which is the signal that no router
+    is available. The file name is keyed on the interface only, a VRF assignment
+    does not change it.
     """
     lease_file = directories['isc_dhclient_dir'] + f'/dhclient_{interface}.lease'
     if not os.path.exists(lease_file):