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

[SmartSwitch] Add tests for reboot of a smart switch #16566

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

vvolam
Copy link
Contributor

@vvolam vvolam commented Jan 17, 2025

Description of PR

Summary: Add sonic-mgmt tests for reboot of a smart switch and individual DPUs
Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • New Test case
    • Skipped for non-supported platforms
  • Test case improvement

Back port request

  • 202012
  • 202205
  • 202305
  • 202311
  • 202405
  • 202411

Approach

What is the motivation for this PR?

Supporting different types of reboots for smart switch

How did you do it?

  • Extend existing reboot() method for a smart switch as well to reboot the DPUs.
  • Add a test case to reboot all the DPUs individually

How did you verify/test it?

  • Verified for any regressions on normal switch.
  • Verified on x86_64-nvidia_sn4280-r0 smartswitch platform

Any platform specific information?

-Smartswitch topology

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

Documentation

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@nissampa
Copy link
Contributor

lgtm

@oleksandrivantsiv
Copy link
Contributor

@congh-nvidia, @JibinBao pleae review

Returns:
True if the current node is a SmartSwitch, else False
"""
return "DPUS" in self.facts
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you give an example for self.facts?

"""
return "DPUS" in self.facts

def is_dpu(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

For dpu, we have a dpuhosts and dpuhost fixture, if you want to operate the dpu, maybe you can use dpuhosts
#15695

@param dpu_id: DPU ID
@param dpu_name: DPU name
"""
from tests.common.reboot import REBOOT_TYPE_HISTOYR_QUEUE, sync_reboot_history_queue_with_dut
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we combine it with line 7 'from tests.common.reboot import reboot_ss_ctrl_dict as reboot_dict'

@param dpu_name: Name of the DPU
@param dut_datetime: Datetime of DUT when reboot initiated
"""
pytest_assert(wait_until(120, 30, 0, check_dpu_ping_status, duthost, dpu_ip),
Copy link
Contributor

Choose a reason for hiding this comment

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

After reboot dpu, is 120s enough for the dpu to be up?

"""
duthost = duthosts[enum_rand_one_per_hwsku_hostname]

for index in range(num_dpu_modules):
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we have 4 or 8 dpus for one smartswitch, perform_and_check_reboot might take more than 3 minutes for one dpu. Can we tests the dpus in parallel so that we can save much test running time?

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.

6 participants