From 123b54cbc09e6794c3f48230879e0cbd981d2c27 Mon Sep 17 00:00:00 2001 From: Aayush Gadia Date: Thu, 10 Oct 2024 16:26:53 -0700 Subject: [PATCH 1/5] added check for running state of snmp-subagent --- tests/common/helpers/snmp_helpers.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/tests/common/helpers/snmp_helpers.py b/tests/common/helpers/snmp_helpers.py index f89b7b7ab4..2b000021b5 100644 --- a/tests/common/helpers/snmp_helpers.py +++ b/tests/common/helpers/snmp_helpers.py @@ -10,20 +10,32 @@ DEF_WAIT_TIMEOUT = 300 DEF_CHECK_INTERVAL = 10 +SNMP_SUBAGENT_WAIT_TIMEOUT = 120 +SNMP_SUBAGENT_CHECK_INTERVAL = 5 global_snmp_facts = {} +def is_snmp_subagent_running(duthost): + cmd = "docker exec snmp supervisorctl status snmp-subagent" + output = duthost.shell(cmd) + if "RUNNING" in output["stdout"]: + return True + return False + + def _get_snmp_facts(localhost, host, version, community, is_dell, include_swap, module_ignore_errors): snmp_facts = localhost.snmp_facts(host=host, version=version, community=community, is_dell=is_dell, module_ignore_errors=module_ignore_errors, include_swap=include_swap) return snmp_facts -def _update_snmp_facts(localhost, host, version, community, is_dell, include_swap): +def _update_snmp_facts(localhost, host, version, community, is_dell, include_swap, duthost): global global_snmp_facts try: + pytest_assert(wait_until(SNMP_SUBAGENT_WAIT_TIMEOUT, SNMP_SUBAGENT_CHECK_INTERVAL, 0, is_snmp_subagent_running, duthost), + "SNMP Sub-Agent is not in Running state") global_snmp_facts = _get_snmp_facts(localhost, host, version, community, is_dell, include_swap, module_ignore_errors=False) except RunAnsibleModuleFail as e: @@ -34,7 +46,7 @@ def _update_snmp_facts(localhost, host, version, community, is_dell, include_swa return True -def get_snmp_facts(localhost, host, version, community, is_dell=False, module_ignore_errors=False, +def get_snmp_facts(duthost, localhost, host, version, community, is_dell=False, module_ignore_errors=False, wait=False, include_swap=False, timeout=DEF_WAIT_TIMEOUT, interval=DEF_CHECK_INTERVAL): if not wait: return _get_snmp_facts(localhost, host, version, community, is_dell, include_swap, module_ignore_errors) @@ -42,7 +54,7 @@ def get_snmp_facts(localhost, host, version, community, is_dell=False, module_ig global global_snmp_facts pytest_assert(wait_until(timeout, interval, 0, _update_snmp_facts, localhost, host, version, - community, is_dell, include_swap), "Timeout waiting for SNMP facts") + community, is_dell, include_swap, duthost), "Timeout waiting for SNMP facts") return global_snmp_facts From 501951b6f51edd906d8c38c217a87d0c193d5a4f Mon Sep 17 00:00:00 2001 From: Aayush Gadia Date: Thu, 10 Oct 2024 16:27:10 -0700 Subject: [PATCH 2/5] updated get_snmp_facts by passing duthost as param --- tests/snmp/test_snmp_cpu.py | 4 ++-- tests/snmp/test_snmp_default_route.py | 2 +- tests/snmp/test_snmp_fdb.py | 2 +- tests/snmp/test_snmp_interfaces.py | 6 +++--- tests/snmp/test_snmp_link_local.py | 2 +- tests/snmp/test_snmp_lldp.py | 2 +- tests/snmp/test_snmp_loopback.py | 2 +- tests/snmp/test_snmp_memory.py | 4 ++-- tests/snmp/test_snmp_pfc_counters.py | 2 +- tests/snmp/test_snmp_phy_entity.py | 2 +- tests/snmp/test_snmp_psu.py | 4 ++-- tests/snmp/test_snmp_queue.py | 2 +- tests/snmp/test_snmp_v2mib.py | 2 +- 13 files changed, 18 insertions(+), 18 deletions(-) diff --git a/tests/snmp/test_snmp_cpu.py b/tests/snmp/test_snmp_cpu.py index e03c34d57e..101ad1f535 100644 --- a/tests/snmp/test_snmp_cpu.py +++ b/tests/snmp/test_snmp_cpu.py @@ -39,7 +39,7 @@ def test_snmp_cpu(duthosts, enum_rand_one_per_hwsku_hostname, localhost, creds_a # Gather facts with SNMP version 2 snmp_facts = get_snmp_facts( - localhost, host=hostip, version="v2c", + duthost, localhost, host=hostip, version="v2c", community=creds_all_duts[duthost.hostname]["snmp_rocommunity"], is_dell=True, wait=True)['ansible_facts'] assert int(snmp_facts['ansible_ChStackUnitCpuUtil5sec']) @@ -53,7 +53,7 @@ def test_snmp_cpu(duthosts, enum_rand_one_per_hwsku_hostname, localhost, creds_a # Gather facts with SNMP version 2 snmp_facts = get_snmp_facts( - localhost, host=hostip, version="v2c", + duthost, localhost, host=hostip, version="v2c", community=creds_all_duts[duthost.hostname]["snmp_rocommunity"], is_dell=True, wait=True)['ansible_facts'] # Pull CPU utilization via shell diff --git a/tests/snmp/test_snmp_default_route.py b/tests/snmp/test_snmp_default_route.py index 1b476853ed..11fdc847db 100644 --- a/tests/snmp/test_snmp_default_route.py +++ b/tests/snmp/test_snmp_default_route.py @@ -21,7 +21,7 @@ def test_snmp_default_route(duthosts, enum_rand_one_per_hwsku_frontend_hostname, hostip = duthost.host.options['inventory_manager'].get_host( duthost.hostname).vars['ansible_host'] snmp_facts = get_snmp_facts( - localhost, host=hostip, version="v2c", + duthost, localhost, host=hostip, version="v2c", community=creds_all_duts[duthost.hostname]["snmp_rocommunity"], wait=True)['ansible_facts'] dut_result = duthost.shell(r'show ip route 0.0.0.0/0 | grep "\*"') diff --git a/tests/snmp/test_snmp_fdb.py b/tests/snmp/test_snmp_fdb.py index 22a09087d5..711b4bf6eb 100644 --- a/tests/snmp/test_snmp_fdb.py +++ b/tests/snmp/test_snmp_fdb.py @@ -130,7 +130,7 @@ def test_snmp_fdb_send_tagged(ptfadapter, duthosts, rand_one_dut_hostname, hostip = duthost.host.options['inventory_manager'].get_host( duthost.hostname).vars['ansible_host'] snmp_facts = get_snmp_facts( - localhost, host=hostip, version="v2c", + duthost, localhost, host=hostip, version="v2c", community=creds_all_duts[duthost.hostname]["snmp_rocommunity"], wait=True)['ansible_facts'] assert 'snmp_fdb' in snmp_facts assert 'snmp_interfaces' in snmp_facts diff --git a/tests/snmp/test_snmp_interfaces.py b/tests/snmp/test_snmp_interfaces.py index 956e13237d..54a95066a2 100644 --- a/tests/snmp/test_snmp_interfaces.py +++ b/tests/snmp/test_snmp_interfaces.py @@ -168,7 +168,7 @@ def test_snmp_interfaces(localhost, creds_all_duts, duthosts, enum_rand_one_per_ config_facts = duthost.config_facts( host=duthost.hostname, source="persistent", namespace=namespace)['ansible_facts'] snmp_facts = get_snmp_facts( - localhost, host=hostip, version="v2c", + duthost, localhost, host=hostip, version="v2c", community=creds_all_duts[duthost.hostname]["snmp_rocommunity"], wait=True)['ansible_facts'] snmp_ifnames = [v['name'] @@ -193,7 +193,7 @@ def test_snmp_mgmt_interface(localhost, creds_all_duts, duthosts, enum_rand_one_ duthost.hostname).vars['ansible_host'] snmp_facts = get_snmp_facts( - localhost, host=hostip, version="v2c", + duthost, localhost, host=hostip, version="v2c", community=creds_all_duts[duthost.hostname]["snmp_rocommunity"], wait=True)['ansible_facts'] config_facts = duthost.config_facts( host=duthost.hostname, source="persistent")['ansible_facts'] @@ -227,7 +227,7 @@ def test_snmp_interfaces_mibs(duthosts, enum_rand_one_per_hwsku_hostname, localh hostip = duthost.host.options['inventory_manager'].get_host( duthost.hostname).vars['ansible_host'] snmp_facts = get_snmp_facts( - localhost, host=hostip, version="v2c", + duthost, localhost, host=hostip, version="v2c", community=creds_all_duts[duthost.hostname]["snmp_rocommunity"], wait=True)['ansible_facts'] config_facts = duthost.config_facts( host=duthost.hostname, source="persistent", namespace=namespace)['ansible_facts'] diff --git a/tests/snmp/test_snmp_link_local.py b/tests/snmp/test_snmp_link_local.py index 7f8f58ff68..8486bc02d9 100644 --- a/tests/snmp/test_snmp_link_local.py +++ b/tests/snmp/test_snmp_link_local.py @@ -31,7 +31,7 @@ def test_snmp_link_local_ip(duthosts, hostip = duthost.host.options['inventory_manager'].get_host( duthost.hostname).vars['ansible_host'] snmp_facts = get_snmp_facts( - localhost, host=hostip, version="v2c", + duthost, localhost, host=hostip, version="v2c", community=creds_all_duts[duthost.hostname]["snmp_rocommunity"], wait=True)['ansible_facts'] # Get link local IP of mamangement interface diff --git a/tests/snmp/test_snmp_lldp.py b/tests/snmp/test_snmp_lldp.py index 7b51fdf112..dd43dd22aa 100644 --- a/tests/snmp/test_snmp_lldp.py +++ b/tests/snmp/test_snmp_lldp.py @@ -43,7 +43,7 @@ def test_snmp_lldp(duthosts, enum_rand_one_per_hwsku_hostname, localhost, creds_ duthost.hostname).vars['ansible_host'] snmp_facts = get_snmp_facts( - localhost, host=hostip, version="v2c", + duthost, localhost, host=hostip, version="v2c", community=creds_all_duts[duthost.hostname]["snmp_rocommunity"], wait=True)['ansible_facts'] mg_facts = {} for asic_id in duthost.get_asic_ids(): diff --git a/tests/snmp/test_snmp_loopback.py b/tests/snmp/test_snmp_loopback.py index 4a8feab2b5..5d480373a1 100644 --- a/tests/snmp/test_snmp_loopback.py +++ b/tests/snmp/test_snmp_loopback.py @@ -23,7 +23,7 @@ def test_snmp_loopback(duthosts, enum_rand_one_per_hwsku_frontend_hostname, hostip = duthost.host.options['inventory_manager'].get_host( duthost.hostname).vars['ansible_host'] snmp_facts = get_snmp_facts( - localhost, host=hostip, version="v2c", + duthost, localhost, host=hostip, version="v2c", community=creds_all_duts[duthost.hostname]["snmp_rocommunity"], wait=True)['ansible_facts'] config_facts = duthost.config_facts( host=duthost.hostname, source="persistent")['ansible_facts'] diff --git a/tests/snmp/test_snmp_memory.py b/tests/snmp/test_snmp_memory.py index 5e7397d8b9..298c9752d9 100644 --- a/tests/snmp/test_snmp_memory.py +++ b/tests/snmp/test_snmp_memory.py @@ -88,7 +88,7 @@ def test_snmp_memory(duthosts, enum_rand_one_per_hwsku_hostname, localhost, cred # Allow the test to retry a few times before claiming failure. for _ in range(3): snmp_facts = get_snmp_facts( - localhost, host=host_ip, version="v2c", + duthost, localhost, host=host_ip, version="v2c", community=creds_all_duts[duthost.hostname]["snmp_rocommunity"], wait=True)['ansible_facts'] facts = collect_memory(duthost) # net-snmp calculate cached memory as cached + sreclaimable @@ -175,7 +175,7 @@ def test_snmp_swap(duthosts, enum_rand_one_per_hwsku_hostname, localhost, creds_ "Swap is not on for this device, snmp does not support swap related queries when swap isn't on") snmp_facts = get_snmp_facts( - localhost, host=host_ip, version="v2c", include_swap=True, + duthost, localhost, host=host_ip, version="v2c", include_swap=True, community=creds_all_duts[duthost.hostname]["snmp_rocommunity"], wait=True)['ansible_facts'] snmp_total_swap = snmp_facts['ansible_sysTotalSwap'] snmp_free_swap = snmp_facts['ansible_sysTotalFreeSwap'] diff --git a/tests/snmp/test_snmp_pfc_counters.py b/tests/snmp/test_snmp_pfc_counters.py index 9313613ef9..6f3e50f487 100644 --- a/tests/snmp/test_snmp_pfc_counters.py +++ b/tests/snmp/test_snmp_pfc_counters.py @@ -14,7 +14,7 @@ def test_snmp_pfc_counters(duthosts, enum_rand_one_per_hwsku_frontend_hostname, duthost.hostname).vars['ansible_host'] snmp_facts = get_snmp_facts( - localhost, host=hostip, version="v2c", + duthost, localhost, host=hostip, version="v2c", community=creds_all_duts[duthost.hostname]["snmp_rocommunity"], wait=True)['ansible_facts'] # Check PFC counters diff --git a/tests/snmp/test_snmp_phy_entity.py b/tests/snmp/test_snmp_phy_entity.py index dfc6490fe4..18421fbb69 100644 --- a/tests/snmp/test_snmp_phy_entity.py +++ b/tests/snmp/test_snmp_phy_entity.py @@ -209,7 +209,7 @@ def get_entity_and_sensor_mib(duthost, localhost, creds_all_duts): hostip = duthost.host.options['inventory_manager'].get_host( duthost.hostname).vars['ansible_host'] snmp_facts = get_snmp_facts( - localhost, host=hostip, version="v2c", + duthost, localhost, host=hostip, version="v2c", community=creds_all_duts[duthost.hostname]["snmp_rocommunity"], wait=True)['ansible_facts'] entity_mib = {} sensor_mib = {} diff --git a/tests/snmp/test_snmp_psu.py b/tests/snmp/test_snmp_psu.py index 05d2ce4942..9553b1d9d6 100644 --- a/tests/snmp/test_snmp_psu.py +++ b/tests/snmp/test_snmp_psu.py @@ -20,7 +20,7 @@ def test_snmp_numpsu(duthosts, enum_supervisor_dut_hostname, localhost, creds_al duthost.hostname).vars['ansible_host'] snmp_facts = get_snmp_facts( - localhost, host=hostip, version="v2c", + duthost, localhost, host=hostip, version="v2c", community=creds_all_duts[duthost.hostname]["snmp_rocommunity"], wait=True)['ansible_facts'] res = duthost.shell("psuutil numpsus", module_ignore_errors=True) @@ -41,7 +41,7 @@ def test_snmp_psu_status(duthosts, enum_supervisor_dut_hostname, localhost, cred hostip = duthost.host.options['inventory_manager'].get_host( duthost.hostname).vars['ansible_host'] snmp_facts = get_snmp_facts( - localhost, host=hostip, version="v2c", + duthost, localhost, host=hostip, version="v2c", community=creds_all_duts[duthost.hostname]["snmp_rocommunity"], wait=True)['ansible_facts'] psus_on = 0 diff --git a/tests/snmp/test_snmp_queue.py b/tests/snmp/test_snmp_queue.py index e4455d9af9..afe6804a48 100644 --- a/tests/snmp/test_snmp_queue.py +++ b/tests/snmp/test_snmp_queue.py @@ -58,7 +58,7 @@ def test_snmp_queues(duthosts, enum_rand_one_per_hwsku_hostname, localhost, cred q_interfaces[intf[intf_idx]] = set() q_interfaces[intf[intf_idx]].add(intf[queue_idx]) - snmp_facts = get_snmp_facts(localhost, host=hostip, version="v2c", + snmp_facts = get_snmp_facts(duthost, localhost, host=hostip, version="v2c", community=creds_all_duts[duthost.hostname]["snmp_rocommunity"], wait=True)['ansible_facts'] diff --git a/tests/snmp/test_snmp_v2mib.py b/tests/snmp/test_snmp_v2mib.py index c17d48b096..0b0540733b 100644 --- a/tests/snmp/test_snmp_v2mib.py +++ b/tests/snmp/test_snmp_v2mib.py @@ -19,7 +19,7 @@ def test_snmp_v2mib(duthosts, enum_rand_one_per_hwsku_hostname, localhost, creds host_ip = duthost.host.options['inventory_manager'].get_host( duthost.hostname).vars['ansible_host'] snmp_facts = get_snmp_facts( - localhost, host=host_ip, version="v2c", + duthost, localhost, host=host_ip, version="v2c", community=creds_all_duts[duthost.hostname]["snmp_rocommunity"], wait=True)['ansible_facts'] dut_facts = duthost.setup()['ansible_facts'] debian_ver = duthost.shell('cat /etc/debian_version')['stdout'] From 1ed05bd2d5395e25d18e2703a98952855595aa44 Mon Sep 17 00:00:00 2001 From: Aayush Gadia Date: Thu, 10 Oct 2024 17:15:05 -0700 Subject: [PATCH 3/5] linter issues --- tests/common/helpers/snmp_helpers.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/common/helpers/snmp_helpers.py b/tests/common/helpers/snmp_helpers.py index 2b000021b5..61a8dc1cc9 100644 --- a/tests/common/helpers/snmp_helpers.py +++ b/tests/common/helpers/snmp_helpers.py @@ -34,8 +34,10 @@ def _update_snmp_facts(localhost, host, version, community, is_dell, include_swa global global_snmp_facts try: - pytest_assert(wait_until(SNMP_SUBAGENT_WAIT_TIMEOUT, SNMP_SUBAGENT_CHECK_INTERVAL, 0, is_snmp_subagent_running, duthost), - "SNMP Sub-Agent is not in Running state") + pytest_assert( + wait_until(SNMP_SUBAGENT_WAIT_TIMEOUT, SNMP_SUBAGENT_CHECK_INTERVAL, 0, + is_snmp_subagent_running, duthost), + "SNMP Sub-Agent is not in Running state") global_snmp_facts = _get_snmp_facts(localhost, host, version, community, is_dell, include_swap, module_ignore_errors=False) except RunAnsibleModuleFail as e: From 375c029511b9dc5e205b8368baa73fc6de899b64 Mon Sep 17 00:00:00 2001 From: Aayush Gadia Date: Fri, 11 Oct 2024 13:20:17 -0700 Subject: [PATCH 4/5] updated get_snmp_facts params --- tests/cacl/test_cacl_function.py | 8 +++++--- tests/mvrf/test_mgmtvrf.py | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/cacl/test_cacl_function.py b/tests/cacl/test_cacl_function.py index 0e853be7d1..712ad618d3 100644 --- a/tests/cacl/test_cacl_function.py +++ b/tests/cacl/test_cacl_function.py @@ -34,7 +34,8 @@ def test_cacl_function(duthosts, enum_rand_one_per_hwsku_hostname, localhost, cr logging.warning("Will not check NTP connection. ntplib is not installed.") # Ensure we can gather basic SNMP facts from the device. Should fail on timeout - get_snmp_facts(localhost, + get_snmp_facts(duthost, + localhost, host=dut_mgmt_ip, version="v2c", community=creds['snmp_rocommunity'], @@ -83,7 +84,7 @@ def test_cacl_function(duthosts, enum_rand_one_per_hwsku_hostname, localhost, cr pytest_assert(res.is_failed, "SSH did not timeout when expected. {}".format(res.get('msg', ''))) # Ensure we CANNOT gather basic SNMP facts from the device - res = get_snmp_facts(localhost, host=dut_mgmt_ip, version='v2c', community=creds['snmp_rocommunity'], + res = get_snmp_facts(duthost, localhost, host=dut_mgmt_ip, version='v2c', community=creds['snmp_rocommunity'], module_ignore_errors=True) pytest_assert('ansible_facts' not in res and "No SNMP response received before timeout" in res.get('msg', '')) @@ -114,7 +115,8 @@ def test_cacl_function(duthosts, enum_rand_one_per_hwsku_hostname, localhost, cr duthost.file(path="/tmp/config_service_acls.sh", state="absent") # Ensure we can gather basic SNMP facts from the device once again. Should fail on timeout - get_snmp_facts(localhost, + get_snmp_facts(duthost, + localhost, host=dut_mgmt_ip, version="v2c", community=creds['snmp_rocommunity'], diff --git a/tests/mvrf/test_mgmtvrf.py b/tests/mvrf/test_mgmtvrf.py index 8a184579fc..772e1755b0 100644 --- a/tests/mvrf/test_mgmtvrf.py +++ b/tests/mvrf/test_mgmtvrf.py @@ -176,7 +176,7 @@ def test_ping(self, duthost): duthost.ping() def test_snmp_fact(self, localhost, duthost, creds): - get_snmp_facts(localhost, host=duthost.mgmt_ip, version="v2c", community=creds['snmp_rocommunity']) + get_snmp_facts(duthost, localhost, host=duthost.mgmt_ip, version="v2c", community=creds['snmp_rocommunity']) class TestMvrfOutbound(): From ef47030149fb6f6c7fbb2b4773b75c8dfcdea182 Mon Sep 17 00:00:00 2001 From: Aayush Gadia Date: Tue, 29 Oct 2024 12:34:44 -0700 Subject: [PATCH 5/5] added PR feedback to _update_snmp_facts fn --- tests/common/helpers/snmp_helpers.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/common/helpers/snmp_helpers.py b/tests/common/helpers/snmp_helpers.py index 61a8dc1cc9..c187e509f2 100644 --- a/tests/common/helpers/snmp_helpers.py +++ b/tests/common/helpers/snmp_helpers.py @@ -10,8 +10,6 @@ DEF_WAIT_TIMEOUT = 300 DEF_CHECK_INTERVAL = 10 -SNMP_SUBAGENT_WAIT_TIMEOUT = 120 -SNMP_SUBAGENT_CHECK_INTERVAL = 5 global_snmp_facts = {} @@ -20,7 +18,9 @@ def is_snmp_subagent_running(duthost): cmd = "docker exec snmp supervisorctl status snmp-subagent" output = duthost.shell(cmd) if "RUNNING" in output["stdout"]: + logger.info("SNMP Sub-Agent is Running") return True + logger.info("SNMP Sub-Agent is Not Running") return False @@ -34,10 +34,7 @@ def _update_snmp_facts(localhost, host, version, community, is_dell, include_swa global global_snmp_facts try: - pytest_assert( - wait_until(SNMP_SUBAGENT_WAIT_TIMEOUT, SNMP_SUBAGENT_CHECK_INTERVAL, 0, - is_snmp_subagent_running, duthost), - "SNMP Sub-Agent is not in Running state") + snmp_subagent_running = is_snmp_subagent_running(duthost) global_snmp_facts = _get_snmp_facts(localhost, host, version, community, is_dell, include_swap, module_ignore_errors=False) except RunAnsibleModuleFail as e: @@ -45,7 +42,7 @@ def _update_snmp_facts(localhost, host, version, community, is_dell, include_swa global_snmp_facts = {} return False - return True + return snmp_subagent_running and True def get_snmp_facts(duthost, localhost, host, version, community, is_dell=False, module_ignore_errors=False,