Modified SNMP, LLDP and PC testcases for T2 topology #2935

oxygen980 · 2021-02-05T06:26:28Z

Description of PR

Modified testcases for T2 topology, these are very small changes mainly replaced 'rand_one_dut_hostname' with 'enum_rand_one_per_hwsku_frontend_hostname' for multi-dut.

Summary:
Fixes # (issue)

Type of change

Bug fix
Testbed and Framework(new/improvement)
[x ] Test case(new/improvement)

Approach

What is the motivation for this PR?

Modified testcases to run on multi-dut

How did you do it?

Replaced 'rand_one_dut_hostname' with 'enum_rand_one_per_hwsku_frontend_hostname'

How did you verify/test it?

Modified and ran on T2 topology

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

anshuv-mfst · 2021-03-13T02:21:45Z

@yxieca, @wangxin - could you please review.
@eswaranb - please add Arista reviewers

yxieca · 2021-03-21T17:28:48Z

/azp run

azure-pipelines · 2021-03-21T17:28:58Z

Azure Pipelines successfully started running 1 pipeline(s).

shubav · 2021-03-22T16:23:49Z

@wangxin would you know what the root cause is? These are dhcp failures unrelated to the test cases modified in this commit. Could you help resolve this?

oxygen980 · 2021-03-24T05:14:02Z

@wangxin Could you please run again, the failure is not from the code change. Thanks

shubav · 2021-03-30T17:54:55Z

The pipeline checks have passed.
@wangxin - could you please review? Thanks

tests/pc/test_po_cleanup.py

wangxin · 2021-03-31T10:20:49Z

tests/snmp/test_snmp_interfaces.py

    """compare the snmp facts between observed states and target state"""

-    duthost = duthosts[enum_dut_hostname]
+    duthost = duthosts[enum_rand_one_per_hwsku_hostname]
    hostip = duthost.host.options['inventory_manager'].get_host(duthost.hostname).vars['ansible_host']

    snmp_facts = localhost.snmp_facts(host=hostip, version="v2c", community=creds["snmp_rocommunity"])['ansible_facts']


Should the last case in this file be updated from

def test_snmp_interfaces_mibs(duthosts, rand_one_dut_hostname, localhost, creds):

to

def test_snmp_interfaces_mibs(duthosts, enum_rand_one_per_hwsku_hostname, localhost, creds):

?

wangxin · 2021-03-31T10:24:54Z

tests/snmp/test_snmp_interfaces.py

@@ -120,9 +120,9 @@ def verify_snmp_speed(facts, snmp_facts, results):
    return results

 @pytest.mark.bsl
-def test_snmp_interfaces(localhost, creds, duthosts, enum_dut_hostname, enum_asic_index):
+def test_snmp_interfaces(localhost, creds, duthosts, enum_rand_one_per_hwsku_hostname, enum_asic_index):


This will check interfaces on supervisor node too, right? Was the original intention to use fixture enum_rand_one_per_hwsku_frontend_hostname?

@wangxin the reason on doing enum_rand_one_per_hwsku_hostname is because the other test test_snmp_mgmt_interface is applicable to both supervisor and front end node and we found if you have both enum_rand_one_per_hwsku_hostname and enum_rand_one_per_hwsku_frontend_hostname in same module correct nodes are not selected, I will add and if condition to skip this test for supervisor for now

wangxin · 2021-03-31T10:28:43Z

tests/snmp/test_snmp_memory.py

    host_ip = duthost.host.options['inventory_manager'].get_host(duthost.hostname).vars['ansible_host']
    snmp_facts = localhost.snmp_facts(host=host_ip, version="v2c",
                                      community=creds["snmp_rocommunity"])['ansible_facts']
-    facts = collect_memory(duthosts, rand_one_dut_hostname)
+    facts = collect_memory(duthosts, enum_rand_one_per_hwsku_hostname)


Suggest to change the arguments definition of collect_memory to:

def collect_memory(duthost):

Then it would be unnecessary to get duthost using code like duthost = duthosts[enum_rand_one_per_hwsku_hostname].
And the code here can be just:

facts = collect_memory(duthost)

wangxin · 2021-03-31T10:32:56Z

tests/snmp/test_snmp_phy_entity.py

@@ -428,7 +441,7 @@ def _get_transceiver_sensor_data(duthost, name):


 @pytest.mark.disable_loganalyzer
-def test_turn_off_pdu_and_check_psu_info(duthost, localhost, creds, pdu_controller):
+def test_turn_off_psu_and_check_psu_info(duthosts, enum_rand_one_per_hwsku_hostname, snmp_physical_entity_info, localhost, creds, pdu_controller):


Currently the pdu_controller definition is like below:

@pytest.fixture(scope="module") def pdu_controller(duthosts, rand_one_dut_hostname, conn_graph_facts, pdu):

So, the pdu_controller object used by this test case could be a PDU controller for other node in current test bed. How to address this issue?

wangxin · 2021-03-31T10:37:55Z

tests/snmp/test_snmp_cpu.py

@@ -5,12 +5,12 @@
 logger = logging.getLogger(__name__)

 pytestmark = [
-    pytest.mark.topology('any'),
+    pytest.mark.topology('any', 't2'),


The supported topology is already any. It's unnecessary to add t2 here.

wangxin · 2021-03-31T10:38:49Z

tests/snmp/test_snmp_lldp.py


 pytestmark = [
-    pytest.mark.topology('t0', 't1'),
+    pytest.mark.topology('t0', 't1', 't2'),


Change the topology mark to any instead of 't0', 't1', 't2'?

wangxin · 2021-03-31T10:40:38Z

tests/lldp/test_lldp.py

@@ -4,22 +4,22 @@
 logger = logging.getLogger(__name__)

 pytestmark = [
-    pytest.mark.topology('t0', 't1'),
+    pytest.mark.topology('t0', 't1', 't2'),


Change the topology mark to any instead of 't0', 't1', 't2'?

wangxin · 2021-03-31T10:43:20Z

tests/snmp/test_snmp_pfc_counters.py

@@ -1,12 +1,12 @@
 import pytest

 pytestmark = [
-    pytest.mark.topology('any'),
+    pytest.mark.topology('any', 't2'),


The supported topology is already any. It's unnecessary to add t2 here.

tests/snmp/test_snmp_phy_entity.py

…present, changing pdu_controller to use enum_rand_one_per_hwsku_hostname, replace rand_one_hostname to enum_rand_one_per_hwsku_hostname in tests

…s as testcase is using.

wangxin · 2021-04-09T02:28:20Z

tests/snmp/test_snmp_psu.py

-def test_snmp_psu_status(duthosts, rand_one_dut_hostname, localhost, creds):
-    duthost = duthosts[rand_one_dut_hostname]
+def test_snmp_psu_status(duthosts, enum_supervisor_dut_hostname, localhost, creds):
+    duthost = duthosts[enum_supervisor_dut_hostname]


I see. Thanks!

What is the motivation for this PR? Modified testcases to run on multi-dut How did you do it? Replaced 'rand_one_dut_hostname' with 'enum_rand_one_per_hwsku_frontend_hostname' Co-authored-by: falodiya <renu.falodiy@nokia.com>

oxygen980 requested a review from a team as a code owner February 5, 2021 06:26

oxygen980 force-pushed the snmp_lldp branch from 93d010c to 95218aa Compare February 10, 2021 19:43

oxygen980 changed the title ~~Modified SNMP and LLDP testcases for T2 topology~~ Modified SNMP, LLDP and PC testcases for T2 topology Feb 10, 2021

oxygen980 force-pushed the snmp_lldp branch from 95218aa to 62d03f2 Compare March 17, 2021 18:31

oxygen980 force-pushed the snmp_lldp branch from 8a3c63f to 3ed4d16 Compare March 29, 2021 20:30

wangxin requested changes Mar 31, 2021

View reviewed changes

oxygen980 force-pushed the snmp_lldp branch 2 times, most recently from 49f8e1d to 7c20e70 Compare April 1, 2021 23:04

falodiya added 5 commits April 1, 2021 19:05

Resovling conflicts

50a0844

rebase and fixed lldp, portchannel and snmp testcases for T2 chassis

354d82d

making changes to address comments, removing t2 markers where any is …

2344398

…present, changing pdu_controller to use enum_rand_one_per_hwsku_hostname, replace rand_one_hostname to enum_rand_one_per_hwsku_hostname in tests

address failure in snmp_lldp test

7c20e70

Changed creds to creds_all_dut to take care of same dut info for cred…

99c78dc

…s as testcase is using.

oxygen980 mentioned this pull request Apr 3, 2021

Modified crm/tacacs/autorestart testcases to use 'enum_rand_one_*' instead of 'rand_one_dut_hostname' #3272

Merged

3 tasks

oxygen980 requested a review from wangxin April 6, 2021 01:49

wangxin approved these changes Apr 9, 2021

View reviewed changes

wangxin merged commit 05c5fa9 into sonic-net:master Apr 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modified SNMP, LLDP and PC testcases for T2 topology #2935

Modified SNMP, LLDP and PC testcases for T2 topology #2935

oxygen980 commented Feb 5, 2021

anshuv-mfst commented Mar 13, 2021

yxieca commented Mar 21, 2021

azure-pipelines bot commented Mar 21, 2021

shubav commented Mar 22, 2021

oxygen980 commented Mar 24, 2021

shubav commented Mar 30, 2021

wangxin Mar 31, 2021

wangxin Mar 31, 2021

oxygen980 Apr 1, 2021

wangxin Mar 31, 2021

wangxin Mar 31, 2021

wangxin Mar 31, 2021

wangxin Mar 31, 2021

wangxin Mar 31, 2021

wangxin Mar 31, 2021

wangxin Apr 9, 2021

Modified SNMP, LLDP and PC testcases for T2 topology #2935

Modified SNMP, LLDP and PC testcases for T2 topology #2935

Conversation

oxygen980 commented Feb 5, 2021

Description of PR

Type of change

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

anshuv-mfst commented Mar 13, 2021

yxieca commented Mar 21, 2021

azure-pipelines bot commented Mar 21, 2021

shubav commented Mar 22, 2021

oxygen980 commented Mar 24, 2021

shubav commented Mar 30, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment