Merge pull request #4797 from c-po/frrender-vrf-dhcp

frrender: T7927: de-nest DHCP and PPPoE interface section for VRFs

3 files changed, 99 insertions, 41 deletions

diff --git a/python/vyos/configdict.py b/python/vyos/configdict.py
index d91d88d88..ea244247c 100644
--- a/python/vyos/configdict.py
+++ b/python/vyos/configdict.py
@@ -355,47 +355,48 @@ def is_source_interface(conf, interface, intftype=None):
 
 def get_dhcp_interfaces(conf, vrf=None):
     """ Common helper functions to retrieve all interfaces from current CLI
-    sessions that have DHCP configured. """
+    sessions that have DHCP configured. Only DHCP interfaces in the defined VRF
+    will be returned. If vrf is None - the default VRF interfaces will be
+    returned """
+
     dhcp_interfaces = {}
-    dict = conf.get_config_dict(['interfaces'], get_first_key=True)
-    if not dict:
+    interface_dict = conf.get_config_dict(['interfaces'], get_first_key=True)
+    if not interface_dict:
         return dhcp_interfaces
 
-    def check_dhcp(config):
-        ifname = config['ifname']
+    def check_dhcp(if_config: dict, vrf=None) -> dict:
+        ifname = if_config['ifname']
         tmp = {}
-        if 'address' in config and 'dhcp' in config['address']:
+        if 'address' in if_config and 'dhcp' in if_config['address']:
             options = {}
-            if dict_search('dhcp_options.default_route_distance', config) != None:
-                options.update({'dhcp_options' : config['dhcp_options']})
-            if 'vrf' in config:
-                if vrf == config['vrf']: tmp.update({ifname : options})
+            if dict_search('dhcp_options.default_route_distance', if_config) != None:
+                options.update({'dhcp_options' : if_config['dhcp_options']})
+            if 'vrf' in if_config:
+                if vrf == if_config['vrf']: tmp.update({ifname : options})
             else:
                 if vrf is None: tmp.update({ifname : options})
 
         return tmp
 
-    for section, interface in dict.items():
+    for section, interface in interface_dict.items():
         for ifname in interface:
-            # always reset config level, as get_interface_dict() will alter it
-            conf.set_level([])
             # we already have a dict representation of the config from get_config_dict(),
             # but with the extended information from get_interface_dict() we also
             # get the DHCP client default-route-distance default option if not specified.
             _, ifconfig = get_interface_dict(conf, ['interfaces', section], ifname)
 
-            tmp = check_dhcp(ifconfig)
+            tmp = check_dhcp(ifconfig, vrf=vrf)
             dhcp_interfaces.update(tmp)
             # check per VLAN interfaces
             for vif, vif_config in ifconfig.get('vif', {}).items():
-                tmp = check_dhcp(vif_config)
+                tmp = check_dhcp(vif_config, vrf=vrf)
                 dhcp_interfaces.update(tmp)
             # check QinQ VLAN interfaces
             for vif_s, vif_s_config in ifconfig.get('vif_s', {}).items():
-                tmp = check_dhcp(vif_s_config)
+                tmp = check_dhcp(vif_s_config, vrf=vrf)
                 dhcp_interfaces.update(tmp)
                 for vif_c, vif_c_config in vif_s_config.get('vif_c', {}).items():
-                    tmp = check_dhcp(vif_c_config)
+                    tmp = check_dhcp(vif_c_config, vrf=vrf)
                     dhcp_interfaces.update(tmp)
 
     return dhcp_interfaces
@@ -404,7 +405,7 @@ def get_pppoe_interfaces(conf, vrf=None):
     """ Common helper functions to retrieve all interfaces from current CLI
     sessions that have DHCP configured. """
     pppoe_interfaces = {}
-    conf.set_level([])
+    conf.set_level([]) # required for list_nodes()
     for ifname in conf.list_nodes(['interfaces', 'pppoe']):
         # always reset config level, as get_interface_dict() will alter it
         conf.set_level([])
diff --git a/python/vyos/frrender.py b/python/vyos/frrender.py
index 75824fcf9..decec8aa2 100644
--- a/python/vyos/frrender.py
+++ b/python/vyos/frrender.py
@@ -14,15 +14,25 @@
 # along with this library.  If not, see <http://www.gnu.org/licenses/>.
 
 """
-Library used to interface with FRRs mgmtd introduced in version 10.0
+Helper class attached to vyos-configd to interfact between our CLI configuration
+and FRR. Class will render one full FRR configuration and apply this via
+frr-reload.py, if the configuration has no errors.
+
+Will fail early if the rendered configuration has any errors.
 """
 
 import os
 
+from copy import deepcopy
 from time import sleep
 
+from vyos.config import Config
+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.utils.dict import dict_search
+from vyos.utils.dict import dict_set_nested
 from vyos.utils.file import write_file
 from vyos.utils.process import cmd
 from vyos.utils.process import rc_cmd
@@ -58,12 +68,7 @@ ripng_daemon = 'ripngd'
 zebra_daemon = 'zebra'
 nhrp_daemon = 'nhrpd'
 
-def get_frrender_dict(conf, argv=None) -> dict:
-    from copy import deepcopy
-    from vyos.config import config_dict_merge
-    from vyos.configdict import get_dhcp_interfaces
-    from vyos.configdict import get_pppoe_interfaces
-
+def get_frrender_dict(conf: Config, argv=None) -> dict:
     # We need to re-set the CLI path to the root level, as this function uses
     # conf.exists() with an absolute path form the CLI root
     conf.set_level([])
@@ -435,17 +440,10 @@ def get_frrender_dict(conf, argv=None) -> dict:
 
     # T3680 - get a list of all interfaces currently configured to use DHCP
     tmp = get_dhcp_interfaces(conf)
-    if tmp:
-        if 'static' in dict:
-            dict['static'].update({'dhcp' : tmp})
-        else:
-            dict.update({'static' : {'dhcp' : tmp}})
+    if tmp: dict_set_nested('static.dhcp', tmp, dict)
+
     tmp = get_pppoe_interfaces(conf)
-    if tmp:
-        if 'static' in dict:
-            dict['static'].update({'pppoe' : tmp})
-        else:
-            dict.update({'static' : {'pppoe' : tmp}})
+    if tmp: dict_set_nested('static.pppoe', tmp, dict)
 
     # keep a re-usable list of dependent VRFs
     dependent_vrfs_default = {}
@@ -573,16 +571,17 @@ def get_frrender_dict(conf, argv=None) -> dict:
                 static = conf.get_config_dict(static_vrf_path, key_mangling=('-', '_'),
                                               get_first_key=True,
                                               no_tag_node_value_mangle=True)
-                # T3680 - get a list of all interfaces currently configured to use DHCP
-                tmp = get_dhcp_interfaces(conf, vrf_name)
-                if tmp: static.update({'dhcp' : tmp})
-                tmp = get_pppoe_interfaces(conf, vrf_name)
-                if tmp: static.update({'pppoe' : tmp})
-
                 vrf['name'][vrf_name]['protocols'].update({'static': static})
             elif conf.exists_effective(static_vrf_path):
                 vrf['name'][vrf_name]['protocols'].update({'static': {'deleted' : ''}})
 
+            # T3680 - get a list of all interfaces currently configured to use DHCP
+            tmp = get_dhcp_interfaces(conf, vrf_name)
+            if tmp: dict_set_nested(f'name.{vrf_name}.protocols.static.dhcp', tmp, vrf)
+
+            tmp = get_pppoe_interfaces(conf, vrf_name)
+            if tmp: dict_set_nested(f'name.{vrf_name}.protocols.static.pppoe', tmp, vrf)
+
             vrf_vni_path = ['vrf', 'name', vrf_name, 'vni']
             if conf.exists(vrf_vni_path):
                 vrf_config.update({'vni': conf.return_value(vrf_vni_path)})
diff --git a/smoketest/scripts/cli/test_vrf.py b/smoketest/scripts/cli/test_vrf.py
index a69068c6e..3a3f0e5b9 100755
--- a/smoketest/scripts/cli/test_vrf.py
+++ b/smoketest/scripts/cli/test_vrf.py
@@ -20,6 +20,7 @@ import unittest
 
 from json import loads
 from jmespath import search
+from time import sleep
 
 from base_vyostest_shim import VyOSUnitTestSHIM
 
@@ -35,6 +36,7 @@ from vyos.utils.process import cmd
 from vyos.utils.system import sysctl_read
 from vyos.template import inc_ip
 from vyos.utils.process import process_named_running
+from vyos.xml_ref import default_value
 
 base_path = ['vrf']
 vrfs = ['red', 'green', 'blue', 'foo-bar', 'baz_foo']
@@ -773,6 +775,62 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
         self.cli_delete(base)
         self.cli_commit()
 
+    def test_dhcp_vrf_default_route(self):
+        # T7927 - when retrieving a default route via DHCP, check that additional
+        # calls into FRRender() keep the DHCP route in place
+
+        vrf_name = 'red15'
+        default_gateway = '192.0.2.1'
+        dhcp_if_server = 'veth0'
+        dhcp_if_client = 'veth1'
+
+        default_distance = default_value(['interfaces', 'virtual-ethernet',
+                                          dhcp_if_client, 'dhcp-options',
+                                          'default-route-distance'])
+
+        dhcp_pool_base = ['service', 'dhcp-server', 'shared-network-name',
+                          'FOO-4', 'subnet', '192.0.2.0/24']
+        veth_base = ['interfaces', 'virtual-ethernet']
+
+        # Start DHCP Server in VRF connected via veth pair to default VRF
+        self.cli_set(['vrf', 'name', vrf_name, 'table', '48752'])
+        self.cli_set(veth_base + [dhcp_if_server, 'address', '192.0.2.1/24'])
+        self.cli_set(veth_base + [dhcp_if_server, 'peer-name', dhcp_if_client])
+        self.cli_set(veth_base + [dhcp_if_client, 'peer-name', dhcp_if_server])
+        self.cli_set(veth_base + [dhcp_if_client, 'vrf', vrf_name])
+
+        self.cli_set(['service', 'dhcp-server', 'listen-interface', dhcp_if_server])
+        self.cli_set(dhcp_pool_base + ['option', 'default-router', default_gateway])
+        self.cli_set(dhcp_pool_base + ['range', 'uno', 'start', '192.0.2.10'])
+        self.cli_set(dhcp_pool_base + ['range', 'uno', 'stop', '192.0.2.30'])
+        self.cli_set(dhcp_pool_base + ['subnet-id', '1'])
+
+        self.cli_commit()
+
+        # Start DHCP client in VRF
+        self.cli_set(['interfaces', 'virtual-ethernet', dhcp_if_client, 'address', 'dhcp'])
+        self.cli_commit()
+
+        # We need to wait until DHCP client has started and an IP address has been received
+        sleep(8)
+
+        frrconfig = self.getFRRconfig('^vrf red', stop_section='^exit-vrf')
+        self.assertIn(f' ip route 0.0.0.0/0 {default_gateway} {dhcp_if_client} '\
+                      f'tag 210 {default_distance}', frrconfig)
+
+        # Change anything in FRR to re-trigger config generation. DHCP route
+        # must still be present
+        self.cli_set(['protocols', 'static', 'route', '10.0.0.0/24', 'blackhole'])
+        self.cli_commit()
+
+        frrconfig = self.getFRRconfig('^vrf red', stop_section='^exit-vrf')
+        self.assertIn(f' ip route 0.0.0.0/0 {default_gateway} {dhcp_if_client} '\
+                      f'tag 210 {default_distance}', frrconfig)
+
+        self.cli_delete(['interfaces', 'virtual-ethernet'])
+        self.cli_delete(['service', 'dhcp-server'])
+        self.cli_delete(['vrf', 'name', vrf_name])
+
     def test_dhcpv6_single_pool(self):
         # Prepare the vrf and other options
         table = '100'