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

[voq] [chassis] Bug fixes to get iface_namingmode tests to work on multi-asic linecards of a T2 chassis #6528

Merged
merged 1 commit into from
Apr 24, 2023

Conversation

sanmalho-git
Copy link
Contributor

@sanmalho-git sanmalho-git commented Oct 12, 2022

Description of PR

Summary:
Fixes # (issue)

Type of change

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

Back port request

  • 201911
  • 202012
  • 202205

Approach

What is the motivation for this PR?

Currently, the following test isfailing on multi-asic linecards on a T2 VoQ chassis.

  • show pfc counters: This fails as the Inband and Recycle ports show in the 'show pfc counters' table while they are not expected.

How did you do it?

  • show pfc counters:
    • Treat Inband and Recycle ports on a VoQ chassis as internal ports. Since the inband and recycle ports do not show up in 'show' commands unless we specify '-d all' option, they should be treated similar to internal ports.

How did you verify/test it?

Ran the test against a multi-asic linecard in a T2 VoQ chassis.

Any platform specific information?

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

Documentation

@azure-pipelines
Copy link

The pre-commit check detected issues in the files touched by this pull request.
The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame
author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

Detailed pre-commit check results:
trim trailing whitespace.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook

Fixing ansible/library/port_alias.py
Fixing tests/iface_namingmode/test_iface_namingmode.py

fix end of files.........................................................Passed
check yaml...........................................(no files to check)Skipped
check for added large files..............................................Passed
check python ast.........................................................Passed
autopep8.................................................................Failed
- hook id: autopep8
- files were modified by this hook
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

ansible/library/port_alias.py:3:1: F403 'from ansible.module_utils.basic import *' used; unable to detect undefined names
ansible/library/port_alias.py:6:1: F401 'traceback' imported but unused
ansible/library/port_alias.py:7:1: F401 'subprocess' imported but unused
ansible/library/port_alias.py:8:1: F401 'operator.itemgetter' imported but unused
ansible/library/port_alias.py:9:1: F401 'itertools.groupby' imported but unused
ansible/library/port_alias.py:10:1: F401 'collections.defaultdict' imported but unused
ansible/library/port_alias.py:22:121: E501 line too long (130 > 120 characters)
ansible/library/port_alias.py:23:121: E501 line too long (121 > 120 characters)
ansible/library/port_alias.py:27:121: E501 line too long (121 > 120 characters)
ansible/library/port_alias.py:121:121: E501 line too long (138 > 120 characters)
ansible/library/port_alias.py:219:14: F405 'AnsibleModule' may be undefined, or defined from star imports: ansible.module_utils.basic
ansible/library/port_alias.py:255:58: E711 comparison to None should be 'if cond is not None:'
ansible/library/port_alias.py:258:52: E711 comparison to None should be 'if cond is not None:'
ansible/library/port_alias.py:269:13: F841 local variable 'e' is assigned to but never used
tests/iface_namingmode/test_iface_namingmode.py:126:121: E501 line too long (121 > 120 characters)
tests/iface_namingmode/test_iface_namingmode.py:141:121: E501 line too long (164 > 120 characters)
tests/iface_namingmode/test_iface_namingmode.py:188:1: E266 too many leading '#' for block comment
tests/iface_namingmode/test_iface_namingmode.py:236:121: E501 line too long (132 > 120 characters)
tests/iface_namingmode/test_iface_namingmode.py:258:121: E501 line too long (132 > 120 characters)
tests/iface_namingmode/test_iface_namingmode.py:303:121: E501 line too long (127 > 120 characters)
tests/iface_namingmode/test_iface_namingmode.py:464:73: W605 invalid escape sequence '|'
tests/iface_namingmode/test_iface_namingmode.py:464:77: W605 invalid escape sequence '|'
tests/iface_namingmode/test_iface_namingmode.py:484:76: W605 invalid escape sequence '|'
tests/iface_namingmode/test_iface_namingmode.py:484:80: W605 invalid escape sequence '|'
tests/iface_namingmode/test_iface_namingmode.py:739:121: E501 line too long (127 > 120 characters)
tests/iface_namingmode/test_iface_namingmode.py:755:29: E711 comparison to None should be 'if cond is not None:'
tests/iface_namingmode/test_iface_namingmode.py:814:121: E501 line too long (130 > 120 characters)
tests/iface_namingmode/test_iface_namingmode.py:868:121: E501 line too long (147 > 120 characters)
tests/iface_namingmode/test_iface_namingmode.py:871:121: E501 line too long (133 > 120 characters)
tests/iface_namingmode/test_iface_namingmode.py:974:121: E501 line too long (131 > 120 characters)

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

  1. 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>

Copy link
Contributor

@SuvarnaMeenakshi SuvarnaMeenakshi left a comment

Choose a reason for hiding this comment

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

Please modify as per comments.

@abdosi
Copy link
Contributor

abdosi commented Dec 9, 2022

@sanmalho-git Can you please check Suvarna comments ?

@sanmalho-git
Copy link
Contributor Author

I will close this PR once the sonic-utilities PR is merged into 202205

@rlhui
Copy link

rlhui commented Mar 8, 2023

I will close this PR once the sonic-utilities PR is merged into 202205

@sanmalho-git - which sonic-utilities PR is being referred to here? Is this PR still needed? Thanks.

@sanmalho-git
Copy link
Contributor Author

@rlhui sonic-net/sonic-utilities#2525 is the sonic-utilities PR that is required if we want to close this PR

…s of a T2 chassis

 - Treat Inband and Recycle ports on a VoQ chassis as internal ports

    Since the inband and recycle ports do not show up in 'show' commands
    unless we specify '-d all' option, they should be treated similar to internal ports.

    This fixes iface_namingmode::test_show_pfc_counters test case
@mssonicbld
Copy link
Collaborator

The pre-commit check detected issues in the files touched by this pull request.
The detected issues may be old or new. For new issues, please try to fix them.

For old issues, it is not mandatory to fix them because they were not caused by this change. It is unfair to blame
author of this pull request. But if you can take extra effort to fix the old issues as well, that would be great!

Detailed pre-commit check results:
trim trailing whitespace.................................................Failed
- hook id: trailing-whitespace
- exit code: 1
- files were modified by this hook

Fixing ansible/library/port_alias.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

ansible/library/port_alias.py:5:1: F401 'traceback' imported but unused
ansible/library/port_alias.py:6:1: F401 'subprocess' imported but unused
ansible/library/port_alias.py:7:1: F401 'operator.itemgetter' imported but unused
ansible/library/port_alias.py:8:1: F401 'itertools.groupby' imported but unused
ansible/library/port_alias.py:9:1: F401 'collections.defaultdict' imported but unused
...
[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>

Copy link
Contributor

@SuvarnaMeenakshi SuvarnaMeenakshi left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@SuvarnaMeenakshi SuvarnaMeenakshi left a comment

Choose a reason for hiding this comment

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

lgtm

@SuvarnaMeenakshi
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@SuvarnaMeenakshi SuvarnaMeenakshi merged commit 0a9af9b into sonic-net:master Apr 24, 2023
wangxin pushed a commit that referenced this pull request Apr 26, 2023
…s of a T2 chassis (#6528)

What is the motivation for this PR?
Currently, the following test isfailing on multi-asic linecards on a T2 VoQ chassis.

show pfc counters: This fails as the Inband and Recycle ports show in the 'show pfc counters' table while they are not expected.
How did you do it?
show pfc counters:
Treat Inband and Recycle ports on a VoQ chassis as internal ports. Since the inband and recycle ports do not show up in 'show' commands unless we specify '-d all' option, they should be treated similar to internal ports.
How did you verify/test it?
Ran the test against a multi-asic linecard in a T2 VoQ chassis.
mrkcmo pushed a commit to Azarack/sonic-mgmt that referenced this pull request Oct 3, 2023
…s of a T2 chassis (sonic-net#6528)

What is the motivation for this PR?
Currently, the following test isfailing on multi-asic linecards on a T2 VoQ chassis.

show pfc counters: This fails as the Inband and Recycle ports show in the 'show pfc counters' table while they are not expected.
How did you do it?
show pfc counters:
Treat Inband and Recycle ports on a VoQ chassis as internal ports. Since the inband and recycle ports do not show up in 'show' commands unless we specify '-d all' option, they should be treated similar to internal ports.
How did you verify/test it?
Ran the test against a multi-asic linecard in a T2 VoQ chassis.
AharonMalkin pushed a commit to AharonMalkin/sonic-mgmt that referenced this pull request Jan 25, 2024
…s of a T2 chassis (sonic-net#6528)

What is the motivation for this PR?
Currently, the following test isfailing on multi-asic linecards on a T2 VoQ chassis.

show pfc counters: This fails as the Inband and Recycle ports show in the 'show pfc counters' table while they are not expected.
How did you do it?
show pfc counters:
Treat Inband and Recycle ports on a VoQ chassis as internal ports. Since the inband and recycle ports do not show up in 'show' commands unless we specify '-d all' option, they should be treated similar to internal ports.
How did you verify/test it?
Ran the test against a multi-asic linecard in a T2 VoQ chassis.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

7 participants