From 9a7d5b27cd908473b7cd9e16fc0d1bbe7e983d2e Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Fri, 4 Sep 2026 22:14:45 +0200 Subject: static: T9278: reconcile FRR config after every DHCP lease event The default route derived from "interfaces address dhcp" is rendered from the DHCP lease file, so it is only known at lease time. Until now the sole runtime path into staticd was the one-shot vtysh injection in dhclient-enter-hooks.d/03-vyos-ipwrapper. That injection races the FRR reload of the very commit which started the DHCP client: dhclient runs with "-nw", thus the commit does not wait for a lease, and when BOUND arrives mid-reload frr_alive() can report FRR as down. The route is then installed into the kernel only and FRR never learns about it. Nothing recovers afterwards, as FRRender.generate() short-circuits on an unchanged configuration dict. The self-healing re-render used by "protocols static route dhcp-interface" was gated on /tmp/static_dhcp_interfaces, which never lists plain "address dhcp" interfaces - protocols_static.py does not even run on an interface-only commit, as no config-mode dependency points to it. Derive the DHCP dependent interface list from the configuration dict itself, both for the default VRF and for every named VRF, and use it for FRR change detection. Drop the interface list gate in the dhclient exit hook so any lease event requests a re-render. Also poll for the lease and for the rendered route in the affected smoketests instead of relying on a fixed sleep. --- python/vyos/frrender.py | 44 ++++++++++++- python/vyos/template.py | 8 +++ smoketest/scripts/cli/test_protocols_static.py | 75 ++++++++++++++-------- src/conf_mode/protocols_static.py | 15 ++--- .../98-vyos-static-routes-dhclient-hook | 11 ++-- 5 files changed, 109 insertions(+), 44 deletions(-) diff --git a/python/vyos/frrender.py b/python/vyos/frrender.py index 266da3d3d..b3f2718ac 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,41 @@ 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 dhcp-interface " + - the default route implied by "interfaces address dhcp" + + Both variants exist in the default VRF as well as inside any named VRF. + """ + interfaces = set() + + def _add_from_static(static_conf): + if not isinstance(static_conf, dict): + return + for prefix_options in static_conf.get('route', {}).values(): + interfaces.update(prefix_options.get('dhcp_interface', [])) + 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 +740,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_.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): diff --git a/smoketest/scripts/cli/test_protocols_static.py b/smoketest/scripts/cli/test_protocols_static.py index a87675ad2..f18549059 100755 --- a/smoketest/scripts/cli/test_protocols_static.py +++ b/smoketest/scripts/cli/test_protocols_static.py @@ -202,6 +202,39 @@ class TestProtocolsStatic(VyOSUnitTestSHIM.TestCase): # always forward to base class super().tearDown() + def wait_for_dhcp_router(self, interface, timeout=30): + """The DHCP client is started in the background (dhclient -nw), thus a + commit returns before a lease was acquired. In addition, moving an + interface into a VRF releases and re-acquires the lease. Wait until the + DHCP hook reported a router for the given interface.""" + result, router = self.wait_for_result( + lambda: get_dhcp_router(interface), + lambda tmp: tmp is not None, + pause=1, + timeout=timeout, + ) + self.assertTrue( + result, f'No DHCP router received on interface "{interface}"' + ) + return router + + def assert_in_frrconfig(self, needle, timeout=30, **kwargs): + """Assert that needle shows up in the FRR configuration. A DHCP lease + event reconciles the FRR configuration asynchronously via the dhclient + hook, thus we can not check the configuration only once.""" + def check(): + frrconfig = self.getFRRconfig(**kwargs) + if needle in frrconfig: + return True + return frrconfig + + result, frrconfig = self.wait_for_result( + check, True, pause=1, timeout=timeout + ) + self.assertTrue( + result, f"Expected '{needle}' in FRR config:\n{frrconfig}" + ) + def test_01_static(self): self.cli_set(['vrf', 'name', 'black', 'table', '43210']) for route, route_config in routes.items(): @@ -605,12 +638,9 @@ class TestProtocolsStatic(VyOSUnitTestSHIM.TestCase): self.cli_set(interface_path + ['address', 'dhcp']) self.cli_commit() - # Wait for dhclient to receive IP address and default gateway - sleep(5) - - router = get_dhcp_router(interface) - frrconfig = self.getFRRconfig() - self.assertIn(rf'ip route 0.0.0.0/0 {router} {interface} tag 210 {default_distance}', frrconfig) + router = self.wait_for_dhcp_router(interface) + route_str = f'ip route 0.0.0.0/0 {router} {interface} tag 210 {default_distance}' + self.assert_in_frrconfig(route_str) # T6991: Default route is missing when there is no "protocols static" # CLI node entry @@ -621,8 +651,7 @@ class TestProtocolsStatic(VyOSUnitTestSHIM.TestCase): self.cli_commit() # Re-check FRR configuration that default route is still present - frrconfig = self.getFRRconfig() - self.assertIn(rf'ip route 0.0.0.0/0 {router} {interface} tag 210 {default_distance}', frrconfig) + self.assert_in_frrconfig(route_str) self.cli_delete(interface_path + ['address']) self.cli_commit() @@ -648,7 +677,10 @@ class TestProtocolsStatic(VyOSUnitTestSHIM.TestCase): self.cli_set(interface_path + ['vrf', vrf]) self.cli_commit() - router = get_dhcp_router(interface) + # Moving the interface into a VRF stops and restarts the DHCP client, + # thus the lease is released and re-acquired - we must not read the + # released (empty) lease information here + router = self.wait_for_dhcp_router(interface) route_str = ( rf'ip route 0.0.0.0/0 {router} {interface} tag 210 {default_distance}' ) @@ -660,7 +692,7 @@ class TestProtocolsStatic(VyOSUnitTestSHIM.TestCase): return frrconfig result, config = self.wait_for_result( - check_default_route, True, pause=1, timeout=10 + check_default_route, True, pause=1, timeout=30 ) # First clean interfaces from VRF so that VRF can be deleted @@ -695,8 +727,9 @@ class TestProtocolsStatic(VyOSUnitTestSHIM.TestCase): # Commit configuration self.cli_commit() - # Wait for dhclient to receive IP address - sleep(5) + # Wait for dhclient to receive an IP address and default gateway - the + # dhcp-interface routes are rendered from the lease + router = self.wait_for_dhcp_router(dhcp_interface) # Configure static routes with dhcp-interface dhcp_routes = { @@ -734,17 +767,11 @@ class TestProtocolsStatic(VyOSUnitTestSHIM.TestCase): f'Interface {dhcp_interface} should be in hook interface list', ) - # Get the DHCP router for verification - router = get_dhcp_router(dhcp_interface) - self.assertIsNotNone(router, 'DHCP router should be available') - # Verify FRR configuration contains the static routes with DHCP router - frrconfig = self.getFRRconfig('ip route', end_marker='') - for route in dhcp_routes.keys(): expected_route = f'ip route {route} {router} {dhcp_interface}' - self.assertIn(expected_route, frrconfig, f'Static route {route} '\ - 'with dhcp-interface should be in FRR config') + self.assert_in_frrconfig(expected_route, start_section='ip route', + end_marker='') # Test table-based routes with dhcp-interface table_id = '100' @@ -754,15 +781,11 @@ class TestProtocolsStatic(VyOSUnitTestSHIM.TestCase): self.cli_commit() # Verify table route in FRR config - frrconfig = self.getFRRconfig('ip route', end_marker='') expected_table_route = ( f'ip route {table_route} {router} {dhcp_interface} table {table_id}' ) - self.assertIn( - expected_table_route, - frrconfig, - f'Table static route {table_route} with dhcp-interface should be in FRR config', - ) + self.assert_in_frrconfig(expected_table_route, + start_section='ip route', end_marker='') # Clean up - remove DHCP configuration self.cli_delete(interface_path + ['address']) diff --git a/src/conf_mode/protocols_static.py b/src/conf_mode/protocols_static.py index 0f3f735a7..7d2b4949e 100755 --- a/src/conf_mode/protocols_static.py +++ b/src/conf_mode/protocols_static.py @@ -24,6 +24,7 @@ from vyos.configverify import has_frr_protocol_in_dict from vyos.configverify import verify_common_route_maps from vyos.configverify import verify_vrf from vyos.frrender import FRRender +from vyos.frrender import get_dhcp_route_interfaces from vyos.frrender import get_frrender_dict from vyos.utils.dict import dict_search from vyos.utils.file import write_file @@ -99,15 +100,11 @@ def generate(config_dict): static = vrf and dict_search(f'vrf.name.{vrf}.protocols.static', config_dict) or config_dict['static'] - # Collect interfaces that have DHCP configuration for DHCP hooks - dhcp_interfaces = set() - - # Check for DHCP interfaces in route configurations - if 'route' in static: - for prefix, prefix_options in static['route'].items(): - if 'dhcp_interface' in prefix_options: - for interface_name in prefix_options['dhcp_interface']: - dhcp_interfaces.add(interface_name) + # Collect interfaces that have DHCP configuration for DHCP hooks. This must + # be derived from the entire config_dict and not from the (possibly + # VRF-narrowed) static dict above, as DHCP_HOOK_IFLIST is a single global + # file - a per VRF invocation would otherwise clobber the list. + dhcp_interfaces = get_dhcp_route_interfaces(config_dict) # Write the interface list for DHCP hooks or clean up if empty if dhcp_interfaces: diff --git a/src/etc/dhcp/dhclient-exit-hooks.d/98-vyos-static-routes-dhclient-hook b/src/etc/dhcp/dhclient-exit-hooks.d/98-vyos-static-routes-dhclient-hook index 7038bf25e..4f207411e 100755 --- a/src/etc/dhcp/dhclient-exit-hooks.d/98-vyos-static-routes-dhclient-hook +++ b/src/etc/dhcp/dhclient-exit-hooks.d/98-vyos-static-routes-dhclient-hook @@ -14,12 +14,11 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -DHCP_HOOK_IFLIST="/tmp/static_dhcp_interfaces" - -# Only run if there are static routes with dhcp-interface configured -if ! { [ -f $DHCP_HOOK_IFLIST ] && grep -qw $interface $DHCP_HOOK_IFLIST; }; then - return 0 -fi +# The FRR static route configuration embeds the DHCP gateway of an interface, +# either for "protocols static route dhcp-interface " or for +# the default route implied by "interfaces address dhcp". The +# gateway is only known at lease time, so any lease event must reconcile the +# rendered configuration with the current lease (T8465). # Re-generate the config on the following events: # - BOUND: always re-generate -- cgit v1.2.3