Add scope into JSON patch of existing GCU testcases for Multi ASIC. by xincunli-sonic · Pull Request #14098 · sonic-net/sonic-mgmt

xincunli-sonic · 2024-08-13T19:56:43Z

Description of PR

Summary:
Improve existing GCU test cases for multi asic.

Type of change

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

Back port request

Approach

What is the motivation for this PR?

In multi asic, by default, there is no namespace when we do replace or remove operation, which will fail due to the path is incomplete.

How did you do it?

When testcase is running, the apply-patch wrapper in test code will inject the localhost namespace into payload.

How did you verify/test it?

Any platform specific information?

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

Documentation

xincunli-sonic · 2024-08-15T17:29:51Z

/azpw run

mssonicbld · 2024-08-15T17:29:53Z

/AzurePipelines run

azure-pipelines · 2024-08-15T17:30:05Z

Azure Pipelines successfully started running 1 pipeline(s).

tests/generic_config_updater/gu_utils.py

wen587

lgtm

qiluo-msft · 2024-08-28T00:35:10Z

def apply_patch(duthost, json_data, dest_file):

why not just fix json_data directly? keep the code here generic.

Refers to: tests/generic_config_updater/gu_utils.py:37 in dfe4379. [](commit_id = dfe4379, deletion_comment = False)

xincunli-sonic · 2024-08-28T05:26:33Z

def apply_patch(duthost, json_data, dest_file):

IIUC, both way will be working in this test path, but fix json data may cause many current and future efforts, in this wrapper's change, the caller will not need care it is multiasic or not when preparing data.

In reply to: 2313842978

Refers to: tests/generic_config_updater/gu_utils.py:37 in dfe4379. [](commit_id = dfe4379, deletion_comment = False)

mssonicbld · 2024-09-10T16:48:55Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Failed - hook id: trailing-whitespace - exit code: 1 - files were modified by this hook Fixing tests/generic_config_updater/gu_utils.py fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Passed flake8...............................................(no files to check)Skipped check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

xincunli-sonic · 2024-09-11T00:27:12Z

/azpw run

mssonicbld · 2024-09-11T00:27:15Z

/AzurePipelines run

azure-pipelines · 2024-09-11T00:27:25Z

Azure Pipelines successfully started running 1 pipeline(s).

xincunli-sonic · 2024-09-12T17:17:18Z

/azpw run

mssonicbld · 2024-09-12T17:17:21Z

/AzurePipelines run

azure-pipelines · 2024-09-12T17:17:45Z

Azure Pipelines successfully started running 1 pipeline(s).

qiluo-msft · 2024-09-16T20:12:44Z

Please resolve conflict.

qiluo-msft · 2024-09-16T20:36:34Z

def apply_patch(duthost, json_data, dest_file):

Offline discussed, suggest change each test file. The abstraction level at JsonPatch is not proper.

In reply to: 2314344959

Refers to: tests/generic_config_updater/gu_utils.py:37 in dfe4379. [](commit_id = dfe4379, deletion_comment = False)

xincunli-sonic · 2024-10-17T16:04:02Z

/AzurePipelines run

azure-pipelines · 2024-10-17T16:04:11Z

Commenter does not have sufficient privileges for PR 14098 in repo sonic-net/sonic-mgmt

xincunli-sonic · 2024-10-17T16:05:21Z

/azpw run

mssonicbld · 2024-10-17T16:05:24Z

/AzurePipelines run

azure-pipelines · 2024-10-17T16:05:35Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2024-10-17T16:06:55Z

The pre-commit check detected issues in the files touched by this pull request.
The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:
trim trailing whitespace.................................................Failed - hook id: trailing-whitespace - exit code: 1 - files were modified by this hook Fixing tests/common/gu_utils.py fix end of files.........................................................Passed check yaml...........................................(no files to check)Skipped check for added large files..............................................Passed check python ast.........................................................Passed flake8...................................................................Failed - hook id: flake8 - exit code: 1 tests/common/gu_utils.py:25:1: E302 expected 2 blank lines, found 1 flake8...............................................(no files to check)Skipped check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run
the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt
docker container.
Ensure that the pre-commit package is installed:

sudo pip install pre-commit

Go to repository root folder
Install the pre-commit hooks:

pre-commit install

Use pre-commit to check staged file:

pre-commit

Alternatively, you can check committed files using:

pre-commit run --from-ref <commit_id> --to-ref <commit_id>

xincunli-sonic · 2024-11-13T18:27:12Z

/azpw run

mssonicbld · 2024-11-13T18:27:14Z

/AzurePipelines run

azure-pipelines · 2024-11-13T18:27:25Z

Azure Pipelines successfully started running 1 pipeline(s).

…onic-net#14098) ### Description of PR Summary: Improve existing GCU test cases for multi asic. ### Approach #### What is the motivation for this PR? In multi asic, by default, there is no namespace when we do replace or remove operation, which will fail due to the path is incomplete. #### How did you do it? When testcase is running, the apply-patch wrapper in test code will inject the localhost namespace into payload.

…16065) Description of PR For GCU test suites, configuration changes applied via patches sometimes target a specific ASIC namespace or apply globally to the card (localhost), depending on the feature functionality. With the recent merged PR #14098, configuration changes are now applied to both localhost and all ASIC namespaces for all multi-ASIC platforms by default, which is incorrect. Examples: Configuration paths like /BGP_NEIGHBOR/ should apply only to the ASIC namespace. Paths like /SYSLOG_SERVER/ should apply only to the localhost namespace. This PR addresses the issue by introducing a missing flag for "asic_specific" configurations. It adapts the calls appropriately in each test suite based on the targeted configuration scope (ASIC namespace or localhost). Approach How did you verify/test it? Ran vs-kvm tests. Attaching sample results for test_portchannel.py and test_eth_interface.py:

…onic-net#16065) Description of PR For GCU test suites, configuration changes applied via patches sometimes target a specific ASIC namespace or apply globally to the card (localhost), depending on the feature functionality. With the recent merged PR sonic-net#14098, configuration changes are now applied to both localhost and all ASIC namespaces for all multi-ASIC platforms by default, which is incorrect. Examples: Configuration paths like /BGP_NEIGHBOR/ should apply only to the ASIC namespace. Paths like /SYSLOG_SERVER/ should apply only to the localhost namespace. This PR addresses the issue by introducing a missing flag for "asic_specific" configurations. It adapts the calls appropriately in each test suite based on the targeted configuration scope (ASIC namespace or localhost). Approach How did you verify/test it? Ran vs-kvm tests. Attaching sample results for test_portchannel.py and test_eth_interface.py:

Add inject namespace for test case.

dfe4379

xincunli-sonic requested review from qiluo-msft and wen587 August 16, 2024 18:25

wen587 reviewed Aug 27, 2024

View reviewed changes

tests/generic_config_updater/gu_utils.py Outdated Show resolved Hide resolved

wen587 approved these changes Aug 28, 2024

View reviewed changes

Add patch decorator

58ba902

Fix format

a5b855b

Remove dup code

2513ec7

xincunli-sonic added 2 commits October 16, 2024 22:05

Merge from master

a160f4a

Refactor with populate action for multi asic.

0668381

qiluo-msft approved these changes Nov 13, 2024

View reviewed changes

xincunli-sonic changed the title ~~Add inject namespace for test case.~~ Add scope into JSON patch of existing GCU testcases for Multi ASIC. Nov 14, 2024

qiluo-msft merged commit 0c934e3 into sonic-net:master Nov 14, 2024

okaravasi mentioned this pull request Dec 13, 2024

Apply GCU changes to the appropriate namespace scope for each suite #16065

Merged

8 tasks

xincunli-sonic mentioned this pull request Jan 30, 2025

Multi-asic support for ApplyPatchDb API for gNMI sonic-net/sonic-buildimage#20874

Open

okaravasi mentioned this pull request Oct 3, 2025

Test Gap: GCU PRs back-porting in 2405 #20915

Closed

Conversation

xincunli-sonic commented Aug 13, 2024 • 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

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!

xincunli-sonic commented Aug 15, 2024

Uh oh!

mssonicbld commented Aug 15, 2024

Uh oh!

azure-pipelines bot commented Aug 15, 2024

Uh oh!

Uh oh!

wen587 left a comment

Choose a reason for hiding this comment

Uh oh!

qiluo-msft commented Aug 28, 2024

Uh oh!

xincunli-sonic commented Aug 28, 2024

Uh oh!

mssonicbld commented Sep 10, 2024

Uh oh!

xincunli-sonic commented Sep 11, 2024

Uh oh!

mssonicbld commented Sep 11, 2024

Uh oh!

azure-pipelines bot commented Sep 11, 2024

Uh oh!

xincunli-sonic commented Sep 12, 2024

Uh oh!

mssonicbld commented Sep 12, 2024

Uh oh!

azure-pipelines bot commented Sep 12, 2024

Uh oh!

qiluo-msft commented Sep 16, 2024

Uh oh!

qiluo-msft commented Sep 16, 2024

Uh oh!

xincunli-sonic commented Oct 17, 2024

Uh oh!

azure-pipelines bot commented Oct 17, 2024

Uh oh!

xincunli-sonic commented Oct 17, 2024

Uh oh!

mssonicbld commented Oct 17, 2024

Uh oh!

azure-pipelines bot commented Oct 17, 2024

Uh oh!

mssonicbld commented Oct 17, 2024

Uh oh!

xincunli-sonic commented Nov 13, 2024

Uh oh!

mssonicbld commented Nov 13, 2024

Uh oh!

azure-pipelines bot commented Nov 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

xincunli-sonic commented Aug 13, 2024 •

edited

Loading