vrf: bgp: T3523: bugfix Kernel route-map deployment

Commit 4f9aa30f ("vrf: bgp: T3523: add route-map support for kernel routes")
added the possibility to also filter BGP routes towards the OS kernel, but the
smoketests failed. Reason was a non working CLI command applied to bgpd.

Thus the VRF route-map and the BGP configuration is now split into two templates,
one to be used for each daemon (zebra and bgpd).

Nevertheless one more bug was found in vyos.frr which currently does not suppoort
calling modify_section() inside a configuration "block". See [1] for more info.

[1]: https://phabricator.vyos.net/T3529

8 files changed, 58 insertions, 44 deletions

diff --git a/data/templates/frr/bgp.frr.tmpl b/data/templates/frr/bgpd.frr.tmpl
index 0245c875e..57b9ba8d6 100644
--- a/data/templates/frr/bgp.frr.tmpl
+++ b/data/templates/frr/bgpd.frr.tmpl
@@ -214,15 +214,6 @@
 {%   endif %}
 {% endmacro %}
 !
-{% if vrf is defined and vrf is not none and route_map is defined and route_map is not none %}
-vrf {{ vrf }}
- ip protocol bgp route-map {{ route_map }}
- exit-vrf
-!
-{% elif route_map is defined and route_map is not none %}
-ip protocol bgp route-map {{ route_map }}
-{% endif %}
-!
 router bgp {{ local_as }} {{ 'vrf ' ~ vrf if vrf is defined and vrf is not none }}
 {% if parameters is defined and parameters.ebgp_requires_policy is defined %}
  bgp ebgp-requires-policy
diff --git a/data/templates/frr/isis.frr.tmpl b/data/templates/frr/isis.frr.tmpl
index c74ac3101..433f10892 100644
--- a/data/templates/frr/isis.frr.tmpl
+++ b/data/templates/frr/isis.frr.tmpl
@@ -1,13 +1,4 @@
 !
-{% if vrf is defined and vrf is not none and route_map is defined and route_map is not none %}
-vrf {{ vrf }}
- ip protocol isis route-map {{ route_map }}
- exit-vrf
-!
-{% elif route_map is defined and route_map is not none %}
-ip protocol isis route-map {{ route_map }}
-{% endif %}
-!
 router isis VyOS {{ 'vrf ' + vrf if vrf is defined and vrf is not none }}
  net {{ net }}
 {% if dynamic_hostname is defined %}
diff --git a/data/templates/frr/ospf.frr.tmpl b/data/templates/frr/ospf.frr.tmpl
index 6283ad4e5..36aa699a9 100644
--- a/data/templates/frr/ospf.frr.tmpl
+++ b/data/templates/frr/ospf.frr.tmpl
@@ -50,15 +50,6 @@ interface {{ iface }} {{ 'vrf ' + vrf if vrf is defined and vrf is not none }}
 {%   endfor %}
 {% endif %}
 !
-{% if vrf is defined and vrf is not none and route_map is defined and route_map is not none %}
-vrf {{ vrf }}
- ip protocol ospf route-map {{ route_map }}
- exit-vrf
-!
-{% elif route_map is defined and route_map is not none %}
-ip protocol ospf route-map {{ route_map }}
-{% endif %}
-!
 router ospf {{ 'vrf ' + vrf if vrf is defined and vrf is not none }}
 {% if access_list is defined and access_list is not none %}
 {%   for acl, acl_config in access_list.items() %}
diff --git a/data/templates/frr/vrf.route-map.frr.tmpl b/data/templates/frr/vrf.route-map.frr.tmpl
new file mode 100644
index 000000000..cb0e07616
--- /dev/null
+++ b/data/templates/frr/vrf.route-map.frr.tmpl
@@ -0,0 +1,10 @@
+!
+{% if vrf is defined and vrf is not none and route_map is defined and route_map is not none %}
+vrf {{ vrf }}
+ ip protocol {{ protocol }} route-map {{ route_map }}
+ exit-vrf
+!
+{% elif route_map is defined and route_map is not none %}
+ip protocol {{ protocol }} route-map {{ route_map }}
+{% endif %}
+!
diff --git a/smoketest/scripts/cli/test_protocols_bgp.py b/smoketest/scripts/cli/test_protocols_bgp.py
index 08697eebf..0ed66657c 100755
--- a/smoketest/scripts/cli/test_protocols_bgp.py
+++ b/smoketest/scripts/cli/test_protocols_bgp.py
@@ -144,6 +144,7 @@ class TestProtocolsBGP(VyOSUnitTestSHIM.TestCase):
 
     def tearDown(self):
         self.cli_delete(['policy'])
+        self.cli_delete(['vrf'])
         self.cli_delete(base_path)
         self.cli_commit()
 
@@ -624,6 +625,7 @@ class TestProtocolsBGP(VyOSUnitTestSHIM.TestCase):
             self.cli_set(vrf_base + ['table', table])
             self.cli_set(vrf_base + ['protocols', 'bgp', 'local-as', ASN])
             self.cli_set(vrf_base + ['protocols', 'bgp', 'parameters', 'router-id', router_id])
+            self.cli_set(vrf_base + ['protocols', 'bgp', 'route-map', route_map_in])
             table = str(int(table) + 1000)
 
         self.cli_commit()
@@ -631,9 +633,16 @@ class TestProtocolsBGP(VyOSUnitTestSHIM.TestCase):
         for vrf in vrfs:
             # Verify FRR bgpd configuration
             frrconfig = self.getFRRconfig(f'router bgp {ASN} vrf {vrf}')
-
             self.assertIn(f'router bgp {ASN} vrf {vrf}', frrconfig)
             self.assertIn(f' bgp router-id {router_id}', frrconfig)
 
+            # CCC: Currently this is not working as FRR() class does not support
+            # route-maps for multiple vrfs because the modify_section() only works
+            # on lines and not text blocks.
+            #
+            # vrfconfig = self.getFRRconfig(f'vrf {vrf}')
+            # zebra_route_map = f' ip protocol bgp route-map {route_map_in}'
+            # self.assertIn(zebra_route_map, vrfconfig)
+
 if __name__ == '__main__':
     unittest.main(verbosity=2)
\ No newline at end of file
diff --git a/src/conf_mode/protocols_bgp.py b/src/conf_mode/protocols_bgp.py
index 2b0cb1fe2..a51fe6d72 100755
--- a/src/conf_mode/protocols_bgp.py
+++ b/src/conf_mode/protocols_bgp.py
@@ -233,10 +233,14 @@ def verify(bgp):
 
 def generate(bgp):
     if not bgp or 'deleted' in bgp:
-        bgp['new_frr_config'] = ''
+        bgp['frr_bgpd_config'] = ''
+        bgp['frr_zebra_config'] = ''
         return None
 
-    bgp['new_frr_config'] = render_to_string('frr/bgp.frr.tmpl', bgp)
+    bgp['protocol'] = 'bgp' # required for frr/vrf.route-map.frr.tmpl
+    bgp['frr_zebra_config'] = render_to_string('frr/vrf.route-map.frr.tmpl', bgp)
+    bgp['frr_bgpd_config']  = render_to_string('frr/bgpd.frr.tmpl', bgp)
+
     return None
 
 def apply(bgp):
@@ -248,7 +252,8 @@ def apply(bgp):
 
     # The route-map used for the FIB (zebra) is part of the zebra daemon
     frr_cfg.load_configuration(zebra_daemon)
-    frr_cfg.modify_section(r'\s+ip protocol bgp route-map [-a-zA-Z0-9.]+$', '', ' ')
+    frr_cfg.modify_section(r'(\s+)?ip protocol bgp route-map [-a-zA-Z0-9.]+$', '', '(\s|!)')
+    frr_cfg.add_before(r'(ip prefix-list .*|route-map .*|line vty)', bgp['frr_zebra_config'])
     frr_cfg.commit_configuration(zebra_daemon)
 
     # Generate empty helper string which can be ammended to FRR commands, it
@@ -259,14 +264,17 @@ def apply(bgp):
 
     frr_cfg.load_configuration(bgp_daemon)
     frr_cfg.modify_section(f'^router bgp \d+{vrf}$', '')
-    frr_cfg.add_before(r'(ip prefix-list .*|route-map .*|line vty)', bgp['new_frr_config'])
+    frr_cfg.add_before(r'(ip prefix-list .*|route-map .*|line vty)', bgp['frr_bgpd_config'])
     frr_cfg.commit_configuration(bgp_daemon)
 
-    # If FRR config is blank, rerun the blank commit x times due to frr-reload
+    # If FRR config is blank, re-run the blank commit x times due to frr-reload
     # behavior/bug not properly clearing out on one commit.
-    if bgp['new_frr_config'] == '':
+    if bgp['frr_bgpd_config'] == '':
         for a in range(5):
             frr_cfg.commit_configuration(bgp_daemon)
+    if bgp['frr_zebra_config'] == '':
+        for a in range(5):
+            frr_cfg.commit_configuration(zebra_daemon)
 
     # Save configuration to /run/frr/config/frr.conf
     frr.save_configuration()
diff --git a/src/conf_mode/protocols_isis.py b/src/conf_mode/protocols_isis.py
index e3f57c6ba..ef21e0055 100755
--- a/src/conf_mode/protocols_isis.py
+++ b/src/conf_mode/protocols_isis.py
@@ -190,10 +190,13 @@ def verify(isis):
 
 def generate(isis):
     if not isis or 'deleted' in isis:
-        isis['new_frr_config'] = ''
+        isis['frr_isisd_config'] = ''
+        isis['frr_zebra_config'] = ''
         return None
 
-    isis['new_frr_config'] = render_to_string('frr/isis.frr.tmpl', isis)
+    isis['protocol'] = 'isis' # required for frr/vrf.route-map.frr.tmpl
+    isis['frr_zebra_config'] = render_to_string('frr/vrf.route-map.frr.tmpl', isis)
+    isis['frr_isisd_config'] = render_to_string('frr/isis.frr.tmpl', isis)
     return None
 
 def apply(isis):
@@ -205,7 +208,8 @@ def apply(isis):
 
     # The route-map used for the FIB (zebra) is part of the zebra daemon
     frr_cfg.load_configuration(zebra_daemon)
-    frr_cfg.modify_section(r'\s+ip protocol isis route-map [-a-zA-Z0-9.]+$', '', ' ')
+    frr_cfg.modify_section(r'(\s+)?ip protocol isis route-map [-a-zA-Z0-9.]+$', '', '(\s|!)')
+    frr_cfg.add_before(r'(ip prefix-list .*|route-map .*|line vty)', isis['frr_zebra_config'])
     frr_cfg.commit_configuration(zebra_daemon)
 
     # Generate empty helper string which can be ammended to FRR commands, it
@@ -223,14 +227,17 @@ def apply(isis):
         for interface in isis[key]:
             frr_cfg.modify_section(f'^interface {interface}{vrf}$', '')
 
-    frr_cfg.add_before(r'(ip prefix-list .*|route-map .*|line vty)', isis['new_frr_config'])
+    frr_cfg.add_before(r'(ip prefix-list .*|route-map .*|line vty)', isis['frr_isisd_config'])
     frr_cfg.commit_configuration(isis_daemon)
 
     # If FRR config is blank, rerun the blank commit x times due to frr-reload
     # behavior/bug not properly clearing out on one commit.
-    if isis['new_frr_config'] == '':
+    if isis['frr_isisd_config'] == '':
         for a in range(5):
             frr_cfg.commit_configuration(isis_daemon)
+    if isis['frr_zebra_config'] == '':
+        for a in range(5):
+            frr_cfg.commit_configuration(zebra_daemon)
 
     # Save configuration to /run/frr/config/frr.conf
     frr.save_configuration()
diff --git a/src/conf_mode/protocols_ospf.py b/src/conf_mode/protocols_ospf.py
index b5fef0c64..21eb8e447 100755
--- a/src/conf_mode/protocols_ospf.py
+++ b/src/conf_mode/protocols_ospf.py
@@ -171,10 +171,13 @@ def verify(ospf):
 
 def generate(ospf):
     if not ospf or 'deleted' in ospf:
-        ospf['new_frr_config'] = ''
+        ospf['frr_ospfd_config'] = ''
+        ospf['frr_zebra_config'] = ''
         return None
 
-    ospf['new_frr_config'] = render_to_string('frr/ospf.frr.tmpl', ospf)
+    ospf['protocol'] = 'ospf' # required for frr/vrf.route-map.frr.tmpl
+    ospf['frr_zebra_config'] = render_to_string('frr/vrf.route-map.frr.tmpl', ospf)
+    ospf['frr_ospfd_config']   = render_to_string('frr/ospf.frr.tmpl', ospf)
     return None
 
 def apply(ospf):
@@ -186,7 +189,8 @@ def apply(ospf):
 
     # The route-map used for the FIB (zebra) is part of the zebra daemon
     frr_cfg.load_configuration(zebra_daemon)
-    frr_cfg.modify_section(r'\s+ip protocol ospf route-map [-a-zA-Z0-9.]+$', '', ' ')
+    frr_cfg.modify_section(r'(\s+)?ip protocol ospf route-map [-a-zA-Z0-9.]+$', '', '(\s|!)')
+    frr_cfg.add_before(r'(ip prefix-list .*|route-map .*|line vty)', ospf['frr_zebra_config'])
     frr_cfg.commit_configuration(zebra_daemon)
 
     # Generate empty helper string which can be ammended to FRR commands, it
@@ -204,14 +208,17 @@ def apply(ospf):
         for interface in ospf[key]:
             frr_cfg.modify_section(f'^interface {interface}{vrf}$', '')
 
-    frr_cfg.add_before(r'(ip prefix-list .*|route-map .*|line vty)', ospf['new_frr_config'])
+    frr_cfg.add_before(r'(ip prefix-list .*|route-map .*|line vty)', ospf['frr_ospfd_config'])
     frr_cfg.commit_configuration(ospf_daemon)
 
     # If FRR config is blank, rerun the blank commit x times due to frr-reload
     # behavior/bug not properly clearing out on one commit.
-    if ospf['new_frr_config'] == '':
+    if ospf['frr_ospfd_config'] == '':
         for a in range(5):
             frr_cfg.commit_configuration(ospf_daemon)
+    if ospf['frr_zebra_config'] == '':
+        for a in range(5):
+            frr_cfg.commit_configuration(zebra_daemon)
 
     # Save configuration to /run/frr/config/frr.conf
     frr.save_configuration()