Fix test_neighbor_mac on t1-lag and t1-64-lag by byu343 · Pull Request #13314 · sonic-net/sonic-mgmt

byu343 · 2024-06-14T23:11:41Z

Description of PR

Summary:
Fixes # (issue)

Type of change

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

Back port request

Approach

What is the motivation for this PR?

A recent change applies this test to the topo of t1 and t1-variants:
#12952

The test can pass on topo t1 without any change.

For t1-lag or t1-64-lag, the Ethernet0 to test is in portchannel, so IP address cannot be set to it.

How did you do it?

The fix here is to remove the intf from the portchanenl before testing it.
Skip the test if Ethernet0 is not available or down at the beginning of test
Assert Ethernet0 is in routed state after removing it from portchannel

How did you verify/test it?

Run the test on t1-lag and t1-64-lag topo.

The test can pass
Verified in the log that removing and adding the member to PortChannel succeeded

Any platform specific information?

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

Documentation

lolyu · 2024-06-18T14:51:03Z

tests/arp/test_neighbor_mac.py

better to add a wait_until logic here to wait and check.

lolyu

LGTM

StormLiangMS · 2024-06-20T13:53:53Z

@byu343 conflicts need to resolve.

mssonicbld · 2024-06-20T18:38:48Z

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/arp/test_neighbor_mac.py:57:121: E501 line too long (162 > 120 characters) 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:

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.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

mssonicbld · 2024-06-20T21:51:53Z

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/arp/test_neighbor_mac.py:59:85: E502 the backslash is redundant between brackets tests/arp/test_neighbor_mac.py:59:85: E211 whitespace before '[' 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:

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.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

wsycqyz · 2024-06-21T00:45:06Z

Please re-run the PR test.

StormLiangMS · 2024-06-21T12:43:24Z

/azp run Azure.sonic-mgmt

azure-pipelines · 2024-06-21T12:43:34Z

Azure Pipelines successfully started running 1 pipeline(s).

ZhaohuiS · 2024-06-24T05:31:41Z

/azp run

azure-pipelines · 2024-06-24T05:31:46Z

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

ZhaohuiS

LGTM

ZhaohuiS · 2024-06-25T00:47:15Z

/azp run

azure-pipelines · 2024-06-25T00:47:21Z

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

wsycqyz · 2024-06-25T01:38:00Z

Have triggered to rerun the failed jobs.

StormLiangMS · 2024-06-25T05:47:54Z

/azp run Azure.sonic-mgmt

azure-pipelines · 2024-06-25T05:48:04Z

Azure Pipelines successfully started running 1 pipeline(s).

StormLiangMS · 2024-06-26T04:05:37Z

@byu343 code conflict.

byu343 · 2024-07-02T17:32:16Z

@byu343 code conflict.

Code conflict has been fixed

A recent change sets this test to t1-only: sonic-net#12952 The test can pass on topo t1 without any change. For t1-lag or t1-64-lag, the Ethernet0 to test is in portchannel, so IP address cannot be set to it. The fix here is to delete the intf from the portchanenl before testing it.

StormLiangMS · 2024-07-04T04:27:55Z

/azp run Azure.sonic-mgmt

azure-pipelines · 2024-07-04T04:28:04Z

Azure Pipelines successfully started running 1 pipeline(s).

* Fix test_neighbor_mac on t1-lag and t1-64-lag A recent change sets this test to t1-only: sonic-net#12952 The test can pass on topo t1 without any change. For t1-lag or t1-64-lag, the Ethernet0 to test is in portchannel, so IP address cannot be set to it. The fix here is to delete the intf from the portchanenl before testing it.

mssonicbld · 2024-07-10T01:23:50Z

Cherry-pick PR to 202311: #13607

mssonicbld · 2024-07-10T01:23:51Z

Cherry-pick PR to 202405: #13608

* Fix test_neighbor_mac on t1-lag and t1-64-lag A recent change sets this test to t1-only: #12952 The test can pass on topo t1 without any change. For t1-lag or t1-64-lag, the Ethernet0 to test is in portchannel, so IP address cannot be set to it. The fix here is to delete the intf from the portchanenl before testing it.

* Fix test_neighbor_mac on t1-lag and t1-64-lag A recent change sets this test to t1-only: sonic-net#12952 The test can pass on topo t1 without any change. For t1-lag or t1-64-lag, the Ethernet0 to test is in portchannel, so IP address cannot be set to it. The fix here is to delete the intf from the portchanenl before testing it.

mssonicbld · 2024-07-30T07:05:49Z

Cherry-pick PR to 202305: #13879

* Fix test_neighbor_mac on t1-lag and t1-64-lag A recent change sets this test to t1-only: #12952 The test can pass on topo t1 without any change. For t1-lag or t1-64-lag, the Ethernet0 to test is in portchannel, so IP address cannot be set to it. The fix here is to delete the intf from the portchanenl before testing it.

* Fix test_neighbor_mac on t1-lag and t1-64-lag A recent change sets this test to t1-only: sonic-net#12952 The test can pass on topo t1 without any change. For t1-lag or t1-64-lag, the Ethernet0 to test is in portchannel, so IP address cannot be set to it. The fix here is to delete the intf from the portchanenl before testing it.

wsycqyz approved these changes Jun 17, 2024

View reviewed changes

lolyu reviewed Jun 18, 2024

View reviewed changes

lolyu approved these changes Jun 20, 2024

View reviewed changes

byu343 force-pushed the fix-test-neighbor-mac branch from f364049 to 17062f4 Compare June 20, 2024 18:34

byu343 force-pushed the fix-test-neighbor-mac branch from 35d0086 to d811bec Compare June 20, 2024 22:01

byu343 force-pushed the fix-test-neighbor-mac branch from d811bec to 4a444eb Compare June 21, 2024 01:05

ZhaohuiS approved these changes Jun 24, 2024

View reviewed changes

siqbal1986 self-requested a review June 25, 2024 19:17

siqbal1986 approved these changes Jun 25, 2024

View reviewed changes

byu343 force-pushed the fix-test-neighbor-mac branch from 4a444eb to a96e16f Compare July 2, 2024 17:31

byu343 added 3 commits July 3, 2024 16:24

Assert when not in routed status

4b536a5

Change sleep to wait_until

100cbdd

byu343 force-pushed the fix-test-neighbor-mac branch from a96e16f to 1bb3018 Compare July 3, 2024 23:24

Fix format issue of line too long

96f18cd

byu343 force-pushed the fix-test-neighbor-mac branch from 1bb3018 to 96f18cd Compare July 5, 2024 17:24

bingwang-ms merged commit 35efc04 into sonic-net:master Jul 10, 2024

bingwang-ms added Approved for 202311 branch Approved for 202405 branch labels Jul 10, 2024

mssonicbld added the Created PR to 202311 branch label Jul 10, 2024

mssonicbld added the Created PR to 202405 branch label Jul 10, 2024

This was referenced Jul 10, 2024

[action] [PR:13314] Fix test_neighbor_mac on t1-lag and t1-64-lag #13607

Merged

[action] [PR:13314] Fix test_neighbor_mac on t1-lag and t1-64-lag #13608

Merged

mssonicbld added Included in 202311 branch and removed Created PR to 202311 branch labels Jul 10, 2024

mssonicbld added Included in 202405 branch and removed Created PR to 202405 branch labels Jul 12, 2024

ZhaohuiS added Request for 202305 branch Approved for 202305 branch labels Jul 30, 2024

mssonicbld added the Created PR to 202305 branch label Jul 30, 2024

mssonicbld mentioned this pull request Jul 30, 2024

[action] [PR:13314] Fix test_neighbor_mac on t1-lag and t1-64-lag #13879

Merged

8 tasks

mssonicbld added Included in 202305 branch and removed Created PR to 202305 branch labels Jul 30, 2024

Conversation

byu343 commented Jun 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of PR

Type of change

Back port request

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

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

Documentation

Uh oh!

lolyu Jun 18, 2024

Choose a reason for hiding this comment

Uh oh!

byu343 Jun 19, 2024

Choose a reason for hiding this comment

Uh oh!

lolyu left a comment

Choose a reason for hiding this comment

Uh oh!

StormLiangMS commented Jun 20, 2024

Uh oh!

mssonicbld commented Jun 20, 2024

Uh oh!

mssonicbld commented Jun 20, 2024

Uh oh!

wsycqyz commented Jun 21, 2024

Uh oh!

StormLiangMS commented Jun 21, 2024

Uh oh!

azure-pipelines bot commented Jun 21, 2024

Uh oh!

ZhaohuiS commented Jun 24, 2024

Uh oh!

azure-pipelines bot commented Jun 24, 2024

Uh oh!

ZhaohuiS left a comment

Choose a reason for hiding this comment

Uh oh!

ZhaohuiS commented Jun 25, 2024

Uh oh!

azure-pipelines bot commented Jun 25, 2024

Uh oh!

wsycqyz commented Jun 25, 2024

Uh oh!

StormLiangMS commented Jun 25, 2024

Uh oh!

azure-pipelines bot commented Jun 25, 2024

Uh oh!

StormLiangMS commented Jun 26, 2024

Uh oh!

byu343 commented Jul 2, 2024

Uh oh!

StormLiangMS commented Jul 4, 2024

Uh oh!

azure-pipelines bot commented Jul 4, 2024

Uh oh!

mssonicbld commented Jul 10, 2024

Uh oh!

mssonicbld commented Jul 10, 2024

Uh oh!

mssonicbld commented Jul 30, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

byu343 commented Jun 14, 2024 •

edited

Loading