Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[action] [PR:15174] tests-common: Handle PortInUseException for SSHConsoleConn #15295

Merged
merged 1 commit into from
Nov 1, 2024

Conversation

matthew-soulsby
Copy link
Contributor

Description of PR

Addresses #15092

Summary:
This PR adds handling for the PortInUseException, which is thrown when an SSHConsoleConn is successfully established (most prominently in dut_console tests) but the console port is occupied, resulting in the connection being terminated by the host.

Type of change

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

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • 202405

Approach

What is the motivation for this PR?

This PR is intended to add more resilience to the dut_console tests, as if a testbed with a blocked port was selected for a nightly test, it would cause the dut_console tests to fail during setup. This provides a method for auto-recovery for these cases.

How did you do it?

By completing the following:

  • Add connection types for each of the configuration menu types (Digi, Cisco, and Sonic) to allow for different command sequences to be accounted for
  • Add function to clear a DUT's console port
  • Add retry logic to add resilience to main DUT connection set up

How did you verify/test it?

This process was conducted on 5 testbeds, with at least one of each config menu type.

Steps conducted during testing
  1. SSH into testbed using the console IP and console port - simulating a blocked port
  2. In a separate terminal, run any dut_console test individually
  3. During test setup, the connection from step 1 was terminated successfully
  4. The test then runs successfully
Example test output
  • Successful port reset (Digi config):

Screenshot 2024-10-25 165601

  • Successful port reset (Sonic config):

Screenshot 2024-10-25 183646

  • Sample failure
    • Simulating config menu type is not defined (causing the port clear function to exit early), and a more descriptive error message is thrown regarding why the connection to the DUT is failing:

Screenshot 2024-10-25 153008

Any platform specific information?

N/A - this is a generalised solution which accounts for as many types of configuration menus as possible.

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

N/A

Documentation

N/A

* Add Digi, Sonic and Cisco Config connection types (to differentiate
between commands required to clear a port)
* Add function to clear a DUT's console port
* Add retry to set up DUT console connection after clearing port

  Signed-off-by: Matt Soulsby [email protected]
Copy link
Collaborator

@StormLiangMS StormLiangMS left a comment

Choose a reason for hiding this comment

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

LGTM

@StormLiangMS StormLiangMS merged commit 6ed7869 into sonic-net:202311 Nov 1, 2024
12 checks passed
@bingwang-ms
Copy link
Collaborator

@matthew-soulsby, @StormLiangMS The change caused some issue in 202311 branch. Can you please check?

        # Set up console host
        host = None
        for attempt in range(1, 4):
            try:
                host = ConsoleHost(console_type=console_type,
                                   console_host=console_host,
                                   console_port=console_port,
                                   sonic_username=creds['sonicadmin_user'],
                                   sonic_password=sonic_password,
                                   console_username=console_username,
                                   console_password=creds['console_password'][console_type])
                break
            except Exception as e:
                logger.warning(f"Attempt {attempt}/3 failed: {e}")
                continue
        else:
>           raise Exception("Failed to set up connection to console port. See warning logs for details.")
E           Exception: Failed to set up connection to console port. See warning logs for details.

@matthew-soulsby
Copy link
Contributor Author

Hi @bingwang-ms, sure I'll look into this asap

@matthew-soulsby
Copy link
Contributor Author

Just to update for visibility, this was an issue with our internal repository diverging a couple of months ago, and this change reverting that erroneously. The issue has been resolved internally

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