From b082684271b18288baff8f7f26141593ee041763 Mon Sep 17 00:00:00 2001 From: Neetha John Date: Thu, 21 Jan 2021 09:59:15 -0800 Subject: [PATCH] [ecn] Add tests for ecnconfig command (#1372) - What I did Added unit tests for ecnconfig command Updated 'show ecn' command to use run_command wrapper Updated 'config ecn' command to allow configuration of drop probabilities Cleanup trailing spaces in ecnconfig script and add hooks to support unit tests - How to verify it Ran the new ecn_test.py and it passed Signed-off-by: Neetha John --- config/main.py | 8 +- scripts/ecnconfig | 66 +++++++++++++---- setup.py | 3 +- show/main.py | 8 +- tests/ecn_input/__init__.py | 0 tests/ecn_input/ecn_test_vectors.py | 105 ++++++++++++++++++++++++++ tests/ecn_test.py | 111 ++++++++++++++++++++++++++++ tests/mock_tables/config_db.json | 15 ++++ 8 files changed, 295 insertions(+), 21 deletions(-) create mode 100644 tests/ecn_input/__init__.py create mode 100644 tests/ecn_input/ecn_test_vectors.py create mode 100644 tests/ecn_test.py diff --git a/config/main.py b/config/main.py index 57d15a75c7cc..34d01955f2c4 100644 --- a/config/main.py +++ b/config/main.py @@ -3533,8 +3533,11 @@ def remove_reasons(counter_name, reasons, verbose): @click.option('-ymin', metavar='', type=int, help="Set yellow min threshold") @click.option('-gmax', metavar='', type=int, help="Set green max threshold") @click.option('-gmin', metavar='', type=int, help="Set green min threshold") +@click.option('-rdrop', metavar='', type=click.IntRange(0, 100), help="Set red drop probability") +@click.option('-ydrop', metavar='', type=click.IntRange(0, 100), help="Set yellow drop probability") +@click.option('-gdrop', metavar='', type=click.IntRange(0, 100), help="Set green drop probability") @click.option('-v', '--verbose', is_flag=True, help="Enable verbose output") -def ecn(profile, rmax, rmin, ymax, ymin, gmax, gmin, verbose): +def ecn(profile, rmax, rmin, ymax, ymin, gmax, gmin, rdrop, ydrop, gdrop, verbose): """ECN-related configuration tasks""" log.log_info("'ecn -profile {}' executing...".format(profile)) command = "ecnconfig -p %s" % profile @@ -3544,6 +3547,9 @@ def ecn(profile, rmax, rmin, ymax, ymin, gmax, gmin, verbose): if ymin is not None: command += " -ymin %d" % ymin if gmax is not None: command += " -gmax %d" % gmax if gmin is not None: command += " -gmin %d" % gmin + if rdrop is not None: command += " -rdrop %d" % rdrop + if ydrop is not None: command += " -ydrop %d" % ydrop + if gdrop is not None: command += " -gdrop %d" % gdrop if verbose: command += " -vv" clicommon.run_command(command, display_cmd=verbose) diff --git a/scripts/ecnconfig b/scripts/ecnconfig index f2e05376ecc4..cdebf21e799a 100755 --- a/scripts/ecnconfig +++ b/scripts/ecnconfig @@ -48,12 +48,25 @@ ECN status: queue 3: on """ import argparse +import json import os import sys import swsssdk from tabulate import tabulate +# mock the redis for unit test purposes # +try: + if os.environ["UTILITIES_UNIT_TESTING"] == "2": + modules_path = os.path.join(os.path.dirname(__file__), "..") + tests_path = os.path.join(modules_path, "tests") + sys.path.insert(0, modules_path) + sys.path.insert(0, tests_path) + import mock_tables.dbconnector + +except KeyError: + pass + WRED_PROFILE_TABLE_NAME = "WRED_PROFILE" WRED_CONFIG_FIELDS = { "gmax": "green_max_threshold", @@ -78,11 +91,12 @@ lossless_queues = ['3', '4'] class EcnConfig(object): """ - Process aclstat + Process ecnconfig """ - def __init__(self, verbose): + def __init__(self, filename, verbose): self.ports = [] self.queues = [] + self.filename = filename self.verbose = verbose # Set up db connections @@ -109,8 +123,8 @@ class EcnConfig(object): for profile_name, profile_data in wred_profiles.items(): if profile_name == profile: - return profile_data - + return profile_data + return None def validate_profile_data(self, profile_data): @@ -125,7 +139,7 @@ class EcnConfig(object): if 'threshold' in field: if value.isdigit() == False: print("Invalid %s (%s). %s should be an non-negative integer." % (key, value, key)) - result = False + result = False elif 'probability' in field: if value.isdigit() == False or int(value) < 0 or int(value) > 100: @@ -133,9 +147,9 @@ class EcnConfig(object): result = False if result == False: - return result + return result - # check if min threshold is no larger than max threshold + # check if min threshold is no larger than max threshold colors = ['g', 'y', 'r'] for color in colors: if (WRED_CONFIG_FIELDS[color + 'min'] in profile_data and @@ -145,11 +159,11 @@ class EcnConfig(object): max_thresh = int(profile_data[WRED_CONFIG_FIELDS[color + 'max']]) if min_thresh > max_thresh: - print("Invalid %s (%d) and %s (%d). %s should be no smaller than %s" % + print("Invalid %s (%d) and %s (%d). %s should be smaller than %s" % (color + 'min', min_thresh, color + 'max', max_thresh, color + 'min', color + 'max')) - result = False + result = False - return result + return result def set_wred_threshold(self, profile, threshold, value): if os.geteuid() != 0: @@ -159,15 +173,23 @@ class EcnConfig(object): if self.verbose: print("Setting %s value to %s" % (field, value)) self.db.mod_entry(WRED_PROFILE_TABLE_NAME, profile, {field: value}) + if self.filename is not None: + prof_table = self.db.get_table(WRED_PROFILE_TABLE_NAME) + with open(self.filename, "w") as fd: + json.dump(prof_table, fd) def set_wred_prob(self, profile, drop_color, value): if os.geteuid() != 0: sys.exit("Root privileges required for this operation") - + field = WRED_CONFIG_FIELDS[drop_color] if self.verbose: print("Setting %s value to %s%%" % (field, value)) self.db.mod_entry(WRED_PROFILE_TABLE_NAME, profile, {field: value}) + if self.filename is not None: + prof_table = self.db.get_table(WRED_PROFILE_TABLE_NAME) + with open(self.filename, "w") as fd: + json.dump(prof_table, fd) class EcnQ(object): """ @@ -253,21 +275,35 @@ def main(): parser.add_argument('command', nargs='?', choices=['on', 'off'], type=str, help='turn on/off ecn', default=None) parser.add_argument('-q', '--queue', type=str, help='specify queue index list: 3,4', default=None) + parser.add_argument('-f', '--filename', help='file used by mock tests', type=str, default=None) + + if os.environ.get("UTILITIES_UNIT_TESTING", "0") == "2": + sys.argv.extend(['-f', '/tmp/ecnconfig']) args = parser.parse_args() try: if args.list or args.profile: - prof_cfg = EcnConfig(args.verbose) + prof_cfg = EcnConfig(args.filename, args.verbose) if args.list: - if len(sys.argv) > (3 if args.verbose else 2): + arg_len_max = 2 + if args.verbose: + arg_len_max += 1 + if args.filename: + arg_len_max += 2 + if len(sys.argv) > arg_len_max: raise Exception("Input arguments error. No set options allowed when -l[ist] specified") prof_cfg.list() elif args.profile: - if len(sys.argv) < (5 if args.verbose else 4): + arg_len_min = 4 + if args.verbose: + arg_len_min += 1 + if args.filename: + arg_len_min += 2 + if len(sys.argv) < arg_len_min: raise Exception("Input arguments error. Specify at least one threshold parameter to set") - # get current configuration data + # get current configuration data wred_profile_data = prof_cfg.get_profile_data(args.profile) if wred_profile_data is None: raise Exception("Input arguments error. Invalid WRED profile %s" % (args.profile)) diff --git a/setup.py b/setup.py index adb725727b40..2af6829029b3 100644 --- a/setup.py +++ b/setup.py @@ -61,7 +61,8 @@ 'mock_tables/asic2/*.json', 'filter_fdb_input/*', 'pfcwd_input/*', - 'wm_input/*'] + 'wm_input/*', + 'ecn_input/*'] }, scripts=[ 'scripts/aclshow', diff --git a/show/main.py b/show/main.py index bd4ed3837cfd..bce69063ed0f 100644 --- a/show/main.py +++ b/show/main.py @@ -15,7 +15,7 @@ from utilities_common.db import Db from . import acl -from . import bgp_common +from . import bgp_common from . import chassis_modules from . import dropcounters from . import feature @@ -1449,11 +1449,11 @@ def policer(policer_name, verbose): # 'ecn' command ("show ecn") # @cli.command('ecn') -def ecn(): +@click.option('--verbose', is_flag=True, help="Enable verbose output") +def ecn(verbose): """Show ECN configuration""" cmd = "ecnconfig -l" - proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, shell=True, text=True) - click.echo(proc.stdout.read()) + run_command(cmd, display_cmd=verbose) # diff --git a/tests/ecn_input/__init__.py b/tests/ecn_input/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/ecn_input/ecn_test_vectors.py b/tests/ecn_input/ecn_test_vectors.py new file mode 100644 index 000000000000..44bc9ad1531b --- /dev/null +++ b/tests/ecn_input/ecn_test_vectors.py @@ -0,0 +1,105 @@ +ecn_show_config_output="""\ +Profile: AZURE_LOSSLESS +----------------------- ------- +red_max_threshold 2097152 +wred_green_enable true +ecn ecn_all +green_min_threshold 1048576 +red_min_threshold 1048576 +wred_yellow_enable true +yellow_min_threshold 1048576 +green_max_threshold 2097152 +green_drop_probability 5 +yellow_max_threshold 2097152 +wred_red_enable true +yellow_drop_probability 5 +red_drop_probability 5 +----------------------- ------- + +""" + +testData = { + 'ecn_show_config' : {'cmd' : ['show'], + 'args' : [], + 'rc' : 0, + 'rc_output': ecn_show_config_output + }, + 'ecn_cfg_gmin' : {'cmd' : ['config'], + 'args' : ['-profile', 'AZURE_LOSSLESS', '-gmin', '1048600'], + 'rc' : 0, + 'cmp_args' : ['AZURE_LOSSLESS,green_min_threshold,1048600'] + }, + 'ecn_cfg_gmax' : {'cmd' : ['config'], + 'args' : ['-profile', 'AZURE_LOSSLESS', '-gmax', '2097153'], + 'rc' : 0, + 'cmp_args' : ['AZURE_LOSSLESS,green_max_threshold,2097153'] + }, + 'ecn_cfg_ymin' : {'cmd' : ['config'], + 'args' : ['-profile', 'AZURE_LOSSLESS', '-ymin', '1048600'], + 'rc' : 0, + 'cmp_args' : ['AZURE_LOSSLESS,yellow_min_threshold,1048600'] + }, + 'ecn_cfg_ymax' : {'cmd' : ['config'], + 'args' : ['-profile', 'AZURE_LOSSLESS', '-ymax', '2097153'], + 'rc' : 0, + 'cmp_args' : ['AZURE_LOSSLESS,yellow_max_threshold,2097153'] + }, + 'ecn_cfg_rmin' : {'cmd' : ['config'], + 'args' : ['-profile', 'AZURE_LOSSLESS', '-rmin', '1048600'], + 'rc' : 0, + 'cmp_args' : ['AZURE_LOSSLESS,red_min_threshold,1048600'] + }, + 'ecn_cfg_rmax' : {'cmd' : ['config'], + 'args' : ['-profile', 'AZURE_LOSSLESS', '-rmax', '2097153'], + 'rc' : 0, + 'cmp_args' : ['AZURE_LOSSLESS,red_max_threshold,2097153'] + }, + 'ecn_cfg_rdrop' : {'cmd' : ['config'], + 'args' : ['-profile', 'AZURE_LOSSLESS', '-rdrop', '10'], + 'rc' : 0, + 'cmp_args' : ['AZURE_LOSSLESS,red_drop_probability,10'] + }, + 'ecn_cfg_ydrop' : {'cmd' : ['config'], + 'args' : ['-profile', 'AZURE_LOSSLESS', '-ydrop', '11'], + 'rc' : 0, + 'cmp_args' : ['AZURE_LOSSLESS,yellow_drop_probability,11'] + }, + 'ecn_cfg_gdrop' : {'cmd' : ['config'], + 'args' : ['-profile', 'AZURE_LOSSLESS', '-gdrop', '12'], + 'rc' : 0, + 'cmp_args' : ['AZURE_LOSSLESS,green_drop_probability,12'] + }, + 'ecn_cfg_multi_set' : {'cmd' : ['config'], + 'args' : ['-profile', 'AZURE_LOSSLESS', '-gdrop', '12', '-gmax', '2097153'], + 'rc' : 0, + 'cmp_args' : ['AZURE_LOSSLESS,green_drop_probability,12', + 'AZURE_LOSSLESS,green_max_threshold,2097153' + ] + }, + 'ecn_cfg_gmin_gmax_invalid' : {'cmd' : ['config'], + 'args' : ['-profile', 'AZURE_LOSSLESS', '-gmax', '2097153', '-gmin', '2097154'], + 'rc' : 1, + 'rc_msg' : 'Invalid gmin (2097154) and gmax (2097153). gmin should be smaller than gmax' + }, + 'ecn_cfg_ymin_ymax_invalid' : {'cmd' : ['config'], + 'args' : ['-profile', 'AZURE_LOSSLESS', '-ymax', '2097153', '-ymin', '2097154'], + 'rc' : 1, + 'rc_msg' : 'Invalid ymin (2097154) and ymax (2097153). ymin should be smaller than ymax' + }, + 'ecn_cfg_rmin_rmax_invalid' : {'cmd' : ['config'], + 'args' : ['-profile', 'AZURE_LOSSLESS', '-rmax', '2097153', '-rmin', '2097154'], + 'rc' : 1, + 'rc_msg' : 'Invalid rmin (2097154) and rmax (2097153). rmin should be smaller than rmax' + }, + 'ecn_cfg_rmax_invalid' : {'cmd' : ['config'], + 'args' : ['-profile', 'AZURE_LOSSLESS', '-rmax', '-2097153'], + 'rc' : 1, + 'rc_msg' : 'Invalid rmax (-2097153). rmax should be an non-negative integer' + }, + 'ecn_cfg_rdrop_invalid' : {'cmd' : ['config'], + 'args' : ['-profile', 'AZURE_LOSSLESS', '-rdrop', '105'], + 'rc' : 1, + 'rc_msg' : 'Invalid value for "-rdrop": 105 is not in the valid range of 0 to 100' + } + + } diff --git a/tests/ecn_test.py b/tests/ecn_test.py new file mode 100644 index 000000000000..1a0b54c32bce --- /dev/null +++ b/tests/ecn_test.py @@ -0,0 +1,111 @@ +import json +import os +import sys + +from click.testing import CliRunner + +import config.main as config +from .ecn_input.ecn_test_vectors import * +import show.main as show + +test_path = os.path.dirname(os.path.abspath(__file__)) +modules_path = os.path.dirname(test_path) +scripts_path = os.path.join(modules_path, "scripts") +sys.path.insert(0, test_path) +sys.path.insert(0, modules_path) + + +class TestEcnConfig(object): + @classmethod + def setup_class(cls): + os.environ["PATH"] += os.pathsep + scripts_path + os.environ['UTILITIES_UNIT_TESTING'] = "2" + print("SETUP") + + def test_ecn_show_config(self): + self.executor(testData['ecn_show_config']) + + def test_ecn_config_gmin(self): + self.executor(testData['ecn_cfg_gmin']) + + def test_ecn_config_gmax(self): + self.executor(testData['ecn_cfg_gmax']) + + def test_ecn_config_ymin(self): + self.executor(testData['ecn_cfg_ymin']) + + def test_ecn_config_ymax(self): + self.executor(testData['ecn_cfg_ymax']) + + def test_ecn_config_rmin(self): + self.executor(testData['ecn_cfg_gmin']) + + def test_ecn_config_rmax(self): + self.executor(testData['ecn_cfg_gmax']) + + def test_ecn_config_gdrop(self): + self.executor(testData['ecn_cfg_gdrop']) + + def test_ecn_config_ydrop(self): + self.executor(testData['ecn_cfg_ydrop']) + + def test_ecn_config_rdrop(self): + self.executor(testData['ecn_cfg_rdrop']) + + def test_ecn_config_multi_set(self): + self.executor(testData['ecn_cfg_multi_set']) + + def test_ecn_config_gmin_gmax_invalid(self): + self.executor(testData['ecn_cfg_gmin_gmax_invalid']) + + def test_ecn_config_ymin_ymax_invalid(self): + self.executor(testData['ecn_cfg_ymin_ymax_invalid']) + + def test_ecn_config_rmin_rmax_invalid(self): + self.executor(testData['ecn_cfg_rmin_rmax_invalid']) + + def test_ecn_config_rmax_invalid(self): + self.executor(testData['ecn_cfg_rmax_invalid']) + + def test_ecn_config_rdrop_invalid(self): + self.executor(testData['ecn_cfg_rdrop_invalid']) + + def executor(self, input): + runner = CliRunner() + + if 'show' in input['cmd']: + exec_cmd = show.cli.commands["ecn"] + else: + exec_cmd = config.config.commands["ecn"] + + result = runner.invoke(exec_cmd, input['args']) + + print(result.exit_code) + print(result.output) + + if input['rc'] == 0: + assert result.exit_code == 0 + else: + assert result.exit_code != 0 + + if 'cmp_args' in input: + fd = open('/tmp/ecnconfig', 'r') + prof_data = json.load(fd) + for args in input['cmp_args']: + profile, name, value = args.split(',') + assert(prof_data[profile][name] == value) + fd.close() + + if 'rc_msg' in input: + assert input['rc_msg'] in result.output + + if 'rc_output' in input: + assert result.output == input['rc_output'] + + @classmethod + def teardown_class(cls): + os.environ['PATH'] = os.pathsep.join(os.environ['PATH'].split(os.pathsep)[:-1]) + os.environ['UTILITIES_UNIT_TESTING'] = "0" + if os.path.isfile('/tmp/ecnconfig'): + os.remove('/tmp/ecnconfig') + print("TEARDOWN") diff --git a/tests/mock_tables/config_db.json b/tests/mock_tables/config_db.json index a8da97f3eb86..a535a93ec9b5 100644 --- a/tests/mock_tables/config_db.json +++ b/tests/mock_tables/config_db.json @@ -1440,5 +1440,20 @@ "insecure": "True", "disable": "False", "port": "6443" + }, + "WRED_PROFILE|AZURE_LOSSLESS": { + "red_max_threshold": "2097152", + "wred_green_enable": "true", + "ecn": "ecn_all", + "green_min_threshold": "1048576", + "red_min_threshold": "1048576", + "wred_yellow_enable": "true", + "yellow_min_threshold": "1048576", + "green_max_threshold": "2097152", + "green_drop_probability": "5", + "yellow_max_threshold": "2097152", + "wred_red_enable": "true", + "yellow_drop_probability": "5", + "red_drop_probability": "5" } }