Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Snmp fixes - test_snmp_default_route and test_snmp_queue_counters #15279

Merged
merged 2 commits into from
Nov 28, 2024

Conversation

sanjair-git
Copy link
Contributor

Description of PR

Summary:
Fixes # (issue)

  • This PR fixes test issue introduced as part of #3537 for test_snmp_default_route test, and
  • General fix for test_snmp_queue_counters test teardown.

Type of change

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

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • 202405

Approach

What is the motivation for this PR?

  • After PR #3537 introduction, CLI command output for 'show ip route 0.0.0.0/0' has been changed and a new word 'recursive' gets added. Hence sonic-mgmt needs to be modified to support this new change. For example, "* 11.0.0.145 recursive via iBGP"
  • During teardown of 'test_snmp_queue_counters' test, sometimes we see the following error while recopying the config_db json file for the duthost.
E           tests.common.errors.RunAnsibleModuleFail: run module copy failed, Ansible Results =>
E           {"changed": false, "failed": true, "msg": "Source /etc/sonic/orig_config_db1.json not found"}

complex_args = {'dest': '/etc/sonic/config_db1.json', 'remote_src': True, 'src': '/etc/sonic/orig_config_db1.json'}
filename   = '/data/tests/snmp/test_snmp_queue_counters.py'
function_name = 'teardown'

How did you do it?

  • Handle 'recursive' word as well while parsing for ip-address in test_snmp_default_route test case.
  • Make sure the duthost is same during test call and teardown in test_snmp_queue_counters test case.

How did you verify/test it?

  • Ran all the above-mentioned test cases on a T2 chassis and made sure tests passed with expected behavior.

Any platform specific information?

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

Documentation

image

@arlakshm
Copy link
Contributor

@sanjair-git, can you please resolve the conflicts

@sanjair-git
Copy link
Contributor Author

@sanjair-git, can you please resolve the conflicts

Done @arlakshm. Thanks

@arlakshm arlakshm merged commit 73a2ccf into sonic-net:master Nov 28, 2024
16 checks passed
sreejithsreekumaran pushed a commit to sreejithsreekumaran/sonic-mgmt that referenced this pull request Nov 29, 2024
…unters (sonic-net#15279)

Summary:


This PR fixes test issue introduced as part of sonic-net#3537 for test_snmp_default_route test, and
General fix for test_snmp_queue_counters test teardown.
Type of change

Approach
What is the motivation for this PR?
After PR sonic-net#3537 introduction, CLI command output for 'show ip route 0.0.0.0/0' has been changed and a new word 'recursive' gets added. Hence sonic-mgmt needs to be modified to support this new change. For example, "* 11.0.0.145 recursive via iBGP"
During teardown of 'test_snmp_queue_counters' test, sometimes we see the following error while recopying the config_db json file for the duthost.
E           tests.common.errors.RunAnsibleModuleFail: run module copy failed, Ansible Results =>
E           {"changed": false, "failed": true, "msg": "Source /etc/sonic/orig_config_db1.json not found"}

complex_args = {'dest': '/etc/sonic/config_db1.json', 'remote_src': True, 'src': '/etc/sonic/orig_config_db1.json'}
filename   = '/data/tests/snmp/test_snmp_queue_counters.py'
function_name = 'teardown'
How did you do it?
Handle 'recursive' word as well while parsing for ip-address in test_snmp_default_route test case.
Make sure the duthost is same during test call and teardown in test_snmp_queue_counters test case.
How did you verify/test it?
Ran all the above-mentioned test cases on a T2 chassis and made sure tests passed with expected behavior.
@bingwang-ms
Copy link
Collaborator

@sanjair-git Does this change impact non-T2 platform? Since if recursive is not present, the strip changes nothing.
ip = ip.strip("*, ,recursive")

@sanjair-git
Copy link
Contributor Author

@sanjair-git Does this change impact non-T2 platform? Since if recursive is not present, the strip changes nothing. ip = ip.strip("*, ,recursive")

It is not impacted. For T0 & T1 platforms, I could see 'test_snmp_default_route' test passing.
https://elastictest.org/scheduler/publictestplan/6747b5a7d587148c92f86921?testcase=snmp%2Ftest_snmp_default_route.py&type=console

https://elastictest.org/scheduler/publictestplan/6747b5a6b40a2cd68e60fe76?searchTestCase=default&testcase=snmp%2Ftest_snmp_default_route.py&type=console

mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Dec 17, 2024
…unters (sonic-net#15279)

Summary:


This PR fixes test issue introduced as part of sonic-net#3537 for test_snmp_default_route test, and
General fix for test_snmp_queue_counters test teardown.
Type of change

Approach
What is the motivation for this PR?
After PR sonic-net#3537 introduction, CLI command output for 'show ip route 0.0.0.0/0' has been changed and a new word 'recursive' gets added. Hence sonic-mgmt needs to be modified to support this new change. For example, "* 11.0.0.145 recursive via iBGP"
During teardown of 'test_snmp_queue_counters' test, sometimes we see the following error while recopying the config_db json file for the duthost.
E           tests.common.errors.RunAnsibleModuleFail: run module copy failed, Ansible Results =>
E           {"changed": false, "failed": true, "msg": "Source /etc/sonic/orig_config_db1.json not found"}

complex_args = {'dest': '/etc/sonic/config_db1.json', 'remote_src': True, 'src': '/etc/sonic/orig_config_db1.json'}
filename   = '/data/tests/snmp/test_snmp_queue_counters.py'
function_name = 'teardown'
How did you do it?
Handle 'recursive' word as well while parsing for ip-address in test_snmp_default_route test case.
Make sure the duthost is same during test call and teardown in test_snmp_queue_counters test case.
How did you verify/test it?
Ran all the above-mentioned test cases on a T2 chassis and made sure tests passed with expected behavior.
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202405: #16103

mssonicbld pushed a commit that referenced this pull request Dec 17, 2024
…unters (#15279)

Summary:


This PR fixes test issue introduced as part of #3537 for test_snmp_default_route test, and
General fix for test_snmp_queue_counters test teardown.
Type of change

Approach
What is the motivation for this PR?
After PR #3537 introduction, CLI command output for 'show ip route 0.0.0.0/0' has been changed and a new word 'recursive' gets added. Hence sonic-mgmt needs to be modified to support this new change. For example, "* 11.0.0.145 recursive via iBGP"
During teardown of 'test_snmp_queue_counters' test, sometimes we see the following error while recopying the config_db json file for the duthost.
E           tests.common.errors.RunAnsibleModuleFail: run module copy failed, Ansible Results =>
E           {"changed": false, "failed": true, "msg": "Source /etc/sonic/orig_config_db1.json not found"}

complex_args = {'dest': '/etc/sonic/config_db1.json', 'remote_src': True, 'src': '/etc/sonic/orig_config_db1.json'}
filename   = '/data/tests/snmp/test_snmp_queue_counters.py'
function_name = 'teardown'
How did you do it?
Handle 'recursive' word as well while parsing for ip-address in test_snmp_default_route test case.
Make sure the duthost is same during test call and teardown in test_snmp_queue_counters test case.
How did you verify/test it?
Ran all the above-mentioned test cases on a T2 chassis and made sure tests passed with expected behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants