From 7547b699c1afdf3e37f288eb8805b8fb559e5105 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Sun, 21 Mar 2021 13:52:25 +0100
Subject: ospf: vrf: T2271: ease FRR interaction for config reload

Instead of multiple if/else paths, use a common vrf string variable which is
either populated or not. In addtion when interfaces are configured for a given
VRF, harden the regex for config reload.
---
 smoketest/scripts/cli/test_protocols_ospf.py | 25 ++++++++++++-------------
 src/conf_mode/protocols_ospf.py              | 20 ++++++++++----------
 2 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/smoketest/scripts/cli/test_protocols_ospf.py b/smoketest/scripts/cli/test_protocols_ospf.py
index 385f50918..8d94c86cb 100755
--- a/smoketest/scripts/cli/test_protocols_ospf.py
+++ b/smoketest/scripts/cli/test_protocols_ospf.py
@@ -288,16 +288,16 @@ class TestProtocolsOSPF(VyOSUnitTestSHIM.TestCase):
 
 
     def test_ospf_11_vrfs(self):
-        vrfs = ['red', 'green', 'blue']
         # It is safe to assume that when the basic VRF test works, all
         # other OSPF related features work, as we entirely inherit the CLI
         # templates and Jinja2 FRR template.
         table = '1000'
-        for vrf in vrfs:
-            vrf_base = ['vrf', 'name', vrf]
-            self.cli_set(vrf_base + ['table', table])
-            self.cli_set(vrf_base + ['protocols', 'ospf'])
-            table = str(int(table) + 1000)
+        vrf = 'blue'
+        vrf_base = ['vrf', 'name', vrf]
+        vrf_iface = 'eth1'
+        self.cli_set(vrf_base + ['table', table])
+        self.cli_set(vrf_base + ['protocols', 'ospf', 'interface', vrf_iface])
+        self.cli_set(['interfaces', 'ethernet', vrf_iface, 'vrf', vrf])
 
         # Also set a default VRF OSPF config
         self.cli_set(base_path)
@@ -309,14 +309,13 @@ class TestProtocolsOSPF(VyOSUnitTestSHIM.TestCase):
         self.assertIn(f' auto-cost reference-bandwidth 100', frrconfig)
         self.assertIn(f' timers throttle spf 200 1000 10000', frrconfig) # defaults
 
-        for vrf in vrfs:
-            frrconfig = self.getFRRconfig(f'router ospf vrf {vrf}')
-            self.assertIn(f'router ospf vrf {vrf}', frrconfig)
-            self.assertIn(f' auto-cost reference-bandwidth 100', frrconfig)
-            self.assertIn(f' timers throttle spf 200 1000 10000', frrconfig) # defaults
-
-            self.cli_delete(['vrf', 'name', vrf])
+        frrconfig = self.getFRRconfig(f'router ospf vrf {vrf}')
+        self.assertIn(f'router ospf vrf {vrf}', frrconfig)
+        self.assertIn(f' auto-cost reference-bandwidth 100', frrconfig)
+        self.assertIn(f' timers throttle spf 200 1000 10000', frrconfig) # defaults
 
+        self.cli_delete(['vrf', 'name', vrf])
+        self.cli_delete(['interfaces', 'ethernet', vrf_iface, 'vrf'])
 
 if __name__ == '__main__':
     unittest.main(verbosity=2)
diff --git a/src/conf_mode/protocols_ospf.py b/src/conf_mode/protocols_ospf.py
index 08347aa52..a655eaeca 100755
--- a/src/conf_mode/protocols_ospf.py
+++ b/src/conf_mode/protocols_ospf.py
@@ -154,11 +154,10 @@ def verify(ospf):
                                   f'concurrently for {interface}!')
 
             if 'vrf' in ospf:
-                # If interface specific options are set, we must ensure that
-                # the interface is bound to our requesting VRF. Due to the VyOS
-                # priorities the interface is bound to the VRF after creation
-                # of the VRF itself, and before any routing protocol is
-                # configured.
+            # If interface specific options are set, we must ensure that the
+            # interface is bound to our requesting VRF. Due to the VyOS
+            # priorities the interface is bound to the VRF after creation of
+            # the VRF itself, and before any routing protocol is configured.
                 vrf = ospf['vrf']
                 tmp = get_interface_config(interface)
                 if 'master' not in tmp or tmp['master'] != vrf:
@@ -179,17 +178,18 @@ def apply(ospf):
     frr_cfg = frr.FRRConfig()
     frr_cfg.load_configuration(frr_daemon)
 
+    # Generate empty helper string which can be ammended to FRR commands,
+    # it will be either empty (default VRF) or contain the "vrf <name" statement
+    vrf = ''
     if 'vrf' in ospf:
-        vrf = ospf['vrf']
-        frr_cfg.modify_section(f'^router ospf vrf {vrf}$', '')
-    else:
-        frr_cfg.modify_section('^router ospf$', '')
+        vrf = ' vrf ' + ospf['vrf']
 
+    frr_cfg.modify_section(f'^router ospf{vrf}$', '')
     for key in ['interface', 'interface_removed']:
         if key not in ospf:
             continue
         for interface in ospf[key]:
-            frr_cfg.modify_section(f'^interface {interface}$', '')
+            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.commit_configuration(frr_daemon)
-- 
cgit v1.2.3