From 8119ba25b7453d685a475694142e293c2a3a5341 Mon Sep 17 00:00:00 2001
From: Akhilesh Samineni <47657796+AkhileshSamineni@users.noreply.github.com>
Date: Wed, 20 Jan 2021 10:28:14 +0530
Subject: [PATCH] Validations checks while creating and deleting a Portchannel
 (#1326)

Added below validation checks when a portchannel is created by the user:
1 if a given portchannel name is valid or not
2 if a given portchannel name is already created by user (exists in CONFIG_DB)

Added below validation checks when a portchannel is attempted for deletion by the user:
1 if a given portchannel name is valid or not
2 if a given portchannel name exists in exists in CONFIG_DB or not (throw an error if it does not exist)

Signed-off-by: Akhilesh Samineni <akhilesh.samineni@broadcom.com>
---
 config/main.py            | 17 ++++++++++++++
 tests/portchannel_test.py | 48 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/config/main.py b/config/main.py
index 5816f02f960c..57d15a75c7cc 100644
--- a/config/main.py
+++ b/config/main.py
@@ -1356,7 +1356,15 @@ def portchannel(ctx, namespace):
 @click.pass_context
 def add_portchannel(ctx, portchannel_name, min_links, fallback):
     """Add port channel"""
+    if is_portchannel_name_valid(portchannel_name) != True:
+        ctx.fail("{} is invalid!, name should have prefix '{}' and suffix '{}'"
+                 .format(portchannel_name, CFG_PORTCHANNEL_PREFIX, CFG_PORTCHANNEL_NO))
+
     db = ctx.obj['db']
+
+    if is_portchannel_present_in_db(db, portchannel_name):
+        ctx.fail("{} already exists!".format(portchannel_name))
+
     fvs = {'admin_status': 'up',
            'mtu': '9100'}
     if min_links != 0:
@@ -1370,7 +1378,16 @@ def add_portchannel(ctx, portchannel_name, min_links, fallback):
 @click.pass_context
 def remove_portchannel(ctx, portchannel_name):
     """Remove port channel"""
+    if is_portchannel_name_valid(portchannel_name) != True:
+        ctx.fail("{} is invalid!, name should have prefix '{}' and suffix '{}'"
+                 .format(portchannel_name, CFG_PORTCHANNEL_PREFIX, CFG_PORTCHANNEL_NO))
+
     db = ctx.obj['db']
+
+    # Dont proceed if the port channel does not exist
+    if is_portchannel_present_in_db(db, portchannel_name) is False:
+        ctx.fail("{} is not present.".format(portchannel_name))
+
     if len([(k, v) for k, v in db.get_table('PORTCHANNEL_MEMBER') if k == portchannel_name]) != 0:
         click.echo("Error: Portchannel {} contains members. Remove members before deleting Portchannel!".format(portchannel_name))
     else:
diff --git a/tests/portchannel_test.py b/tests/portchannel_test.py
index a07e20539262..1ccf19495f61 100644
--- a/tests/portchannel_test.py
+++ b/tests/portchannel_test.py
@@ -13,6 +13,54 @@ def setup_class(cls):
         os.environ['UTILITIES_UNIT_TESTING'] = "1"
         print("SETUP")
 
+    def test_add_portchannel_with_invalid_name(self):
+        runner = CliRunner()
+        db = Db()
+        obj = {'db':db.cfgdb}
+
+        # add a portchannel with invalid name
+        result = runner.invoke(config.config.commands["portchannel"].commands["add"], ["PortChan005"], obj=obj)
+        print(result.exit_code)
+        print(result.output)
+        assert result.exit_code != 0
+        assert "Error: PortChan005 is invalid!, name should have prefix 'PortChannel' and suffix '<0-9999>'" in result.output
+
+    def test_delete_portchannel_with_invalid_name(self):
+        runner = CliRunner()
+        db = Db()
+        obj = {'db':db.cfgdb}
+
+        # delete a portchannel with invalid name
+        result = runner.invoke(config.config.commands["portchannel"].commands["del"], ["PortChan005"], obj=obj)
+        print(result.exit_code)
+        print(result.output)
+        assert result.exit_code != 0
+        assert "Error: PortChan005 is invalid!, name should have prefix 'PortChannel' and suffix '<0-9999>'" in result.output
+
+    def test_add_existing_portchannel_again(self):
+        runner = CliRunner()
+        db = Db()
+        obj = {'db':db.cfgdb}
+
+        # add a portchannel which is already created
+        result = runner.invoke(config.config.commands["portchannel"].commands["add"], ["PortChannel0001"], obj=obj)
+        print(result.exit_code)
+        print(result.output)
+        assert result.exit_code != 0
+        assert "Error: PortChannel0001 already exists!" in result.output
+
+    def test_delete_non_existing_portchannel(self):
+        runner = CliRunner()
+        db = Db()
+        obj = {'db':db.cfgdb}
+
+        # delete a portchannel which is not present
+        result = runner.invoke(config.config.commands["portchannel"].commands["del"], ["PortChannel0005"], obj=obj)
+        print(result.exit_code)
+        print(result.output)
+        assert result.exit_code != 0
+        assert "Error: PortChannel0005 is not present." in result.output
+        
     def test_add_portchannel_member_with_invalid_name(self):
         runner = CliRunner()
         db = Db()