From 103de86a3dbe2199fea7cb209fd985f07eebf84c Mon Sep 17 00:00:00 2001 From: Dev Ojha <47282568+devadityaojha@users.noreply.github.com> Date: Thu, 19 Aug 2021 17:58:54 -0700 Subject: [PATCH] =?UTF-8?q?[CLI][MPLS][Show]=20Fixed=20show=20and=20config?= =?UTF-8?q?=20mpls=20cli=20bug=20where=20invalid=20interfaces=20would=20pa?= =?UTF-8?q?s=E2=80=A6=20(#1770)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Added unit tests for both single and multi ASIC platform --- config/main.py | 8 +- show/interfaces/__init__.py | 19 +++-- tests/mpls_test.py | 148 ++++++++++++++++++++++++++++++++++-- utilities_common/cli.py | 11 +++ 4 files changed, 171 insertions(+), 15 deletions(-) diff --git a/config/main.py b/config/main.py index c4782ebfe0f4..786ae67d403d 100644 --- a/config/main.py +++ b/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"}) diff --git a/show/interfaces/__init__.py b/show/interfaces/__init__.py index 458d3cc25116..6cb2febc1911 100644 --- a/show/interfaces/__init__.py +++ b/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 = [] diff --git a/tests/mpls_test.py b/tests/mpls_test.py index b0c2d4fb805d..57336cd63ff5 100644 --- a/tests/mpls_test.py +++ b/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] +Try "remove --help" for help. + +Error: interface Ethernet8 doesn`t exist +""" +invalid_interface_add_output = """\ +Usage: add [OPTIONS] +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,6 +158,26 @@ 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() @@ -118,12 +185,27 @@ def test_config_mpls_remove(self): 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,6 +299,26 @@ 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() @@ -209,12 +326,27 @@ def test_config_mpls_masic_remove(self): 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): diff --git a/utilities_common/cli.py b/utilities_common/cli.py index 15941e28b32b..59aa36a7c054 100644 --- a/utilities_common/cli.py +++ b/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 +