From ef7f52e5ec411208dd42d59669b99a3d6a3d5a65 Mon Sep 17 00:00:00 2001 From: Xin Wang Date: Thu, 4 Mar 2021 04:25:31 +0000 Subject: [PATCH] Add verbose option for suppressing unnecessary log When execute an ansible module, the detailed arguments and module result are logged at DEBUG level by default. Sometimes, the module result could be too huge to be logged even at DEBUG level. This change added keyword option "verbose" to the method in AnsibleHostBase class for executing ansible modules. Default value of "verbose" is "True". If "verbose" is set to "False", only simplified messages will be logged at DEBUG level. This change also updated some test scripts and the sanity check scripts to use this option. Signed-off-by: Xin Wang --- tests/bgp/bgp_helpers.py | 4 ++-- tests/common/devices/base.py | 21 ++++++++++++++----- tests/common/devices/sonic.py | 6 +++--- tests/common/devices/sonic_asic.py | 2 +- tests/common/plugins/sanity_check/__init__.py | 4 ++-- tests/common/plugins/sanity_check/checks.py | 4 ++-- tests/route/test_route_perf.py | 7 ++++--- 7 files changed, 30 insertions(+), 18 deletions(-) diff --git a/tests/bgp/bgp_helpers.py b/tests/bgp/bgp_helpers.py index 3f5e4217f67..78f39d045d7 100644 --- a/tests/bgp/bgp_helpers.py +++ b/tests/bgp/bgp_helpers.py @@ -89,7 +89,7 @@ def parse_exabgp_dump(host): Parse the dump file of exabgp, and build a set for checking routes """ routes = set() - output_lines = host.shell("cat {}".format(DUMP_FILE))['stdout_lines'] + output_lines = host.shell("cat {}".format(DUMP_FILE), verbose=False)['stdout_lines'] for line in output_lines: routes.add(line) return routes @@ -100,7 +100,7 @@ def parse_rib(host, ip_ver): """ routes = {} cmd = "vtysh -c \"show bgp ipv%d json\"" % ip_ver - route_data = json.loads(host.shell(cmd)['stdout']) + route_data = json.loads(host.shell(cmd, verbose=False)['stdout']) for ip, nexthops in route_data['routes'].iteritems(): aspath = set() for nexthop in nexthops: diff --git a/tests/common/devices/base.py b/tests/common/devices/base.py index 5d5982a04e0..cc66367bd45 100644 --- a/tests/common/devices/base.py +++ b/tests/common/devices/base.py @@ -55,9 +55,15 @@ def _run(self, *module_args, **complex_args): previous_frame = inspect.currentframe().f_back filename, line_number, function_name, lines, index = inspect.getframeinfo(previous_frame) - logging.debug("{}::{}#{}: [{}] AnsibleModule::{}, args={}, kwargs={}"\ - .format(filename, function_name, line_number, self.hostname, - self.module_name, json.dumps(module_args), json.dumps(complex_args))) + verbose = complex_args.pop('verbose', True) + + if verbose: + logging.debug("{}::{}#{}: [{}] AnsibleModule::{}, args={}, kwargs={}"\ + .format(filename, function_name, line_number, self.hostname, + self.module_name, json.dumps(module_args), json.dumps(complex_args))) + else: + logging.debug("{}::{}#{}: [{}] AnsibleModule::{} executing..."\ + .format(filename, function_name, line_number, self.hostname, self.module_name)) module_ignore_errors = complex_args.pop('module_ignore_errors', False) module_async = complex_args.pop('module_async', False) @@ -70,8 +76,13 @@ def run_module(module_args, complex_args): return pool, result res = self.module(*module_args, **complex_args)[self.hostname] - logging.debug("{}::{}#{}: [{}] AnsibleModule::{} Result => {}"\ - .format(filename, function_name, line_number, self.hostname, self.module_name, json.dumps(res))) + + if verbose: + logging.debug("{}::{}#{}: [{}] AnsibleModule::{} Result => {}"\ + .format(filename, function_name, line_number, self.hostname, self.module_name, json.dumps(res))) + else: + logging.debug("{}::{}#{}: [{}] AnsibleModule::{} done"\ + .format(filename, function_name, line_number, self.hostname, self.module_name)) if (res.is_failed or 'exception' in res) and not module_ignore_errors: raise RunAnsibleModuleFail("run module {} failed".format(self.module_name), res) diff --git a/tests/common/devices/sonic.py b/tests/common/devices/sonic.py index e05952ec8d7..bac89738b68 100644 --- a/tests/common/devices/sonic.py +++ b/tests/common/devices/sonic.py @@ -333,7 +333,7 @@ def get_monit_services_status(self): """ monit_services_status = {} - services_status_result = self.shell("sudo monit status", module_ignore_errors=True) + services_status_result = self.shell("sudo monit status", module_ignore_errors=True, verbose=False) exit_code = services_status_result["rc"] if exit_code != 0: @@ -1079,7 +1079,7 @@ def get_extended_minigraph_facts(self, tbinfo): def run_redis_cli_cmd(self, redis_cmd): cmd = "/usr/bin/redis-cli {}".format(redis_cmd) - return self.command(cmd) + return self.command(cmd, verbose=False) def get_asic_name(self): asic = "unknown" @@ -1099,7 +1099,7 @@ def get_asic_name(self): return asic def get_running_config_facts(self): - return self.config_facts(host=self.hostname, source='running')['ansible_facts'] + return self.config_facts(host=self.hostname, source='running', verbose=False)['ansible_facts'] def get_vlan_intfs(self): ''' diff --git a/tests/common/devices/sonic_asic.py b/tests/common/devices/sonic_asic.py index 81ccaae34b8..e1f8f9ecdd7 100644 --- a/tests/common/devices/sonic_asic.py +++ b/tests/common/devices/sonic_asic.py @@ -146,7 +146,7 @@ def run_redis_cli_cmd(self, redis_cmd): if self.namespace != DEFAULT_NAMESPACE: redis_cli = "/usr/bin/redis-cli" cmd = "sudo ip netns exec {} {} {}".format(self.namespace, redis_cli,redis_cmd) - return self.sonichost.command(cmd) + return self.sonichost.command(cmd, verbose=False) # for single asic platforms there are not Namespaces, so the redis-cli command is same the DUT host return self.sonichost.run_redis_cli_cmd(redis_cmd) diff --git a/tests/common/plugins/sanity_check/__init__.py b/tests/common/plugins/sanity_check/__init__.py index de86c3b5ad2..4d0db02f7d7 100644 --- a/tests/common/plugins/sanity_check/__init__.py +++ b/tests/common/plugins/sanity_check/__init__.py @@ -22,7 +22,7 @@ def is_check_item(member): ''' Function to filter for valid check items - Used in conjuction with inspect.getmembers to make sure that only valid check functions/fixtures executed + Used in conjuction with inspect.getmembers to make sure that only valid check functions/fixtures executed Valid check items must meet the following criteria: - Is a function @@ -82,7 +82,7 @@ def print_logs(duthosts): logger.info("Run commands to print logs, logs to be collected on {}:\n{}"\ .format(dut.hostname, json.dumps(constants.PRINT_LOGS, indent=4))) for cmd in constants.PRINT_LOGS.values(): - res = dut.shell(cmd, module_ignore_errors=True) + res = dut.shell(cmd, module_ignore_errors=True, verbose=False) logger.info("cmd='%s', output:\n%s" % (cmd, json.dumps(res["stdout_lines"], indent=4))) diff --git a/tests/common/plugins/sanity_check/checks.py b/tests/common/plugins/sanity_check/checks.py index b4adf5e164d..c55b4116119 100644 --- a/tests/common/plugins/sanity_check/checks.py +++ b/tests/common/plugins/sanity_check/checks.py @@ -125,7 +125,7 @@ def _check(): for asic in dut.asics: ip_interfaces = [] cfg_facts = asic.config_facts(host=dut.hostname, - source="persistent")['ansible_facts'] + source="persistent", verbose=False)['ansible_facts'] phy_interfaces = [k for k, v in cfg_facts["PORT"].items() if "admin_status" in v and v["admin_status"] == "up"] if "PORTCHANNEL_INTERFACE" in cfg_facts: ip_interfaces = cfg_facts["PORTCHANNEL_INTERFACE"].keys() @@ -340,7 +340,7 @@ def _check(): ip_snd=lower_tor_mgmt_ip, ip_tgt=lower_tor_ping_tgt_ip, hw_snd=lower_tor_intf_mac) - + ptf_arp_pkt = testutils.simple_arp_packet(ip_tgt=ptf_arp_tgt_ip, ip_snd=ptf_arp_tgt_ip, arp_op=2) diff --git a/tests/route/test_route_perf.py b/tests/route/test_route_perf.py index 4ccf97ef614..c3d0bc82ad5 100644 --- a/tests/route/test_route_perf.py +++ b/tests/route/test_route_perf.py @@ -126,12 +126,12 @@ def generate_route_file(duthost, prefixes, str_intf_nexthop, dir, op): route_data.append(route_command) # Copy json file to DUT - duthost.copy(content=json.dumps(route_data, indent=4), dest=dir) + duthost.copy(content=json.dumps(route_data, indent=4), dest=dir, verbose=False) def count_routes(host): num = host.shell( 'sonic-db-cli ASIC_DB eval "return #redis.call(\'keys\', \'{}*\')" 0'.format(ROUTE_TABLE_NAME), - module_ignore_errors=True)['stdout'] + module_ignore_errors=True, verbose=True)['stdout'] return int(num) def exec_routes(duthost, prefixes, str_intf_nexthop, op): @@ -174,7 +174,8 @@ def _check_num_routes(expected_num_routes): end_time = datetime.now() # Check route entries are correct - asic_route_keys = duthost.shell('sonic-db-cli ASIC_DB eval "return redis.call(\'keys\', \'{}*\')" 0'.format(ROUTE_TABLE_NAME))['stdout_lines'] + asic_route_keys = duthost.shell('sonic-db-cli ASIC_DB eval "return redis.call(\'keys\', \'{}*\')" 0'\ + .format(ROUTE_TABLE_NAME), verbose=False)['stdout_lines'] asic_prefixes = [] for key in asic_route_keys: json_obj = key[len(ROUTE_TABLE_NAME) + 1 : ]