Revert "Remove suppress-fib-pending CLI and make route_check.py check suppress-fib in BGP configuration" (#3477)

Reverts #3331
BGP zebra enhancements is merged to master branch #19717

Reverting the revert of bgp suppress pending feature to enable it in master branch

Author: dgsudharsan

config/main.py

@@ -2376,6 +2376,20 @@ def synchronous_mode(sync_mode):
               config reload -y \n
     Option 2. systemctl restart swss""" % sync_mode)
 
+
+#
+# 'suppress-fib-pending' command ('config suppress-fib-pending ...')
+#
+@config.command('suppress-fib-pending')
+@click.argument('state', metavar='<enabled|disabled>', required=True, type=click.Choice(['enabled', 'disabled']))
+@clicommon.pass_db
+def suppress_pending_fib(db, state):
+    ''' Enable or disable pending FIB suppression. Once enabled,
+        BGP will not advertise routes that are not yet installed in the hardware '''
+
+    config_db = db.cfgdb
+    config_db.mod_entry('DEVICE_METADATA', 'localhost', {"suppress-fib-pending": state})
+
 #
 # 'yang_config_validation' command ('config yang_config_validation ...')
 #

doc/Command-Reference.md

@@ -2612,6 +2612,26 @@ This command displays the routing policy that takes precedence over the other ro
       Exit routemap
   ```
 
+**show suppress-fib-pending**
+
+This command is used to show the status of suppress pending FIB feature.
+When enabled, BGP will not advertise routes which aren't yet offloaded.
+
+- Usage:
+  ```
+  show suppress-fib-pending
+  ```
+
+- Examples:
+  ```
+  admin@sonic:~$ show suppress-fib-pending
+  Enabled
+  ```
+  ```
+  admin@sonic:~$ show suppress-fib-pending
+  Disabled
+  ```
+
 **show bgp device-global**
 
 This command displays BGP device global configuration.
@@ -2724,6 +2744,24 @@ This command is used to remove particular IPv4 or IPv6 BGP neighbor configuratio
   admin@sonic:~$ sudo config bgp remove neighbor SONIC02SPINE
   ```
 
+**config suppress-fib-pending**
+
+This command is used to enable or disable announcements of routes not yet installed in the HW.
+Once enabled, BGP will not advertise routes which aren't yet offloaded.
+
+- Usage:
+  ```
+  config suppress-fib-pending <enabled|disabled>
+  ```
+
+- Examples:
+  ```
+  admin@sonic:~$ sudo config suppress-fib-pending enabled
+  ```
+  ```
+  admin@sonic:~$ sudo config suppress-fib-pending disabled 
+  ```
+
 **config bgp device-global tsa/w-ecmp**
 
 This command is used to manage BGP device global configuration.

scripts/route_check.py

@@ -328,16 +328,6 @@ def get_asicdb_routes(namespace):
     return (selector, subs, sorted(rt))
 
 
-def is_bgp_suppress_fib_pending_enabled(namespace):
-    """
-    Retruns True if FIB suppression is enabled in BGP config, False otherwise
-    """
-    show_run_cmd = ['show', 'runningconfiguration', 'bgp', '-n', namespace]
-
-    output = subprocess.check_output(show_run_cmd, text=True)
-    return 'bgp suppress-fib-pending' in output
-
-
 def is_suppress_fib_pending_enabled(namespace):
     """
     Returns True if FIB suppression is enabled, False otherwise
@@ -791,20 +781,19 @@ def check_routes(namespace):
                 results[namespace] = {}
             results[namespace]["Unaccounted_ROUTE_ENTRY_TABLE_entries"] = rt_asic_miss
 
-        if is_bgp_suppress_fib_pending_enabled(namespace):
-            rt_frr_miss = check_frr_pending_routes(namespace)
-
-            if rt_frr_miss:
-                if namespace not in results:
-                    results[namespace] = {}
-                results[namespace]["missed_FRR_routes"] = rt_frr_miss
+        rt_frr_miss = check_frr_pending_routes(namespace)
 
-            if results:
-                if rt_frr_miss and not rt_appl_miss and not rt_asic_miss:
-                    print_message(syslog.LOG_ERR, "Some routes are not set offloaded in FRR{} but all "
-                                  "routes in APPL_DB and ASIC_DB are in sync".format(namespace))
-                    if is_suppress_fib_pending_enabled(namespace):
-                        mitigate_installed_not_offloaded_frr_routes(namespace, rt_frr_miss, rt_appl)
+        if rt_frr_miss:
+            if namespace not in results:
+                results[namespace] = {}
+            results[namespace]["missed_FRR_routes"] = rt_frr_miss
+
+        if results:
+            if rt_frr_miss and not rt_appl_miss and not rt_asic_miss:
+                print_message(syslog.LOG_ERR, "Some routes are not set offloaded in FRR{} \
+                              but all routes in APPL_DB and ASIC_DB are in sync".format(namespace))
+                if is_suppress_fib_pending_enabled(namespace):
+                    mitigate_installed_not_offloaded_frr_routes(namespace, rt_frr_miss, rt_appl)
 
     if results:
         print_message(syslog.LOG_WARNING, "Failure results: {",  json.dumps(results, indent=4), "}")

show/main.py

@@ -2159,6 +2159,17 @@ def peer(db, peer_ip):
     click.echo(tabulate(bfd_body, bfd_headers))
 
 
+# 'suppress-fib-pending' subcommand ("show suppress-fib-pending")
+@cli.command('suppress-fib-pending')
+@clicommon.pass_db
+def suppress_pending_fib(db):
+    """ Show the status of suppress pending FIB feature """
+
+    field_values = db.cfgdb.get_entry('DEVICE_METADATA', 'localhost')
+    state = field_values.get('suppress-fib-pending', 'disabled').title()
+    click.echo(state)
+
+
 # asic-sdk-health-event subcommand ("show asic-sdk-health-event")
 @cli.group(cls=clicommon.AliasedGroup)
 def asic_sdk_health_event():

tests/route_check_test.py

@@ -252,11 +252,8 @@ def run_test(self, ct_data):
 
     def mock_check_output(self, ct_data, *args, **kwargs):
         ns = self.extract_namespace_from_args(args[0])
-        if 'show runningconfiguration bgp' in ' '.join(args[0]):
-            return 'bgp suppress-fib-pending'
-        else:
-            routes = ct_data.get(FRR_ROUTES, {}).get(ns, {})
-            return json.dumps(routes)
+        routes = ct_data.get(FRR_ROUTES, {}).get(ns, {})
+        return json.dumps(routes)
 
     def assert_results(self, ct_data, ret, res):
         expect_ret = ct_data.get(RET, 0)

tests/suppress_pending_fib_test.py

@@ -0,0 +1,34 @@
+from click.testing import CliRunner
+
+import config.main as config
+import show.main as show
+from utilities_common.db import Db
+
+
+class TestSuppressFibPending:
+    def test_synchronous_mode(self):
+        runner = CliRunner()
+
+        db = Db()
+
+        result = runner.invoke(config.config.commands['suppress-fib-pending'], ['enabled'], obj=db)
+        print(result.output)
+        assert result.exit_code == 0
+        assert db.cfgdb.get_entry('DEVICE_METADATA', 'localhost')['suppress-fib-pending'] == 'enabled'
+
+        result = runner.invoke(show.cli.commands['suppress-fib-pending'], obj=db)
+        assert result.exit_code == 0
+        assert result.output == 'Enabled\n'
+
+        result = runner.invoke(config.config.commands['suppress-fib-pending'], ['disabled'], obj=db)
+        print(result.output)
+        assert result.exit_code == 0
+        assert db.cfgdb.get_entry('DEVICE_METADATA', 'localhost')['suppress-fib-pending'] == 'disabled'
+
+        result = runner.invoke(show.cli.commands['suppress-fib-pending'], obj=db)
+        assert result.exit_code == 0
+        assert result.output == 'Disabled\n'
+
+        result = runner.invoke(config.config.commands['suppress-fib-pending'], ['invalid-input'], obj=db)
+        print(result.output)
+        assert result.exit_code != 0