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

Add buffer queue counters test case to telemetry test #12735

Merged
merged 10 commits into from
Jun 5, 2024

Conversation

arfeigin
Copy link
Contributor

@arfeigin arfeigin commented May 6, 2024

Description of PR

This test is checking that the number of buffer queue counters from telemetry point of view is inline when using create_only_config_db_counters optimization.

Summary:
Accompanies "Fix SNMP dropping some of the queue counter when create_only_config_db_buffers is set to true" (sonic-net/sonic-snmpagent#303) which fixes the issue: "The feature "polling only configured ports buffer queue" will break SNMP" (sonic-net/sonic-buildimage#17448).
Accompanies "Test polling only configured buffer queue counters" #11941

Type of change

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

Back port request

  • 201911
  • 202012
  • 202205
  • 202305
  • 202311

Approach

What is the motivation for this PR?

To enhance the bug fix mentioned above, solving an issue with buffer queue counters optimization.

How did you do it?

  • Set "create_only_config_db_buffers" to true in config db, to create only relevant counters
  • Remove one of the buffer queues, Ethernet0|3-4 is chosen arbitrary
  • Using gnmi to query COUNTERS_QUEUE_NAME_MAP for Ethernet0 compare number of queue counters on Ethernet0. It is expected that it will decrease by the number of deleted queue buffers.

How did you verify/test it?

Run the test multiple times on various setups.

Any platform specific information?

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

Any

Documentation

@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.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook

Fixing tests/telemetry/test_telemetry.py

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/telemetry/test_telemetry.py:46:9: F841 local variable 'stdout_output' is assigned to but never used
tests/telemetry/test_telemetry.py:122:121: E501 line too long (124 > 120 characters)
tests/telemetry/test_telemetry.py:135:26: E128 continuation line under-indented for visual indent

flake8...............................................(no files to check)Skipped
...
[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>

@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.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook

Fixing tests/telemetry/test_telemetry.py

fix end of files.........................................................Passed
check yaml...........................................(no files to check)Skipped
check for added large files..............................................Passed
check python ast.........................................................Passed
flake8...................................................................Passed
flake8...............................................(no files to check)Skipped
check conditional mark sort..........................(no files to check)Skipped

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>

@bingwang-ms
Copy link
Collaborator

@qiluo-msft @zbud-msft Can you please help review this PR?

@qiluo-msft qiluo-msft requested a review from FengPan-Frank May 10, 2024 23:24
@qiluo-msft
Copy link
Contributor

@arfeigin Could you check the test failure?

qiluo-msft
qiluo-msft previously approved these changes May 13, 2024
@qiluo-msft
Copy link
Contributor

/azp run Azure.sonic-mgmt

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@FengPan-Frank FengPan-Frank left a comment

Choose a reason for hiding this comment

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

LGTM

@arfeigin
Copy link
Contributor Author

arfeigin commented Jun 3, 2024

/azp run Azure.sonic-mgmt

Copy link

Commenter does not have sufficient privileges for PR 12735 in repo sonic-net/sonic-mgmt

@FengPan-Frank
Copy link
Contributor

/azpw run Azure.sonic-mgmt

@dprital
Copy link

dprital commented Jun 5, 2024

@qiluo-msft , Can you please approve and merge ?

@bingwang-ms bingwang-ms merged commit 28496c3 into sonic-net:master Jun 5, 2024
14 checks passed
@bingwang-ms
Copy link
Collaborator

@qiluo-msft Is this PR required for other release branch?

mssonicbld pushed a commit to mssonicbld/sonic-mgmt that referenced this pull request Jun 10, 2024
* Add buffer queue counters test case to telemetry test
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202405: #13207

@zbud-msft
Copy link
Contributor

@arfeigin Please disable loganalyzer for new testcase, internal tests for this testcase are failing due to loganalyzer.

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.

7 participants