From 3d2aea05df714b54068eca247efcb10cb270c1c7 Mon Sep 17 00:00:00 2001 From: Kenneth Cheung Date: Wed, 5 Jun 2024 01:27:46 -0700 Subject: [PATCH] Small tweaks to improve readability --- scripts/pg-drop | 48 ++++++----- tests/multi_asic_pgdropstat_test.py | 126 ++++++++++++++++------------ 2 files changed, 100 insertions(+), 74 deletions(-) diff --git a/scripts/pg-drop b/scripts/pg-drop index 59c3fba7f33..54ba6901d6e 100755 --- a/scripts/pg-drop +++ b/scripts/pg-drop @@ -50,9 +50,9 @@ class PgDropStat(object): def __init__(self, namespace): self.namespace = namespace + self.ns_list = multi_asic.get_namespace_list(namespace) self.configdb = ConfigDBConnector(namespace=namespace, use_unix_socket_path=True) self.configdb.connect() - self.target_ns = multi_asic.get_namespace_list(namespace) dropstat_dir = get_dropstat_dir() self.port_drop_stats_file = os.path.join(dropstat_dir, 'pg_drop_stats') @@ -60,14 +60,14 @@ class PgDropStat(object): """ Get port ID using object ID """ - port_id = self.get_counter_index(COUNTERS_PG_PORT_MAP, oid) + port_id = self.get_counters_mapdata(COUNTERS_PG_PORT_MAP, oid) if not port_id: print("Port is not available for oid '{}'".format(oid)) sys.exit(1) return port_id # Get all ports - self.counter_port_name_map = self.get_counters_db_map(COUNTERS_PORT_NAME_MAP) + self.counter_port_name_map = self.get_counters_mapall(COUNTERS_PORT_NAME_MAP) if not self.counter_port_name_map: print("COUNTERS_PORT_NAME_MAP is empty!") sys.exit(1) @@ -80,7 +80,7 @@ class PgDropStat(object): self.port_name_map[self.counter_port_name_map[port]] = port # Get PGs for each port - counter_pg_name_map = self.get_counters_db_map(COUNTERS_PG_NAME_MAP) + counter_pg_name_map = self.get_counters_mapall(COUNTERS_PG_NAME_MAP) if not counter_pg_name_map: print("COUNTERS_PG_NAME_MAP is empty!") sys.exit(1) @@ -97,24 +97,24 @@ class PgDropStat(object): "header_prefix": "PG"}, } - def get_counter_index(self, index_map, table_id): - for ns in self.target_ns: + def get_counters_mapdata(self, tablemap, index): + for ns in self.ns_list: counters_db = SonicV2Connector(namespace=ns, use_unix_socket_path=True) counters_db.connect(counters_db.COUNTERS_DB) - index = counters_db.get(counters_db.COUNTERS_DB, index_map, table_id) - if index: - return index + data = counters_db.get(counters_db.COUNTERS_DB, tablemap, index) + if data: + return data return None - def get_counters_db_map(self, map_name): - map = {} - for ns in self.target_ns: + def get_counters_mapall(self, tablemap): + mapdata = {} + for ns in self.ns_list: counters_db = SonicV2Connector(namespace=ns, use_unix_socket_path=True) counters_db.connect(counters_db.COUNTERS_DB) - map_result = counters_db.get_all(counters_db.COUNTERS_DB, map_name) + map_result = counters_db.get_all(counters_db.COUNTERS_DB, tablemap) if map_result: - map.update(map_result) - return map + mapdata.update(map_result) + return mapdata def get_pg_index(self, oid): """ @@ -122,7 +122,7 @@ class PgDropStat(object): oid - object ID for entry in redis """ - pg_index = self.get_counter_index(COUNTERS_PG_INDEX_MAP, oid) + pg_index = self.get_counters_mapdata(COUNTERS_PG_INDEX_MAP, oid) if not pg_index: print("Priority group index is not available for oid '{}'".format(oid)) sys.exit(1) @@ -176,7 +176,7 @@ class PgDropStat(object): old_collected_data = port_drop_ckpt.get(name,{})[full_table_id] if len(port_drop_ckpt) > 0 else 0 idx = int(idx_func(obj_id)) pos = self.header_idx_to_pos[idx] - counter_data = self.get_counter_index(full_table_id, counter_name) + counter_data = self.get_counters_mapdata(full_table_id, counter_name) if counter_data is None: fields[pos] = STATUS_NA elif fields[pos] != STATUS_NA: @@ -208,7 +208,7 @@ class PgDropStat(object): counts = {} table_id = COUNTER_TABLE_PREFIX + oid for counter in counters: - counter_data = self.get_counter_index(table_id, counter) + counter_data = self.get_counters_mapdata(table_id, counter) if counter_data is None: counts[table_id] = 0 else: @@ -221,7 +221,7 @@ class PgDropStat(object): to its PG drop counts. Counts are contained in a dictionary that maps counter oid to its counts. """ - counter_object_name_map = self.get_counters_db_map(object_table) + counter_object_name_map = self.get_counters_mapall(object_table) current_stat_dict = OrderedDict() if not counter_object_name_map: @@ -261,11 +261,12 @@ def main(): epilog=""" Examples: pg-drop -c show +pg-drop -c show --namespace asic0 pg-drop -c clear """) parser.add_argument('-c', '--command', type=str, help='Desired action to perform') - parser.add_argument('-n', '--namespace', type=str, help='Namespace name or all', default=None) + parser.add_argument('-n', '--namespace', type=str, help='Namespace name or skip for all', default=None) args = parser.parse_args() command = args.command @@ -279,13 +280,16 @@ pg-drop -c clear print(e) sys.exit(e.errno) + # Load database config files load_db_config() namespaces = multi_asic.get_namespace_list() if args.namespace and args.namespace not in namespaces: - print("Input arguments error. Namespaces must be one of", *namespaces) + namespacelist = ', '.join(namespaces) + print(f"Input value for '--namespace' / '-n'. Choose from one of ({namespacelist})") sys.exit(1) - pgdropstat = PgDropStat(args.namespace) + # For 'clear' command force applying to all namespaces + pgdropstat = PgDropStat(args.namespace if command != 'clear' else None) if command == 'clear': pgdropstat.clear_drop_counts() diff --git a/tests/multi_asic_pgdropstat_test.py b/tests/multi_asic_pgdropstat_test.py index 25682372180..94bb13011b4 100644 --- a/tests/multi_asic_pgdropstat_test.py +++ b/tests/multi_asic_pgdropstat_test.py @@ -1,7 +1,7 @@ import os import sys from utilities_common.cli import UserCache -from utils import get_result_and_return_code +from .utils import get_result_and_return_code test_path = os.path.dirname(os.path.abspath(__file__)) modules_path = os.path.dirname(test_path) @@ -9,65 +9,87 @@ sys.path.insert(0, test_path) sys.path.insert(0, modules_path) -pg_drop_masic_one_result="""\ +pg_drop_masic_one_result = """\ Ingress PG dropped packets: - Port PG0 PG1 PG2 PG3 PG4 PG5 PG6 PG7 PG8 PG9 PG10 PG11 PG12 PG13 PG14 PG15 --------------- ----- ----- ----- ----- ----- ----- ----- ----- ----- ----- ------ ------ ------ ------ ------ ------ -Ethernet-BP256 N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A -Ethernet-BP260 N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A + Port PG0 PG1 PG2 PG3 PG4 PG5 PG6 PG7 PG8 PG9 PG10 PG11 PG12 PG13\ + PG14 PG15 +-------------- ----- ----- ----- ----- ----- ----- ----- ----- ----- ----- ------ ------ ------ ------\ + ------ ------ +Ethernet-BP256 N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A\ + N/A N/A +Ethernet-BP260 N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A\ + N/A N/A """ -pg_drop_masic_all_result="""\ +pg_drop_masic_all_result = """\ Ingress PG dropped packets: - Port PG0 PG1 PG2 PG3 PG4 PG5 PG6 PG7 PG8 PG9 PG10 PG11 PG12 PG13 PG14 PG15 --------------- ----- ----- ----- ----- ----- ----- ----- ----- ----- ----- ------ ------ ------ ------ ------ ------ - Ethernet0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 - Ethernet4 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 - Ethernet-BP0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 - Ethernet-BP4 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 -Ethernet-BP256 N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A -Ethernet-BP260 N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A + Port PG0 PG1 PG2 PG3 PG4 PG5 PG6 PG7 PG8 PG9 PG10 PG11 PG12 PG13\ + PG14 PG15 +-------------- ----- ----- ----- ----- ----- ----- ----- ----- ----- ----- ------ ------ ------ ------\ + ------ ------ + Ethernet0 0 0 0 0 0 0 0 0 0 0 0 0 0 0\ + 0 0 + Ethernet4 0 0 0 0 0 0 0 0 0 0 0 0 0 0\ + 0 0 + Ethernet-BP0 0 0 0 0 0 0 0 0 0 0 0 0 0 0\ + 0 0 + Ethernet-BP4 0 0 0 0 0 0 0 0 0 0 0 0 0 0\ + 0 0 +Ethernet-BP256 N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A\ + N/A N/A +Ethernet-BP260 N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A N/A\ + N/A N/A """ + class TestMultiAsicPgDropstat(object): - @classmethod - def setup_class(cls): - os.environ["PATH"] += os.pathsep + scripts_path - os.environ['UTILITIES_UNIT_TESTING'] = "2" - os.environ["UTILITIES_UNIT_TESTING_TOPOLOGY"] = "multi_asic" - print("SETUP") + @classmethod + def setup_class(cls): + os.environ["PATH"] += os.pathsep + scripts_path + os.environ['UTILITIES_UNIT_TESTING'] = "2" + os.environ["UTILITIES_UNIT_TESTING_TOPOLOGY"] = "multi_asic" + print("SETUP") + + def test_show_pg_drop_masic_all(self): + return_code, result = get_result_and_return_code([ + 'pg-drop', '-c', 'show' + ]) + print("return_code: {}".format(return_code)) + print("result = {}".format(result)) + assert return_code == 0 + assert result == pg_drop_masic_all_result - def test_show_pg_drop_masic_all(self): - return_code, result = get_result_and_return_code([ - 'pg-drop', '-c', 'show' - ]) - print("return_code: {}".format(return_code)) - print("result = {}".format(result)) - assert return_code == 0 - assert result == pg_drop_masic_all_result + def test_show_pg_drop_masic(self): + return_code, result = get_result_and_return_code([ + 'pg-drop', '-c', 'show', '-n', 'asic1' + ]) + print("return_code: {}".format(return_code)) + print("result = {}".format(result)) + assert return_code == 0 + assert result == pg_drop_masic_one_result - def test_show_pg_drop_masic(self): - return_code, result = get_result_and_return_code([ - 'pg-drop', '-c', 'show', '-n', 'asic1' - ]) - print("return_code: {}".format(return_code)) - print("result = {}".format(result)) - assert return_code == 0 - assert result == pg_drop_masic_one_result + def test_show_pg_drop_masic_not_exist(self): + return_code, result = get_result_and_return_code([ + 'pg-drop', '-c', 'show', '-n', 'asic5' + ]) + print("return_code: {}".format(return_code)) + print("result = {}".format(result)) + assert return_code == 1 + assert result == "Input value for '--namespace' / '-n'. Choose from one of (asic0, asic1)" - def test_show_pg_drop_masic_not_exist(self): - return_code, result = get_result_and_return_code([ - 'pg-drop', '-c', 'show', '-n', 'asic5' - ]) - print("return_code: {}".format(return_code)) - print("result = {}".format(result)) - assert return_code == 1 - assert result == "Input arguments error. Namespaces must be one of asic0 asic1" + def test_clear_pg_drop(self): + return_code, result = get_result_and_return_code([ + 'pg-drop', '-c', 'clear' + ]) + print("return_code: {}".format(return_code)) + print("result = {}".format(result)) + assert return_code == 0 + assert result == "Cleared PG drop counter\n" - @classmethod - def teardown_class(cls): - os.environ["PATH"] = os.pathsep.join(os.environ["PATH"].split(os.pathsep)[:-1]) - os.environ['UTILITIES_UNIT_TESTING'] = "0" - os.environ["UTILITIES_UNIT_TESTING_TOPOLOGY"] = "" - UserCache('pg-drop').remove_all() - print("TEARDOWN") + @classmethod + def teardown_class(cls): + os.environ["PATH"] = os.pathsep.join(os.environ["PATH"].split(os.pathsep)[:-1]) + os.environ['UTILITIES_UNIT_TESTING'] = "0" + os.environ["UTILITIES_UNIT_TESTING_TOPOLOGY"] = "" + UserCache('pg-drop').remove_all() + print("TEARDOWN")