From 263810b25d12dc2435406d57245a113f7e9688c8 Mon Sep 17 00:00:00 2001 From: Muhammad Danish Date: Tue, 29 Nov 2022 06:10:46 -0800 Subject: [PATCH] Update vrf add, del commands for duplicate/non-existing VRFs (#2467) *[VRF] Update vrf add, del commands for duplicate/non-existing VRFs #2467 *What I did Throw an error in case user wants to add a duplicate VRF. Throw an error if user wants to delete a VRF that does not exist. Update is_vrf_exists function to use vrf_name == management as a valid vrf name as well. Update grammar for vrf_name is invalid message. Renamed tests/show_vrf_test.py -> tests/vrf_test.py to correctly represent the file contents. Add test cases for the added/modified lines. --- config/main.py | 14 ++++-- tests/{show_vrf_test.py => vrf_test.py} | 64 ++++++++++++++++++------- 2 files changed, 56 insertions(+), 22 deletions(-) rename tests/{show_vrf_test.py => vrf_test.py} (81%) diff --git a/config/main.py b/config/main.py index 004b40bdb4..d0829eb040 100644 --- a/config/main.py +++ b/config/main.py @@ -379,7 +379,7 @@ def is_vrf_exists(config_db, vrf_name): keys = config_db.get_keys("VRF") if vrf_name in keys: return True - elif vrf_name == "mgmt": + elif vrf_name == "mgmt" or vrf_name == "management": entry = config_db.get_entry("MGMT_VRF_CONFIG", "vrf_global") if entry and entry.get("mgmtVrfEnabled") == "true": return True @@ -5213,10 +5213,12 @@ def add_vrf(ctx, vrf_name): """Add vrf""" config_db = ctx.obj['config_db'] if not vrf_name.startswith("Vrf") and not (vrf_name == 'mgmt') and not (vrf_name == 'management'): - ctx.fail("'vrf_name' is not start with Vrf, mgmt or management!") + ctx.fail("'vrf_name' must begin with 'Vrf' or named 'mgmt'/'management' in case of ManagementVRF.") if len(vrf_name) > 15: ctx.fail("'vrf_name' is too long!") - if (vrf_name == 'mgmt' or vrf_name == 'management'): + if is_vrf_exists(config_db, vrf_name): + ctx.fail("VRF {} already exists!".format(vrf_name)) + elif (vrf_name == 'mgmt' or vrf_name == 'management'): vrf_add_management_vrf(config_db) else: config_db.set_entry('VRF', vrf_name, {"NULL": "NULL"}) @@ -5228,7 +5230,7 @@ def del_vrf(ctx, vrf_name): """Del vrf""" config_db = ctx.obj['config_db'] if not vrf_name.startswith("Vrf") and not (vrf_name == 'mgmt') and not (vrf_name == 'management'): - ctx.fail("'vrf_name' is not start with Vrf, mgmt or management!") + ctx.fail("'vrf_name' must begin with 'Vrf' or named 'mgmt'/'management' in case of ManagementVRF.") if len(vrf_name) > 15: ctx.fail("'vrf_name' is too long!") syslog_table = config_db.get_table("SYSLOG_SERVER") @@ -5237,7 +5239,9 @@ def del_vrf(ctx, vrf_name): syslog_vrf = syslog_data.get("vrf") if syslog_vrf == syslog_vrf_dev: ctx.fail("Failed to remove VRF device: {} is in use by SYSLOG_SERVER|{}".format(syslog_vrf, syslog_entry)) - if (vrf_name == 'mgmt' or vrf_name == 'management'): + if not is_vrf_exists(config_db, vrf_name): + ctx.fail("VRF {} does not exist!".format(vrf_name)) + elif (vrf_name == 'mgmt' or vrf_name == 'management'): vrf_delete_management_vrf(config_db) else: del_interface_bind_to_vrf(config_db, vrf_name) diff --git a/tests/show_vrf_test.py b/tests/vrf_test.py similarity index 81% rename from tests/show_vrf_test.py rename to tests/vrf_test.py index c5853ba443..954fbf692b 100644 --- a/tests/show_vrf_test.py +++ b/tests/vrf_test.py @@ -180,31 +180,61 @@ def test_vrf_bind_unbind(self): assert result.exit_code == 0 assert result.output == expected_output - def test_vrf_del(self): + def test_vrf_add_del(self): runner = CliRunner() db = Db() vrf_obj = {'config_db':db.cfgdb, 'namespace':db.db.namespace} + result = runner.invoke(config.config.commands["vrf"].commands["add"], ["Vrf100"], obj=vrf_obj) + assert ('Vrf100') in db.cfgdb.get_table('VRF') + assert result.exit_code == 0 + + result = runner.invoke(config.config.commands["vrf"].commands["add"], ["Vrf1"], obj=vrf_obj) + assert "VRF Vrf1 already exists!" in result.output + assert ('Vrf1') in db.cfgdb.get_table('VRF') + assert result.exit_code != 0 + expected_output_del = "VRF Vrf1 deleted and all associated IP addresses removed.\n" result = runner.invoke(config.config.commands["vrf"].commands["del"], ["Vrf1"], obj=vrf_obj) assert result.exit_code == 0 assert result.output == expected_output_del assert ('Vrf1') not in db.cfgdb.get_table('VRF') - expected_output_del = "VRF Vrf101 deleted and all associated IP addresses removed.\n" - result = runner.invoke(config.config.commands["vrf"].commands["del"], ["Vrf101"], obj=vrf_obj) - assert result.exit_code == 0 - assert result.output == expected_output_del - assert ('Vrf101') not in db.cfgdb.get_table('VRF') - - expected_output_del = "VRF Vrf102 deleted and all associated IP addresses removed.\n" - result = runner.invoke(config.config.commands["vrf"].commands["del"], ["Vrf102"], obj=vrf_obj) - assert result.exit_code == 0 - assert result.output == expected_output_del - assert ('Vrf102') not in db.cfgdb.get_table('VRF') + result = runner.invoke(config.config.commands["vrf"].commands["del"], ["Vrf200"], obj=vrf_obj) + assert result.exit_code != 0 + assert ('Vrf200') not in db.cfgdb.get_table('VRF') + assert "VRF Vrf200 does not exist!" in result.output - expected_output_del = "VRF Vrf103 deleted and all associated IP addresses removed.\n" - result = runner.invoke(config.config.commands["vrf"].commands["del"], ["Vrf103"], obj=vrf_obj) - assert result.exit_code == 0 - assert result.output == expected_output_del - assert ('Vrf103') not in db.cfgdb.get_table('VRF') + def test_invalid_vrf_name(self): + db = Db() + runner = CliRunner() + obj = {'config_db':db.cfgdb} + expected_output = """\ +Error: 'vrf_name' must begin with 'Vrf' or named 'mgmt'/'management' in case of ManagementVRF. +""" + result = runner.invoke(config.config.commands["vrf"].commands["add"], ["vrf-blue"], obj=obj) + assert result.exit_code != 0 + assert ('vrf-blue') not in db.cfgdb.get_table('VRF') + assert expected_output in result.output + + result = runner.invoke(config.config.commands["vrf"].commands["add"], ["VRF2"], obj=obj) + assert result.exit_code != 0 + assert ('VRF2') not in db.cfgdb.get_table('VRF') + assert expected_output in result.output + + result = runner.invoke(config.config.commands["vrf"].commands["add"], ["VrF10"], obj=obj) + assert result.exit_code != 0 + assert ('VrF10') not in db.cfgdb.get_table('VRF') + assert expected_output in result.output + + result = runner.invoke(config.config.commands["vrf"].commands["del"], ["vrf-blue"], obj=obj) + assert result.exit_code != 0 + assert expected_output in result.output + + result = runner.invoke(config.config.commands["vrf"].commands["del"], ["VRF2"], obj=obj) + assert result.exit_code != 0 + assert expected_output in result.output + + result = runner.invoke(config.config.commands["vrf"].commands["del"], ["VrF10"], obj=obj) + assert result.exit_code != 0 + assert expected_output in result.output