Update vrf add, del commands for duplicate/non-existing VRFs (sonic-net#2467)

*[VRF] Update vrf add, del commands for duplicate/non-existing VRFs sonic-net#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.

Author: mdanish-kh

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)

tests/show_vrf_test.py renamed to 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