vrf: T6189: render FRR L3VNI configuration when creating VRF instance

When adding and removing VRF instances on the fly it was noticed that the vni
statement under the VRF instance in FRR vanishes. This was caused by a race
condition which was previously designed to fix another bug.

The wierd design of a Python helper below the VRF tree to only generate the
VNI configuration nodes is now gone and all is rendered in the proper place.

6 files changed, 46 insertions, 141 deletions

diff --git a/data/configd-include.json b/data/configd-include.json
index dc00f0698..0c767f987 100644
--- a/data/configd-include.json
+++ b/data/configd-include.json
@@ -108,6 +108,5 @@
 "vpn_openconnect.py",
 "vpn_pptp.py",
 "vpn_sstp.py",
-"vrf.py",
-"vrf_vni.py"
+"vrf.py"
 ]
diff --git a/data/templates/frr/zebra.vrf.route-map.frr.j2 b/data/templates/frr/zebra.vrf.route-map.frr.j2
index f1cc6fe66..8ebb82511 100644
--- a/data/templates/frr/zebra.vrf.route-map.frr.j2
+++ b/data/templates/frr/zebra.vrf.route-map.frr.j2
@@ -1,10 +1,6 @@
 !
 {% if name is vyos_defined %}
 {%     for vrf, vrf_config in name.items() %}
-{#         code path required for vrf_vni.py as we will only render the required VR configuration and not all of them #}
-{%         if only_vrf is vyos_defined and vrf is not vyos_defined(only_vrf) %}
-{%             continue %}
-{%         endif %}
 vrf {{ vrf }}
 {%         if vrf_config.ip.nht.no_resolve_via_default is vyos_defined %}
  no ip nht resolve-via-default
@@ -25,7 +21,7 @@ vrf {{ vrf }}
  ipv6 protocol {{ protocol_name }} route-map {{ protocol_config.route_map }}
 {%             endfor %}
 {%         endif %}
-{%         if vrf_config.vni is vyos_defined and no_vni is not vyos_defined %}
+{%         if vrf_config.vni is vyos_defined %}
  vni {{ vrf_config.vni }}
 {%         endif %}
 exit-vrf
diff --git a/interface-definitions/vrf.xml.in b/interface-definitions/vrf.xml.in
index 94ed96e4b..a20be995a 100644
--- a/interface-definitions/vrf.xml.in
+++ b/interface-definitions/vrf.xml.in
@@ -120,20 +120,7 @@
               <constraintErrorMessage>VRF routing table must be in range from 100 to 65535</constraintErrorMessage>
             </properties>
           </leafNode>
-          <leafNode name="vni" owner="${vyos_conf_scripts_dir}/vrf_vni.py $VAR(../@)">
-            <properties>
-              <help>Virtual Network Identifier</help>
-              <!-- must be after BGP to keep correct order when removing L3VNIs in FRR -->
-              <priority>822</priority>
-              <valueHelp>
-                <format>u32:0-16777214</format>
-                <description>VXLAN virtual network identifier</description>
-              </valueHelp>
-              <constraint>
-                <validator name="numeric" argument="--range 0-16777214"/>
-              </constraint>
-            </properties>
-          </leafNode>
+          #include <include/vni.xml.i>
         </children>
       </tagNode>
     </children>
diff --git a/smoketest/scripts/cli/test_vrf.py b/smoketest/scripts/cli/test_vrf.py
index f6e4181c0..243397dc2 100755
--- a/smoketest/scripts/cli/test_vrf.py
+++ b/smoketest/scripts/cli/test_vrf.py
@@ -18,7 +18,6 @@ import re
 import os
 import unittest
 
-from netifaces import interfaces
 from base_vyostest_shim import VyOSUnitTestSHIM
 
 from vyos.configsession import ConfigSessionError
@@ -27,6 +26,7 @@ from vyos.ifconfig import Section
 from vyos.utils.file import read_file
 from vyos.utils.network import get_interface_config
 from vyos.utils.network import is_intf_addr_assigned
+from vyos.utils.network import interface_exists
 from vyos.utils.system import sysctl_read
 
 base_path = ['vrf']
@@ -60,7 +60,7 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
         self.cli_delete(base_path)
         self.cli_commit()
         for vrf in vrfs:
-            self.assertNotIn(vrf, interfaces())
+            self.assertFalse(interface_exists(vrf))
 
     def test_vrf_vni_and_table_id(self):
         base_table = '1000'
@@ -89,7 +89,7 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
         iproute2_config = read_file('/etc/iproute2/rt_tables.d/vyos-vrf.conf')
         for vrf in vrfs:
             description = f'VyOS-VRF-{vrf}'
-            self.assertTrue(vrf in interfaces())
+            self.assertTrue(interface_exists(vrf))
             vrf_if = Interface(vrf)
             # validate proper interface description
             self.assertEqual(vrf_if.get_alias(), description)
@@ -131,7 +131,7 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
         loopbacks = ['127.0.0.1', '::1']
         for vrf in vrfs:
             # Ensure VRF was created
-            self.assertIn(vrf, interfaces())
+            self.assertTrue(interface_exists(vrf))
             # Verify IP forwarding is 1 (enabled)
             self.assertEqual(sysctl_read(f'net.ipv4.conf.{vrf}.forwarding'), '1')
             self.assertEqual(sysctl_read(f'net.ipv6.conf.{vrf}.forwarding'), '1')
@@ -171,7 +171,7 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
         self.cli_commit()
 
         # Check if VRF has been created
-        self.assertTrue(vrf in interfaces())
+        self.assertTrue(interface_exists(vrf))
 
         table = str(int(table) + 1)
         self.cli_set(base + ['table', table])
@@ -228,7 +228,7 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
             next_hop = f'192.0.{table}.1'
             prefix = f'10.0.{table}.0/24'
 
-            self.assertTrue(vrf in interfaces())
+            self.assertTrue(interface_exists(vrf))
 
             frrconfig = self.getFRRconfig(f'vrf {vrf}')
             self.assertIn(f' vni {table}', frrconfig)
@@ -261,7 +261,7 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
         # Apply VRF config
         self.cli_commit()
         # Ensure VRF got created
-        self.assertIn(vrf, interfaces())
+        self.assertTrue(interface_exists(vrf))
         # ... and IP addresses are still assigned
         for address in addresses:
             self.assertTrue(is_intf_addr_assigned(interface, address))
@@ -293,7 +293,7 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
         loopbacks = ['127.0.0.1', '::1']
         for vrf in vrfs:
             # Ensure VRF was created
-            self.assertIn(vrf, interfaces())
+            self.assertTrue(interface_exists(vrf))
             # Verify IP forwarding is 0 (disabled)
             self.assertEqual(sysctl_read(f'net.ipv4.conf.{vrf}.forwarding'), '0')
             self.assertEqual(sysctl_read(f'net.ipv6.conf.{vrf}.forwarding'), '0')
@@ -425,7 +425,7 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
         # Verify VRF configuration
         table = base_table
         for vrf in vrfs:
-            self.assertTrue(vrf in interfaces())
+            self.assertTrue(interface_exists(vrf))
 
             frrconfig = self.getFRRconfig(f'vrf {vrf}')
             self.assertIn(f' vni {table}', frrconfig)
@@ -447,7 +447,7 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
         # Verify VRF configuration
         table = base_table
         for vrf in vrfs:
-            self.assertTrue(vrf in interfaces())
+            self.assertTrue(interface_exists(vrf))
 
             frrconfig = self.getFRRconfig(f'vrf {vrf}')
             self.assertIn(f' vni {table}', frrconfig)
@@ -470,13 +470,39 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
         # Verify VRF configuration
         table = base_table
         for vrf in vrfs:
-            self.assertTrue(vrf in interfaces())
+            self.assertTrue(interface_exists(vrf))
 
             frrconfig = self.getFRRconfig(f'vrf {vrf}')
             self.assertIn(f' vni {table}', frrconfig)
             # Increment table ID for the next run
             table = str(int(table) + 2)
 
+
+        # add a new VRF with VNI - this must not delete any existing VRF/VNI
+        purple = 'purple'
+        table = str(int(table) + 10)
+        self.cli_set(base_path + ['name', purple, 'table', table])
+        self.cli_set(base_path + ['name', purple, 'vni', table])
+
+        # commit changes
+        self.cli_commit()
+
+        # Verify VRF configuration
+        table = base_table
+        for vrf in vrfs:
+            self.assertTrue(interface_exists(vrf))
+
+            frrconfig = self.getFRRconfig(f'vrf {vrf}')
+            self.assertIn(f' vni {table}', frrconfig)
+            # Increment table ID for the next run
+            table = str(int(table) + 2)
+
+        # Verify purple VRF/VNI
+        self.assertTrue(interface_exists(purple))
+        table = str(int(table) + 10)
+        frrconfig = self.getFRRconfig(f'vrf {purple}')
+        self.assertIn(f' vni {table}', frrconfig)
+
         # Now delete all the VNIs
         for vrf in vrfs:
             base = base_path + ['name', vrf]
@@ -487,11 +513,16 @@ class VRFTest(VyOSUnitTestSHIM.TestCase):
 
         # Verify no VNI is defined
         for vrf in vrfs:
-            self.assertTrue(vrf in interfaces())
+            self.assertTrue(interface_exists(vrf))
 
             frrconfig = self.getFRRconfig(f'vrf {vrf}')
             self.assertNotIn('vni', frrconfig)
 
+        # Verify purple VNI remains
+        self.assertTrue(interface_exists(purple))
+        frrconfig = self.getFRRconfig(f'vrf {purple}')
+        self.assertIn(f' vni {table}', frrconfig)
+
     def test_vrf_ip_ipv6_nht(self):
         table = '6910'
 
diff --git a/src/conf_mode/vrf.py b/src/conf_mode/vrf.py
index 587309005..8d8c234c0 100755
--- a/src/conf_mode/vrf.py
+++ b/src/conf_mode/vrf.py
@@ -130,11 +130,6 @@ def get_config(config=None):
     tmp = {'policy' : {'route-map' : conf.get_config_dict(['policy', 'route-map'],
                                                           get_first_key=True)}}
 
-    # L3VNI setup is done via vrf_vni.py as it must be de-configured (on node
-    # deletetion prior to the BGP process. Tell the Jinja2 template no VNI
-    # setup is needed
-    vrf.update({'no_vni' : ''})
-
     # Merge policy dict into "regular" config dict
     vrf = dict_merge(tmp, vrf)
     return vrf
diff --git a/src/conf_mode/vrf_vni.py b/src/conf_mode/vrf_vni.py
deleted file mode 100644
index 8dab164d7..000000000
--- a/src/conf_mode/vrf_vni.py
+++ /dev/null
@@ -1,103 +0,0 @@
-#!/usr/bin/env python3
-#
-# Copyright (C) 2023-2024 VyOS maintainers and contributors
-#
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License version 2 or later as
-# published by the Free Software Foundation.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
-from sys import argv
-from sys import exit
-
-from vyos.config import Config
-from vyos.template import render_to_string
-from vyos import ConfigError
-from vyos import frr
-from vyos import airbag
-airbag.enable()
-
-def get_config(config=None):
-    if config:
-        conf = config
-    else:
-        conf = Config()
-
-    vrf_name = None
-    if len(argv) > 1:
-        vrf_name = argv[1]
-    else:
-        return None
-
-    # Using duplicate L3VNIs makes no sense - it's also forbidden in FRR,
-    # thus VyOS CLI must deny this, too. Instead of getting only the dict for
-    # the requested VRF and den comparing it with depenent VRfs to not have any
-    # duplicate we will just grad ALL VRFs by default but only render/apply
-    # the configuration for the requested VRF - that makes the code easier and
-    # hopefully less error prone
-    vrf = conf.get_config_dict(['vrf'], key_mangling=('-', '_'),
-                               no_tag_node_value_mangle=True,
-                               get_first_key=True)
-
-    # Store name of VRF we are interested in for FRR config rendering
-    vrf.update({'only_vrf' : vrf_name})
-
-    return vrf
-
-def verify(vrf):
-    if not vrf:
-        return
-
-    if len(argv) < 2:
-        raise ConfigError('VRF parameter not specified when valling vrf_vni.py')
-
-    if 'name' in vrf:
-        vni_ids = []
-        for name, vrf_config in vrf['name'].items():
-            # VRF VNI (Virtual Network Identifier) must be unique on the system
-            if 'vni' in vrf_config:
-                if vrf_config['vni'] in vni_ids:
-                    raise ConfigError(f'VRF "{name}" VNI is not unique!')
-                vni_ids.append(vrf_config['vni'])
-
-    return None
-
-def generate(vrf):
-    if not vrf:
-        return
-
-    vrf['new_frr_config'] = render_to_string('frr/zebra.vrf.route-map.frr.j2', vrf)
-    return None
-
-def apply(vrf):
-    frr_daemon = 'zebra'
-
-    # add configuration to FRR
-    frr_cfg = frr.FRRConfig()
-    frr_cfg.load_configuration(frr_daemon)
-    # There is only one VRF inside the dict as we read only one in get_config()
-    if vrf and 'only_vrf' in vrf:
-        vrf_name = vrf['only_vrf']
-        frr_cfg.modify_section(f'^vrf {vrf_name}', stop_pattern='^exit-vrf', remove_stop_mark=True)
-    if vrf and 'new_frr_config' in vrf:
-        frr_cfg.add_before(frr.default_add_before, vrf['new_frr_config'])
-    frr_cfg.commit_configuration(frr_daemon)
-
-    return None
-
-if __name__ == '__main__':
-    try:
-        c = get_config()
-        verify(c)
-        generate(c)
-        apply(c)
-    except ConfigError as e:
-        print(e)
-        exit(1)