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

test_snmp_queue_counters.py/test_telemetry.py config_reload and snmpwwalk output time delay fix, test_snmp_queue_counters.py multi-asic KeyError fix #15688

Merged
merged 3 commits into from
Nov 22, 2024

Conversation

harjotsinghpawra
Copy link
Contributor

@harjotsinghpawra harjotsinghpawra commented Nov 21, 2024

test_snmp_queue_counters.py/test_telemetry.py config_reload and snmpwalk output time delay fix, test_snmp_queue_counters.py multi-asic KeyError fix

Description of PR

Scripts:
test_snmp_queue_counters.py
test_telemetry

/////////////////////////////////////////////////
First Issue :
When we run these scripts sometimes based on the platform and image along with other factors it takes some time for ports to come up and buffer queues to be generated and then further Snmp OID or even gnmi info to be genrated .

In script we immediately try to snmpwalk after all docker are up . But interfaces are still not up so no oid is generated .
Snmpwalk says No Such Instance currently exists at this OID whihc script count as 1 counter being created when none is created, which causes test case to fail.

enum_rand_one_per_hwsku_frontend_hostname = 'mth64-m5-2'
get_bfr_queue_cntrs_cmd = 'docker exec snmp snmpwalk -v2c -c public 1.74.23.17 1.3.6.1.4.1.9.9.580.1.5.5.1.4.1'
hostip = '1.74.23.17'
multicast_expected_diff = 16
queue_counters_cnt_post = 1
queue_counters_cnt_pre = 1
unicast_expected_diff = 8

["docker exec snmp snmpwalk -v2c -c public 1.74.23.17 1.3.6.1.4.1.9.9.580.1.5.5.1.4.1"], kwargs={}
12:37:54 base._run L0108 �[35mDEBUG �[0m| /data/tests/common/devices/multi_asic.py::_run_on_asics#134: [mth64-m5-2] AnsibleModule::shell Result => {"changed": true, "stdout": "iso.3.6.1.4.1.9.9.580.1.5.5.1.4.1 = No Such Instance currently exists at this OID", "stderr": "", "rc": 0, "cmd": "docker exec snmp snmpwalk -v2c -c public 1.74.23.17 1.3.6.1.4.1.9.9.580.1.5.5.1.4.1", "start": "2024-08-28 12:37:55.343677", "end": "2024-08-28 12:37:55.452104", "delta": "0:00:00.108427", "msg": "", "invocation": {"module_args": {"_raw_params": "docker exec snmp snmpwalk -v2c -c public 1.74.23.17 1.3.6.1.4.1.9.9.580.1.5.5.1.4.1", "_uses_shell": true, "warn": false, "stdin_add_newline": true, "strip_empty_ends": true, "argv": null, "chdir": null, "executable": null, "creates": null, "removes": null, "stdin": null}}, "stdout_lines": ["iso.3.6.1.4.1.9.9.580.1.5.5.1.4.1 = No Such Instance currently exists at this OID"], "stderr_lines": [], "_ansible_no_log": null, "failed": false}

//////////////////////////////////////////////////
Second issue :
In test_snmp_queue_counters script in multi-asic case we choose a buffer_queue of first interface mentioned in BUFFER_QUEUE config and then we try to match that, also we search asic.namepace in queue name which is invalid check which causes buffer_queue_to_del to be None.

This in turn fails the test case by saying that KeyError: None when we try to delete buffer
result = testfunction(**testargs)
File "/var/src/sonic-mgmt/tests/snmp/test_snmp_queue_counters.py", line 123, in test_snmp_queue_counters
del data['BUFFER_QUEUE'][buffer_queue_to_del]
KeyError: None

Summary:
Fixes #15683 and #15686

Type of change

Test script delays and condition fix mentioned in How did you do it?

  • 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?

How did you do it?

1.) added necessary checks so that all the interfaces are up and oid's are generated only then take command output.
2.) changed wrong logic of multi asic buffer queue selection and alsoimproved it to work for both single and multi-asic system.
3.) Also added extra check where i match the OID's of counters generated by snmp with queuestat output because they should match queuestat gives the latest information.

How did you verify/test it?

Ran it on local CISCO platforms and its passing

Any platform specific information?

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

Documentation

…alk output time delay fix, test_snmp_queue_counters.py multi-asic KeyError fix
@mssonicbld
Copy link
Collaborator

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...........................................(no files to check)Skipped
check for added large files..............................................Passed
check python ast.........................................................Passed
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

tests/snmp/test_snmp_queue_counters.py:17:1: E302 expected 2 blank lines, found 1
tests/snmp/test_snmp_queue_counters.py:19:71: E203 whitespace before ','
tests/snmp/test_snmp_queue_counters.py:21:1: E302 expected 2 blank lines, found 1
tests/snmp/test_snmp_queue_counters.py:24:1: E302 expected 2 blank lines, found 1
tests/snmp/test_snmp_queue_counters.py:31:1: E302 expected 2 blank lines, found 1
tests/snmp/test_snmp_queue_counters.py:90:25: E222 multiple spaces after operator
tests/snmp/test_snmp_queue_counters.py:129:23: E127 continuation line over-indented for visual indent
tests/snmp/test_snmp_queue_counters.py:139:23: E127 continuation line over-indented for visual indent
tests/snmp/test_snmp_queue_counters.py:143:5: E303 too many blank lines (2)
tests/telemetry/test_telemetry.py:54:1: E302 expected 2 blank lines, found 1
tests/telemetry/test_telemetry.py:61:1: E302 expected 2 blank lines, found 1
...
[truncated extra lines, please run pre-commit locally to view full check results]

To run the pre-commit checks locally, you can follow below steps:

  1. Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
    the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
    docker container.
  2. Ensure that the pre-commit package is installed:
sudo pip install pre-commit
  1. Go to repository root folder
  2. Install the pre-commit hooks:
pre-commit install
  1. Use pre-commit to check staged file:
pre-commit
  1. Alternatively, you can check committed files using:
pre-commit run --from-ref <commit_id> --to-ref <commit_id>

@harjotsinghpawra harjotsinghpawra changed the title test_snmp_queue_counters.py/test_telemetry.py config_reload and snmpw… test_snmp_queue_counters.py/test_telemetry.py config_reload and snmpwwalk output time delay fix, test_snmp_queue_counters.py multi-asic KeyError fix Nov 22, 2024
@harjotsinghpawra harjotsinghpawra marked this pull request as ready for review November 22, 2024 00:46
@vperumal
Copy link
Contributor

Hi @abdosi @yejianquan - Kindly help review and merge

@vperumal
Copy link
Contributor

This help fix - Fixes #15683 and #15686

Copy link
Collaborator

@yejianquan yejianquan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yejianquan yejianquan merged commit 21317ed into sonic-net:master Nov 22, 2024
16 checks passed
@mssonicbld
Copy link
Collaborator

@harjotsinghpawra PR conflicts with 202405 branch

@yejianquan
Copy link
Collaborator

Hi @harjotsinghpawra , please resolve the conflict and create a separate PR

yejianquan pushed a commit that referenced this pull request Nov 27, 2024
…g_reload and snmpwwalk output time delay fix, test_snmp_queue_counters.py multi-asic KeyError fix (#15735)

Description of PR
Cherry pick PR #15688 for 202405
Resolve conflit issues -

This PR also resolves following issues along with ones mentioned in above PR -
Fixed multi-asic issues where right buffer queues were not selected
asic namespace should not be checked in queue name

co-authorized by: [email protected]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants