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

Add multi-asic support for dropconfig #3735

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

Conversation

arista-hpandya
Copy link
Contributor

@arista-hpandya arista-hpandya commented Jan 27, 2025

What I did

  • Added support for iterating over all namespaces (ns) when none specified
  • Added a test cases to verify multi-asic behaviour
  • Introduced a wrapper class to handle the mutli-asic functionality

This script was missed during the initial efforts to add multi-asic support.

How I did it

This is in accordance to the issue sonic-net/sonic-buildimage#15148 which tracks the multi-asic support for scripts in sonic-utilities.

How to verify it

The changes were verified by:

Running all the unit tests in sonic-utilities
Verifying the changes manually on a single-asic linecards
Verifying the changes manually on a multi-asic linecards

Screenshots of the manual tests:

Running on all namespaces when none specified

image


Running commands on specific namespaces

image


When an invalid namespace name is passed

image

Previous command output (if the output of a command-line utility has changed)

Single asic command outputs remain untouched

New command output (if the output of a command-line utility has changed)

New multi asic command outputs will start with the prefix: `For namespace: "

@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).

@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).

@arista-hpandya
Copy link
Contributor Author

@arlakshm @wenyiz2021 for awareness

@arista-hpandya
Copy link
Contributor Author

arista-hpandya commented Feb 14, 2025

A gentle nudge to review the PR @vmittal-msft @tjchadaga . Thank you!

Copy link
Contributor

@vmittal-msft vmittal-msft left a comment

Choose a reason for hiding this comment

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

Changes looks fine. Could you get chance to test ToR to make sure it doesn't break anything ?

@arista-hpandya
Copy link
Contributor Author

Thanks for reviewing @vmittal-msft! Here are the snapshots from ToR device testing as requested:

image
image
image

@vmittal-msft
Copy link
Contributor

Thanks. Please go ahead and merge.

@arista-hpandya
Copy link
Contributor Author

Resolved conflict for 202405 cherry-pick: #3775

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

3 participants