Fix post-reboot Ansible readiness check by LinJin23 · Pull Request #26636 · sonic-net/sonic-mgmt

LinJin23 · 2026-07-28T23:33:43Z

Description of PR

Summary:

Wait for the DUT to successfully execute an Ansible command after post-reboot SSH startup before reading its uptime.

Fixes #

Type of change

Back port request

Tracking issue/work item for backport/cherry-pick request (GitHub issue or Microsoft ADO): N/A
Failure type: Flaky post-reboot readiness issue

Tested branch

Test result

202605: SONiC.20260510.08 - test_link_down passed 10 out of 10 runs across three physical Arista T0 testbeds: https://elastictest.org/scheduler/testplan/6a718773eb2c1b23503d3013

Approach

What is the motivation for this PR?

SSH can become available before the DUT is ready to execute Ansible commands through privilege escalation, causing a flaky uptime -s failure after reboot.

How did you do it?

Added a bounded readiness check that waits for a lightweight Ansible command to succeed before reading the DUT uptime.

How did you verify/test it?

Ran the targeted pre-commit checks for tests/platform_tests/test_link_down.py. The test also passed 10 out of 10 runs across three physical Arista T0 testbeds using SONiC.20260510.08.

Any platform specific information?

No.

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

Not applicable.

Documentation

Not applicable.

Wait for a lightweight Ansible command to succeed after SSH startup before reading DUT uptime. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0d08e59d-63c7-4189-9be9-29045dd91d20 Signed-off-by: Lin Jin <linjin@microsoft.com>

mssonicbld · 2026-07-28T23:33:51Z

/azp run

azure-pipelines · 2026-07-28T23:33:51Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

azure-pipelines · 2026-07-28T23:34:07Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Copilot

Pull request overview

Improves the robustness of the test_link_status_on_host_reboot workflow by adding a bounded post-reboot readiness check to ensure the DUT can successfully execute an Ansible command (including privilege escalation) before reading uptime, reducing flakiness where SSH is up but Ansible/become isn’t fully ready yet.

Changes:

Import wait_until and introduce a wait_for_dut_ready() helper that polls a lightweight Ansible command (true) until it succeeds.
Add bounded timeout/interval constants for the new post-reboot readiness polling.
Invoke the readiness check immediately after wait_for_startup() and before dut.get_up_time().

azure-pipelines · 2026-07-28T23:36:48Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

mssonicbld · 2026-07-29T00:14:51Z

This PR has backport request label(s) for branch(es): 202605, but is missing required test information. Please make sure you tick the tested branch(es) in the Tested branch section and provide test evidence (e.g., 202605: <test result>) in the Test result section as well in your PR description.

_{---Powered by SONiC BuildBot}

liamkearney-msft · 2026-07-29T01:37:24Z

    return duts_and_ports


+def wait_for_dut_ready(dut):


should we use an existing library function for this? eg. "wait_critical_process" ?
the reboot tests (eg. test_reboot.py / reboot_utils/py / process_utils.py) should already have infrastructure for this. We shouldnt reinvent the wheel in this test

wait_critical_processes() waits for full SONiC process readiness, which is more than needed here. This check only verifies that the Ansible/become command path is ready before calling get_up_time() . I could not find an existing lightweight helper for this purpose.

wait for startup should do this - is it not sufficient? That function is used for many tests for ssh readyness - it should be robust

I would opt to fix that function instead if its not doing what it should be.

Thanks for the suggestion. I'll update the code accordingly.

LinJin23 · 2026-07-29T02:08:29Z

/azp run

azure-pipelines · 2026-07-29T02:08:46Z

Azure Pipelines: Successfully started running 1 pipeline(s).

mssonicbld · 2026-07-29T06:15:49Z

The Tested branch section has been ticked and Test result is provided for branch(es): 202605. Added label(s): Tested for 202605 Branch.

_{---Powered by SONiC BuildBot}

Signed-off-by: Lin Jin <linjin@microsoft.com>

mssonicbld · 2026-08-04T13:20:07Z

/azp run

azure-pipelines · 2026-08-04T13:20:27Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

tests/common/reboot.py:251

The Ansible-readiness wait currently uses a fixed ANSIBLE_READY_TIMEOUT regardless of how much of timeout was already spent waiting for SSH. If timeout is meant as the overall startup budget, compute an ansible_timeout from the remaining time to avoid waiting longer than intended.

    logger.info('waiting for Ansible commands to become ready on {}'.format(hostname))

    def is_ansible_ready():
        result = duthost.command("true", module_ignore_errors=True)
        return result.is_successful

    pytest_assert(
        wait_until(ANSIBLE_READY_TIMEOUT, ANSIBLE_READY_INTERVAL, 0, is_ansible_ready),
        "DUT {} did not become ready for Ansible commands after SSH startup".format(hostname),
    )

tests/common/reboot.py:212

To keep timeout acting as an end-to-end bound for wait_for_startup(), it helps to record a function-level start timestamp before the SSH polling begins so the subsequent Ansible-readiness wait can consume only the remaining budget.

This issue also appears on line 242 of the same file.

def wait_for_startup(duthost, localhost, delay, timeout, port=SONIC_SSH_PORT,
                     wait_for_ansible=True):

LinJin23 · 2026-08-04T23:32:06Z

/azp run

azure-pipelines · 2026-08-04T23:32:22Z

Azure Pipelines: Successfully started running 1 pipeline(s).

judyjoseph · 2026-08-25T23:08:05Z

@StormLiangMS could you please help review/merge

StormLiangMS · 2026-09-01T04:40:04Z

@LinJin23 — ❌ I found one blocking cleanup issue in the current head (6ec2c1e4).

In tests/common/reboot.py, the new pytest_assert(...) readiness timeout raises pytest Failed, which derives from BaseException, not Exception. Therefore it bypasses reboot()'s existing except Exception handler around wait_for_startup(). That handler is responsible for disconnecting the console session and terminating the thread pool, so an Ansible-readiness timeout can leave both resources active.

Please either raise a normal Exception here, consistent with the SSH timeout failures earlier in wait_for_startup(), or guarantee cleanup with finally/explicit handling of pytest.fail.Exception.

The shared-helper placement and command("true") probe otherwise look sound. Non-blocking: please also confirm that the fixed 90-second readiness limit is sufficient for slow physical chassis/platform recovery paths, since this now applies by default to all callers.

Signed-off-by: Lin Jin <linjin@microsoft.com>

mssonicbld · 2026-09-01T07:27:04Z

/azp run

azure-pipelines · 2026-09-01T07:27:22Z

Azure Pipelines: Successfully started running 1 pipeline(s).

+    duthost.meta("clear_facts")
+
+    if not wait_for_ansible:
+        return


+    if not wait_for_ansible:
+        return
+
+    logger.info('waiting for Ansible commands to become ready on {}'.format(hostname))


+        return
+
+    logger.info('waiting for Ansible commands to become ready on {}'.format(hostname))
+


+    logger.info('waiting for Ansible commands to become ready on {}'.format(hostname))
+
+    def is_ansible_ready():
+        result = duthost.command("true", module_ignore_errors=True)


Signed-off-by: Lin Jin <linjin@microsoft.com>

mssonicbld · 2026-09-15T22:58:35Z

/azp run

azure-pipelines · 2026-09-15T22:58:52Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Signed-off-by: Lin Jin <linjin@microsoft.com>

mssonicbld · 2026-09-16T07:23:13Z

/azp run

azure-pipelines · 2026-09-16T07:23:31Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Copilot AI lite review requested due to automatic review settings July 28, 2026 23:33

github-actions Bot requested review from Junchao-Mellanox, vaibhavhd and xwjiang-ms July 28, 2026 23:34

Copilot started reviewing on behalf of LinJin23 July 28, 2026 23:34 View session

Copilot AI reviewed Jul 28, 2026

View reviewed changes

LinJin23 marked this pull request as ready for review July 28, 2026 23:36

LinJin23 requested review from gauravnagesh-msft, judyjoseph, liamkearney-msft and vvolam July 28, 2026 23:36

LinJin23 added the Request for 202605 branch label Jul 28, 2026

liamkearney-msft reviewed Jul 29, 2026

View reviewed changes

mssonicbld added the Tested for 202605 branch Tested for 202605 branch label Jul 29, 2026

Harden common reboot startup readiness

6ec2c1e

Signed-off-by: Lin Jin <linjin@microsoft.com>

Copilot AI review requested due to automatic review settings August 4, 2026 13:20

github-actions Bot requested review from lolyu, wangxin and yutongzhang-microsoft August 4, 2026 13:20

Copilot started reviewing on behalf of LinJin23 August 4, 2026 13:20 View session

Copilot AI reviewed Aug 4, 2026

View reviewed changes

judyjoseph previously approved these changes Aug 4, 2026

View reviewed changes

liamkearney-msft previously approved these changes Aug 4, 2026

View reviewed changes

Fix Ansible readiness timeout cleanup

4591ce8

Signed-off-by: Lin Jin <linjin@microsoft.com>

LinJin23 dismissed stale reviews from liamkearney-msft and judyjoseph via 4591ce8 September 1, 2026 07:26

github-advanced-security Bot found potential problems Sep 1, 2026

View reviewed changes

Use reboot timeout for Ansible readiness

ea8b066

Signed-off-by: Lin Jin <linjin@microsoft.com>

Initialize SSH retry result state

b26b15b

Signed-off-by: Lin Jin <linjin@microsoft.com>

		return

		logger.info('waiting for Ansible commands to become ready on {}'.format(hostname))

Conversation

LinJin23 commented Jul 28, 2026 • 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

Tested branch

Test result

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!

mssonicbld commented Jul 28, 2026

Uh oh!

azure-pipelines Bot commented Jul 28, 2026

Uh oh!

azure-pipelines Bot commented Jul 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

azure-pipelines Bot commented Jul 28, 2026

Uh oh!

mssonicbld commented Jul 29, 2026

Uh oh!

liamkearney-msft Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

LinJin23 Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

liamkearney-msft Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

liamkearney-msft Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

LinJin23 Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

LinJin23 commented Jul 29, 2026

Uh oh!

azure-pipelines Bot commented Jul 29, 2026

Uh oh!

mssonicbld commented Jul 29, 2026

Uh oh!

mssonicbld commented Aug 4, 2026

Uh oh!

azure-pipelines Bot commented Aug 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

LinJin23 commented Aug 4, 2026

Uh oh!

azure-pipelines Bot commented Aug 4, 2026

Uh oh!

judyjoseph commented Aug 25, 2026

Uh oh!

StormLiangMS commented Sep 1, 2026

Uh oh!

mssonicbld commented Sep 1, 2026

Uh oh!

azure-pipelines Bot commented Sep 1, 2026

Uh oh!

mssonicbld commented Sep 15, 2026

Uh oh!

azure-pipelines Bot commented Sep 15, 2026

Uh oh!

mssonicbld commented Sep 16, 2026

Uh oh!

azure-pipelines Bot commented Sep 16, 2026

Uh oh!

Reviewers

LinJin23 commented Jul 28, 2026 •

edited

Loading