Skip to content

[framework] Add DUT connection by console port#2204

Merged
bingwang-ms merged 5 commits intosonic-net:masterfrom
bingwang-ms:add_connection_by_console
Jan 5, 2021
Merged

[framework] Add DUT connection by console port#2204
bingwang-ms merged 5 commits intosonic-net:masterfrom
bingwang-ms:add_connection_by_console

Conversation

@bingwang-ms
Copy link
Collaborator

@bingwang-ms bingwang-ms commented Sep 11, 2020

Description of PR

Summary:
Fixes # (issue)

Type of change

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

Approach

What is the motivation for this PR?

As we all know, Ansible run commands via SSH connection from mgmt port. However, the connection is not stable or not usable for some scenes, like vrf test, reboot test and etc. Therefore, we need a more robust way to connect and run command on DUT. This commit add enables framework to connect DUT by console port.

How did you do it?

  1. The feature is implemented based on netmiko.
    Cueerntly, two types of console connection in star lab are supported, that is console on Digi (digi_console) and console servers using telnet. Some interfaces are needed to be overwriten if more console types are to support.
  2. The console type, address and port of console hub are defined in inventory file,
str-7260cx3-acs-1:
      ansible_host: 10.64.247.224
      hwsku: Arista-7260CX3-D108C8
      pdu_host: pdu-106
      console_type: telnet_console
      console_host: 10.3.146.16
      console_port: 2015

and console username and password are managed by Ansible secret vars.
3. Add a new fixture duthost_console for establishing connection to DUT by console port.
4. All public interfaces are inherited from netmiko. But only send_command is tested for now.
https://ktbyers.github.io/netmiko/docs/netmiko/index.html#netmiko.BaseConnection.send_command

How did you verify/test it?

I write a demo script to verify it

import pytest
import logging

pytestmark = [
    pytest.mark.topology('t0'),
]

def test_telnet(duthost_console):
    out = duthost_console.send_command("show version")
    logging.info(out)

Any platform specific information?

No.

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

N/A.

Documentation

Please refer to Netmiko doc
https://ktbyers.github.io/netmiko/docs/netmiko/index.html

@lgtm-com
Copy link

lgtm-com bot commented Sep 11, 2020

This pull request introduces 7 alerts when merging 373675e19b9011dca73c87308234ac9c429f5189 into 647b57e - view on LGTM.com

new alerts:

  • 6 for Unused import
  • 1 for First argument to super() is not enclosing class

@bingwang-ms bingwang-ms requested a review from a team September 11, 2020 07:48
Copy link
Contributor

Choose a reason for hiding this comment

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

How do we handle console connections that require Active Directory password. This is the case for most of the new DUTs (for eg. DX10s) that are onboarded and some Fanout switches. I think we will not want to have AD creds in secrets vars file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great question. The auth method used in Lab highly depends on Microsoft AD. Perhaps we could try to add a public account for test.

Copy link
Contributor

Choose a reason for hiding this comment

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

This method seems to be returning return_msg

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks. I missed it. I will post an update to fix it.

Copy link
Contributor

Choose a reason for hiding this comment

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

This condition is repeating in the line 74. Is there a reason to check twice?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch. The check in ln68-71 is not necessary. But the check in ln74-77 must be kept to handle the case when password is not required to login.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This assumes that all the console servers use the same username/password?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch. I need to come up with a better method to manage account info for various console server.

@lguohan
Copy link
Contributor

lguohan commented Oct 29, 2020

most of the part looks good to me. maybe we can get it merged first and then iterate?

@bingwang-ms
Copy link
Collaborator Author

most of the part looks good to me. maybe we can get it merged first and then iterate?

Thanks. Now I'm waiting for this PR #2257 (Pdu and Console connection info in connection graph), which puts console meta data in connection graph. Currently, I put them in inventory file. But it shouldn't be too much work.

As we all know, Ansible run commands via SSH connection from mgmt port.
However, the connection is not stable or not usable for some scenes,
like vrf test, reboot test and etc. Therefore, we need a more robust way
to connect and run command on DUT (or other hosts). This commit add
enables framework to connect DUT by console port.
@bingwang-ms bingwang-ms force-pushed the add_connection_by_console branch from 78c597d to dff6cb1 Compare December 31, 2020 05:00
1. Support alt password for console and SNOiC
2. Support new console server 'menu_ports'

Signed-off-by: bingwang <bingwang@microsoft.com>
@bingwang-ms bingwang-ms force-pushed the add_connection_by_console branch from 0859016 to d401bdc Compare December 31, 2020 05:13
@bingwang-ms
Copy link
Collaborator Author

@yxieca @vaibhavhd @wangxin Sorry for the late update. Please help to review again.
Major updates:

  1. Support console login via menu ports
  2. Support multi password for console server and SONiC

Please note that the current code reads console information from inventory file and reads creds from secret file. The next step is to put console information into connect_graph_facts. I will work with @sujinmkang to complete PR #2257

@lgtm-com
Copy link

lgtm-com bot commented Dec 31, 2020

This pull request introduces 1 alert when merging ba7cdb5 into 20d729d - view on LGTM.com

new alerts:

  • 1 for Unused import

Signed-off-by: bingwang <bingwang@microsoft.com>
Signed-off-by: bingwang <bingwang@microsoft.com>
@bingwang-ms bingwang-ms merged commit ad4fb76 into sonic-net:master Jan 5, 2021
kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
…ance submodule head (sonic-net#14029)

utilities:
* a4f141f1 2023-01-10 | [sfputil] Firmware download/upgrade CLI support for QSFP-DD (sonic-net#1947) (sonic-net#2349) (HEAD -> 202205) [CliveNi]

swss-common:
* 41fcad8 2023-01-30 | Increase the netlink buffer size from 3MB to 16MB. (sonic-net#739) (HEAD -> 202205) [KISHORE KUNAL]

sairedis:
* 5ce9990 2023-02-27 | [Dual-ToR] update sai.profile with SAI_ADDITIONAL_MAC_ENABLED attribute if corresponding arg passed to syncd (sonic-net#1201) (HEAD -> 202205) [Andriy Yurkiv]
* 3c2e0c5 2023-02-23 | Use new value of STATE_DB FAST_REBOOT entry (sonic-net#1196) [Aryeh Feigin]
* fe7756f 2023-02-28 | [submodule][SAI]Advance SAI head (sonic-net#1210) (github/202205) [Richard.Yu]

platform-common:
* 321a8e7 2022-09-23 | Cdb fw upgrade (sonic-net#308) (HEAD -> 202205) [CliveNi]

swss:
* ceea558 2023-02-28 | [orchagent]: Get bridge port ID from orchagent cache instead of SAI API (sonic-net#2657) (HEAD -> 202205) [Lawrence Lee]
* bd04e24 2023-03-01 | [dualtor] Fix neighbor miss when mux is not ready (sonic-net#2676) (HEAD -> 202205) [Longxiang Lyu]
* 7d87a90 2023-02-28 | [ci] Fix pipeline error about team5 not found. (sonic-net#2684) [Liu Shilong]
* 93a924c 2023-02-27 | [aclorch] Fixed issue sonic-net#2204.Support IN_PORTS qualifer in MIRRORV6 table. (sonic-net#2668) [Rajkumar-Marvell]
* 9d87ec4 2023-02-23 | swss: Fix Invalid port oid messages generated because of voq counters. (sonic-net#2653) [Sambath Kumar Balasubramanian]

Signed-off-by: Ying Xie <ying.xie@microsoft.com>
kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
swss:
* 143cd44 2023-03-04 | Revert "[aclorch] Fixed issue sonic-net#2204.Support IN_PORTS qualifer in MIRRORV6 table. (sonic-net#2668)" (sonic-net#2687) (HEAD -> 202205, github/202205) [StormLiangMS]
* 25812f8 2023-02-06 | [test_mux] add sleep in test_NH (sonic-net#2648) [Nikola Dancejic]

Signed-off-by: Ying Xie <ying.xie@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants