sonic-mgmt icon indicating copy to clipboard operation
sonic-mgmt copied to clipboard

Use lldp0/lldp1 instead of lldp, since container lldp was removed from the host

Open mannytaheri opened this issue 1 year ago • 6 comments
trafficstars

Description of PR

test_lldp_neighbor testcase fails since container lldp was removed form the host

admin@ixre-board7:~$ docker exec -i lldp lldpcli show chassis Error response from daemon: No such container: lldp

for this reason, we will test either lldp0 or lldp1 based on randomly selected Asic.

Summary: Fixes # (issue)

Type of change

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

Back port request

  • [ ] 201911
  • [ ] 202012
  • [ ] 202205
  • [ ] 202305
  • [ ] 202311

Approach

What is the motivation for this PR?

since container lldp was removed from the host, we will test on lldp0 or lldp1 to test the test_lldp_neighbor testcase.

How did you do it?

Use enum_rand_one_frontend_asic_index to randomly select an Asic index Use the Asic index to select either lldp0 or lldp1

How did you verify/test it?

Tested on a multi cards multi Asics chassis.

Any platform specific information?

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

Documentation

mannytaheri avatar Jun 06 '24 20:06 mannytaheri

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/lldp/test_lldp.py:59:121: E501 line too long (122 > 120 characters)
tests/lldp/test_lldp.py:65:81: E127 continuation line over-indented for visual indent

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>

mssonicbld avatar Jun 06 '24 20:06 mssonicbld

The pre-commit check is mandatory. Please help have a look.

wsycqyz avatar Jun 07 '24 02:06 wsycqyz

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/lldp/test_lldp.py:66:81: E127 continuation line over-indented for visual indent

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>

mssonicbld avatar Jun 10 '24 16:06 mssonicbld

please retest with latest sonic-mgmt as fix is merged

rlhui avatar Jun 12 '24 17:06 rlhui

https://github.com/sonic-net/sonic-mgmt/pull/13190

rlhui avatar Jun 12 '24 17:06 rlhui

This change is still needed. @abdosi can you sign-off on this PR

arlakshm avatar Jul 10 '24 17:07 arlakshm

/azp run

yejianquan avatar Aug 08 '24 23:08 yejianquan

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines[bot] avatar Aug 08 '24 23:08 azure-pipelines[bot]

lldp test failed in the PR test, please check

yejianquan avatar Aug 09 '24 07:08 yejianquan

@mannytaheri please resolve conflicts

rlhui avatar Sep 18 '24 17:09 rlhui

@mannytaheri, please resolve conflicts

arlakshm avatar Sep 22 '24 23:09 arlakshm

@mannytaheri - reminder to please resolve conflicts, thanks.

rlhui avatar Oct 22 '24 02:10 rlhui

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/lldp/test_lldp.py:75:5: F841 local variable 'internal_port_list' is assigned to but never used

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>

mssonicbld avatar Oct 23 '24 14:10 mssonicbld

@mannytaheri PR conflicts with 202405 branch

mssonicbld avatar Oct 31 '24 23:10 mssonicbld

@mannytaheri Could you please add a new PR for 202405 branch as this one had conflicts. Thanks

YatishSVC avatar Nov 04 '24 23:11 YatishSVC

Create https://github.com/sonic-net/sonic-mgmt/pull/15385 for cherry-picking.

Please be note that, in current PR, supposedly Manny wanted to reduce test enumerations by using enum_rand_one_frontend_asic_index instead of enum_frontend_asic_index.

But it shouldn't work because it's on check_lldp_neighbor rather than test_lldp_neighbor.

In my cherry-pick PR I ignored the change because it's not harmful to enumerate other asics

yejianquan avatar Nov 06 '24 06:11 yejianquan