Merge pull request #3393 from vyos/mergify/bp/sagitta/pr-3392

bgp: T6189: L3VPN connectivity is broken after re-enabling VRF (backport #3392)

3 files changed, 28 insertions, 126 deletions

diff --git a/src/conf_mode/protocols_bgp.py b/src/conf_mode/protocols_bgp.py
index 4df97d133..44409c0e3 100755
--- a/src/conf_mode/protocols_bgp.py
+++ b/src/conf_mode/protocols_bgp.py
@@ -31,6 +31,7 @@ from vyos.utils.dict import dict_search
 from vyos.utils.network import get_interface_vrf
 from vyos.utils.network import is_addr_assigned
 from vyos.utils.process import process_named_running
+from vyos.utils.process import call
 from vyos import ConfigError
 from vyos import frr
 from vyos import airbag
@@ -50,13 +51,8 @@ def get_config(config=None):
 
     # eqivalent of the C foo ? 'a' : 'b' statement
     base = vrf and ['vrf', 'name', vrf, 'protocols', 'bgp'] or base_path
-    bgp = conf.get_config_dict(
-        base,
-        key_mangling=('-', '_'),
-        get_first_key=True,
-        no_tag_node_value_mangle=True,
-        with_recursive_defaults=True,
-    )
+    bgp = conf.get_config_dict(base, key_mangling=('-', '_'),
+                               get_first_key=True, no_tag_node_value_mangle=True)
 
     bgp['dependent_vrfs'] = conf.get_config_dict(['vrf', 'name'],
                                                  key_mangling=('-', '_'),
@@ -75,22 +71,29 @@ def get_config(config=None):
     if vrf:
         bgp.update({'vrf' : vrf})
         # We can not delete the BGP VRF instance if there is a L3VNI configured
+        # FRR L3VNI must be deleted first otherwise we will see error:
+        # "FRR error: Please unconfigure l3vni 3000"
         tmp = ['vrf', 'name', vrf, 'vni']
-        if conf.exists(tmp):
-            bgp.update({'vni' : conf.return_value(tmp)})
+        if conf.exists_effective(tmp):
+            bgp.update({'vni' : conf.return_effective_value(tmp)})
         # We can safely delete ourself from the dependent vrf list
         if vrf in bgp['dependent_vrfs']:
             del bgp['dependent_vrfs'][vrf]
 
-    bgp['dependent_vrfs'].update({'default': {'protocols': {
-        'bgp': conf.get_config_dict(base_path, key_mangling=('-', '_'),
-                                    get_first_key=True,
-                                    no_tag_node_value_mangle=True)}}})
+        bgp['dependent_vrfs'].update({'default': {'protocols': {
+            'bgp': conf.get_config_dict(base_path, key_mangling=('-', '_'),
+                                        get_first_key=True,
+                                        no_tag_node_value_mangle=True)}}})
+
     if not conf.exists(base):
         # If bgp instance is deleted then mark it
         bgp.update({'deleted' : ''})
         return bgp
 
+    # We have gathered the dict representation of the CLI, but there are default
+    # options which we need to update into the dictionary retrived.
+    bgp = conf.merge_defaults(bgp, recursive=True)
+
     # We also need some additional information from the config, prefix-lists
     # and route-maps for instance. They will be used in verify().
     #
@@ -242,10 +245,6 @@ def verify(bgp):
                 if verify_vrf_as_import(bgp['vrf'], tmp_afi, bgp['dependent_vrfs']):
                     raise ConfigError(f'Cannot delete VRF instance "{bgp["vrf"]}", ' \
                                       'unconfigure "import vrf" commands!')
-            # We can not delete the BGP instance if a L3VNI instance exists
-            if 'vni' in bgp:
-                raise ConfigError(f'Cannot delete VRF instance "{bgp["vrf"]}", ' \
-                                  f'unconfigure VNI "{bgp["vni"]}" first!')
         else:
             # We are running in the default VRF context, thus we can not delete
             # our main BGP instance if there are dependent BGP VRF instances.
@@ -254,7 +253,11 @@ def verify(bgp):
                     if vrf != 'default':
                         if dict_search('protocols.bgp', vrf_options):
                             raise ConfigError('Cannot delete default BGP instance, ' \
-                                              'dependent VRF instance(s) exist!')
+                                              'dependent VRF instance(s) exist(s)!')
+                        if 'vni' in vrf_options:
+                            raise ConfigError('Cannot delete default BGP instance, ' \
+                                              'dependent L3VNI exists!')
+
         return None
 
     if 'system_as' not in bgp:
@@ -607,6 +610,13 @@ def generate(bgp):
     return None
 
 def apply(bgp):
+    if 'deleted' in bgp:
+        # We need to ensure that the L3VNI is deleted first.
+        # This is not possible with old config backend
+        # priority bug
+        if {'vrf', 'vni'} <= set(bgp):
+            call('vtysh -c "conf t" -c "vrf {vrf}" -c "no vni {vni}"'.format(**bgp))
+
     bgp_daemon = 'bgpd'
 
     # Save original configuration prior to starting any commit actions
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)