From 4f5f45ebb7177cf2f7254012e7fd37a71c69b5f3 Mon Sep 17 00:00:00 2001
From: Sangita Maity <sangitamaity0211@gmail.com>
Date: Thu, 5 Dec 2019 14:54:22 -0800
Subject: [PATCH 01/14] [config] Add Initial draft of Breakout Mode subcommand

Signed-off-by: Sangita Maity <sangitamaity0211@gmail.com>

Integrating sonicMgmt API

Signed-off-by: Sangita Maity <sangitamaity0211@gmail.com>
---
 config/main.py | 254 ++++++++++++++++++++++++++++++++++++++++++++++++-
 setup.py       |   2 +-
 2 files changed, 254 insertions(+), 2 deletions(-)

diff --git a/config/main.py b/config/main.py
index 6f2f9eeedf..ea3e9c0fb7 100755
--- a/config/main.py
+++ b/config/main.py
@@ -16,16 +16,33 @@
 from swsssdk import ConfigDBConnector
 from swsssdk import SonicV2Connector
 from minigraph import parse_device_desc_xml
+from config_mgmt import configMgmt
 
 import aaa
 import mlnx
 
+from collections import OrderedDict
+import ast
+
+
+# import port config file path
+from sfputil.main import get_platform_and_hwsku
+from portconfig import get_port_config_file_name, parse_platform_json_file
+from portconfig import get_child_ports
+
 CONTEXT_SETTINGS = dict(help_option_names=['-h', '--help', '-?'])
 
 SONIC_CFGGEN_PATH = '/usr/local/bin/sonic-cfggen'
 SYSLOG_IDENTIFIER = "config"
+PORT_STR = "Ethernet"
+INTF_KEY = "interfaces"
+
+(platform, hwsku) =  get_platform_and_hwsku()
+BREAKOUT_CFG_FILE = get_port_config_file_name(hwsku, platform)
+
 VLAN_SUB_INTERFACE_SEPARATOR = '.'
 
+
 # ========================== Syslog wrappers ==========================
 
 def log_debug(msg):
@@ -62,9 +79,122 @@ def log_error(msg):
     raise click.Abort()
 
 #
-# Helper functions
+# Breakout Mode Helper functions
 #
 
+# Read given JSON file
+def readJsonFile(fileName):
+    try:
+        with open(fileName) as f:
+            result = json.load(f)
+    except Exception as e:
+        raise Exception(str(e))
+    return result
+
+def _get_option(ctx,args,incomplete):
+    """ Provides dynamic mode option as per user argument i.e. interface name """
+    global all_mode_options
+    interface_name = args[-1]
+
+    if not os.path.isfile(BREAKOUT_CFG_FILE) or not BREAKOUT_CFG_FILE.endswith('.json'):
+        return []
+    else:
+        breakout_file_input = readJsonFile(BREAKOUT_CFG_FILE)
+        if interface_name in breakout_file_input[INTF_KEY]:
+            breakout_mode_list = [v["breakout_modes"] for i ,v in breakout_file_input[INTF_KEY].items() if i == interface_name][0]
+            breakout_mode_options = []
+            for i in breakout_mode_list.split(','):
+                    breakout_mode_options.append(i)
+            all_mode_options = [str(c) for c in breakout_mode_options if incomplete in c]
+            return all_mode_options
+
+
+def shutdown_interfaces(ctx, del_intf_dict):
+    """ shut down all the interfaces before deletion """
+    for intf in del_intf_dict.keys():
+        config_db = ctx.obj['config_db']
+        if get_interface_naming_mode() == "alias":
+            interface_name = interface_alias_to_name(intf)
+            if intf is None:
+                click.echo("[ERROR] interface name is None!")
+                return False
+        if interface_name_is_valid(intf) is False:
+            click.echo("[ERROR] Interface name is invalid. Please enter a valid interface name!!")
+            return False
+        if intf.startswith(PORT_STR):
+            config_db.mod_entry("PORT", intf, {"admin_status": "down"})
+        else:
+            click.secho("[ERROR] Could not get the correct interface name, exiting", fg='red')
+            return False
+    return True
+
+
+def _validate_interface_mode(ctx, BREAKOUT_CFG_FILE, interface_name, target_brkout_mode, cur_brkout_mode):
+    """ Validate Parent interface and user selected mode before starting deletetion or addition process """
+    breakout_file_input = readJsonFile(BREAKOUT_CFG_FILE)["interfaces"]
+
+    if interface_name not in breakout_file_input:
+        click.secho("[ERROR] {} is not a Parent port. So, Breakout Mode is not available on this port".format(interface_name), fg='red')
+        return False
+
+    # Check whether target breakout mode is available for the user-selected interface or not
+    if target_brkout_mode not in breakout_file_input[interface_name]["breakout_modes"]:
+        click.secho('[ERROR] Target mode {} is not available for the port {}'. format(target_brkout_mode, interface_name), fg='red')
+        return False
+
+    # Get config db context
+    config_db = ctx.obj['config_db']
+    port_dict = config_db.get_table('PORT')
+
+    # Check whether there is any port in config db.
+    if not port_dict:
+        click.echo("port_dict is None!")
+        return False
+
+    # Check whether the  user-selected interface is part of  'port' table in config db.
+    if interface_name not in port_dict.keys():
+        click.secho("[ERROR] {} is not in port_dict".format(interface_name))
+        return False
+    click.echo("\nRunning Breakout Mode : {} \nTarget Breakout Mode : {}".format(cur_brkout_mode, target_brkout_mode))
+    if (cur_brkout_mode == target_brkout_mode):
+        click.secho("[WARNING] No action will be taken as current and desired Breakout Mode are same.", fg='magenta')
+        sys.exit(0)
+    return True
+
+def load_configMgmt(verbose):
+    """ Load config for the commands which are capable of change in config DB. """
+    try:
+        # TODO: set allowExtraTables to False, i.e we should have yang models for
+        # each table in Config. [TODO: Create Yang model for each Table]
+        # cm = configMgmt(debug=verbose, allowExtraTables=False)
+        cm = configMgmt(debug=verbose, allowExtraTables=True)
+        return cm
+    except Exception as e:
+        raise Exception("Failed to load the config. Error: {}".format(str(e)))
+
+def breakout_Ports(cm, delPorts=list(), addPorts=list(), portJson=dict(), \
+    force=False, loadDefConfig=True, verbose=False):
+
+    deps, ret = cm.breakOutPort(delPorts=delPorts, addPorts = addPorts, \
+                    portJson=portJson, force=force, loadDefConfig=loadDefConfig)
+    # check if DPB failed
+    if ret == False:
+        if not force and deps:
+            print("Dependecies Exist. No further action will be taken")
+            print("*** Printing dependecies ***")
+            for dep in deps:
+                print(dep)
+            sys.exit(0)
+        else:
+            print("[ERROR] Port breakout Failed!!! Opting Out")
+            raise click.Abort()
+
+        return
+
+
+#
+# Helper functions
+#
 
 def run_command(command, display_cmd=False, ignore_error=False):
     """Run bash command and print output to stdout
@@ -1378,6 +1508,127 @@ def speed(ctx, interface_name, interface_speed, verbose):
         command += " -vv"
     run_command(command, display_cmd=verbose)
 
+
+
+#
+# 'breakout' subcommand
+#
+@interface.command()
+@click.argument('interface_name', metavar='<interface_name>', required=True)
+@click.argument('mode', required=True, type=click.STRING, autocompletion=_get_option)
+@click.option('-f', '--force-remove-dependencies', is_flag=True,  help='Clear all depenedecies internally first.')
+@click.option('-l', '--load-predefined-config', is_flag=True,  help='load predefied user configuration (alias, lanes, speed etc) first.')
+@click.option('-y', '--yes', is_flag=True, callback=_abort_if_false, expose_value=False, prompt='Do you want to Breakout the port, continue?')
+@click.option('-v', '--verbose', is_flag=True, help="Enable verbose output")
+@click.pass_context
+def breakout(ctx, interface_name, mode, verbose, force_remove_dependencies, load_predefined_config):
+
+    """ Set interface breakout mode """
+
+    if not os.path.isfile(BREAKOUT_CFG_FILE) or not BREAKOUT_CFG_FILE.endswith('.json'):
+        click.secho("[ERROR] Breakout feature is not available without platform.json file", fg='red')
+        raise click.Abort()
+
+    # Connect to config db and get the context
+    config_db = ConfigDBConnector()
+    config_db.connect()
+    ctx.obj['config_db'] = config_db
+
+    target_brkout_mode = mode
+
+    # Get current breakout mode
+    cur_brkout_dict = config_db.get_table('BREAKOUT_CFG')
+    cur_brkout_mode = cur_brkout_mode = cur_brkout_dict[interface_name]["brkout_mode"]
+
+    # Validate Interface and Breakout mode
+    if not _validate_interface_mode(ctx, BREAKOUT_CFG_FILE, interface_name, mode, cur_brkout_mode):
+        raise click.Abort()
+
+    """ Interface Deletion Logic """
+    # Get list of interfaces to be deleted
+    del_ports = get_child_ports(interface_name, cur_brkout_mode, BREAKOUT_CFG_FILE)
+    del_intf_dict = {intf: del_ports[intf]["speed"] for intf in del_ports}
+    del_intf_dict = {intf: del_ports[intf]["speed"] for intf in del_ports}
+
+    if del_intf_dict:
+        """ shut down all the interface before deletion """
+        ret = shutdown_interfaces(ctx, del_intf_dict)
+        if not ret:
+            raise click.Abort()
+        click.echo("\nPorts to be deleted : \n {}".format(json.dumps(del_intf_dict, indent=4)))
+
+    else:
+        click.secho("[ERROR] del_intf_dict is None! No interfaces are there to be deleted", fg='red')
+        raise click.Abort()
+
+
+    """ Interface Addition Logic """
+    # Get list of interfaces to be added
+    add_ports = get_child_ports(interface_name, target_brkout_mode, BREAKOUT_CFG_FILE)
+    add_intf_dict = {intf: add_ports[intf]["speed"] for intf in add_ports}
+
+    if add_intf_dict:
+        click.echo("Ports to be added : \n {}".format(json.dumps(add_intf_dict, indent=4)))
+    else:
+        click.secho("[ERROR] port_dict is None!", fg='red')
+        raise click.Abort()
+
+    """ Special Case: Dont delete those ports  where the current mode and speed of the parent port
+                      remains unchanged to limit the traffic impact """
+
+    click.secho("\nAfter running Logic to limit the impact", fg="cyan", underline=True)
+    matched_item = [intf for intf, speed in del_intf_dict.items() if intf in add_intf_dict.keys() and speed == add_intf_dict[intf]]
+
+    # Remove the interface which remains unchanged from both del_intf_dict and add_intf_dict
+    map(del_intf_dict.pop, matched_item)
+    map(add_intf_dict.pop, matched_item)
+
+    click.secho("\nFinal list of ports to be deleted : \n {} \nFinal list of ports to be added :  \n {}".format(json.dumps(del_intf_dict, indent=4), json.dumps(add_intf_dict, indent=4), fg='green', blink=True))
+    if len(add_intf_dict.keys()) == 0:
+        click.secho("[ERROR] add_intf_dict is None! No interfaces are there to be added", fg='red')
+        raise click.Abort()
+
+    port_dict = {}
+    for intf in add_intf_dict:
+        if intf in add_ports.keys():
+            port_dict[intf] = add_ports[intf]
+
+    # writing JSON object
+    with open('new_port_config.json', 'w') as f:
+        json.dump(port_dict, f, indent=4)
+
+    # Start Interation with Dy Port BreakOut Config Mgmt
+    try:
+        """ Load config for the commands which are capable of change in config DB """
+        cm = load_configMgmt(verbose)
+
+        """ Delete all ports if forced else print dependencies using configMgmt API """
+        final_delPorts = [intf for intf in del_intf_dict.keys()]
+        """ Add ports with its attributes using configMgmt API """
+        final_addPorts = [intf for intf in port_dict.keys()]
+        portJson = dict(); portJson['PORT'] = port_dict
+        # breakout_Ports will abort operation on failure, So no need to check return
+        breakout_Ports(cm, delPorts=final_delPorts, addPorts = final_addPorts, \
+            portJson=portJson, force=force_remove_dependencies, \
+            loadDefConfig=load_predefined_config, verbose=verbose)
+
+        # Set Current Breakout mode in config DB
+        brkout_cfg_keys = config_db.get_keys('BREAKOUT_CFG')
+        if interface_name.decode("utf-8") not in  brkout_cfg_keys:
+            click.secho("[ERROR] {} is not present in 'BREAKOUT_CFG' Table!".\
+                format(interface_name), fg='red')
+            raise click.Abort()
+        config_db.set_entry("BREAKOUT_CFG", interface_name,\
+            {'brkout_mode': target_brkout_mode})
+        click.secho("Breakout process got successfully completed.".\
+            format(interface_name),  fg="cyan", underline=True)
+
+    except Exception as e:
+        click.secho("Failed to break out Port. Error: {}".format(str(e)), \
+            fg='magenta')
+        sys.exit(0)
+
+
 def _get_all_mgmtinterface_keys():
     """Returns list of strings containing mgmt interface keys 
     """
@@ -1400,6 +1651,7 @@ def mgmt_ip_restart_services():
     cmd="systemctl restart ntp-config"
     os.system (cmd)
 
+
 #
 # 'ip' subgroup ('config interface ip ...')
 #
diff --git a/setup.py b/setup.py
index 69717fb186..6bcc4c08c4 100644
--- a/setup.py
+++ b/setup.py
@@ -130,9 +130,9 @@
     # - sonic-config-engine
     # - swsssdk
     # - tabulate
+    # - click
     install_requires=[
         'click-default-group',
-        'click',
         'natsort'
     ],
     setup_requires= [

From 26469596ddc4e2850948cdd307139b452322ffdd Mon Sep 17 00:00:00 2001
From: Sangita Maity <sangitamaity0211@gmail.com>
Date: Wed, 13 May 2020 22:25:40 +0000
Subject: [PATCH 02/14] Fix LGTM issues

Signed-off-by: Sangita Maity <sangitamaity0211@gmail.com>
---
 config/main.py | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/config/main.py b/config/main.py
index 6905619d6a..bf43835276 100755
--- a/config/main.py
+++ b/config/main.py
@@ -20,13 +20,9 @@
 import mlnx
 import nat
 
-from collections import OrderedDict
-import ast
-
-
 # import port config file path
 from sfputil.main import get_platform_and_hwsku
-from portconfig import get_port_config_file_name, parse_platform_json_file
+from portconfig import get_port_config_file_name
 from portconfig import get_child_ports
 
 CONTEXT_SETTINGS = dict(help_option_names=['-h', '--help', '-?'])
@@ -166,7 +162,7 @@ def shutdown_interfaces(ctx, del_intf_dict):
         config_db = ctx.obj['config_db']
         if get_interface_naming_mode() == "alias":
             interface_name = interface_alias_to_name(intf)
-            if intf is None:
+            if interface_name is None:
                 click.echo("[ERROR] interface name is None!")
                 return False
         if interface_name_is_valid(intf) is False:
@@ -2003,7 +1999,7 @@ def breakout(ctx, interface_name, mode, verbose, force_remove_dependencies, load
 
     # Get current breakout mode
     cur_brkout_dict = config_db.get_table('BREAKOUT_CFG')
-    cur_brkout_mode = cur_brkout_mode = cur_brkout_dict[interface_name]["brkout_mode"]
+    cur_brkout_mode = cur_brkout_dict[interface_name]["brkout_mode"]
 
     # Validate Interface and Breakout mode
     if not _validate_interface_mode(ctx, BREAKOUT_CFG_FILE, interface_name, mode, cur_brkout_mode):
@@ -2013,7 +2009,6 @@ def breakout(ctx, interface_name, mode, verbose, force_remove_dependencies, load
     # Get list of interfaces to be deleted
     del_ports = get_child_ports(interface_name, cur_brkout_mode, BREAKOUT_CFG_FILE)
     del_intf_dict = {intf: del_ports[intf]["speed"] for intf in del_ports}
-    del_intf_dict = {intf: del_ports[intf]["speed"] for intf in del_ports}
 
     if del_intf_dict:
         """ shut down all the interface before deletion """

From 8410e7da1766cdf1ee3d53115c7b0e84104a4b66 Mon Sep 17 00:00:00 2001
From: Sangita Maity <sangitamaity0211@gmail.com>
Date: Thu, 14 May 2020 00:33:13 +0000
Subject: [PATCH 03/14] Modifying ClassName and adding Func^Con to warn user
 about extra tables

Signed-off-by: Sangita Maity <sangitamaity0211@gmail.com>
---
 config/main.py | 41 ++++++++++++++++++++++++++++++++---------
 1 file changed, 32 insertions(+), 9 deletions(-)

diff --git a/config/main.py b/config/main.py
index bf43835276..3778145392 100755
--- a/config/main.py
+++ b/config/main.py
@@ -14,7 +14,7 @@
 import ipaddress
 from swsssdk import ConfigDBConnector, SonicV2Connector, SonicDBConfig
 from minigraph import parse_device_desc_xml
-from config_mgmt import configMgmt
+from config_mgmt import ConfigMgmt, ConfigMgmtDPB
 
 import aaa
 import mlnx
@@ -208,17 +208,37 @@ def _validate_interface_mode(ctx, BREAKOUT_CFG_FILE, interface_name, target_brko
         sys.exit(0)
     return True
 
-def load_configMgmt(verbose):
+def load_ConfigMgmt(verbose):
     """ Load config for the commands which are capable of change in config DB. """
     try:
-        # TODO: set allowExtraTables to False, i.e we should have yang models for
-        # each table in Config. [TODO: Create Yang model for each Table]
-        # cm = configMgmt(debug=verbose, allowExtraTables=False)
-        cm = configMgmt(debug=verbose, allowExtraTables=True)
+        cm = ConfigMgmtDPB(debug=verbose)
         return cm
     except Exception as e:
         raise Exception("Failed to load the config. Error: {}".format(str(e)))
 
+"""
+Funtion to warn user about extra tables while Dynamic Port Breakout(DPB).
+confirm: re-confirm from user to proceed.
+Config Tables Without Yang model considered extra tables.
+cm =  instance of config MGMT class.
+"""
+def breakout_warnUser_extraTables(cm, final_delPorts, confirm=True):
+
+    try:
+        # check if any extra tables exist
+        eTables = cm.tablesWithOutYang()
+        if len(eTables):
+            # find relavent tables in extra tables, i.e. one which can have deleted
+            # ports
+            tables = cm.configWithKeys(configIn=eTables, keys=final_delPorts)
+            click.secho("Below Config can not be verified, It may cause harm "\
+                "to the system\n {}".format(json.dumps(tables, indent=2)))
+            click.confirm('Do you wish to Continue?', abort=True)
+    except Exception as e:
+        raise Exception("Failed in breakout_warnUser_extraTables. Error: {}".format(str(e)))
+
+    return
+
 def breakout_Ports(cm, delPorts=list(), addPorts=list(), portJson=dict(), \
     force=False, loadDefConfig=True, verbose=False):
 
@@ -2060,11 +2080,14 @@ def breakout(ctx, interface_name, mode, verbose, force_remove_dependencies, load
     # Start Interation with Dy Port BreakOut Config Mgmt
     try:
         """ Load config for the commands which are capable of change in config DB """
-        cm = load_configMgmt(verbose)
+        cm = load_ConfigMgmt(verbose)
 
-        """ Delete all ports if forced else print dependencies using configMgmt API """
+        """ Delete all ports if forced else print dependencies using ConfigMgmt API """
         final_delPorts = [intf for intf in del_intf_dict.keys()]
-        """ Add ports with its attributes using configMgmt API """
+        """ Warn user if tables without yang models exist and have final_delPorts """
+        breakout_warnUser_extraTables(cm, final_delPorts, confirm=True)
+        # prompt
+        """ Add ports with its attributes using ConfigMgmt API """
         final_addPorts = [intf for intf in port_dict.keys()]
         portJson = dict(); portJson['PORT'] = port_dict
         # breakout_Ports will abort operation on failure, So no need to check return

From 2b410e61bb7dd0f022c33df8cb9b119e11417838 Mon Sep 17 00:00:00 2001
From: Sangita Maity <sangitamaity0211@gmail.com>
Date: Tue, 26 May 2020 21:03:48 +0000
Subject: [PATCH 04/14] Addressed review comments after discussion

Signed-off-by: Sangita Maity <sangitamaity0211@gmail.com>
---
 config/main.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/config/main.py b/config/main.py
index 70a74d1d56..083476bb99 100755
--- a/config/main.py
+++ b/config/main.py
@@ -14,7 +14,7 @@
 import ipaddress
 from swsssdk import ConfigDBConnector, SonicV2Connector, SonicDBConfig
 from minigraph import parse_device_desc_xml
-from config_mgmt import ConfigMgmt, ConfigMgmtDPB
+from config_mgmt import ConfigMgmtDPB
 
 import aaa
 import mlnx
@@ -246,7 +246,7 @@ def breakout_warnUser_extraTables(cm, final_delPorts, confirm=True):
     return
 
 def breakout_Ports(cm, delPorts=list(), addPorts=list(), portJson=dict(), \
-    force=False, loadDefConfig=True, verbose=False):
+    force=False, loadDefConfig=False, verbose=False):
 
     deps, ret = cm.breakOutPort(delPorts=delPorts, addPorts = addPorts, \
                     portJson=portJson, force=force, loadDefConfig=loadDefConfig)

From a1266b83237d8ae4e63657f203e4393b5cb67275 Mon Sep 17 00:00:00 2001
From: Sangita Maity <sangitamaity0211@gmail.com>
Date: Mon, 15 Jun 2020 21:33:10 +0000
Subject: [PATCH 05/14] [DOC] Added documentation for newly added 'config
 interface breakout' sub-command.

Signed-off-by: Sangita Maity <sangitamaity0211@gmail.com>
---
 doc/Command-Reference.md | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md
index e6bb9ba6df..4410df2d4a 100644
--- a/doc/Command-Reference.md
+++ b/doc/Command-Reference.md
@@ -2557,6 +2557,7 @@ This sub-section explains the following list of configuration on the interfaces.
 3) shutdown - to administratively shut down the interface
 4) speed - to set the interface speed
 5) startup - to bring up the administratively shutdown interface
+6) breakout - to set interface breakout mode
 
 From 201904 release onwards, the “config interface” command syntax is changed and the format is as follows:
 
@@ -2859,6 +2860,36 @@ This command is used to configure the mtu for the Physical interface. Use the va
   admin@sonic:~$ sudo config interface mtu Ethernet64 1500
   ```
 
+**config interface breakout**
+
+This command is used to set breakout mode available for user-specified interface.
+kindly use, double tab i.e. <tab><tab> to see the available breakout option customized for each interface provided by the user.
+
+- Usage:
+  ```
+  sudo config interface breakout  --help
+  Usage: config interface breakout [OPTIONS] <interface_name> MODE
+
+    Set interface breakout mode
+
+    Options:
+      -f, --force-remove-dependencies
+                                      Clear all depenedecies internally first.
+      -l, --load-predefined-config    load predefied user configuration (alias,
+                                      lanes, speed etc) first.
+      -y, --yes
+      -v, --verbose                   Enable verbose output
+      -?, -h, --help                  Show this message and exit.
+  ```
+- Example :
+  ```
+  admin@sonic:~$ sudo config interface breakout  Ethernet0 <tab><tab>
+  <tab provides option for breakout mode>
+  1x100G[40G]  2x50G        4x25G[10G]
+
+  admin@sonic:~$ sudo config interface breakout  Ethernet0 4x25G[10G] -f -l -v -y
+  ```
+
 Go Back To [Beginning of the document](#) or [Beginning of this section](#interfaces)
 
 

From 40ecffdd2c4a2625033409c40931c45f48e164cc Mon Sep 17 00:00:00 2001
From: Sangita Maity <sangitamaity0211@gmail.com>
Date: Thu, 18 Jun 2020 22:40:34 +0000
Subject: [PATCH 06/14] Modified  front-panel interface naming validation from
 hardcoded value to general naming convention

Signed-off-by: Sangita Maity <sangitamaity0211@gmail.com>
---
 config/main.py | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/config/main.py b/config/main.py
index 038c8c8a7f..cdac608c8b 100755
--- a/config/main.py
+++ b/config/main.py
@@ -31,7 +31,6 @@
 SONIC_GENERATED_SERVICE_PATH = '/etc/sonic/generated_services.conf'
 SONIC_CFGGEN_PATH = '/usr/local/bin/sonic-cfggen'
 SYSLOG_IDENTIFIER = "config"
-PORT_STR = "Ethernet"
 INTF_KEY = "interfaces"
 
 (platform, hwsku) =  get_platform_and_hwsku()
@@ -172,10 +171,17 @@ def shutdown_interfaces(ctx, del_intf_dict):
             if interface_name is None:
                 click.echo("[ERROR] interface name is None!")
                 return False
+
         if interface_name_is_valid(intf) is False:
             click.echo("[ERROR] Interface name is invalid. Please enter a valid interface name!!")
             return False
-        if intf.startswith(PORT_STR):
+
+        port_dict = config_db.get_table('PORT')
+        if not port_dict:
+            click.echo("port_dict is None!")
+            return False
+
+        if intf in port_dict.keys():
             config_db.mod_entry("PORT", intf, {"admin_status": "down"})
         else:
             click.secho("[ERROR] Could not get the correct interface name, exiting", fg='red')

From 3fd71c463264c78d5915c7e6907880be9ea02cc5 Mon Sep 17 00:00:00 2001
From: Sangita Maity <sangitamaity0211@gmail.com>
Date: Fri, 19 Jun 2020 00:08:58 +0000
Subject: [PATCH 07/14] Addressed Review comments

Signed-off-by: Sangita Maity <sangitamaity0211@gmail.com>
---
 config/main.py | 1 +
 setup.py       | 4 ----
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/config/main.py b/config/main.py
index cdac608c8b..f7491d8abc 100755
--- a/config/main.py
+++ b/config/main.py
@@ -2193,6 +2193,7 @@ def breakout(ctx, interface_name, mode, verbose, force_remove_dependencies, load
             {'brkout_mode': target_brkout_mode})
         click.secho("Breakout process got successfully completed.".\
             format(interface_name),  fg="cyan", underline=True)
+        click.echo("Please note loaded setting will be lost after system reboot. To preserve setting, run `config save`.")
 
     except Exception as e:
         click.secho("Failed to break out Port. Error: {}".format(str(e)), \
diff --git a/setup.py b/setup.py
index 344fb3edc7..09e893e3f3 100644
--- a/setup.py
+++ b/setup.py
@@ -145,12 +145,8 @@
     # - sonic-config-engine
     # - swsssdk
     # - tabulate
-    # - click
     install_requires=[
-        'click-default-group',
-
         'click',
-
         'natsort'
     ],
     setup_requires= [

From 43e2404f41d3139e70289160a391880f593214a5 Mon Sep 17 00:00:00 2001
From: Sangita Maity <sangitamaity0211@gmail.com>
Date: Fri, 19 Jun 2020 18:04:53 +0000
Subject: [PATCH 08/14] Changed the location of finding breakout_cfg_file and
 variable type

Signed-off-by: Sangita Maity <sangitamaity0211@gmail.com>
---
 config/main.py | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/config/main.py b/config/main.py
index f7491d8abc..2db02d31b1 100755
--- a/config/main.py
+++ b/config/main.py
@@ -33,9 +33,6 @@
 SYSLOG_IDENTIFIER = "config"
 INTF_KEY = "interfaces"
 
-(platform, hwsku) =  get_platform_and_hwsku()
-BREAKOUT_CFG_FILE = get_port_config_file_name(hwsku, platform)
-
 VLAN_SUB_INTERFACE_SEPARATOR = '.'
 ASIC_CONF_FILENAME = 'asic.conf'
 DEFAULT_CONFIG_DB_FILE = '/etc/sonic/config_db.json'
@@ -131,6 +128,17 @@ def get_command(self, ctx, cmd_name):
 except KeyError, TypeError:
     raise click.Abort()
 
+#
+# Load breakout config file for Dynamic breakout mode
+#
+
+try:
+    (platform, hwsku) =  get_platform_and_hwsku()
+    breakout_cfg_file = get_port_config_file_name(hwsku, platform)
+except Exception as e :
+    click.secho("Breakout config file not found with error:{}".format(str(e)), fg='red')
+    raise click.Abort()
+
 #
 # Breakout Mode Helper functions
 #
@@ -149,10 +157,10 @@ def _get_option(ctx,args,incomplete):
     global all_mode_options
     interface_name = args[-1]
 
-    if not os.path.isfile(BREAKOUT_CFG_FILE) or not BREAKOUT_CFG_FILE.endswith('.json'):
+    if not os.path.isfile(breakout_cfg_file) or not breakout_cfg_file.endswith('.json'):
         return []
     else:
-        breakout_file_input = readJsonFile(BREAKOUT_CFG_FILE)
+        breakout_file_input = readJsonFile(breakout_cfg_file)
         if interface_name in breakout_file_input[INTF_KEY]:
             breakout_mode_list = [v["breakout_modes"] for i ,v in breakout_file_input[INTF_KEY].items() if i == interface_name][0]
             breakout_mode_options = []
@@ -189,9 +197,9 @@ def shutdown_interfaces(ctx, del_intf_dict):
     return True
 
 
-def _validate_interface_mode(ctx, BREAKOUT_CFG_FILE, interface_name, target_brkout_mode, cur_brkout_mode):
+def _validate_interface_mode(ctx, breakout_cfg_file, interface_name, target_brkout_mode, cur_brkout_mode):
     """ Validate Parent interface and user selected mode before starting deletetion or addition process """
-    breakout_file_input = readJsonFile(BREAKOUT_CFG_FILE)["interfaces"]
+    breakout_file_input = readJsonFile(breakout_cfg_file)["interfaces"]
 
     if interface_name not in breakout_file_input:
         click.secho("[ERROR] {} is not a Parent port. So, Breakout Mode is not available on this port".format(interface_name), fg='red')
@@ -2094,7 +2102,7 @@ def breakout(ctx, interface_name, mode, verbose, force_remove_dependencies, load
 
     """ Set interface breakout mode """
 
-    if not os.path.isfile(BREAKOUT_CFG_FILE) or not BREAKOUT_CFG_FILE.endswith('.json'):
+    if not os.path.isfile(breakout_cfg_file) or not breakout_cfg_file.endswith('.json'):
         click.secho("[ERROR] Breakout feature is not available without platform.json file", fg='red')
         raise click.Abort()
 
@@ -2110,12 +2118,12 @@ def breakout(ctx, interface_name, mode, verbose, force_remove_dependencies, load
     cur_brkout_mode = cur_brkout_dict[interface_name]["brkout_mode"]
 
     # Validate Interface and Breakout mode
-    if not _validate_interface_mode(ctx, BREAKOUT_CFG_FILE, interface_name, mode, cur_brkout_mode):
+    if not _validate_interface_mode(ctx, breakout_cfg_file, interface_name, mode, cur_brkout_mode):
         raise click.Abort()
 
     """ Interface Deletion Logic """
     # Get list of interfaces to be deleted
-    del_ports = get_child_ports(interface_name, cur_brkout_mode, BREAKOUT_CFG_FILE)
+    del_ports = get_child_ports(interface_name, cur_brkout_mode, breakout_cfg_file)
     del_intf_dict = {intf: del_ports[intf]["speed"] for intf in del_ports}
 
     if del_intf_dict:
@@ -2132,7 +2140,7 @@ def breakout(ctx, interface_name, mode, verbose, force_remove_dependencies, load
 
     """ Interface Addition Logic """
     # Get list of interfaces to be added
-    add_ports = get_child_ports(interface_name, target_brkout_mode, BREAKOUT_CFG_FILE)
+    add_ports = get_child_ports(interface_name, target_brkout_mode, breakout_cfg_file)
     add_intf_dict = {intf: add_ports[intf]["speed"] for intf in add_ports}
 
     if add_intf_dict:

From 15831d12ba30a55c9473d6992b2518fcde10fd99 Mon Sep 17 00:00:00 2001
From: Sangita Maity <sangitamaity0211@gmail.com>
Date: Fri, 19 Jun 2020 19:39:42 +0000
Subject: [PATCH 09/14] Deleted space

Signed-off-by: Sangita Maity <sangitamaity0211@gmail.com>
---
 config/main.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config/main.py b/config/main.py
index 2db02d31b1..afad79d7b8 100755
--- a/config/main.py
+++ b/config/main.py
@@ -135,7 +135,7 @@ def get_command(self, ctx, cmd_name):
 try:
     (platform, hwsku) =  get_platform_and_hwsku()
     breakout_cfg_file = get_port_config_file_name(hwsku, platform)
-except Exception as e :
+except Exception as e:
     click.secho("Breakout config file not found with error:{}".format(str(e)), fg='red')
     raise click.Abort()
 

From 7a4bf36105c73a1c10e1e8d973cd3c00052f5911 Mon Sep 17 00:00:00 2001
From: Sangita Maity <sangitamaity0211@gmail.com>
Date: Fri, 19 Jun 2020 22:07:22 +0000
Subject: [PATCH 10/14] Started using common Library function

Signed-off-by: Sangita Maity <sangitamaity0211@gmail.com>
---
 config/main.py | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/config/main.py b/config/main.py
index afad79d7b8..45387f272c 100755
--- a/config/main.py
+++ b/config/main.py
@@ -16,16 +16,13 @@
 from minigraph import parse_device_desc_xml
 from config_mgmt import ConfigMgmtDPB
 from utilities_common.intf_filter import parse_interface_in_filter
+from utilities_common.util_base import UtilHelper
+from portconfig import get_child_ports, get_port_config_file_name
 
 import aaa
 import mlnx
 import nat
 
-# import port config file path
-from sfputil.main import get_platform_and_hwsku
-from portconfig import get_port_config_file_name
-from portconfig import get_child_ports
-
 CONTEXT_SETTINGS = dict(help_option_names=['-h', '--help', '-?'])
 
 SONIC_GENERATED_SERVICE_PATH = '/etc/sonic/generated_services.conf'
@@ -129,11 +126,18 @@ def get_command(self, ctx, cmd_name):
     raise click.Abort()
 
 #
-# Load breakout config file for Dynamic breakout mode
+# Load breakout config file for Dynamic Port Breakout
 #
 
 try:
-    (platform, hwsku) =  get_platform_and_hwsku()
+    # Load the helper class
+    helper = UtilHelper()
+    (platform, hwsku) = helper.get_platform_and_hwsku()
+except Exception as e:
+    click.secho("Failed to get platform and hwsku with error:{}".format(str(e)), fg='red')
+    raise click.Abort()
+
+try:
     breakout_cfg_file = get_port_config_file_name(hwsku, platform)
 except Exception as e:
     click.secho("Breakout config file not found with error:{}".format(str(e)), fg='red')

From fd347e9eeaf00164a9fe280490d58ca5ca5f95db Mon Sep 17 00:00:00 2001
From: Sangita Maity <sangitamaity0211@gmail.com>
Date: Mon, 22 Jun 2020 05:14:55 +0000
Subject: [PATCH 11/14] addressed review comments on breakout mode options
 variable

Signed-off-by: Sangita Maity <sangitamaity0211@gmail.com>
---
 config/main.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/config/main.py b/config/main.py
index 45387f272c..70946c2543 100755
--- a/config/main.py
+++ b/config/main.py
@@ -158,7 +158,7 @@ def readJsonFile(fileName):
 
 def _get_option(ctx,args,incomplete):
     """ Provides dynamic mode option as per user argument i.e. interface name """
-    global all_mode_options
+    all_mode_options = []
     interface_name = args[-1]
 
     if not os.path.isfile(breakout_cfg_file) or not breakout_cfg_file.endswith('.json'):
@@ -173,7 +173,6 @@ def _get_option(ctx,args,incomplete):
             all_mode_options = [str(c) for c in breakout_mode_options if incomplete in c]
             return all_mode_options
 
-
 def shutdown_interfaces(ctx, del_intf_dict):
     """ shut down all the interfaces before deletion """
     for intf in del_intf_dict.keys():

From 5221c8f1aa01c475db8233da70733ed27008b815 Mon Sep 17 00:00:00 2001
From: Sangita Maity <sangitamaity0211@gmail.com>
Date: Fri, 26 Jun 2020 17:54:07 +0000
Subject: [PATCH 12/14] Changed the passing variables due to config_mgmt API
 changes

Signed-off-by: Sangita Maity <sangitamaity0211@gmail.com>
---
 config/main.py | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/config/main.py b/config/main.py
index 70946c2543..84dbb7a24b 100755
--- a/config/main.py
+++ b/config/main.py
@@ -263,11 +263,11 @@ def breakout_warnUser_extraTables(cm, final_delPorts, confirm=True):
 
     return
 
-def breakout_Ports(cm, delPorts=list(), addPorts=list(), portJson=dict(), \
-    force=False, loadDefConfig=False, verbose=False):
+def breakout_Ports(cm, delPorts=list(), portJson=dict(), force=False, \
+    loadDefConfig=False, verbose=False):
 
-    deps, ret = cm.breakOutPort(delPorts=delPorts, addPorts = addPorts, \
-                    portJson=portJson, force=force, loadDefConfig=loadDefConfig)
+    deps, ret = cm.breakOutPort(delPorts=delPorts,  portJson=portJson, \
+                    force=force, loadDefConfig=loadDefConfig)
     # check if DPB failed
     if ret == False:
         if not force and deps:
@@ -2185,13 +2185,12 @@ def breakout(ctx, interface_name, mode, verbose, force_remove_dependencies, load
         final_delPorts = [intf for intf in del_intf_dict.keys()]
         """ Warn user if tables without yang models exist and have final_delPorts """
         breakout_warnUser_extraTables(cm, final_delPorts, confirm=True)
-        # prompt
-        """ Add ports with its attributes using ConfigMgmt API """
-        final_addPorts = [intf for intf in port_dict.keys()]
+
+        # Create a dictionary containing all the added ports with its capabilities like alias, lanes, speed etc.
         portJson = dict(); portJson['PORT'] = port_dict
+
         # breakout_Ports will abort operation on failure, So no need to check return
-        breakout_Ports(cm, delPorts=final_delPorts, addPorts = final_addPorts, \
-            portJson=portJson, force=force_remove_dependencies, \
+        breakout_Ports(cm, delPorts=final_delPorts, portJson=portJson, force=force_remove_dependencies, \
             loadDefConfig=load_predefined_config, verbose=verbose)
 
         # Set Current Breakout mode in config DB

From a7f55b599502573d071f02492d7bca28e1d2fe32 Mon Sep 17 00:00:00 2001
From: Sangita Maity <sangitamaity0211@gmail.com>
Date: Wed, 22 Jul 2020 18:36:11 +0000
Subject: [PATCH 13/14] Addressed review comments

Signed-off-by: Sangita Maity <sangitamaity0211@gmail.com>
---
 config/main.py | 31 ++++++++-----------------------
 1 file changed, 8 insertions(+), 23 deletions(-)

diff --git a/config/main.py b/config/main.py
index 84dbb7a24b..e50c80a37d 100755
--- a/config/main.py
+++ b/config/main.py
@@ -28,13 +28,11 @@
 SONIC_GENERATED_SERVICE_PATH = '/etc/sonic/generated_services.conf'
 SONIC_CFGGEN_PATH = '/usr/local/bin/sonic-cfggen'
 SYSLOG_IDENTIFIER = "config"
-INTF_KEY = "interfaces"
-
 VLAN_SUB_INTERFACE_SEPARATOR = '.'
 ASIC_CONF_FILENAME = 'asic.conf'
 DEFAULT_CONFIG_DB_FILE = '/etc/sonic/config_db.json'
 NAMESPACE_PREFIX = 'asic'
-
+INTF_KEY = "interfaces"
 
 INIT_CFG_FILE = '/etc/sonic/init_cfg.json'
 
@@ -43,13 +41,11 @@
 SYSTEMCTL_ACTION_RESET_FAILED="reset-failed"
 
 DEFAULT_NAMESPACE = ''
-
 CFG_LOOPBACK_PREFIX = "Loopback"
 CFG_LOOPBACK_PREFIX_LEN = len(CFG_LOOPBACK_PREFIX)
 CFG_LOOPBACK_NAME_TOTAL_LEN_MAX = 11
 CFG_LOOPBACK_ID_MAX_VAL = 999
 CFG_LOOPBACK_NO="<0-999>"
-
 # ========================== Syslog wrappers ==========================
 
 def log_debug(msg):
@@ -199,7 +195,6 @@ def shutdown_interfaces(ctx, del_intf_dict):
             return False
     return True
 
-
 def _validate_interface_mode(ctx, breakout_cfg_file, interface_name, target_brkout_mode, cur_brkout_mode):
     """ Validate Parent interface and user selected mode before starting deletetion or addition process """
     breakout_file_input = readJsonFile(breakout_cfg_file)["interfaces"]
@@ -240,14 +235,13 @@ def load_ConfigMgmt(verbose):
     except Exception as e:
         raise Exception("Failed to load the config. Error: {}".format(str(e)))
 
-"""
-Funtion to warn user about extra tables while Dynamic Port Breakout(DPB).
-confirm: re-confirm from user to proceed.
-Config Tables Without Yang model considered extra tables.
-cm =  instance of config MGMT class.
-"""
 def breakout_warnUser_extraTables(cm, final_delPorts, confirm=True):
-
+    """
+    Funtion to warn user about extra tables while Dynamic Port Breakout(DPB).
+    confirm: re-confirm from user to proceed.
+    Config Tables Without Yang model considered extra tables.
+    cm =  instance of config MGMT class.
+    """
     try:
         # check if any extra tables exist
         eTables = cm.tablesWithOutYang()
@@ -260,7 +254,6 @@ def breakout_warnUser_extraTables(cm, final_delPorts, confirm=True):
             click.confirm('Do you wish to Continue?', abort=True)
     except Exception as e:
         raise Exception("Failed in breakout_warnUser_extraTables. Error: {}".format(str(e)))
-
     return
 
 def breakout_Ports(cm, delPorts=list(), portJson=dict(), force=False, \
@@ -279,10 +272,8 @@ def breakout_Ports(cm, delPorts=list(), portJson=dict(), force=False, \
         else:
             print("[ERROR] Port breakout Failed!!! Opting Out")
             raise click.Abort()
-
         return
 
-
 #
 # Helper functions
 #
@@ -2088,11 +2079,10 @@ def speed(ctx, interface_name, interface_speed, verbose):
         command += " -vv"
     run_command(command, display_cmd=verbose)
 
-
-
 #
 # 'breakout' subcommand
 #
+
 @interface.command()
 @click.argument('interface_name', metavar='<interface_name>', required=True)
 @click.argument('mode', required=True, type=click.STRING, autocompletion=_get_option)
@@ -2102,9 +2092,7 @@ def speed(ctx, interface_name, interface_speed, verbose):
 @click.option('-v', '--verbose', is_flag=True, help="Enable verbose output")
 @click.pass_context
 def breakout(ctx, interface_name, mode, verbose, force_remove_dependencies, load_predefined_config):
-
     """ Set interface breakout mode """
-
     if not os.path.isfile(breakout_cfg_file) or not breakout_cfg_file.endswith('.json'):
         click.secho("[ERROR] Breakout feature is not available without platform.json file", fg='red')
         raise click.Abort()
@@ -2140,7 +2128,6 @@ def breakout(ctx, interface_name, mode, verbose, force_remove_dependencies, load
         click.secho("[ERROR] del_intf_dict is None! No interfaces are there to be deleted", fg='red')
         raise click.Abort()
 
-
     """ Interface Addition Logic """
     # Get list of interfaces to be added
     add_ports = get_child_ports(interface_name, target_brkout_mode, breakout_cfg_file)
@@ -2210,7 +2197,6 @@ def breakout(ctx, interface_name, mode, verbose, force_remove_dependencies, load
             fg='magenta')
         sys.exit(0)
 
-
 def _get_all_mgmtinterface_keys():
     """Returns list of strings containing mgmt interface keys 
     """
@@ -2233,7 +2219,6 @@ def mgmt_ip_restart_services():
     cmd="systemctl restart ntp-config"
     os.system (cmd)
 
-
 #
 # 'mtu' subcommand
 #

From 16cdf4d8067631bea8a8bc939fde34eb4fb33bbd Mon Sep 17 00:00:00 2001
From: Sangita Maity <sangitamaity0211@gmail.com>
Date: Wed, 22 Jul 2020 19:09:59 +0000
Subject: [PATCH 14/14] Addressed few more review comments

Signed-off-by: Sangita Maity <sangitamaity0211@gmail.com>
---
 config/main.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/config/main.py b/config/main.py
index e50c80a37d..75eb7e2451 100755
--- a/config/main.py
+++ b/config/main.py
@@ -196,7 +196,7 @@ def shutdown_interfaces(ctx, del_intf_dict):
     return True
 
 def _validate_interface_mode(ctx, breakout_cfg_file, interface_name, target_brkout_mode, cur_brkout_mode):
-    """ Validate Parent interface and user selected mode before starting deletetion or addition process """
+    """ Validate Parent interface and user selected mode before starting deletion or addition process """
     breakout_file_input = readJsonFile(breakout_cfg_file)["interfaces"]
 
     if interface_name not in breakout_file_input:
@@ -237,7 +237,7 @@ def load_ConfigMgmt(verbose):
 
 def breakout_warnUser_extraTables(cm, final_delPorts, confirm=True):
     """
-    Funtion to warn user about extra tables while Dynamic Port Breakout(DPB).
+    Function to warn user about extra tables while Dynamic Port Breakout(DPB).
     confirm: re-confirm from user to proceed.
     Config Tables Without Yang model considered extra tables.
     cm =  instance of config MGMT class.
@@ -264,13 +264,13 @@ def breakout_Ports(cm, delPorts=list(), portJson=dict(), force=False, \
     # check if DPB failed
     if ret == False:
         if not force and deps:
-            print("Dependecies Exist. No further action will be taken")
-            print("*** Printing dependecies ***")
+            click.echo("Dependecies Exist. No further action will be taken")
+            click.echo("*** Printing dependecies ***")
             for dep in deps:
-                print(dep)
+                click.echo(dep)
             sys.exit(0)
         else:
-            print("[ERROR] Port breakout Failed!!! Opting Out")
+            click.echo("[ERROR] Port breakout Failed!!! Opting Out")
             raise click.Abort()
         return