Skip to content

Use paramiko for ssh connection in test_wr_arp#2037

Merged
wangxin merged 3 commits intosonic-net:masterfrom
wangxin:fix-wr-arp-pr
Aug 7, 2020
Merged

Use paramiko for ssh connection in test_wr_arp#2037
wangxin merged 3 commits intosonic-net:masterfrom
wangxin:fix-wr-arp-pr

Conversation

@wangxin
Copy link
Copy Markdown
Collaborator

@wangxin wangxin commented Aug 5, 2020

Description of PR

Summary:
Fixes # (issue)
The test_wr_arp.py test failed for two reasons:

  1. After reboot, the ptf ssh key on dut host is lost. Executing remote command on dut from ptf failed.
  2. This test script needs to do warm reboot on dut and would cause some error messages in syslog. The test will fail when loganalyzer is enabled.

Type of change

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

Approach

What is the motivation for this PR?

The test_wr_arp script put ssh key of ptfhost on dut to have password less
ssh connection. Then the ptfhost can execute command remote on dut
through this ssh connection. The problem is that the ssh key saved in
~/.ssh/authorized_keys on dut host may not persist after warm reboot.
It would also be a problem when secure boot feature is introduced. This
PR updated the test_wr_arp script to use paramiko for ssh connection
and remote command execution between ptfhost and dut.

How did you do it?

  • Update the ptf test script to use paramiko for SSH connection with dut
  • Disabled loganalyzer because test_wr_arp need to do warm-reboot on dut

How did you verify/test it?

Test run the test_wr_arp.py script on latest 201911 image.

Any platform specific information?

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

Documentation

The test_wr_arp script put ssh key of ptfhost on dut to have password less
ssh connection. Then the ptfhost can execute command remote on dut
through this ssh connection. The problem is that the ssh key saved in
~/.ssh/authorized_keys on dut host may not persist after warm reboot.
It would also be a problem when secure boot feature is introduced. This
PR updated the test_wr_arp script to use paramiko for ssh connection
and remote command execution between ptfhost and dut.

Changes:
* Update the ptf test script to use paramiko for SSH connection with dut
* Disabled loganalyzer because test_wr_arp need to do warm-reboot on dut

Signed-off-by: Xin Wang <[email protected]>
@wangxin wangxin requested a review from a team August 5, 2020 09:43
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Aug 5, 2020

This pull request introduces 1 alert when merging 5e1cf6f into 9da74ca - view on LGTM.com

new alerts:

  • 1 for Unused import

stdout = so.readlines()
stderr = se.readlines()
self.log('executed command {}, stdout={}, stderr={}'.format(cmd, stdout, stderr))
return_code = 0
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we get return code of exec_command? Is it possible to ignore some error if we always return 0 here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The paramiko library uses pattern of sending cmd string and receiving returned results in streams. There is no return code returned by the exec_command method of the paramiko.SSHClient.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we reuse this class?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Sure, pushed a new commit.

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Aug 6, 2020

This pull request introduces 2 alerts when merging 9db07ea into 4f02bb5 - view on LGTM.com

new alerts:

  • 2 for Unused import

@wangxin
Copy link
Copy Markdown
Collaborator Author

wangxin commented Aug 6, 2020

retest this please

@wangxin wangxin merged commit 2c1da34 into sonic-net:master Aug 7, 2020
@wangxin wangxin deleted the fix-wr-arp-pr branch September 24, 2020 02:33
kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
sonic-swss

1aa40f7 Remove port serdes object before removing port (sonic-net#2152)
876d690 [doc] Updating Policer config in Configuration manual (sonic-net#2144)

sonic-utilities
dfed952 show_platfom_info not run for simx (sonic-net#2042)
71fdee7 [aclshow] fix aclshow when clear is called before counters are populated (sonic-net#2037)
a48a027 [sonic-package-manager] implement blocking feature state change (sonic-net#2035)
c51871d [ci] Fix python dependencies reference path. (sonic-net#2060)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants