Fix unreachable DUT mgmt_ip in IPv6 only test case by maipbui · Pull Request #12492 · sonic-net/sonic-mgmt

maipbui · 2024-04-17T20:03:47Z

Description of PR

Summary: Fix unreachable DUT mgmt_ip in IPv6 only test case
Fixes # (issue)

Microsoft ADO 27674168, 27674176

Type of change

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

Back port request

201911
202012
202205
202305 [action] [PR:12492] Fix unreachable DUT mgmt_ip in IPv6 only test case #12573
202311 [action] [PR:12492] Fix unreachable DUT mgmt_ip in IPv6 only test case #12574

Approach

What is the motivation for this PR?

#12303 introduced a bug that DUT mgmt_ipv4 is unreachable in a IPv6 only test case, causing test_rw_user_ipv6_only, test_ro_user_ipv6_only, test_snmp_ipv6_only failed with paramiko.ssh_exception.NoValidConnectionsError or socket.timeout: timed out

How did you do it?

If mgmt_ip is unreachable, try mgmt_ipv6

How did you verify/test it?

Verified on internal and 202305 branch on physical DUT

Any platform specific information?

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

Documentation

Signed-off-by: Mai Bui <[email protected]>

tests/common/utilities.py

tests/conftest.py

This reverts commit b7230b1.

Signed-off-by: Mai Bui <[email protected]>

qiluo-msft

Please also check with other reviewers.

sdszhang · 2024-04-19T00:48:35Z

@maipbui I tested this change locally in KVM t0 testbed, there is a failure, can you please take a look?
You can follow the steps in #12377 to enable ipv6 in docker for KVM testbed.

$./run_tests.sh -n vms-kvm-t0 -d vlab-01 -c ip/test_mgmt_ipv6_only.py::test_ro_user_ipv6_only -f vtestbed.yaml -i ../ansible/veos_vtb -u -E -m individual -u
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

output = {'failed': True, 'changed': True, 'stdout': '', 'stderr': "Warning: Permanently added 'fec0::ffff:afa:1' (RSA) to the ...fec0::ffff:afa:1' (RSA) to the list of known hosts.", 'Permission denied, please try again.'], '_ansible_no_log': None}
exp_val1 = 'test', exp_val2 = 'remote_user'

    def check_output(output, exp_val1, exp_val2):
>       pytest_assert(not output['failed'], output['stderr'])
E       Failed: Warning: Permanently added 'fec0::ffff:afa:1' (RSA) to the list of known hosts.
E       Permission denied, please try again.

exp_val1   = 'test'
exp_val2   = 'remote_user'
output     = {'failed': True, 'changed': True, 'stdout': '', 'stderr': "Warning: Permanently added 'fec0::ffff:afa:1' (RSA) to the ...fec0::ffff:afa:1' (RSA) to the list of known hosts.", 'Permission denied, please try again.'], '_ansible_no_log': None}

tests/common/utilities.py

Signed-off-by: Mai Bui <[email protected]>

mssonicbld · 2024-04-19T16:38:20Z

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/common/utilities.py:21:1: F401 'socket' imported but unused tests/common/utilities.py:26:1: F401 'paramiko.ssh_exception.NoValidConnectionsError' imported but unused 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>

Signed-off-by: Mai Bui <[email protected]>

qiluo-msft · 2024-04-19T21:28:55Z

tests/common/utilities.py

-    return pwd
+def get_dut_current_passwd(ipv4_address, ipv6_address, username, passwords):
+    try:
+        return _paramiko_ssh(ipv4_address, username, passwords)


return

original this function return a password string. you want to return a pair instead? Is it an unexpected behavior change? #Closed

If you would like to keep old bahavior, suggest use a different variable name pwd -> passwd.

updated, thanks for catching this

Signed-off-by: Mai Bui <[email protected]>

maipbui · 2024-04-22T20:07:19Z

Tested on 202305 and internal on physical DUT, syncing offline with Dashuai Zhang for kvm test

sdszhang · 2024-04-23T11:08:39Z

@maipbui I tested this change locally in KVM t0 testbed, there is a failure, can you please take a look? You can follow the steps in #12377 to enable ipv6 in docker for KVM testbed.

$./run_tests.sh -n vms-kvm-t0 -d vlab-01 -c ip/test_mgmt_ipv6_only.py::test_ro_user_ipv6_only -f vtestbed.yaml -i ../ansible/veos_vtb -u -E -m individual -u
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

output = {'failed': True, 'changed': True, 'stdout': '', 'stderr': "Warning: Permanently added 'fec0::ffff:afa:1' (RSA) to the ...fec0::ffff:afa:1' (RSA) to the list of known hosts.", 'Permission denied, please try again.'], '_ansible_no_log': None}
exp_val1 = 'test', exp_val2 = 'remote_user'

    def check_output(output, exp_val1, exp_val2):
>       pytest_assert(not output['failed'], output['stderr'])
E       Failed: Warning: Permanently added 'fec0::ffff:afa:1' (RSA) to the list of known hosts.
E       Permission denied, please try again.

exp_val1   = 'test'
exp_val2   = 'remote_user'
output     = {'failed': True, 'changed': True, 'stdout': '', 'stderr': "Warning: Permanently added 'fec0::ffff:afa:1' (RSA) to the ...fec0::ffff:afa:1' (RSA) to the list of known hosts.", 'Permission denied, please try again.'], '_ansible_no_log': None}

You can skip this error. It seems to be separate issues, not related to your change. I will check it further and reach out if needed. Your current code change looks good to me.

sdszhang

LGTM

## Description of PR Summary: Fix unreachable DUT mgmt_ip in IPv6 only test case ### Type of change - [X] Bug fix - [ ] Testbed and Framework(new/improvement) - [ ] Test case(new/improvement) ## Approach #### What is the motivation for this PR? sonic-net#12303 introduced a bug that DUT mgmt_ipv4 is unreachable in a IPv6 only test case, causing test_rw_user_ipv6_only, test_ro_user_ipv6_only, test_snmp_ipv6_only failed with `paramiko.ssh_exception.NoValidConnectionsError` or `socket.timeout: timed out` #### How did you do it? If mgmt_ip is unreachable, try mgmt_ipv6 #### How did you verify/test it? Verified on internal and 202305 branch on physical DUT

mssonicbld · 2024-04-24T13:07:27Z

Cherry-pick PR to 202305: #12573

mssonicbld · 2024-04-24T13:07:29Z

Cherry-pick PR to 202311: #12574

## Description of PR Summary: Fix unreachable DUT mgmt_ip in IPv6 only test case ### Type of change - [X] Bug fix - [ ] Testbed and Framework(new/improvement) - [ ] Test case(new/improvement) ## Approach #### What is the motivation for this PR? #12303 introduced a bug that DUT mgmt_ipv4 is unreachable in a IPv6 only test case, causing test_rw_user_ipv6_only, test_ro_user_ipv6_only, test_snmp_ipv6_only failed with `paramiko.ssh_exception.NoValidConnectionsError` or `socket.timeout: timed out` #### How did you do it? If mgmt_ip is unreachable, try mgmt_ipv6 #### How did you verify/test it? Verified on internal and 202305 branch on physical DUT

Fix mgmt ipv4 unreachable in ipv6 only test

b7230b1

Signed-off-by: Mai Bui <[email protected]>

maipbui force-pushed the fix_mgmt_ip branch from f91c1ed to b7230b1 Compare April 18, 2024 00:15

maipbui changed the title ~~test~~ Fix unreachable DUT mgmt_ip in IPv6 only test case Apr 18, 2024

maipbui marked this pull request as ready for review April 18, 2024 01:48

maipbui requested review from qiluo-msft and xwjiang-ms April 18, 2024 01:48

yejianquan reviewed Apr 18, 2024

View reviewed changes

tests/common/utilities.py Outdated Show resolved Hide resolved

tests/conftest.py Outdated Show resolved Hide resolved

maipbui added 3 commits April 18, 2024 19:17

Revert "Fix mgmt ipv4 unreachable in ipv6 only test"

de438b1

This reverts commit b7230b1.

address comment

2abd7ac

Signed-off-by: Mai Bui <[email protected]>

fix precommit

3653203

Signed-off-by: Mai Bui <[email protected]>

qiluo-msft previously approved these changes Apr 18, 2024

View reviewed changes

qiluo-msft mentioned this pull request Apr 18, 2024

Adding ipv6_mgmt_only test case into PR testing and fix fixture errors #12393

Merged

8 tasks

maipbui marked this pull request as draft April 18, 2024 22:07

yejianquan reviewed Apr 19, 2024

View reviewed changes

tests/common/utilities.py Outdated Show resolved Hide resolved

address comments

8279393

Signed-off-by: Mai Bui <[email protected]>

maipbui dismissed qiluo-msft’s stale review via 8279393 April 19, 2024 16:35

maipbui added 2 commits April 19, 2024 16:40

fix pre-commit

23e7dfd

Signed-off-by: Mai Bui <[email protected]>

Merge remote-tracking branch 'upstream/master' into fix_mgmt_ip

8aeb2bc

qiluo-msft reviewed Apr 19, 2024

View reviewed changes

fix

fa5493b

Signed-off-by: Mai Bui <[email protected]>

sdszhang approved these changes Apr 23, 2024

View reviewed changes

maipbui marked this pull request as ready for review April 23, 2024 13:47

maipbui requested review from qiluo-msft and yejianquan April 23, 2024 13:47

qiluo-msft approved these changes Apr 23, 2024

View reviewed changes

qiluo-msft merged commit 1376afa into sonic-net:master Apr 23, 2024

maipbui deleted the fix_mgmt_ip branch April 23, 2024 18:35

maipbui added Request for 202305 branch Request for 202311 branch Approved for 202305 branch Approved for 202311 branch labels Apr 24, 2024

mssonicbld added the Created PR to 202305 branch label Apr 24, 2024

mssonicbld added the Created PR to 202311 branch label Apr 24, 2024

This was referenced Apr 24, 2024

[action] [PR:12492] Fix unreachable DUT mgmt_ip in IPv6 only test case #12573

Merged

[action] [PR:12492] Fix unreachable DUT mgmt_ip in IPv6 only test case #12574

Merged

mssonicbld added Included in 202305 branch and removed Created PR to 202305 branch labels Apr 24, 2024

mssonicbld added Included in 202311 branch and removed Created PR to 202311 branch labels Apr 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix unreachable DUT mgmt_ip in IPv6 only test case#12492

Fix unreachable DUT mgmt_ip in IPv6 only test case#12492
qiluo-msft merged 8 commits intosonic-net:masterfrom
maipbui:fix_mgmt_ip

maipbui commented Apr 17, 2024 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

qiluo-msft left a comment

Uh oh!

sdszhang commented Apr 19, 2024 •

edited

Loading

Uh oh!

Uh oh!

mssonicbld commented Apr 19, 2024

Uh oh!

qiluo-msft Apr 19, 2024 •

edited

Loading

Uh oh!

qiluo-msft Apr 19, 2024

Uh oh!

maipbui Apr 22, 2024

Uh oh!

maipbui commented Apr 22, 2024

Uh oh!

sdszhang commented Apr 23, 2024

Uh oh!

sdszhang left a comment

Uh oh!

mssonicbld commented Apr 24, 2024

Uh oh!

mssonicbld commented Apr 24, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

maipbui commented Apr 17, 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!

Uh oh!

Uh oh!

qiluo-msft left a comment

Choose a reason for hiding this comment

Uh oh!

sdszhang commented Apr 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

mssonicbld commented Apr 19, 2024

Uh oh!

qiluo-msft Apr 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Apr 19, 2024

Choose a reason for hiding this comment

Uh oh!

maipbui Apr 22, 2024

Choose a reason for hiding this comment

Uh oh!

maipbui commented Apr 22, 2024

Uh oh!

sdszhang commented Apr 23, 2024

Uh oh!

sdszhang left a comment

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Apr 24, 2024

Uh oh!

mssonicbld commented Apr 24, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

maipbui commented Apr 17, 2024 •

edited

Loading

sdszhang commented Apr 19, 2024 •

edited

Loading

qiluo-msft Apr 19, 2024 •

edited

Loading