From 4f5f45ebb7177cf2f7254012e7fd37a71c69b5f3 Mon Sep 17 00:00:00 2001 From: Sangita Maity 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 Integrating sonicMgmt API Signed-off-by: Sangita Maity --- 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='', 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 Date: Wed, 13 May 2020 22:25:40 +0000 Subject: [PATCH 02/14] Fix LGTM issues Signed-off-by: Sangita Maity --- 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 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 --- 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 Date: Tue, 26 May 2020 21:03:48 +0000 Subject: [PATCH 04/14] Addressed review comments after discussion Signed-off-by: Sangita Maity --- 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 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 --- 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. 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] 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 + + 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 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 --- 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 Date: Fri, 19 Jun 2020 00:08:58 +0000 Subject: [PATCH 07/14] Addressed Review comments Signed-off-by: Sangita Maity --- 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 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 --- 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 Date: Fri, 19 Jun 2020 19:39:42 +0000 Subject: [PATCH 09/14] Deleted space Signed-off-by: Sangita Maity --- 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 Date: Fri, 19 Jun 2020 22:07:22 +0000 Subject: [PATCH 10/14] Started using common Library function Signed-off-by: Sangita Maity --- 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 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 --- 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 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 --- 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 Date: Wed, 22 Jul 2020 18:36:11 +0000 Subject: [PATCH 13/14] Addressed review comments Signed-off-by: Sangita Maity --- 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='', 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 Date: Wed, 22 Jul 2020 19:09:59 +0000 Subject: [PATCH 14/14] Addressed few more review comments Signed-off-by: Sangita Maity --- 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