From 042254e1f9f08c9b4abe22c5ceab8148166033a8 Mon Sep 17 00:00:00 2001 From: rajendra-dendukuri <47423477+rajendra-dendukuri@users.noreply.github.com> Date: Fri, 4 Dec 2020 12:10:43 -0500 Subject: [PATCH 1/6] Kdump improvements (#1284) - Added support for reporting kdump reboot cause to detect and report a kernel crash - Added additional commands to sonic-kdump-config wrapper script to dump Kdump configuration and status information in JSON format - Automatically save Kdump configuration in startup configuration file. This is an added convenience to the user as Kdump configuration changes require a system reboot and user is expected to save the configuration so that they are activated post reboot. - Additional bug fixes and improvements to allow hostcfgd to process Kdump configuration changes in ConfigDB. sonic-kdump-config script will write to ConfigDB. - Moved kdump CLI commands to a dedicated file Signed-off-by: Rajendra Dendukuri --- config/kdump.py | 44 ++++++ config/main.py | 46 +----- scripts/reboot | 21 ++- scripts/sonic-kdump-config | 287 +++++++++++++++++++++++++++++++++---- show/kdump.py | 10 +- show/main.py | 1 + 6 files changed, 320 insertions(+), 89 deletions(-) create mode 100644 config/kdump.py diff --git a/config/kdump.py b/config/kdump.py new file mode 100644 index 0000000000..c99a6485b6 --- /dev/null +++ b/config/kdump.py @@ -0,0 +1,44 @@ +import os +import click +import utilities_common.cli as clicommon +from swsssdk import ConfigDBConnector + +@click.group(cls=clicommon.AbbreviationGroup, name="kdump") +def kdump(): + """ Configure kdump """ + if os.geteuid() != 0: + exit("Root privileges are required for this operation") + +@kdump.command() +def disable(): + """Disable kdump operation""" + config_db = ConfigDBConnector() + if config_db is not None: + config_db.connect() + config_db.mod_entry("KDUMP", "config", {"enabled": "false"}) + +@kdump.command() +def enable(): + """Enable kdump operation""" + config_db = ConfigDBConnector() + if config_db is not None: + config_db.connect() + config_db.mod_entry("KDUMP", "config", {"enabled": "true"}) + +@kdump.command() +@click.argument('kdump_memory', metavar='', required=True) +def memory(kdump_memory): + """Set memory allocated for kdump capture kernel""" + config_db = ConfigDBConnector() + if config_db is not None: + config_db.connect() + config_db.mod_entry("KDUMP", "config", {"memory": kdump_memory}) + +@kdump.command('num-dumps') +@click.argument('kdump_num_dumps', metavar='', required=True, type=int) +def num_dumps(kdump_num_dumps): + """Set max number of dump files for kdump""" + config_db = ConfigDBConnector() + if config_db is not None: + config_db.connect() + config_db.mod_entry("KDUMP", "config", {"num_dumps": kdump_num_dumps}) diff --git a/config/main.py b/config/main.py index b832c45cb9..2316d7e38c 100755 --- a/config/main.py +++ b/config/main.py @@ -28,6 +28,7 @@ from . import chassis_modules from . import console from . import feature +from . import kdump from . import kube from . import mlnx from . import muxcable @@ -878,6 +879,7 @@ def config(ctx): config.add_command(chassis_modules.chassis_modules) config.add_command(console.console) config.add_command(feature.feature) +config.add_command(kdump.kdump) config.add_command(kube.kubernetes) config.add_command(muxcable.muxcable) config.add_command(nat.nat) @@ -1901,50 +1903,6 @@ def shutdown(): """Shut down BGP session(s)""" pass -@config.group(cls=clicommon.AbbreviationGroup) -def kdump(): - """ Configure kdump """ - if os.geteuid() != 0: - exit("Root privileges are required for this operation") - -@kdump.command() -def disable(): - """Disable kdump operation""" - config_db = ConfigDBConnector() - if config_db is not None: - config_db.connect() - config_db.mod_entry("KDUMP", "config", {"enabled": "false"}) - clicommon.run_command("sonic-kdump-config --disable") - -@kdump.command() -def enable(): - """Enable kdump operation""" - config_db = ConfigDBConnector() - if config_db is not None: - config_db.connect() - config_db.mod_entry("KDUMP", "config", {"enabled": "true"}) - clicommon.run_command("sonic-kdump-config --enable") - -@kdump.command() -@click.argument('kdump_memory', metavar='', required=True) -def memory(kdump_memory): - """Set memory allocated for kdump capture kernel""" - config_db = ConfigDBConnector() - if config_db is not None: - config_db.connect() - config_db.mod_entry("KDUMP", "config", {"memory": kdump_memory}) - clicommon.run_command("sonic-kdump-config --memory %s" % kdump_memory) - -@kdump.command('num-dumps') -@click.argument('kdump_num_dumps', metavar='', required=True, type=int) -def num_dumps(kdump_num_dumps): - """Set max number of dump files for kdump""" - config_db = ConfigDBConnector() - if config_db is not None: - config_db.connect() - config_db.mod_entry("KDUMP", "config", {"num_dumps": kdump_num_dumps}) - clicommon.run_command("sonic-kdump-config --num_dumps %d" % kdump_num_dumps) - # 'all' subcommand @shutdown.command() @click.option('-v', '--verbose', is_flag=True, help="Enable verbose output") diff --git a/scripts/reboot b/scripts/reboot index 0d7d400c57..23092a56b6 100755 --- a/scripts/reboot +++ b/scripts/reboot @@ -1,19 +1,28 @@ #!/bin/bash +DEVPATH="/usr/share/sonic/device" +PLAT_REBOOT="platform_reboot" +REBOOT_CAUSE_FILE="/host/reboot-cause/reboot-cause.txt" +REBOOT_TIME=$(date) # Reboot immediately if we run the kdump capture kernel VMCORE_FILE=/proc/vmcore if [ -e $VMCORE_FILE -a -s $VMCORE_FILE ]; then - echo "We have a /proc/vmcore, then we just kdump'ed" - /sbin/reboot + echo "We have a /proc/vmcore, then we just kdump'ed" + echo "User issued 'kdump' command [User: kdump, Time: ${REBOOT_TIME}]" > ${REBOOT_CAUSE_FILE} + sync + PLATFORM=$(grep -oP 'platform=\K\S+' /proc/cmdline) + if [ ! -z "${PLATFORM}" -a -x ${DEVPATH}/${PLATFORM}/${PLAT_REBOOT} ]; then + exec ${DEVPATH}/${PLATFORM}/${PLAT_REBOOT} + fi + # If no platform-specific reboot tool, just run /sbin/reboot + /sbin/reboot + echo 1 > /proc/sys/kernel/sysrq + echo b > /proc/sysrq-trigger fi REBOOT_USER=$(logname) -REBOOT_TIME=$(date) PLATFORM=$(sonic-cfggen -H -v DEVICE_METADATA.localhost.platform) ASIC_TYPE=$(sonic-cfggen -y /etc/sonic/sonic_version.yml -v asic_type) -DEVPATH="/usr/share/sonic/device" -PLAT_REBOOT="platform_reboot" -REBOOT_CAUSE_FILE="/host/reboot-cause/reboot-cause.txt" VERBOSE=no EXIT_NEXT_IMAGE_NOT_EXISTS=4 EXIT_SONIC_INSTALLER_VERIFY_REBOOT=21 diff --git a/scripts/sonic-kdump-config b/scripts/sonic-kdump-config index 0aef37b803..7dbde1d708 100755 --- a/scripts/sonic-kdump-config +++ b/scripts/sonic-kdump-config @@ -22,11 +22,13 @@ import argparse import shlex import os import subprocess +import json from swsssdk import ConfigDBConnector aboot_cfg_template ="/host/image-%s/kernel-cmdline" grub_cfg = "/host/grub/grub.cfg" kdump_cfg = "/etc/default/kdump-tools" +kdump_mem_file = "/sys/kernel/kexec_crash_size" machine_cfg = "/host/machine.conf" ## Same as print(), but output to stderr instead of stdout @@ -80,6 +82,14 @@ def get_current_image(): print_err("Unable to locate current SONiC image") sys.exit(1) +## Read allocated memory size +def get_crash_kernel_size(): + try: + with open(kdump_mem_file, 'r') as fp: + return fp.read().rstrip('\n') + except Exception as e: + return "0" + ## Search which SONiC image is the Next image def get_next_image(): (rc, img, err_str) = run_command("sonic-installer list | grep 'Next: ' | cut -d '-' -f 3-", use_shell=True); @@ -129,7 +139,7 @@ def search_for_crash_kernel(where): ## Search for "crashkernel=X" in /proc/cmdline # -# @return Return the X from "crashkernel=X" in /proc/cmdline +# @return Return the X from "crashkernel=X" in /proc/cmdline # None in case "crashkernel=" is not found def search_for_crash_kernel_in_cmdline(): try: @@ -139,13 +149,88 @@ def search_for_crash_kernel_in_cmdline(): sys.exit(1) return search_for_crash_kernel(cmdline[0]) +def cmd_dump_db(): + print("Read Kdump configuration from db:") + config_db = ConfigDBConnector(use_unix_socket_path=True) + if config_db is not None: + config_db.connect() + table_data = config_db.get_table('KDUMP') + if table_data is not None: + config_data = table_data.get('config') + if config_data is not None: + print(config_data) + else: + print("empty") + +def cmd_dump_config_json(): + kdump_enabled = get_kdump_administrative_mode() + kdump_memory = get_kdump_memory() + kdump_num_dumps = get_kdump_num_dumps() + data = { "enable" : kdump_enabled, \ + "memory" : kdump_memory, \ + "max-dumps" : int(kdump_num_dumps) } + print(json.dumps(data, indent=4)) + +def cmd_dump_kdump_records_json(): + data = dict() + log_files = dict() + (rc_logs, crash_log_filenames, err_str) = run_command("find /var/crash/ -name 'dmesg.*'", use_shell=False); + if rc_logs == 0: + crash_log_filenames.sort(reverse=True) + for f in crash_log_filenames: + log_files[f[11:23]] = f + + core_files = dict() + (rc_cores, crash_vmcore_filenames, err_str) = run_command("find /var/crash/ -name 'kdump.*'", use_shell=False); + if rc_cores == 0: + crash_vmcore_filenames.sort(reverse=True) + for f in crash_vmcore_filenames: + core_files[f[11:23]] = f + + kdump_records = dict() + for k in sorted(log_files.keys(), reverse=True): + try: + f = open(log_files[k], "r") + log_content = f.read() + f.close() + except Exception as e: + log_content = "" + log_lines = log_content.split("\n")[-100:] + kdump_records[k] = { "id" : k, \ + "vmcore-diagnostic-message" : "\n".join(log_lines), \ + "vmcore-diagnostic-message-file" : log_files[k], \ + "vmcore" : "Kernel vmcore not found" } + + for k in sorted(core_files.keys(), reverse=True): + if kdump_records.get(k): + kdump_records[k]["vmcore"] = core_files[k] + else: + kdump_records[k] = { "id" : k, \ + "vmcore-dmesg-file" : "Kernel crash log not found", \ + "vmcore-dmesg" : "", \ + "vmcore" : core_files[k] } + data["kdump-record"] = kdump_records + print(json.dumps(data, indent=4)) + +def cmd_dump_status_json(): + kdump_enabled = get_kdump_administrative_mode() + kdump_oper_state = get_kdump_oper_mode(kdump_enabled) + kdump_memory = get_kdump_memory() + kdump_num_dumps = get_kdump_num_dumps() + data = { "enable" : kdump_enabled, \ + "current-state" : kdump_oper_state, \ + "memory" : kdump_memory, \ + "allocated-memory" : get_crash_kernel_size(), \ + "max-dumps" : int(kdump_num_dumps) } + print(json.dumps(data, indent=4)) + ## Query current configuration to check if kdump is enabled or disabled # # @return True if kdump is enable, False if kdump is not enabled # We read the running configuration to check if kdump is enabled or not def get_kdump_administrative_mode(): kdump_is_enabled = False - config_db = ConfigDBConnector() + config_db = ConfigDBConnector(use_unix_socket_path=True) if config_db is not None: config_db.connect() table_data = config_db.get_table('KDUMP') @@ -160,37 +245,53 @@ def get_kdump_administrative_mode(): else: return False +def get_kdump_oper_mode(kdump_enabled): + (rc, lines, err_str) = run_command("/usr/sbin/kdump-config status", use_shell=False); + if len(lines) >= 1 and ": ready to kdump" in lines[0]: + use_kdump_in_cfg = read_use_kdump() + if use_kdump_in_cfg: + return('Ready') + else: + return('Not Ready') + elif not kdump_enabled: + return('Disabled') + else: + return('Ready after Reboot') + ## Query current configuration for kdump memory # # @return The current memory string used for kdump (read from running configuration) def get_kdump_memory(): - (rc, lines, err_str) = run_command("show kdump memory", use_shell=False) - try: - if rc == 0 and len(lines) == 1: - p = lines[0].find(': ') - if p != -1: - #print('XXX') - #print(lines[0][p+2:]) - #print('XXX') - return lines[0][p+2:] - except Exception: - pass - return "0M-2G:256M,2G-4G:320M,4G-8G:384M,8G-:448M" + memory = "0M-2G:256M,2G-4G:320M,4G-8G:384M,8G-:448M" + config_db = ConfigDBConnector(use_unix_socket_path=True) + if config_db is not None: + config_db.connect() + table_data = config_db.get_table('KDUMP') + if table_data is not None: + config_data = table_data.get('config') + if config_data is not None: + mem = config_data.get('memory') + if mem: + memory = mem + return memory ## Query current configuration for kdump num_dumps # # @return The maximum number of kernel dump files stored locally # (read from running configuration) def get_kdump_num_dumps(): - (rc, lines, err_str) = run_command("show kdump num_dumps", use_shell=False) - try: - if rc == 0 and len(lines) == 1: - p = lines[0].find(': ') - if p != -1: - return int(lines[0][p+2:]) - except Exception: - pass - return 3 + num_dumps = 3 + config_db = ConfigDBConnector(use_unix_socket_path=True) + if config_db is not None: + config_db.connect() + table_data = config_db.get_table('KDUMP') + if table_data is not None: + config_data = table_data.get('config') + if config_data is not None: + num = config_data.get('num_dumps') + if num: + num_dumps = num + return num_dumps ## Read current value for USE_KDUMP in kdump config file # @@ -214,6 +315,11 @@ def write_use_kdump(use_kdump): (rc, lines, err_str) = run_command("/bin/sed -i -e 's/USE_KDUMP=.*/USE_KDUMP=%s/' %s" % (use_kdump, kdump_cfg), use_shell=False); if rc == 0 and type(lines) == list and len(lines) == 0: use_kdump_in_cfg = read_use_kdump() + if use_kdump == 0: + (rc, lines, err_str) = run_command("/usr/sbin/kdump-config unload", use_shell=False) + if rc != 0: + print_err("Error Unable to unload the Kdump kernel '%s'", err_str) + sys.exit(1) if use_kdump_in_cfg != use_kdump: print_err("Unable to write USE_KDUMP into %s" % kdump_cfg) sys.exit(1) @@ -250,6 +356,39 @@ def write_num_dumps(num_dumps): print_err("Error while writing KDUMP_NUM_DUMPS into %s" % kdump_cfg) sys.exit(1) +## Save kdump configuration into the startup configuration +# @kdump_enabled Administrative mode (False/True) +# @memory Amount of memory allocated for the capture kernel +# @num_dumps Max number of core files saved locally +def save_config(kdump_enabled, memory, num_dumps): + + configdb_fname = '/etc/sonic/config_db.json' + + # Read current configuration + if not os.path.exists(configdb_fname): + print_err("Startup configuration not found, Kdump configuration is not saved") + return + else: + try: + with open(configdb_fname) as json_file: + data = json.load(json_file) + except Exception as e: + print_err("Error [%s] while reading startup configuration" % e) + return + + # Rewrite configuration + try: + kdump_data = {'config': {'enabled': '', 'num_dumps': '', 'memory': ''}} + (kdump_data['config'])['enabled'] = str(kdump_enabled).lower() + (kdump_data['config'])['num_dumps'] = str(num_dumps) + (kdump_data['config'])['memory'] = memory + data['KDUMP'] = kdump_data + with open(configdb_fname, 'w') as fp: + json.dump(data, fp, indent=4, sort_keys=False) + print("Kdump configuration has been updated in the startup configuration") + except Exception as e: + print_err("Error [%s] while saving Kdump configuration to startup configuration" % e) + ## Enable kdump # # @param verbose If True, the function will display a few additinal information @@ -293,11 +432,42 @@ def kdump_enable(verbose, kdump_enabled, memory, num_dumps, image, cmdline_file) if changed: rewrite_cfg(lines, cmdline_file) + save_config(kdump_enabled, memory, num_dumps) + else: + save_config(kdump_enabled, memory, num_dumps) write_use_kdump(1) + if crash_kernel_in_cmdline is not None: + (rc, lines, err_str) = run_command("/usr/sbin/kdump-config load", use_shell=False) + if rc != 0: + print_err("Error Unable to unload Kdump the kernel '%s'", err_str) + sys.exit(1) return changed +## Read kdump configuration saved in the startup configuration file +# +# @param config_param If True, the function will display a few additional information +# @return Value of the configuration parameter saved in the startup configuration file +# @return None if the startup configuration file does not exist or the kdump +# configuration parameter is not present in the file. +def get_kdump_config_json(config_param): + configdb_fname = '/etc/sonic/config_db.json' + + # Read the startup configuration file + if not os.path.exists(configdb_fname): + return None + else: + try: + with open(configdb_fname) as json_file: + data = json.load(json_file) + if data.get("KDUMP") is not None and \ + data.get("KDUMP").get("config") is not None: + return data.get("KDUMP").get("config").get(config_param) + except Exception as e: + print_err("Error [%s] while reading startup configuration" % e) + return None + ## Command: Enable kdump # # @param verbose If True, the function will display a few additinal information @@ -353,6 +523,30 @@ def kdump_disable(verbose, kdump_enabled, memory, num_dumps, image, cmdline_file if changed: rewrite_cfg(lines, cmdline_file) + if not os.path.exists('/etc/sonic/config_db.json'): + print_err("Startup configuration not found, Kdump configuration is not saved") + return False + + current_img = get_current_image(); + if verbose: + print("Current image=[%s]\n" % current_img) + lines = [line.rstrip('\n') for line in open(grub_cfg)] + current_img_index = locate_image(lines, "loop=image-"+current_img) + + changed = False + curr_crash_kernel_mem = search_for_crash_kernel(lines[current_img_index]) + if curr_crash_kernel_mem is None: + print("Kdump is already disabled") + else: + lines[current_img_index] = lines[current_img_index].replace("crashkernel="+curr_crash_kernel_mem, "") + changed = True + if verbose: + print("Removed [%s] in grub.cfg" % ("crashkernel="+curr_crash_kernel_mem)) + + if changed: + rewrite_grub_cfg(lines, grub_cfg) + save_config(kdump_enabled, memory, num_dumps) + return changed ## Command: Disable kdump @@ -379,7 +573,7 @@ def cmd_kdump_disable(verbose, image=get_current_image()): ## Command: Set / Get memory # # @param verbose If True, the function will display a few additional information -# @param memory If not None, new value to set. +# @param memory If not None, new value to set. # If None, display current value read from running configuration def cmd_kdump_memory(verbose, memory): if memory is None: @@ -389,14 +583,19 @@ def cmd_kdump_memory(verbose, memory): use_kdump_in_cfg = read_use_kdump() if use_kdump_in_cfg: crash_kernel_in_cmdline = search_for_crash_kernel_in_cmdline() - if memory != crash_kernel_in_cmdline: - cmd_kdump_enable(verbose, False) - print("kdump updated memory will be only operational after the system reboots") + memory_in_db = get_kdump_memory() + memory_in_json = get_kdump_config_json("memory") + if memory != crash_kernel_in_cmdline or memory != memory_in_db or memory != memory_in_json: + cmd_kdump_enable(verbose) + print("Kdump updated memory will be only operational after the system reboots") + else: + num_dumps = get_kdump_num_dumps() + save_config(False, memory, num_dumps) ## Command: Set / Get num_dumps # # @param verbose If True, the function will display a few additional information -# @param memory If not None, new value to set. +# @param memory If not None, new value to set. # If None, display current value read from running configuration def cmd_kdump_num_dumps(verbose, num_dumps): if num_dumps is None: @@ -404,6 +603,9 @@ def cmd_kdump_num_dumps(verbose, num_dumps): print('\n'.join(lines)) else: write_num_dumps(num_dumps) + kdump_enabled = get_kdump_administrative_mode() + kdump_memory = get_kdump_memory() + save_config(kdump_enabled, kdump_memory, num_dumps) ## Command: Display kdump status def cmd_kdump_status(): @@ -459,7 +661,7 @@ def cmd_kdump_file(num_lines, filename): fname = None nb_dumps = get_nb_dumps_in_var_crash() if nb_dumps == 0: - print("There is no kernel core file stored") + print("Kernel crash log not found") else: (rc, lines, err_str) = run_command("find /var/crash/ -name 'dmesg.*'", use_shell=False); if rc == 0 and nb_dumps == len(lines): @@ -471,7 +673,7 @@ def cmd_kdump_file(num_lines, filename): else: print("Invalid record number - Should be between 1 and %d" % nb_dumps) sys.exit(1) - fname = lines[num-1] + fname = sorted(lines, reverse=True)[num-1] else: lines.sort(reverse=True) for x in lines: @@ -496,6 +698,10 @@ def main(): parser = argparse.ArgumentParser(description="kdump configuration and status tool", formatter_class=argparse.RawTextHelpFormatter) + # Dump kdump db configuration + parser.add_argument('--dump-db', action='store_true', + help='Dump kdump db configuration') + # Enable kdump on Current image parser.add_argument('--enable', action='store_true', help='Enable kdump (Current image)') @@ -512,6 +718,19 @@ def main(): parser.add_argument('--status', action='store_true', help='Show kdump status') + # kdump status on Current Image + parser.add_argument('--status-json', action='store_true', + help='Show kdump status in json format') + + # kdump status on Current Image + parser.add_argument('--kdump-records-json', action='store_true', + help='Show kdump records in json format') + + # kdump config on Current Image + # kdump config on Current Image + parser.add_argument('--config-json', action='store_true', + help='Show kdump config in json format') + # Maximum number of kernel core dumps parser.add_argument('--num_dumps', nargs='?', type=int, action='store', default=False, help='Maximum number of kernel dump files stored') @@ -563,6 +782,14 @@ def main(): cmd_kdump_files() elif options.file: cmd_kdump_file(options.lines, options.file[0]) + elif options.dump_db: + cmd_dump_db() + elif options.status_json: + cmd_dump_status_json() + elif options.config_json: + cmd_dump_config_json() + elif options.kdump_records_json: + cmd_dump_kdump_records_json() else: parser.print_help() sys.exit(1) diff --git a/show/kdump.py b/show/kdump.py index 8710736891..9039162836 100644 --- a/show/kdump.py +++ b/show/kdump.py @@ -2,16 +2,14 @@ import utilities_common.cli as clicommon from swsssdk import ConfigDBConnector - # # 'kdump command ("show kdump ...") # -@click.group(cls=clicommon.AliasedGroup) +@click.group(cls=clicommon.AliasedGroup, name="kdump") def kdump(): """Show kdump configuration, status and information """ pass - @kdump.command('enabled') def enabled(): """Show if kdump is enabled or disabled""" @@ -30,7 +28,6 @@ def enabled(): else: click.echo("kdump is disabled") - @kdump.command('status') def status(): """Show kdump status""" @@ -39,7 +36,6 @@ def status(): clicommon.run_command("sonic-kdump-config --num_dumps") clicommon.run_command("sonic-kdump-config --files") - @kdump.command('memory') def memory(): """Show kdump memory information""" @@ -56,7 +52,6 @@ def memory(): kdump_memory = kdump_memory_from_db click.echo("Memory Reserved: {}".format(kdump_memory)) - @kdump.command('num_dumps') def num_dumps(): """Show kdump max number of dump files""" @@ -73,20 +68,17 @@ def num_dumps(): kdump_num_dumps = kdump_num_dumps_from_db click.echo("Maximum number of Kernel Core files Stored: {}".format(kdump_num_dumps)) - @kdump.command('files') def files(): """Show kdump kernel core dump files""" clicommon.run_command("sonic-kdump-config --files") - @kdump.command() @click.argument('record', required=True) @click.argument('lines', metavar='', required=False) def log(record, lines): """Show kdump kernel core dump file kernel log""" cmd = "sonic-kdump-config --file {}".format(record) - if lines is not None: cmd += " --lines {}".format(lines) diff --git a/show/main.py b/show/main.py index 60a3e6b683..d163ac930a 100755 --- a/show/main.py +++ b/show/main.py @@ -143,6 +143,7 @@ def cli(ctx): cli.add_command(dropcounters.dropcounters) cli.add_command(feature.feature) cli.add_command(fgnhg.fgnhg) +cli.add_command(kdump.kdump) cli.add_command(interfaces.interfaces) cli.add_command(kdump.kdump) cli.add_command(kube.kubernetes) From 326e534bb230e0f914ff02c5ece91e4a0afe7916 Mon Sep 17 00:00:00 2001 From: Vaibhav Hemant Dixit Date: Mon, 7 Dec 2020 09:00:39 -0800 Subject: [PATCH 2/6] Fast-reboot: add a new flag to ignore ASIC config checksum verification failures (#1292) To fix the issue Azure/sonic-buildimage#5972 warm-reboot with force flag ignores ASIC config checksum mismatch along with orchagent RESTARTCHECK failure. This commit accounts for a use case when checksum-verification should be ignored but orchagent pause check should not be ignored. The change is to add a new option in fast-reboot script to ignore ASIC checksum verification failures. --- scripts/fast-reboot | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/scripts/fast-reboot b/scripts/fast-reboot index 7d7b8b832f..58ad650ffb 100755 --- a/scripts/fast-reboot +++ b/scripts/fast-reboot @@ -9,6 +9,7 @@ REBOOT_SCRIPT_NAME=$(basename $0) REBOOT_TYPE="${REBOOT_SCRIPT_NAME}" VERBOSE=no FORCE=no +IGNORE_ASIC=no STRICT=no REBOOT_METHOD="/sbin/kexec -e" ASSISTANT_IP_LIST="" @@ -55,6 +56,7 @@ function showHelpAndExit() echo " -h,-? : get this help" echo " -v : turn on verbose" echo " -f : force execution" + echo " -i : ignore MD5-checksum-verification of ASIC configuration files" echo " -r : reboot with /sbin/reboot" echo " -k : reboot with /sbin/kexec -e [default]" echo " -x : execute script with -x flag" @@ -67,7 +69,7 @@ function showHelpAndExit() function parseOptions() { - while getopts "vfh?rkxc:s" opt; do + while getopts "vfih?rkxc:s" opt; do case ${opt} in h|\? ) showHelpAndExit @@ -78,6 +80,9 @@ function parseOptions() f ) FORCE=yes ;; + i ) + IGNORE_ASIC=yes + ;; r ) REBOOT_METHOD="/sbin/reboot" ;; @@ -335,7 +340,7 @@ function reboot_pre_check() ${ASIC_CONFIG_CHECK_SCRIPT} || ASIC_CONFIG_CHECK_EXIT_CODE=$? if [[ "${ASIC_CONFIG_CHECK_EXIT_CODE}" != "${ASIC_CONFIG_CHECK_SUCCESS}" ]]; then - if [[ x"${FORCE}" == x"yes" ]]; then + if [[ x"${IGNORE_ASIC}" == x"yes" ]]; then debug "Ignoring ASIC config checksum failure..." else error "ASIC config may have changed: errno=${ASIC_CONFIG_CHECK_EXIT_CODE}" From 2f263c4e4a5f115cbdc6945d3eb8a4d1f5a3be1b Mon Sep 17 00:00:00 2001 From: shlomibitton <60430976+shlomibitton@users.noreply.github.com> Date: Tue, 8 Dec 2020 12:21:04 +0200 Subject: [PATCH 3/6] [fast-reboot] Fix fast-reboot when NDP entries are present (#1295) * Fix fast-reboot when NDP present The IPv6 address delimiter ':' caused an exception Signed-off-by: Shlomi Bitton * Add a comment --- fdbutil/filter_fdb_entries.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fdbutil/filter_fdb_entries.py b/fdbutil/filter_fdb_entries.py index 5fd7664d81..7328395bd9 100755 --- a/fdbutil/filter_fdb_entries.py +++ b/fdbutil/filter_fdb_entries.py @@ -63,7 +63,7 @@ def get_arp_entries_map(arp_filename, config_db_filename): for key, config in arp.items(): if "NEIGH_TABLE" not in key: continue - table, vlan, ip = tuple(key.split(':')) + table, vlan, ip = tuple(key.split(':', 2)) # split with max of 2 is used here because IPv6 addresses contain ':' causing a creation of a non tuple object if "NEIGH_TABLE" in table and vlan in vlan_cidr \ and ip_address(ip) in ip_network(vlan_cidr[vlan][ip_interface(ip).version]) \ and "neigh" in config: From e22980f768631c9cdfe5014b0d7b73970d558404 Mon Sep 17 00:00:00 2001 From: Blueve <672454911@qq.com> Date: Wed, 9 Dec 2020 09:29:18 +0800 Subject: [PATCH 4/6] [config/console][consutil] Support enable/disable console switch (#1275) * [config/console][consutil] Support enable/disable console switch * Changed the key to aligned with feature table style Signed-off-by: Jing Kan jika@microsoft.com --- config/console.py | 32 +++++++++++++++++++++++ consutil/lib.py | 4 +++ consutil/main.py | 9 ++++++- tests/console_test.py | 60 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 104 insertions(+), 1 deletion(-) diff --git a/config/console.py b/config/console.py index 0a3cfbe5c8..b28aeda672 100644 --- a/config/console.py +++ b/config/console.py @@ -9,6 +9,38 @@ def console(): """Console-related configuration tasks""" pass +# +# 'console enable' group ('config console enable') +# +@console.command('enable') +@clicommon.pass_db +def enable_console_switch(db): + """Enable console switch""" + config_db = db.cfgdb + + table = "CONSOLE_SWITCH" + dataKey1 = 'console_mgmt' + dataKey2 = 'enabled' + + data = { dataKey2 : "yes" } + config_db.mod_entry(table, dataKey1, data) + +# +# 'console disable' group ('config console disable') +# +@console.command('disable') +@clicommon.pass_db +def disable_console_switch(db): + """Disable console switch""" + config_db = db.cfgdb + + table = "CONSOLE_SWITCH" + dataKey1 = 'console_mgmt' + dataKey2 = 'enabled' + + data = { dataKey2 : "no" } + config_db.mod_entry(table, dataKey1, data) + # # 'console add' group ('config console add ...') # diff --git a/consutil/lib.py b/consutil/lib.py index 4ba6477014..0172ac5644 100644 --- a/consutil/lib.py +++ b/consutil/lib.py @@ -22,6 +22,8 @@ ERR_BUSY = 5 CONSOLE_PORT_TABLE = "CONSOLE_PORT" +CONSOLE_SWITCH_TABLE = "CONSOLE_SWITCH" + LINE_KEY = "LINE" CUR_STATE_KEY = "CUR_STATE" @@ -29,6 +31,8 @@ BAUD_KEY = "baud_rate" DEVICE_KEY = "remote_device" FLOW_KEY = "flow_control" +FEATURE_KEY = "console_mgmt" +FEATURE_ENABLED_KEY = "enabled" # STATE_DB Keys STATE_KEY = "state" diff --git a/consutil/main.py b/consutil/main.py index 535ec413c2..b4ee3fa4fe 100644 --- a/consutil/main.py +++ b/consutil/main.py @@ -17,8 +17,15 @@ raise ImportError("%s - required module not found" % str(e)) @click.group() -def consutil(): +@clicommon.pass_db +def consutil(db): """consutil - Command-line utility for interacting with switches via console device""" + config_db = db.cfgdb + data = config_db.get_entry(CONSOLE_SWITCH_TABLE, FEATURE_KEY) + if FEATURE_ENABLED_KEY not in data or data[FEATURE_ENABLED_KEY] == "no": + click.echo("Console switch feature is disabled") + sys.exit(ERR_DISABLE) + SysInfoProvider.init_device_prefix() # 'show' subcommand diff --git a/tests/console_test.py b/tests/console_test.py index 6152c0d0db..c38b7ebb56 100644 --- a/tests/console_test.py +++ b/tests/console_test.py @@ -20,6 +20,24 @@ class TestConfigConsoleCommands(object): def setup_class(cls): print("SETUP") + def test_enable_console_switch(self): + runner = CliRunner() + db = Db() + + result = runner.invoke(config.config.commands["console"].commands["enable"]) + print(result.exit_code) + print(sys.stderr, result.output) + assert result.exit_code == 0 + + def test_disable_console_switch(self): + runner = CliRunner() + db = Db() + + result = runner.invoke(config.config.commands["console"].commands["disable"]) + print(result.exit_code) + print(sys.stderr, result.output) + assert result.exit_code == 0 + def test_console_add_exists(self): runner = CliRunner() db = Db() @@ -465,6 +483,48 @@ def test_sys_info_provider_get_active_console_process_info_nonexists(self): proc = SysInfoProvider.get_active_console_process_info("2") assert proc is None +class TestConsutil(object): + @classmethod + def setup_class(cls): + print("SETUP") + + @mock.patch('consutil.lib.SysInfoProvider.init_device_prefix', mock.MagicMock(return_value=None)) + @mock.patch('consutil.main.show', mock.MagicMock(return_value=None)) + def test_consutil_feature_disabled_null_config(self): + runner = CliRunner() + db = Db() + + result = runner.invoke(consutil.consutil, ['show'], obj=db) + print(result.exit_code) + print(sys.stderr, result.output) + assert result.exit_code == 1 + assert result.output == "Console switch feature is disabled\n" + + @mock.patch('consutil.lib.SysInfoProvider.init_device_prefix', mock.MagicMock(return_value=None)) + @mock.patch('consutil.main.show', mock.MagicMock(return_value=None)) + def test_consutil_feature_disabled_config(self): + runner = CliRunner() + db = Db() + db.cfgdb.set_entry("CONSOLE_SWITCH", "console_mgmt", { "enabled" : "no" }) + + result = runner.invoke(consutil.consutil, ['show'], obj=db) + print(result.exit_code) + print(sys.stderr, result.output) + assert result.exit_code == 1 + assert result.output == "Console switch feature is disabled\n" + + @mock.patch('consutil.lib.SysInfoProvider.init_device_prefix', mock.MagicMock(return_value=None)) + @mock.patch('consutil.main.show', mock.MagicMock(return_value=None)) + def test_consutil_feature_enabled(self): + runner = CliRunner() + db = Db() + db.cfgdb.set_entry("CONSOLE_SWITCH", "console_mgmt", { "enabled" : "yes" }) + + result = runner.invoke(consutil.consutil, ['show'], obj=db) + print(result.exit_code) + print(sys.stderr, result.output) + assert result.exit_code == 0 + class TestConsutilShow(object): @classmethod def setup_class(cls): From 2b4a58c5d82212054c40c082ad0b9e08ec1bdb92 Mon Sep 17 00:00:00 2001 From: Garrick He <32883830+GarrickHe@users.noreply.github.com> Date: Tue, 8 Dec 2020 19:50:41 -0800 Subject: [PATCH 5/6] [sflow] Fix traceback seen for show sflow interface (#1282) Fixed a traceback seen when show sflow interface is executed. Updated the script to follow Python3 conventions. Signed-off-by: Garrick He --- show/sflow.py | 7 ++++--- tests/mock_tables/appl_db.json | 9 +++++++++ tests/sflow_test.py | 29 +++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/show/sflow.py b/show/sflow.py index cae637260a..a84a4bdff1 100644 --- a/show/sflow.py +++ b/show/sflow.py @@ -50,11 +50,12 @@ def show_sflow_interface(config_db): for pname in natsorted(list(port_tbl.keys())): intf_key = 'SFLOW_SESSION_TABLE:' + pname sess_info = sess_db.get_all(sess_db.APPL_DB, intf_key) - if sess_info is None: + if (sess_info is None or sess_info.get('admin_state') is None or + sess_info.get('sample_rate') is None): continue body_info = [pname] - body_info.append(sess_info['admin_state']) - body_info.append(sess_info['sample_rate']) + body_info.append(sess_info.get('admin_state')) + body_info.append(sess_info.get('sample_rate')) body.append(body_info) click.echo(tabulate(body, header, tablefmt='grid')) diff --git a/tests/mock_tables/appl_db.json b/tests/mock_tables/appl_db.json index 9ba4300f98..5cdb65a180 100644 --- a/tests/mock_tables/appl_db.json +++ b/tests/mock_tables/appl_db.json @@ -15,6 +15,15 @@ "admin_state": "up", "sample_rate": "5000" }, + "SFLOW_SESSION_TABLE:Ethernet8": { + "BAD": "BAD" + }, + "SFLOW_SESSION_TABLE:Ethernet12": { + "sample_rate": "5000" + }, + "SFLOW_SESSION_TABLE:Ethernet16": { + "admin_state": "up" + }, "PORT_TABLE:Ethernet0": { "index": "0", "lanes": "0", diff --git a/tests/sflow_test.py b/tests/sflow_test.py index 85c9024162..0e15f1e027 100644 --- a/tests/sflow_test.py +++ b/tests/sflow_test.py @@ -261,6 +261,35 @@ def test_config_sflow_intf_sample_rate(self): return + def test_config_disable_all_intf(self): + db = Db() + runner = CliRunner() + obj = {'db':db.cfgdb} + + # disable all interfaces + result = runner.invoke(config.config.commands["sflow"]. + commands["interface"].commands["disable"], ["all"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code == 0 + + # verify in configDb + sflowSession = db.cfgdb.get_table('SFLOW_SESSION') + assert sflowSession["all"]["admin_state"] == "down" + + def test_config_enable_all_intf(self): + db = Db() + runner = CliRunner() + obj = {'db':db.cfgdb} + # enable all interfaces + result = runner.invoke(config.config.commands["sflow"].commands["interface"]. + commands["enable"], ["all"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code == 0 + + # verify in configDb + sflowSession = db.cfgdb.get_table('SFLOW_SESSION') + assert sflowSession["all"]["admin_state"] == "up" + @classmethod def teardown_class(cls): print("TEARDOWN") From d414970ef759490b51e8721cb3d72a5b916a3e5f Mon Sep 17 00:00:00 2001 From: Mahesh Maddikayala <10645050+smaheshm@users.noreply.github.com> Date: Tue, 8 Dec 2020 20:00:59 -0800 Subject: [PATCH 6/6] [unit test][CLI][pfcwd] Added pfcwd config tests for single and multi ASIC platform. (#1248) * Update Db object to include multi ASIC db clients. * Updated pfcwd CLI commands to use decorator to pass Db object. --- pfcwd/main.py | 53 +++-- tests/pfcwd_input/pfcwd_test_vectors.py | 106 ++++++++++ tests/pfcwd_test.py | 269 +++++++++++++++++++++++- utilities_common/db.py | 22 +- utilities_common/multi_asic.py | 22 +- 5 files changed, 441 insertions(+), 31 deletions(-) diff --git a/pfcwd/main.py b/pfcwd/main.py index c12c93a58c..c55038b9b2 100644 --- a/pfcwd/main.py +++ b/pfcwd/main.py @@ -3,6 +3,8 @@ import click +import utilities_common.cli as clicommon + from natsort import natsorted from sonic_py_common.multi_asic import get_external_ports from tabulate import tabulate @@ -98,10 +100,14 @@ def get_server_facing_ports(db): class PfcwdCli(object): - def __init__(self, namespace=None, display=constants.DISPLAY_ALL): + def __init__( + self, db=None, namespace=None, display=constants.DISPLAY_ALL + ): self.db = None self.config_db = None - self.multi_asic = multi_asic_util.MultiAsic(display, namespace) + self.multi_asic = multi_asic_util.MultiAsic( + display, namespace, db + ) self.table = [] self.all_ports = [] @@ -397,6 +403,7 @@ def big_red_switch(self, big_red_switch): pfcwd_info ) + # Show stats class Show(object): # Show commands @@ -408,19 +415,21 @@ def show(): @multi_asic_util.multi_asic_click_options @click.option('-e', '--empty', is_flag=True) @click.argument('queues', nargs=-1) - def stats(namespace, display, empty, queues): + @clicommon.pass_db + def stats(db, namespace, display, empty, queues): """ Show PFC Watchdog stats per queue """ if (len(queues)): display = constants.DISPLAY_ALL - PfcwdCli(namespace, display).show_stats(empty, queues) + PfcwdCli(db, namespace, display).show_stats(empty, queues) # Show config @show.command() @multi_asic_util.multi_asic_click_options @click.argument('ports', nargs=-1) - def config(namespace, display, ports): + @clicommon.pass_db + def config(db, namespace, display, ports): """ Show PFC Watchdog configuration """ - PfcwdCli(namespace, display).config(ports) + PfcwdCli(db, namespace, display).config(ports) # Start WD @@ -432,7 +441,8 @@ class Start(object): @click.option('--restoration-time', '-r', type=click.IntRange(100, 60000)) @click.argument('ports', nargs=-1) @click.argument('detection-time', type=click.IntRange(100, 5000)) - def start(action, restoration_time, ports, detection_time): + @clicommon.pass_db + def start(db, action, restoration_time, ports, detection_time): """ Start PFC watchdog on port(s). To config all ports, use all as input. @@ -441,51 +451,58 @@ def start(action, restoration_time, ports, detection_time): sudo pfcwd start --action drop ports all detection-time 400 --restoration-time 400 """ - PfcwdCli().start(action, restoration_time, ports, detection_time) + PfcwdCli(db).start( + action, restoration_time, ports, detection_time + ) # Set WD poll interval class Interval(object): @cli.command() @click.argument('poll_interval', type=click.IntRange(100, 3000)) - def interval(poll_interval): + @clicommon.pass_db + def interval(db, poll_interval): """ Set PFC watchdog counter polling interval """ - PfcwdCli().interval(poll_interval) + PfcwdCli(db).interval(poll_interval) # Stop WD class Stop(object): @cli.command() @click.argument('ports', nargs=-1) - def stop(ports): + @clicommon.pass_db + def stop(db, ports): """ Stop PFC watchdog on port(s) """ - PfcwdCli().stop(ports) + PfcwdCli(db).stop(ports) # Set WD default configuration on server facing ports when enable flag is on class StartDefault(object): @cli.command("start_default") - def start_default(): + @clicommon.pass_db + def start_default(db): """ Start PFC WD by default configurations """ - PfcwdCli().start_default() + PfcwdCli(db).start_default() # Enable/disable PFC WD counter polling class CounterPoll(object): @cli.command('counter_poll') @click.argument('counter_poll', type=click.Choice(['enable', 'disable'])) - def counter_poll(counter_poll): + @clicommon.pass_db + def counter_poll(db, counter_poll): """ Enable/disable counter polling """ - PfcwdCli().counter_poll(counter_poll) + PfcwdCli(db).counter_poll(counter_poll) # Enable/disable PFC WD BIG_RED_SWITCH mode class BigRedSwitch(object): @cli.command('big_red_switch') @click.argument('big_red_switch', type=click.Choice(['enable', 'disable'])) - def big_red_switch(big_red_switch): + @clicommon.pass_db + def big_red_switch(db, big_red_switch): """ Enable/disable BIG_RED_SWITCH mode """ - PfcwdCli().big_red_switch(big_red_switch) + PfcwdCli(db).big_red_switch(big_red_switch) def get_pfcwd_clis(): diff --git a/tests/pfcwd_input/pfcwd_test_vectors.py b/tests/pfcwd_input/pfcwd_test_vectors.py index 41ad34e9d7..9b6b7488a2 100644 --- a/tests/pfcwd_input/pfcwd_test_vectors.py +++ b/tests/pfcwd_input/pfcwd_test_vectors.py @@ -7,6 +7,47 @@ Ethernet8 drop 600 600 """ +pfcwd_show_start_config_output_pass = """\ +Changed polling interval to 600ms + PORT ACTION DETECTION TIME RESTORATION TIME +--------- -------- ---------------- ------------------ +Ethernet0 forward 102 101 +Ethernet4 drop 600 600 +Ethernet8 drop 600 600 +""" + +pfcwd_show_start_action_forward_output = """\ +Changed polling interval to 600ms + PORT ACTION DETECTION TIME RESTORATION TIME +--------- -------- ---------------- ------------------ +Ethernet0 forward 302 301 +Ethernet4 forward 302 301 +Ethernet8 forward 302 301 +""" + +pfcwd_show_start_action_alert_output = """\ +Changed polling interval to 600ms + PORT ACTION DETECTION TIME RESTORATION TIME +--------- -------- ---------------- ------------------ +Ethernet0 alert 502 501 +Ethernet4 alert 502 501 +Ethernet8 alert 502 501 +""" + +pfcwd_show_start_action_drop_output = """\ +Changed polling interval to 600ms + PORT ACTION DETECTION TIME RESTORATION TIME +--------- -------- ---------------- ------------------ +Ethernet0 drop 602 601 +Ethernet4 drop 602 601 +Ethernet8 drop 602 601 +""" + +pfcwd_show_start_config_output_fail = """\ +Failed to run command, invalid options: +Ethernet1000 +""" + pfcwd_show_config_single_port_output="""\ Changed polling interval to 600ms PORT ACTION DETECTION TIME RESTORATION TIME @@ -222,6 +263,71 @@ Ethernet-BP260 drop 200 200 """ +show_pfc_config_start_pass = """\ +Changed polling interval to 199ms on asic0 +BIG_RED_SWITCH status is enable on asic0 +Changed polling interval to 199ms on asic1 +BIG_RED_SWITCH status is enable on asic1 + PORT ACTION DETECTION TIME RESTORATION TIME +-------------- -------- ---------------- ------------------ + Ethernet0 forward 102 101 + Ethernet4 drop 200 200 + Ethernet-BP0 drop 200 200 + Ethernet-BP4 forward 102 101 +Ethernet-BP256 drop 200 200 +Ethernet-BP260 drop 200 200 +""" + +show_pfc_config_start_action_drop_masic = """\ +Changed polling interval to 199ms on asic0 +BIG_RED_SWITCH status is enable on asic0 +Changed polling interval to 199ms on asic1 +BIG_RED_SWITCH status is enable on asic1 + PORT ACTION DETECTION TIME RESTORATION TIME +-------------- -------- ---------------- ------------------ + Ethernet0 drop 302 301 + Ethernet4 drop 302 301 + Ethernet-BP0 drop 302 301 + Ethernet-BP4 drop 302 301 +Ethernet-BP256 drop 302 301 +Ethernet-BP260 drop 302 301 +""" + +show_pfc_config_start_action_alert_masic = """\ +Changed polling interval to 199ms on asic0 +BIG_RED_SWITCH status is enable on asic0 +Changed polling interval to 199ms on asic1 +BIG_RED_SWITCH status is enable on asic1 + PORT ACTION DETECTION TIME RESTORATION TIME +-------------- -------- ---------------- ------------------ + Ethernet0 alert 402 401 + Ethernet4 alert 402 401 + Ethernet-BP0 alert 402 401 + Ethernet-BP4 alert 402 401 +Ethernet-BP256 alert 402 401 +Ethernet-BP260 alert 402 401 +""" + +show_pfc_config_start_action_forward_masic = """\ +Changed polling interval to 199ms on asic0 +BIG_RED_SWITCH status is enable on asic0 +Changed polling interval to 199ms on asic1 +BIG_RED_SWITCH status is enable on asic1 + PORT ACTION DETECTION TIME RESTORATION TIME +-------------- -------- ---------------- ------------------ + Ethernet0 forward 702 701 + Ethernet4 forward 702 701 + Ethernet-BP0 forward 702 701 + Ethernet-BP4 forward 702 701 +Ethernet-BP256 forward 702 701 +Ethernet-BP260 forward 702 701 +""" + +show_pfc_config_start_fail = """\ +Failed to run command, invalid options: +Ethernet-500 +""" + show_pfcwd_stats_with_queues = """\ QUEUE STATUS STORM DETECTED/RESTORED TX OK/DROP RX OK/DROP TX LAST OK/DROP RX LAST OK/DROP ----------------- -------- ------------------------- ------------ ------------ ----------------- ----------------- diff --git a/tests/pfcwd_test.py b/tests/pfcwd_test.py index e96d5b7e0c..afbd1d0a74 100644 --- a/tests/pfcwd_test.py +++ b/tests/pfcwd_test.py @@ -5,10 +5,7 @@ from click.testing import CliRunner from utilities_common.db import Db -from .pfcwd_input.pfcwd_test_vectors import ( - testData, show_pfcwd_stats_all, show_pfc_config_all, - show_pfcwd_stats_with_queues, show_pfcwd_config_with_ports -) +from .pfcwd_input.pfcwd_test_vectors import * test_path = os.path.dirname(os.path.abspath(__file__)) modules_path = os.path.dirname(test_path) @@ -61,7 +58,9 @@ def executor(self, testcase): exec_cmd = pfcwd.cli.commands[input['cmd'][0]].commands[input['cmd'][1]] if 'db' in input and input['db']: - result = runner.invoke(exec_cmd, input['args'], obj=db) + result = runner.invoke( + exec_cmd, input['args'], obj=db + ) else: result = runner.invoke(exec_cmd, input['args']) @@ -79,6 +78,132 @@ def executor(self, testcase): if 'rc_output' in input: assert result.output == input['rc_output'] + def test_pfcwd_start_ports_valid(self): + # pfcwd start --action drop --restoration-time 200 Ethernet0 200 + import pfcwd.main as pfcwd + runner = CliRunner() + db = Db() + + # get initial config + result = runner.invoke( + pfcwd.cli.commands["show"].commands["config"], + obj=db + ) + print(result.output) + assert result.output == pfcwd_show_config_output + + result = runner.invoke( + pfcwd.cli.commands["start"], + [ + "--action", "forward", "--restoration-time", "101", + "Ethernet0", "102" + ], + obj=db + ) + print(result.output) + assert result.exit_code == 0 + + # get config after the change + result = runner.invoke( + pfcwd.cli.commands["show"].commands["config"], + obj=db + ) + print(result.output) + assert result.exit_code == 0 + assert result.output == pfcwd_show_start_config_output_pass + + def test_pfcwd_start_actions(self): + # pfcwd start --action fwd --restoration-time 200 Ethernet0 200 + import pfcwd.main as pfcwd + runner = CliRunner() + db = Db() + + # get initial config + result = runner.invoke( + pfcwd.cli.commands["show"].commands["config"], + obj=db + ) + print(result.output) + assert result.output == pfcwd_show_config_output + + result = runner.invoke( + pfcwd.cli.commands["start"], + [ + "--action", "forward", "--restoration-time", "301", + "all", "302" + ], + obj=db + ) + print(result.output) + assert result.exit_code == 0 + + # get config after the change + result = runner.invoke( + pfcwd.cli.commands["show"].commands["config"], + obj=db + ) + print(result.output) + assert result.exit_code == 0 + assert result.output == pfcwd_show_start_action_forward_output + + result = runner.invoke( + pfcwd.cli.commands["start"], + [ + "--action", "alert", "--restoration-time", "501", + "all", "502" + ], + obj=db + ) + print(result.output) + assert result.exit_code == 0 + + # get config after the change + result = runner.invoke( + pfcwd.cli.commands["show"].commands["config"], + obj=db + ) + print(result.output) + assert result.exit_code == 0 + assert result.output == pfcwd_show_start_action_alert_output + + result = runner.invoke( + pfcwd.cli.commands["start"], + [ + "--action", "drop", "--restoration-time", "601", + "all", "602" + ], + obj=db + ) + print(result.output) + assert result.exit_code == 0 + + # get config after the change + result = runner.invoke( + pfcwd.cli.commands["show"].commands["config"], + obj=db + ) + print(result.output) + assert result.exit_code == 0 + assert result.output == pfcwd_show_start_action_drop_output + + def test_pfcwd_start_ports_invalid(self): + # pfcwd start --action drop --restoration-time 200 Ethernet0 200 + import pfcwd.main as pfcwd + runner = CliRunner() + db = Db() + + result = runner.invoke( + pfcwd.cli.commands["start"], + [ + "--action", "forward", "--restoration-time", "101", + "Ethernet1000", "102" + ], + obj=db + ) + print(result.output) + assert result.exit_code == 0 + assert result.output == pfcwd_show_start_config_output_fail + @classmethod def teardown_class(cls): os.environ["PATH"] = os.pathsep.join(os.environ["PATH"].split(os.pathsep)[:-1]) @@ -142,6 +267,140 @@ def test_pfcwd_config_with_ports(self): assert result.exit_code == 0 assert result.output == show_pfcwd_config_with_ports + def test_pfcwd_start_ports_masic_valid(self): + # pfcwd start --action forward --restoration-time 200 Ethernet0 200 + import pfcwd.main as pfcwd + runner = CliRunner() + db = Db() + # get initial config + result = runner.invoke( + pfcwd.cli.commands["show"].commands["config"], + obj=db + ) + print(result.output) + assert result.output == show_pfc_config_all + + result = runner.invoke( + pfcwd.cli.commands["start"], + [ + "--action", "forward", "--restoration-time", "101", + "Ethernet0", "Ethernet-BP4", "102" + ], + obj=db + ) + print(result.output) + assert result.exit_code == 0 + + # get config after the change + result = runner.invoke( + pfcwd.cli.commands["show"].commands["config"], + obj=db + ) + print(result.output) + assert result.exit_code == 0 + assert result.output == show_pfc_config_start_pass + + def test_pfcwd_start_actions_masic(self): + # pfcwd start --action drop --restoration-time 200 Ethernet0 200 + import pfcwd.main as pfcwd + runner = CliRunner() + db = Db() + # get initial config + result = runner.invoke( + pfcwd.cli.commands["show"].commands["config"], + obj=db + ) + print(result.output) + assert result.output == show_pfc_config_all + + result = runner.invoke( + pfcwd.cli.commands["start"], + [ + "--action", "drop", "--restoration-time", "301", + "all", "302" + ], + obj=db + ) + print(result.output) + assert result.exit_code == 0 + + # get config after the change + result = runner.invoke( + pfcwd.cli.commands["show"].commands["config"], + obj=db + ) + print(result.output) + assert result.exit_code == 0 + assert result.output == show_pfc_config_start_action_drop_masic + + result = runner.invoke( + pfcwd.cli.commands["start"], + [ + "--action", "alert", "--restoration-time", "401", + "all", "402" + ], + obj=db + ) + print(result.output) + assert result.exit_code == 0 + + # get config after the change + result = runner.invoke( + pfcwd.cli.commands["show"].commands["config"], + obj=db + ) + print(result.output) + assert result.exit_code == 0 + assert result.output == show_pfc_config_start_action_alert_masic + + result = runner.invoke( + pfcwd.cli.commands["start"], + [ + "--action", "forward", "--restoration-time", "701", + "all", "702" + ], + obj=db + ) + print(result.output) + assert result.exit_code == 0 + + # get config after the change + result = runner.invoke( + pfcwd.cli.commands["show"].commands["config"], + obj=db + ) + print(result.output) + assert result.exit_code == 0 + assert result.output == show_pfc_config_start_action_forward_masic + + def test_pfcwd_start_ports_masic_invalid(self): + # --action drop --restoration-time 200 Ethernet0 Ethernet500 200 + import pfcwd.main as pfcwd + runner = CliRunner() + db = Db() + + result = runner.invoke( + pfcwd.cli.commands["start"], + [ + "--action", "forward", "--restoration-time", "101", + "Ethernet0", "Ethernet-500", "102" + ], + obj=db + ) + print(result.output) + assert result.exit_code == 0 + assert result.output == show_pfc_config_start_fail + + # get config after the command, config shouldn't change + result = runner.invoke( + pfcwd.cli.commands["show"].commands["config"], + obj=db + ) + print(result.output) + assert result.exit_code == 0 + # same as original config + assert result.output == show_pfc_config_all + @classmethod def teardown_class(cls): print("TEARDOWN") diff --git a/utilities_common/db.py b/utilities_common/db.py index bab45d7969..f6b4189190 100644 --- a/utilities_common/db.py +++ b/utilities_common/db.py @@ -1,13 +1,29 @@ +from sonic_py_common import multi_asic from swsssdk import ConfigDBConnector, SonicV2Connector +from utilities_common import constants +from utilities_common.multi_asic import multi_asic_ns_choices + class Db(object): def __init__(self): + self.cfgdb_clients = {} + self.db_clients = {} self.cfgdb = ConfigDBConnector() self.cfgdb.connect() self.db = SonicV2Connector(host="127.0.0.1") - self.db.connect(self.db.APPL_DB) - self.db.connect(self.db.CONFIG_DB) - self.db.connect(self.db.STATE_DB) + for db_id in self.db.get_db_list(): + self.db.connect(db_id) + + self.cfgdb_clients[constants.DEFAULT_NAMESPACE] = self.cfgdb + self.db_clients[constants.DEFAULT_NAMESPACE] = self.db + + if multi_asic.is_multi_asic(): + self.ns_list = multi_asic_ns_choices() + for ns in self.ns_list: + self.cfgdb_clients[ns] = ( + multi_asic.connect_config_db_for_ns(ns) + ) + self.db_clients[ns] = multi_asic.connect_to_all_dbs_for_ns(ns) def get_data(self, table, key): data = self.cfgdb.get_table(table) diff --git a/utilities_common/multi_asic.py b/utilities_common/multi_asic.py index 99096bb0b7..51a6a18b65 100644 --- a/utilities_common/multi_asic.py +++ b/utilities_common/multi_asic.py @@ -8,17 +8,20 @@ class MultiAsic(object): - def __init__(self, display_option=constants.DISPLAY_ALL, - namespace_option=None): + def __init__( + self, display_option=constants.DISPLAY_ALL, namespace_option=None, + db=None + ): self.namespace_option = namespace_option self.display_option = display_option self.current_namespace = None self.is_multi_asic = multi_asic.is_multi_asic() + self.db = db def is_object_internal(self, object_type, cli_object): ''' The function checks if a CLI object is internal and returns true or false. - Internal objects are port or portchannel which are connected to other + Internal objects are port or portchannel which are connected to other ports or portchannels within a multi ASIC device. For single asic, this function is not applicable @@ -120,8 +123,17 @@ def wrapped_run_on_all_asics(self, *args, **kwargs): ns_list = self.multi_asic.get_ns_list_based_on_options() for ns in ns_list: self.multi_asic.current_namespace = ns - self.db = multi_asic.connect_to_all_dbs_for_ns(ns) - self.config_db = multi_asic.connect_config_db_for_ns(ns) + # if object instance already has db connections, use them + if self.multi_asic.db and self.multi_asic.db.cfgdb_clients.get(ns): + self.config_db = self.multi_asic.db.cfgdb_clients[ns] + else: + self.config_db = multi_asic.connect_config_db_for_ns(ns) + + if self.multi_asic.db and self.multi_asic.db.db_clients.get(ns): + self.db = self.multi_asic.db.db_clients[ns] + else: + self.db = multi_asic.connect_to_all_dbs_for_ns(ns) + func(self, *args, **kwargs) return wrapped_run_on_all_asics