[snmp] Add test cases for SONiC physical entity MIB feature #2379

Junchao-Mellanox · 2020-10-21T06:28:47Z

Description of PR

Summary:
Add test cases for SONiC physical entity MIB feature

Type of change

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

Approach

What is the motivation for this PR?

There is no regression test case for physical entity MIB before, and we create a few new test cases to cover the existing/new MIB objects.

How did you do it?

Adjust snmp_facts.py to support new MIB objects
Add test case to verify transceiver related entities
Add test case to verify PSU related entities
Add test case to verify fan related entities
Add test case to verify thermal related entities
Add test case to verify PSU/fan remove

How did you verify/test it?

Run the new regression test cases

Any platform specific information?

N/A

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

N/A

Documentation

Junchao-Mellanox · 2020-10-21T06:31:06Z

Depends on:
sonic-net/sonic-platform-common#134
sonic-net/sonic-platform-daemons#102
sonic-net/sonic-buildimage#5645
sonic-net/sonic-snmpagent#168

Junchao-Mellanox · 2020-10-21T06:53:43Z

retest vsimage please

wangxin · 2020-10-23T03:12:20Z

retest this please

lguohan · 2020-10-24T21:31:36Z

can you please add the test to kvmtest.sh?

Junchao-Mellanox · 2020-10-26T02:34:50Z

kvmtest.sh

Done.

Junchao-Mellanox · 2020-10-27T03:06:37Z

retest vsimage please

Junchao-Mellanox · 2020-10-30T11:20:51Z

retest vsimage please

Junchao-Mellanox · 2020-10-30T12:34:39Z

retest vsimage please

liat-grozovik · 2020-11-01T15:10:19Z

retest vsimage please

liat-grozovik · 2020-11-01T15:12:19Z

@Junchao-Mellanox kindly reminder that this feature will go only to master towards 202012.
As of that we need a way to run snmp test on 201911 which has no such capability. Should it work or need to find a solution to detect if the version expected to support this extended mib?

Junchao-Mellanox · 2020-11-02T01:15:58Z

Hi @liat-grozovik , we can do this in sonic-tool. In sonic-tool, we can disable cases according to the image branch name. So once we merge this test case to our local repo, we need add the case to sonic-tool and disable it on 201911 branch in snmp.cases file.

Junchao-Mellanox · 2020-11-02T01:16:09Z

retest vsimage please

keboliu · 2020-11-04T06:03:41Z

retest vsimage please

keboliu · 2020-11-04T06:04:12Z

retest this please

Junchao-Mellanox · 2020-11-05T01:13:17Z

retest vsimage please

Junchao-Mellanox · 2020-11-06T00:59:22Z

retest vsimage please

Junchao-Mellanox · 2020-11-09T00:52:51Z

retest vsimage please

Junchao-Mellanox · 2020-11-11T01:20:33Z

retest vsimage please

…turn None (for VS image)

Junchao-Mellanox · 2020-11-23T01:00:02Z

retest vsimage please

qiluo-msft · 2020-11-29T05:35:12Z

tests/snmp/test_snmp_phy_entity.py

+ SENSOR_TYPE_PORT_TX_BIAS + 4,
+ SENSOR_TYPE_VOLTAGE]
+
+# Redis Constants


Redis [](start = 2, length = 5)

Suggest remove Redis #Closed

qiluo-msft · 2020-11-29T05:36:57Z

tests/snmp/test_snmp_phy_entity.py

+ assert fan_snmp_fact['entPhysMfgName'] == ''
+ assert fan_snmp_fact['entPhysModelName'] == '' if is_null_str(fan_info['model']) else fan_info['model']
+ assert fan_snmp_fact['entPhysIsFRU'] == REPLACEABLE if fan_info[
+ 'is_replaceable'] == 'True' else NOT_REPLACEABLE


'is_replaceable' [](start = 67, length = 16)

Line breaking at a weird position. #Closed

qiluo-msft · 2020-11-29T05:37:46Z

tests/snmp/test_snmp_phy_entity.py

+ """
+ keys = redis_get_keys(duthost, STATE_DB, PSU_KEY_TEMPLATE.format('*'))
+ if not keys:
+ pytest.skip('PSU information not exists in DB, skipping this test')


not exists [](start = 37, length = 10)

does not exist #Closed

qiluo-msft · 2020-11-29T05:40:45Z

tests/snmp/test_snmp_phy_entity.py

+ """
+ keys = redis_get_keys(duthost, STATE_DB, PSU_KEY_TEMPLATE.format('*'))
+ if not keys:
+ pytest.skip('PSU information not exists in DB, skipping this test')


pytest.skip [](start = 8, length = 11)

There are many pytest.skip(). Seems are a valid failure case.
Do you skip just for backward-compatible? If yes, I think this is not a good method. #Closed

I skip this for vsimage test. I found this testcase will be ran in vsimage, and there is no PSU information in DB.

@wangxin What is best practice to skip test case in vsimage test?

In reply to: 532159260 [](ancestors = 532159260)

You can mark the test case like below to specify that this test case is only applicable for physical testbed, not for vsimage.

@pytest.mark.device_type('physical') def test_xxx

qiluo-msft · 2020-11-29T05:41:41Z

tests/snmp/test_snmp_phy_entity.py

+ assert sensor_snmp_fact['entPhysContainedIn'] == psu_oid
+ assert sensor_snmp_fact['entPhysClass'] == PHYSICAL_CLASS_SENSOR
+ assert sensor_snmp_fact['entPhyParentRelPos'] == sensor_tuple[1]
+ assert sensor_snmp_fact['entPhysName'] == '{} for {}'.format(sensor_tuple[0], psu_name)


'{} for {}'.format(sensor_tuple[0], psu_name) [](start = 50, length = 45)

Appear twice, use a variable? #Closed

qiluo-msft · 2020-11-29T05:42:26Z

tests/snmp/test_snmp_phy_entity.py

+ for item in psu_status:
+ if not item['psu_on']:
+ psu_controller.turn_on_psu(item["psu_id"])
+ time.sleep(5)


time.sleep(5) [](start = 12, length = 13)

What if no sleep? #Closed

The PSU may take a few seconds to power on, so I add a sleep here, this code is similar with some test cases in platform tests.

Better to sleep only once after all PSU turned on?

In reply to: 532159395 [](ancestors = 532159395)

qiluo-msft · 2020-11-29T05:46:36Z

tests/snmp/test_snmp_phy_entity.py

+ :param pattern: Redis key pattern
+ :return: A list of key name in string
+ """
+ cmd = 'redis-cli --raw -n {} KEYS \"{}\"'.format(db_id, pattern)


redis-cli [](start = 11, length = 9)

redis-cli will not aware of multi-DB or multi-namespace. Use sonic-db-cli #Closed

qiluo-msft

As comments

Junchao-Mellanox · 2020-12-01T03:43:00Z

Hi @wangxin , it seems the test was not skipped in vsimage after adding the pytest mark. I see following error in the test result:

11:08:57      @pytest.mark.device_type('physical')
11:08:57      def test_fan_info(duthost, snmp_physical_entity_info):
11:08:57          """
11:08:57          Verify fan information in physical entity mib with redis database
11:08:57          :param duthost: DUT host object
11:08:57          :param snmp_physical_entity_info: Physical entity information from snmp fact
11:08:57          :return:
11:08:57          """
11:08:57          keys = redis_get_keys(duthost, STATE_DB, FAN_KEY_TEMPLATE.format('*'))
11:08:57  >       assert keys, 'Fan information does not exist in DB'
11:08:57  E       AssertionError: Fan information does not exist in DB

wangxin · 2020-12-01T08:31:46Z

If all the test cases in this test script need physical testbed, then it should not be added to the list in kvmtest.sh.

…l physical entites such as fan, PSU

Junchao-Mellanox added 2 commits October 20, 2020 11:34

[snmp] Add test cases for SONiC physical entity MIB feature

090054f

Fix issue found in manual test

ab5f6e8

Junchao-Mellanox mentioned this pull request Oct 21, 2020

[snmp] Add test cases for SONiC physical entity MIB feature Junchao-Mellanox/sonic-mgmt#23

Closed

3 tasks

Junchao-Mellanox added 2 commits October 26, 2020 10:30

Merge branch 'master' into phy-mibs-snmp

c1e0e4c

Add new test cases to kvmtest.sh

e74c91a

yxieca force-pushed the master branch from 885053e to 8f9cb56 Compare October 31, 2020 02:57

liat-grozovik added the Enhancement label Nov 1, 2020

Junchao-Mellanox added 4 commits November 18, 2020 08:57

Merge remote-tracking branch 'upstream/master' into phy-mibs-snmp

510989f

Skip fan snmp test if no fan information in DB

b911e5e

Skip test case if related information is not in DB

6e17ead

Skip test_turn_off_psu_and_check_psu_info test when psu_controller re…

c46c1c4

…turn None (for VS image)

keboliu approved these changes Nov 24, 2020

View reviewed changes

liat-grozovik requested review from wangxin and qiluo-msft November 26, 2020 16:59

qiluo-msft reviewed Nov 29, 2020

View reviewed changes

qiluo-msft requested changes Nov 29, 2020

View reviewed changes

Junchao-Mellanox added 5 commits November 30, 2020 10:26

Fix review comments

bd8e1ac

Fix typo

d118d10

Fix issue found in test

ccff1ab

Sleep 5 seconds after turning all PSU on

d564f3e

Use pytest mark to skip testcases that does not apply to vsimage

95b7dbe

Junchao-Mellanox requested a review from qiluo-msft December 1, 2020 01:51

Remove snmp physical entity test from vs test since it depends on rea…

ca0f285

…l physical entites such as fan, PSU

qiluo-msft approved these changes Dec 2, 2020

View reviewed changes

wangxin approved these changes Dec 2, 2020

View reviewed changes

qiluo-msft merged commit 92cb763 into sonic-net:master Dec 2, 2020

Junchao-Mellanox deleted the phy-mibs-snmp branch December 3, 2020 05:30

daall mentioned this pull request Dec 23, 2020

physical entity data missing from SNMP sonic-net/sonic-buildimage#6283

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[snmp] Add test cases for SONiC physical entity MIB feature #2379

[snmp] Add test cases for SONiC physical entity MIB feature #2379

Junchao-Mellanox commented Oct 21, 2020

Junchao-Mellanox commented Oct 21, 2020

Junchao-Mellanox commented Oct 21, 2020

wangxin commented Oct 23, 2020

lguohan commented Oct 24, 2020

Junchao-Mellanox commented Oct 26, 2020

Junchao-Mellanox commented Oct 27, 2020

Junchao-Mellanox commented Oct 30, 2020

Junchao-Mellanox commented Oct 30, 2020

liat-grozovik commented Nov 1, 2020

liat-grozovik commented Nov 1, 2020

Junchao-Mellanox commented Nov 2, 2020

Junchao-Mellanox commented Nov 2, 2020

keboliu commented Nov 4, 2020

keboliu commented Nov 4, 2020

Junchao-Mellanox commented Nov 5, 2020

Junchao-Mellanox commented Nov 6, 2020

Junchao-Mellanox commented Nov 9, 2020

Junchao-Mellanox commented Nov 11, 2020

Junchao-Mellanox commented Nov 23, 2020

qiluo-msft Nov 29, 2020 •

edited

Loading

qiluo-msft Nov 29, 2020 •

edited

Loading

qiluo-msft Nov 29, 2020 •

edited

Loading

qiluo-msft Nov 29, 2020 •

edited

Loading

Junchao-Mellanox Nov 30, 2020

qiluo-msft Nov 30, 2020

wangxin Dec 1, 2020

qiluo-msft Nov 29, 2020 •

edited

Loading

qiluo-msft Nov 29, 2020 •

edited

Loading

Junchao-Mellanox Nov 30, 2020

qiluo-msft Nov 30, 2020

qiluo-msft Nov 29, 2020 •

edited

Loading

qiluo-msft left a comment

Junchao-Mellanox commented Dec 1, 2020

wangxin commented Dec 1, 2020

[snmp] Add test cases for SONiC physical entity MIB feature #2379

[snmp] Add test cases for SONiC physical entity MIB feature #2379

Conversation

Junchao-Mellanox commented Oct 21, 2020

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

Junchao-Mellanox commented Oct 21, 2020

Junchao-Mellanox commented Oct 21, 2020

wangxin commented Oct 23, 2020

lguohan commented Oct 24, 2020

Junchao-Mellanox commented Oct 26, 2020

Junchao-Mellanox commented Oct 27, 2020

Junchao-Mellanox commented Oct 30, 2020

Junchao-Mellanox commented Oct 30, 2020

liat-grozovik commented Nov 1, 2020

liat-grozovik commented Nov 1, 2020

Junchao-Mellanox commented Nov 2, 2020

Junchao-Mellanox commented Nov 2, 2020

keboliu commented Nov 4, 2020

keboliu commented Nov 4, 2020

Junchao-Mellanox commented Nov 5, 2020

Junchao-Mellanox commented Nov 6, 2020

Junchao-Mellanox commented Nov 9, 2020

Junchao-Mellanox commented Nov 11, 2020

Junchao-Mellanox commented Nov 23, 2020

qiluo-msft Nov 29, 2020 • edited Loading

Choose a reason for hiding this comment

qiluo-msft Nov 29, 2020 • edited Loading

Choose a reason for hiding this comment

qiluo-msft Nov 29, 2020 • edited Loading

Choose a reason for hiding this comment

qiluo-msft Nov 29, 2020 • edited Loading

Choose a reason for hiding this comment

Junchao-Mellanox Nov 30, 2020

Choose a reason for hiding this comment

qiluo-msft Nov 30, 2020

Choose a reason for hiding this comment

wangxin Dec 1, 2020

Choose a reason for hiding this comment

qiluo-msft Nov 29, 2020 • edited Loading

Choose a reason for hiding this comment

qiluo-msft Nov 29, 2020 • edited Loading

Choose a reason for hiding this comment

Junchao-Mellanox Nov 30, 2020

Choose a reason for hiding this comment

qiluo-msft Nov 30, 2020

Choose a reason for hiding this comment

qiluo-msft Nov 29, 2020 • edited Loading

Choose a reason for hiding this comment

qiluo-msft left a comment

Choose a reason for hiding this comment

Junchao-Mellanox commented Dec 1, 2020

wangxin commented Dec 1, 2020

qiluo-msft Nov 29, 2020 •

edited

Loading

qiluo-msft Nov 29, 2020 •

edited

Loading

qiluo-msft Nov 29, 2020 •

edited

Loading

qiluo-msft Nov 29, 2020 •

edited

Loading

qiluo-msft Nov 29, 2020 •

edited

Loading

qiluo-msft Nov 29, 2020 •

edited

Loading

qiluo-msft Nov 29, 2020 •

edited

Loading