[CLI][MPLS][Show] Fixed show and config mpls cli bug where invalid interfaces would pas… (sonic-net#1770)

* Added unit tests for both single and multi ASIC platform

Author: developfast

config/main.py

@@ -4039,7 +4039,9 @@ def add(ctx, interface_name):
         if interface_name is None:
             ctx.fail("'interface_name' is None!")
 
-    table_name = get_interface_table_name(interface_name)
+    table_name = get_interface_table_name(interface_name)  
+    if not clicommon.is_interface_in_config_db(config_db, interface_name):
+        ctx.fail('interface {} doesn`t exist'.format(interface_name))
     if table_name == "":
         ctx.fail("'interface_name' is not valid. Valid names [Ethernet/PortChannel/Vlan]")
     config_db.set_entry(table_name, interface_name, {"mpls": "enable"})
@@ -4059,7 +4061,9 @@ def remove(ctx, interface_name):
         if interface_name is None:
             ctx.fail("'interface_name' is None!")
 
-    table_name = get_interface_table_name(interface_name)
+    table_name = get_interface_table_name(interface_name) 
+    if not clicommon.is_interface_in_config_db(config_db, interface_name):
+        ctx.fail('interface {} doesn`t exist'.format(interface_name))
     if table_name == "":
         ctx.fail("'interface_name' is not valid. Valid names [Ethernet/PortChannel/Vlan]")
     config_db.set_entry(table_name, interface_name, {"mpls": "disable"})

show/interfaces/__init__.py

@@ -340,9 +340,11 @@ def mpls(ctx, interfacename, namespace, display):
            print("Error: Invalid display option command for single asic")
            return
 
+    display = "all" if interfacename else display
     masic = multi_asic_util.MultiAsic(display_option=display, namespace_option=namespace)
     ns_list = masic.get_ns_list_based_on_options()
     intfs_data = {}
+    intf_found = False
 
     for ns in ns_list:
 
@@ -360,11 +362,14 @@ def mpls(ctx, interfacename, namespace, display):
             if len(tokens) != 2:
                 continue
 
-            if (interfacename is not None) and (interfacename != tokens[1]):
-                continue
+            if (interfacename is not None):
+                if (interfacename != ifname):
+                    continue
+                
+                intf_found = True
 
             if (display != "all"):
-                if ("Loopback" in tokens[1]):
+                if ("Loopback" in ifname):
                     continue
 
                 if ifname.startswith("Ethernet") and multi_asic.is_port_internal(ifname, ns):
@@ -377,9 +382,13 @@ def mpls(ctx, interfacename, namespace, display):
             mpls_intf = appl_db.get_all(appl_db.APPL_DB, key)
 
             if 'mpls' not in mpls_intf or mpls_intf['mpls'] == 'disable':
-                intfs_data.update({tokens[1]: 'disable'})
+                intfs_data.update({ifname: 'disable'})
             else:
-                intfs_data.update({tokens[1]: mpls_intf['mpls']}) 
+                intfs_data.update({ifname: mpls_intf['mpls']}) 
+    
+    # Check if interface is valid
+    if (interfacename is not None and not intf_found):
+        ctx.fail('interface {} doesn`t exist'.format(interfacename))    
 
     header = ['Interface', 'MPLS State']
     body = []

tests/mpls_test.py

@@ -50,7 +50,39 @@
 Ethernet-BP4  disable
 """
 
+show_interfaces_mpls_output_interface="""\
+Interface    MPLS State
+-----------  ------------
+Ethernet4    enable
+"""
+
+show_interfaces_mpls_masic_output_interface="""\
+Interface    MPLS State
+-----------  ------------
+Ethernet4    disable
+"""
+
+invalid_interface_remove_output = """\
+Usage: remove [OPTIONS] <interface_name>
+Try "remove --help" for help.
+
+Error: interface Ethernet8 doesn`t exist
+"""
 
+invalid_interface_add_output = """\
+Usage: add [OPTIONS] <interface_name>
+Try "add --help" for help.
+
+Error: interface Ethernet8 doesn`t exist
+""" 
+ 
+invalid_interface_show_output = """\
+Usage: mpls [OPTIONS] [INTERFACENAME]
+Try "mpls --help" for help.
+
+Error: interface Ethernet100 doesn`t exist
+"""
+ 
 modules_path = os.path.join(os.path.dirname(__file__), "..")
 test_path = os.path.join(modules_path, "tests")
 scripts_path = os.path.join(modules_path, "scripts")
@@ -72,12 +104,27 @@ def test_config_mpls_add(self):
 
         result = runner.invoke(
                  config.config.commands["interface"].commands["mpls"].commands["add"],
-                 ["Ethernet8"], obj=obj
+                 ["Ethernet0"], obj=obj
                  )
         print(result.exit_code)
         print(result.output)
         assert result.exit_code == 0
-        assert db.cfgdb.get_entry("INTERFACE", "Ethernet8") == {"mpls": "enable"}
+        assert db.cfgdb.get_entry("INTERFACE", "Ethernet0") == {"mpls": "enable"}
+
+    def test_config_mpls_invalid_interface_add(self):
+        runner = CliRunner()
+        db = Db()
+        obj = {'config_db':db.cfgdb}
+
+        result = runner.invoke(
+                 config.config.commands["interface"].commands["mpls"].commands["add"],
+                 ["Ethernet8"], obj=obj
+                 )
+        print(result.exit_code)
+        print(result.output)
+        assert result.exit_code == 2
+        assert result.output == invalid_interface_add_output
+
 
     def test_show_interfaces_mpls_frontend(self):
 
@@ -111,19 +158,54 @@ def test_show_interfaces_mpls_dall(self):
         assert result.exit_code == 0
         assert result.output == show_interfaces_mpls_output_frontend
 
+    def test_show_interfaces_mpls_asic_interface(self):
+        runner = CliRunner()
+        result = runner.invoke(
+                 show.cli.commands["interfaces"].commands["mpls"],
+                 ["Ethernet4"]
+                 )
+        print(result.output)
+        assert result.exit_code == 0
+        assert result.output == show_interfaces_mpls_output_interface
+    
+    def test_show_interfaces_mpls_asic_invalid_interface(self):
+        runner = CliRunner()
+        result = runner.invoke(
+                 show.cli.commands["interfaces"].commands["mpls"],
+                 ["Ethernet100"]
+                 )
+        print(result.output)
+        assert result.exit_code == 2
+        assert result.output == invalid_interface_show_output 
+    
     def test_config_mpls_remove(self):
         runner = CliRunner()
         db = Db()
         obj = {'config_db':db.cfgdb}
 
         result = runner.invoke(
                  config.config.commands["interface"].commands["mpls"].commands["remove"],
-                 ["Ethernet8"], obj=obj
+                 ["Ethernet0"], obj=obj
                  )
         print(result.exit_code)
         print(result.output)
         assert result.exit_code == 0
-        assert db.cfgdb.get_entry("INTERFACE", "Ethernet8") == {"mpls": "disable"}
+        assert db.cfgdb.get_entry("INTERFACE", "Ethernet0") == {"mpls": "disable"}
+
+    def test_config_mpls_invalid_interface_remove(self):
+        runner = CliRunner()
+        db = Db()
+        obj = {'config_db':db.cfgdb}
+
+        result = runner.invoke(
+                 config.config.commands["interface"].commands["mpls"].commands["remove"],
+                 ["Ethernet8"], obj=obj
+                 )
+        print(result.exit_code)
+        print(result.output)
+        assert result.exit_code == 2
+        assert result.output == invalid_interface_remove_output 
+
 
     @classmethod 
     def teardown_class(cls):
@@ -152,12 +234,27 @@ def test_config_mpls_masic_add(self):
 
         result = runner.invoke(
                  config.config.commands["interface"].commands["mpls"].commands["add"],
-                 ["Ethernet8"], obj=obj
+                 ["Ethernet0"], obj=obj
                  )
         print(result.exit_code)
         print(result.output)
         assert result.exit_code == 0
-        assert db.cfgdb.get_entry("INTERFACE", "Ethernet8") == {"mpls": "enable"}
+        assert db.cfgdb.get_entry("INTERFACE", "Ethernet0") == {"mpls": "enable"}
+
+
+    def test_config_mpls_masic_invalid_interface_add(self):
+        runner = CliRunner()
+        db = Db()
+        obj = {'config_db':db.cfgdb, 'namespace':'asic0'}
+
+        result = runner.invoke(
+                 config.config.commands["interface"].commands["mpls"].commands["add"],
+                 ["Ethernet8"], obj=obj
+                 )
+        print(result.exit_code)
+        print(result.output)
+        assert result.exit_code == 2
+        assert result.output == invalid_interface_add_output 
 
 
     def test_show_interfaces_mpls_masic_frontend(self):
@@ -202,19 +299,54 @@ def test_show_interfaces_mpls_masic_asic_all(self):
         assert result.exit_code == 0
         assert result.output == show_interfaces_mpls_masic_output_asic_all
 
+    def test_show_interfaces_mpls_masic_asic_interface(self):
+        runner = CliRunner()
+        result = runner.invoke(
+                 show.cli.commands["interfaces"].commands["mpls"],
+                 ["Ethernet4"]
+                 )
+        print(result.output)
+        assert result.exit_code == 0
+        assert result.output == show_interfaces_mpls_masic_output_interface
+    
+    def test_show_interfaces_mpls_masic_asic_invalid_interface(self):
+        runner = CliRunner()
+        result = runner.invoke(
+                 show.cli.commands["interfaces"].commands["mpls"],
+                 ["Ethernet100"]
+                 )
+        print(result.output)
+        assert result.exit_code == 2
+        assert result.output == invalid_interface_show_output 
+    
     def test_config_mpls_masic_remove(self):
         runner = CliRunner()
         db = Db()
         obj = {'config_db':db.cfgdb, 'namespace':'asic0'}
 
         result = runner.invoke(
                  config.config.commands["interface"].commands["mpls"].commands["remove"],
-                 ["Ethernet8"], obj=obj
+                 ["Ethernet0"], obj=obj
                  )
         print(result.exit_code)
         print(result.output)
         assert result.exit_code == 0
-        assert db.cfgdb.get_entry("INTERFACE", "Ethernet8") == {"mpls": "disable"}
+        assert db.cfgdb.get_entry("INTERFACE", "Ethernet0") == {"mpls": "disable"}
+
+    def test_config_mpls_masic_invalid_interface_remove(self):
+        runner = CliRunner()
+        db = Db()
+        obj = {'config_db':db.cfgdb, 'namespace':'asic0'}
+
+        result = runner.invoke(
+                 config.config.commands["interface"].commands["mpls"].commands["remove"],
+                 ["Ethernet8"], obj=obj
+                 )
+        print(result.exit_code)
+        print(result.output)
+        assert result.exit_code == 2
+        assert result.output == invalid_interface_remove_output 
+
 
     @classmethod
     def teardown_class(cls):

utilities_common/cli.py

@@ -576,3 +576,14 @@ def interface_is_untagged_member(db, interface_name):
             if (val['tagging_mode'] == 'untagged'):
                 return True
     return False
+
+def is_interface_in_config_db(config_db, interface_name):
+    """ Check if an interface is in CONFIG DB """
+    if (not interface_name in config_db.get_keys('VLAN_INTERFACE') and
+        not interface_name in config_db.get_keys('INTERFACE') and
+        not interface_name in config_db.get_keys('PORTCHANNEL_INTERFACE') and
+        not interface_name == 'null'):
+            return False
+
+    return True
+